[Xen-devel] Interaction between ACPI and dt_unreserved_regions() (WAS: Re: [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range)

[Julien Grall <julien@xxxxxxx>]

(Renaming the title to avoid confusion)

On 17/01/2020 09:06, Wei Xu wrote:
> Hi Julien,
Hi Wei,

> On 2020/1/7 23:12, Julien Grall wrote:
> Sorry for the late reply!
Don't worry, thank you for looking into the bug!

> The PC refers to fdt_num_mem_rsv during init_done.
> But the device_tree_flattened is NULL that the data abort happened.
Ah, I didn't realize that device_tree_flattened where still used 
afterwards. Sorry for the breakage. I really need to setup a devbox with 
ACPI so I can test changes properly.
> So I added below changes and the XEN dom0 can be booted.
>
>      diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>      index 1e83351..1ab80a1 100644
>      --- a/xen/arch/arm/setup.c
>      +++ b/xen/arch/arm/setup.c
>      @@ -392,7 +392,8 @@ void __init discard_initial_modules(void)
>                    !mfn_valid(maddr_to_mfn(e)) )
>                   continue;
>
>      -        dt_unreserved_regions(s, e, init_domheap_pages, 0);
>      +       if( acpi_disabled )
>      +           dt_unreserved_regions(s, e, init_domheap_pages, 0);
While I understand how this is fixing your problem, this unfortunately 
means the memory ranges used by the inital modules (e.g Kernel, Initrd) 
will not be re-used by Xen. So this is a "slight" waste of memory.
There are a few other places where dt_unreserved_regions() is called 
(see setup_mm()). However, in the case of setup_mm() we have a pointer 
to DT as we don't know yet we are running on ACPI systems.
But it feels wrong to try to find out the reserved memory range through 
the DT when ACPI will be used. The DT is either going to be nearly 
empty, or contain the full description of the platform. I don't know 
enough to be able to say if something is going to go wrong.
I am thinking to suggest to create an helper that will do the job for 
you. Something like:
void fwtable_unreserved_regions(paddr_t s, paddr_t e,
                                void (*cb)(paddr_t, paddr_t), int first)
{
   if ( acpi_disabled )
     dt_unreserved_regions(s, e, cb, first);
   else
     cb(s, e);
}

Regarding the else part, this may need some adjustment if we need to 
skip some reserved region for ACPI. On Xen 4.13, we should only have 
usuable RAM in hand (the EFI stub is doing to sorting for us). Do you 
know whether ACPI describes something similar to reserved-memory in DT 
(i.e RAM regions reserved for cma...)?
Cheers,

--
Julien Grall

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