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

Thu Mar 11 14:50:50 UTC 2021

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.

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

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 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"?

Thanks
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72668): https://edk2.groups.io/g/devel/message/72668
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]
-=-=-=-=-=-=-=-=-=-=-=-