[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4] EFI: always map EfiRuntimeServices{Code,Data}
On 27.01.2022 11:45, Julien Grall wrote: > 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. I cannot provide any details. I took over the patch after pinging Sergey more than once, without any reaction. It was him to actually run into the problem in the wild. >> 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. No, the other way around: We'd also need to put in a valid virtual address there for EfiBootServices{Code,Data}. I expect this wasn't done when map_bs was introduced because iirc at that time the code fragment in efi_exit_boot() was entirely dead, present merely for documentation purposes. >> --- 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)? In principle we wouldn't need to, but the final setting of this bit isn't known yet at this point - it'll be known only after parsing the command line (see parse_efi_param()). Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |