Re: [Xen-devel] REGRESSION: Xen 4.13 RC5 fails to bootstrap Dom0 on ARM

[Julien Grall <julien@xxxxxxx>]

Hi,

On 20/12/2019 00:01, Stefano Stabellini wrote:
> On Thu, 19 Dec 2019, Julien Grall wrote:
> > > > In fact most of people on Arm are using GRUB rather than EFI directly as
> > > > this is more friendly to use.
> > > >
> > > > Regarding the devicetree, Xen and Linux will completely ignore the
> > > > memory nodes in Xen if using EFI. This because the EFI memory map will
> > > > give you an overview of the platform with the EFI regions included.
> > > Aha! So in that sense it is a bug in Xen after all, right? (that's what
> > > you're
> > > referring to when you say you now understand what needs to get fixed).
> > Yes. The EFI memory map is a list of existing memory with a type associated to
> > it (Conventional, BootServiceCodes, MemoryMappedIO...).
> >
> > The OS/Hypervisor will have to go through them and check which regions are
> > usuable. Compare to Linux, Xen has limited itself to only a few types.
> >
> > However, I think we can be on a par with Linux here.
> I gave a look at the Linux implementation, the interesting bit is
> drivers/firmware/efi/arm-init.c:is_usable_memory as far as I can tell.
> I also gave a look at the Xen side, which is
> xen/arch/arm/efi/efi-boot.h:efi_process_memory_map_bootinfo. As guessed,
> the two are not quite the same.
>
> One of the main differences is that Linux uses as "System RAM" even
> regions that were marked as EFI_BOOT_SERVICES_CODE/DATA and
> EFI_LOADER_CODE/DATA because they will get freed anyway. Xen doesn't
> do that unless map_bs is set.
Well, map_bs is used to request to map the boot services into Xen PT. In 
other words, you can't consider them as usuable if that option is set.
> I wrote a quick patch to implement the Linux behavior on Xen, only
> lightly tested. I can confirm that I see more memory this way. However,
> I am not sure we actually want to import the Linux behavior wholesale.
This is not what I had in mind, we still need to keep Xen behavior for 
boot services and ensure the region are recorded/skipped as expected 
(see more below).
> Anyway, Roman, could you please let me know if this patch solves the
> issue?
Please see a couple of comments below.

> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index ca655ff003..ad18ff3669 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -149,10 +149,14 @@ static EFI_STATUS __init 
> efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
>   
>       for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
>       {
> -        if ( desc_ptr->Type == EfiConventionalMemory ||
> -             (!map_bs &&
> -              (desc_ptr->Type == EfiBootServicesCode ||
> -               desc_ptr->Type == EfiBootServicesData)) )
> +        if ( desc_ptr->Attribute == EFI_MEMORY_WB &&
This is not quite the same as Linux. They use:

desc_ptr->Attribute & EFI_MEMORY_WB

But I don't see why Attribute should be strictly equal to EFI_MEMORY_WB.

> +             (desc_ptr->Type == EfiConventionalMemory ||
> +              desc_ptr->Type == EfiLoaderCode ||
> +              desc_ptr->Type == EfiLoaderData ||
> +              desc_ptr->Type == EfiACPIReclaimMemory ||
Linux will explictly reserve the ACPI region (see the caller of 
is_usable_memory()). For us this is done in the "else if" part.
> +              desc_ptr->Type == EfiPersistentMemory ||
> +              desc_ptr->Type == EfiBootServicesCode ||
> +              desc_ptr->Type == EfiBootServicesData) )
>           {
>               if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
>               {
> diff --git a/xen/include/efi/efidef.h b/xen/include/efi/efidef.h
> index 86a7e111bf..f46207840f 100644
> --- a/xen/include/efi/efidef.h
> +++ b/xen/include/efi/efidef.h
> @@ -147,6 +147,7 @@ typedef enum {
>       EfiMemoryMappedIO,
>       EfiMemoryMappedIOPortSpace,
>       EfiPalCode,
> +    EfiPersistentMemory,
>       EfiMaxMemoryType
>   } EFI_MEMORY_TYPE;
>   
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel