[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4] EFI: always map EfiRuntimeServices{Code,Data}
Hi Jan, On 12/01/2022 08:45, Jan Beulich wrote: From: Sergey Temerkhanov <s.temerkhanov@xxxxxxxxx> This helps overcome problems observed with some UEFI implementations Would it be possible to provide some details about the platform? This is helpful to track why a workaround is present. which don't set the Attributes field in memery descriptors properly. typo: s/memery/memory/ Signed-off-by: Sergey Temerkhanov <s.temerkhanov@xxxxxxxxx> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- v4: Drop EFI_MEMORY_CACHEABILITY_MASK. Fold with pre-existing if() (into switch()). Style. --- I guess "map_bs" would also want honoring in efi_exit_boot(), but that's yet another patch then I suppose. IIUC, we would need to invalidate the mapping when map_bs was used. Is it correct? If so, then I agree this sounds separate to the issue you are describing here. --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1094,7 +1094,13 @@ static void __init efi_exit_boot(EFI_HAN { EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;- if ( desc->Attribute & EFI_MEMORY_RUNTIME )+ /* + * Runtime services regions are always mapped here. + * Attributes may be adjusted in efi_init_memory(). + */ + if ( (desc->Attribute & EFI_MEMORY_RUNTIME) || + desc->Type == EfiRuntimeServicesCode || + desc->Type == EfiRuntimeServicesData ) My knowledge with the stub is limited. Would you be able to explain why we need to map the runtime services even with !efi_enabled(EFI_RS)? The reason I am asking is if we don't need it, then it would be sufficient to rely on desc->Attribute because you updated it in efi_init_memory(). desc->VirtualStart = desc->PhysicalStart; else desc->VirtualStart = INVALID_VIRTUAL_ADDRESS; @@ -1545,13 +1551,36 @@ void __init efi_init_memory(void) ROUNDUP(desc->PhysicalStart + len, PAGE_SIZE)); }- if ( !efi_enabled(EFI_RS) ||- (!(desc->Attribute & EFI_MEMORY_RUNTIME) && - (!map_bs || - (desc->Type != EfiBootServicesCode && - desc->Type != EfiBootServicesData))) ) + if ( !efi_enabled(EFI_RS) ) continue;+ if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )+ { + switch ( desc->Type ) + { + default: + continue; + + /* + * Adjust runtime services regions. Keep in sync with + * efi_exit_boot(). + */ + case EfiRuntimeServicesCode: + case EfiRuntimeServicesData: + printk(XENLOG_WARNING + "Setting RUNTIME attribute for %013" PRIx64 "-%013" PRIx64 "\n", + desc->PhysicalStart, desc->PhysicalStart + len - 1); + desc->Attribute |= EFI_MEMORY_RUNTIME; + break; + + case EfiBootServicesCode: + case EfiBootServicesData: + if ( !map_bs ) + continue; + break; + } + } The new code is much easier to read :). + desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;smfn = PFN_DOWN(desc->PhysicalStart); Cheers, -- Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |