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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 21/12/2019 01:37, Roman Shaposhnik wrote:
> On Thu, Dec 19, 2019 at 4:01 PM Stefano Stabellini
> <sstabellini@xxxxxxxxxx> 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.
> >
> > 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.
> >
> > Anyway, Roman, could you please let me know if this patch solves the
> > issue?
> Tried the attached patch -- but it seems I can't boot at all with this. Xen
> doesn't print anything on the console either.
Thank you for trying the patch. Do you have earlyprintk enabled for the 
hikey board?
> To Julien's point -- should I reduce the # of types and try again?
From my understanding, the field Attribute is a series of flag telling 
what the region can support.
So it would be possible to have other flags set at the same time as 
EFI_MEMORY_WC. However, the check in the patch below is an == equal and 
would potentially discard a lot of regions (if not all regions).
In other words...

> > 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 should be desc_ptr->Attribute & EFI_MEMORY_WB.

Can you give a spin with this change and see how far you can go?

> > +             (desc_ptr->Type == EfiConventionalMemory ||
> > +              desc_ptr->Type == EfiLoaderCode ||
> > +              desc_ptr->Type == EfiLoaderData ||
> > +              desc_ptr->Type == EfiACPIReclaimMemory ||
> > +              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