[edk2-devel] Conflicting virtual addresses causing Runtime Services issues

[Jon Nettleton]

On Thu, Mar 11, 2021 at 3:51 PM Laszlo Ersek <lersek at redhat.com> wrote:
>
> On 03/11/21 10:48, Jon Nettleton wrote:
> > On Thu, Mar 11, 2021 at 7:54 AM Jon Nettleton via groups.io
> > <jon=solid-run.com at groups.io> wrote:
> >>
> >> On Wed, Mar 10, 2021 at 3:52 PM Laszlo Ersek <lersek at redhat.com> wrote:
> >>>
> >>> On 03/10/21 09:04, Jon Nettleton wrote:
> >>>> I am debugging a failure that I am seeing while using the HoneyComb's
> >>>> spi-nor flash for runtime variable storage.  I am hoping someone on
> >>>> the list can give me some insight as what may be the problem.
> >>>>
> >>>> The problem showed up when we switched the MMIO region for the fspi
> >>>> flash device to be marked as non executable.  reading variables is
> >>>> fine, however writes began throwing an error.
> >>>>
> >>>> [  556.709828] Unable to handle kernel execute from non-executable
> >>>> memory at virtual address 00000000206a3968
> >>>>
> >>>> I have patched the kernel and removed the X86 requirement and enabled
> >>>> the sysfs runtime mappings kernel config so I can get an easy view of
> >>>> the mappings the kernel carries for runtime services.  I then track
> >>>> that virtual address to the MMIO region of nor flash, which makes
> >>>> sense that region is marked as non executable.  The question is why is
> >>>> code being executed from this address range
> >>>>
> >>>> attribute
> >>>> ::::::::::::::
> >>>> 0x8000000000000001
> >>>> ::::::::::::::
> >>>> num_pages
> >>>> ::::::::::::::
> >>>> 0x40
> >>>> ::::::::::::::
> >>>> phys_addr
> >>>> ::::::::::::::
> >>>> 0x20500000
> >>>> ::::::::::::::
> >>>> type
> >>>> ::::::::::::::
> >>>> 0xb
> >>>> ::::::::::::::
> >>>> virt_addr
> >>>> ::::::::::::::
> >>>> 0x20680000
> >>>>
> >>>> So then I patched the PL011 serial driver to be able to log to the
> >>>> console in runtime and I track down the access to Status =
> >>>> Fvb->GetPhysicalAddress(Fvb, &FvVolHdr); in UpdateVariableStore().
> >>>> What I don't understand is why EfiConvertPointer is mapping that
> >>>> pointer into the Virtual address space occupied by the runtime mmio of
> >>>> the flash.  The pointer is being properly remapped.  Here are the
> >>>> pointer addresses in EFI and Kernel Runtime
> >>>>
> >>>> EFI:
> >>>> UpdateVariableStore:156 ECE33968
> >>>> FvbGetPhysicalAddress(BaseAddress=0x20000000)
> >>>>
> >>>> KERNEL:
> >>>> UpdateVariableStore:156 206A3968
> >>>> [  556.709828] Unable to handle kernel execute from non-executable
> >>>> memory at virtual address 00000000206a3968
> >>>>
> >>>> Any insight that anyone could provide would be much appreciated.
> >>>
> >>> Your platform appears misconfigured -- the flash MMIO range appears to
> >>> overlap runtime services code even before SetVirtualAddressMap. The
> >>> virtual address conflict is likely the result of the original physical
> >>> address conflict.
> >>
> >> There is no physical address conflict.  Running in physical mode the pointer
> >> for GetPhysicalAddress is at 0xECE33968 and the MMIO physical
> >> region is 0x20000000 - 0x2FFFFFFF.  Shown as the BaseAddress above.
> >> Obviously I don't use that full MMIO region because our flash is not
> >> 256MB.
> >>
> >>>
> >>> After the virtual address updates, the EfiMemoryMappedIO (0xb) type
> >>> range is mapped at virtual address range [0x20680000, 0x206C0000). The
> >>> GetPhysicalAddress function seems to be located at offset 0x23968 in
> >>> that range (with 0x1C698 bytes to go in the range). That's inexplicable.
> >>
> >> Yes, exactly why I am reaching out.
> >>
> >>>
> >>> What is the physical base address of the flash? What are the PCDs used
> >>> in "ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c"?
> >>
> >> We don't use the ArmPlatformPkg
> >>
> >> The code is available here.
> >> https://github.com/SolidRun/edk2-platforms/tree/LX2160_UEFI_ACPI_EAR3-lx2160cex7/Silicon/NXP/Drivers/SpiNorFlashDxe
> >>
> >> Thanks
> >>>
> >
> > Found the root of my problem and fixed it.  As used from other devices
> > they are relocating the individual pointer for each function in FvbProtocol
> > of the DXE.  This is done in ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c,
> > as well as lots of vendor drivers.  I was able to dump the addresses being used
> > in RuntimeDriverConvertPointer and could locate the pointer to
> > GetPhysicalAddress.
> >
> > Convert 0xEC8C1648:0xEC8B0000:0x203D0000
> > New virtual address is then 203E1648 which is fine.
> >
> > However then later down in the remapping I found
> > Convert 0x203E1648:0x20000000:0x20800000
> >
> > And this is where the pointer gets remapped again and into the
> > MMIO space of the nor flash.  If I remove the calls to
> > ConvertPointer for the FvbProtocol I am still seeing those addresses
> > getting remapped but only once and runtime works as expected.
> >
> > I am seeing that in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> > &mVariableModuleGlobal->FvbInstance->* are all being converted.  It is
> > possible this is a long standing bug and it just so happens that our
> > configuration
> > has caused a conflict and exposed it.
>
> Yes, this is curious, I noticed it too yesterday, trying to see where
> the FVB protocol member function pointers were converted. I found that
> OVMF's flash driver (OvmfPkg/QemuFlashFvbServicesRuntimeDxe) didn't do
> it, but MdeModulePkg/Universal/Variable/RuntimeDxe did. That was
> certainly strange, as the variable driver is a consumer of the protocol
> (not the producer thereof), so I'd say it has no business poking new
> values into the protocol interface structure.
>
> So the issue (IIUC) on your platform is that the function pointers are
> doubly-converted, once by your flash driver (which is arguably the right
> thing), and another time by the variable driver.

Correct, that seems to be what is happening

>
> I recommend filing a bug about this, at
> <https://bugzilla.tianocore.org/>, against "VariableDxe.c".

Sure

>
> I suggest CC'ing the "UEFI Variable modules" reviewers on the BZ as
> well, from "Maintainers.txt" -- FWIW, I'm CC'ing Hao and Liming on this
> email, now.
>
> ... It feels as if the Platform Init specification should speak to the
> FVB[2] protocol's runtime aspects... But (as of v1.7), I don't see
> anything runtime-related in that section. In fact I've checked the whole
> PI v1.7 spec for "runtime", and the closest matches are under
> EFI_VARIABLE_WRITE_ARCH_PROTOCOL. FVB2 is not spelled out as a
> (potential) runtime dependency of EFI_VARIABLE_WRITE_ARCH_PROTOCOL. The
> only explicitly runtime protocol (unrelated to runtime services) is
> "Status Code Runtime Protocol".
>
> So what we're seeing here is an edk2-ism, I believe.

I checked the commits and the code that does that conversion is quite old.

>
> I tried seeing if one of the "tour beyond BIOS" edk2 whitepapers said
> anything on FVB -- but the only reference I found (in "Implementing
> UEFI Authenticated Variables in SMM with EDKII", version 2) mentions
> only *SMM* FVB.
>
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-white-papers
>
> I think we should have a TianoCore BZ about this, even if we don' change
> the code as a result -- which driver is responsible for updating the
> runtime FVB protocol member functions should be spelled out somewhere
> "searchable".
>
> ... Strangely, the other flash (FVB) driver in edk2,
> ArmPlatformPkg/Drivers/NorFlashDxe, *does* perform the conversion
> itself! See NorFlashVirtualNotifyEvent().
>
> I don't understand that. Is it possible that, with
> "ArmPlatformPkg/Drivers/NorFlashDxe" too, the conversion happens
> *twice*, but (at least) one of those mappings is "identity"?

I believe all the drivers that are doing the conversion are getting
double mappings.  It just so happens our second mapping got poked
into an area that we had marked non-executable which started the
debugging and led to the issue.

>
> Thanks
> Laszlo
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72669): https://edk2.groups.io/g/devel/message/72669
Mute This Topic: https://groups.io/mt/81222488/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-