| Message ID | 20200926205542.9261-5-julien@xen.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Unbreak ACPI | expand |
On Sat, 26 Sep 2020, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Since commit 6e3e77120378 "xen/arm: setup: Relocate the Device-Tree > later on in the boot", the device-tree will not be kept mapped when > using ACPI. > > However, a few places are calling dt_unreserved_regions() which expects > a valid DT. This will lead to a crash. > > As the DT should not be used for ACPI (other than for detecting the > modules), a new function fw_unreserved_regions() is introduced. > > It will behave the same way on DT system. On ACPI system, it will > unreserve the whole region. The patch is good. Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> I have a small suggestion for improvement that could be done on commit: given that bootinfo is actually used on EFI systems (granted, not bootinfo.reserved_mem but bootinfo.mem, see xen/arch/arm/efi/efi-boot.h:efi_process_memory_map_bootinfo) so technically bootinfo could be in-use with ACPI, maybe we could add a comment on top of xen/include/asm-arm/setup.h:bootinfo to say that reserved_mem is device tree only? > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > > Is there any region we should exclude on ACPI? > --- > xen/arch/arm/kernel.c | 2 +- > xen/arch/arm/setup.c | 22 +++++++++++++++++----- > xen/include/asm-arm/setup.h | 2 +- > 3 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c > index 032923853f2c..ab78689ed2a6 100644 > --- a/xen/arch/arm/kernel.c > +++ b/xen/arch/arm/kernel.c > @@ -307,7 +307,7 @@ static __init int kernel_decompress(struct bootmodule *mod) > * Free the original kernel, update the pointers to the > * decompressed kernel > */ > - dt_unreserved_regions(addr, addr + size, init_domheap_pages, 0); > + fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0); > > return 0; > } > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 35e5bee04efa..7fcff9af2a7e 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -196,8 +196,9 @@ static void __init processor_id(void) > processor_setup(); > } > > -void __init dt_unreserved_regions(paddr_t s, paddr_t e, > - void (*cb)(paddr_t, paddr_t), int first) > +static void __init dt_unreserved_regions(paddr_t s, paddr_t e, > + void (*cb)(paddr_t, paddr_t), > + int first) > { > int i, nr = fdt_num_mem_rsv(device_tree_flattened); > > @@ -244,6 +245,17 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e, > cb(s, e); > } > > +void __init fw_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); > +} > + > + > + > struct bootmodule __init *add_boot_module(bootmodule_kind kind, > paddr_t start, paddr_t size, > bool domU) > @@ -405,7 +417,7 @@ void __init discard_initial_modules(void) > !mfn_valid(maddr_to_mfn(e)) ) > continue; > > - dt_unreserved_regions(s, e, init_domheap_pages, 0); > + fw_unreserved_regions(s, e, init_domheap_pages, 0); > } > > mi->nr_mods = 0; > @@ -712,7 +724,7 @@ static void __init setup_mm(void) > n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages)); > } > > - dt_unreserved_regions(s, e, init_boot_pages, 0); > + fw_unreserved_regions(s, e, init_boot_pages, 0); > > s = n; > } > @@ -765,7 +777,7 @@ static void __init setup_mm(void) > if ( e > bank_end ) > e = bank_end; > > - dt_unreserved_regions(s, e, init_boot_pages, 0); > + fw_unreserved_regions(s, e, init_boot_pages, 0); > s = n; > } > } > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index 2f8f24e286ed..34df23247b87 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -96,7 +96,7 @@ int construct_dom0(struct domain *d); > void create_domUs(void); > > void discard_initial_modules(void); > -void dt_unreserved_regions(paddr_t s, paddr_t e, > +void fw_unreserved_regions(paddr_t s, paddr_t e, > void (*cb)(paddr_t, paddr_t), int first); > > size_t boot_fdt_info(const void *fdt, paddr_t paddr); > -- > 2.17.1 >
Hi Stefano, On 01/10/2020 00:40, Stefano Stabellini wrote: > On Sat, 26 Sep 2020, Julien Grall wrote: > I have a small suggestion for improvement that could be done on commit: > given that bootinfo is actually used on EFI systems (granted, not > bootinfo.reserved_mem but bootinfo.mem, see > xen/arch/arm/efi/efi-boot.h:efi_process_memory_map_bootinfo) so > technically bootinfo could be in-use with ACPI, maybe we could add a > comment on top of xen/include/asm-arm/setup.h:bootinfo to say that > reserved_mem is device tree only? That's fine with me. I will need to resend the rest of the series, so I will update it at the same time. Cheers,
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c index 032923853f2c..ab78689ed2a6 100644 --- a/xen/arch/arm/kernel.c +++ b/xen/arch/arm/kernel.c @@ -307,7 +307,7 @@ static __init int kernel_decompress(struct bootmodule *mod) * Free the original kernel, update the pointers to the * decompressed kernel */ - dt_unreserved_regions(addr, addr + size, init_domheap_pages, 0); + fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0); return 0; } diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 35e5bee04efa..7fcff9af2a7e 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -196,8 +196,9 @@ static void __init processor_id(void) processor_setup(); } -void __init dt_unreserved_regions(paddr_t s, paddr_t e, - void (*cb)(paddr_t, paddr_t), int first) +static void __init dt_unreserved_regions(paddr_t s, paddr_t e, + void (*cb)(paddr_t, paddr_t), + int first) { int i, nr = fdt_num_mem_rsv(device_tree_flattened); @@ -244,6 +245,17 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e, cb(s, e); } +void __init fw_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); +} + + + struct bootmodule __init *add_boot_module(bootmodule_kind kind, paddr_t start, paddr_t size, bool domU) @@ -405,7 +417,7 @@ void __init discard_initial_modules(void) !mfn_valid(maddr_to_mfn(e)) ) continue; - dt_unreserved_regions(s, e, init_domheap_pages, 0); + fw_unreserved_regions(s, e, init_domheap_pages, 0); } mi->nr_mods = 0; @@ -712,7 +724,7 @@ static void __init setup_mm(void) n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages)); } - dt_unreserved_regions(s, e, init_boot_pages, 0); + fw_unreserved_regions(s, e, init_boot_pages, 0); s = n; } @@ -765,7 +777,7 @@ static void __init setup_mm(void) if ( e > bank_end ) e = bank_end; - dt_unreserved_regions(s, e, init_boot_pages, 0); + fw_unreserved_regions(s, e, init_boot_pages, 0); s = n; } } diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index 2f8f24e286ed..34df23247b87 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -96,7 +96,7 @@ int construct_dom0(struct domain *d); void create_domUs(void); void discard_initial_modules(void); -void dt_unreserved_regions(paddr_t s, paddr_t e, +void fw_unreserved_regions(paddr_t s, paddr_t e, void (*cb)(paddr_t, paddr_t), int first); size_t boot_fdt_info(const void *fdt, paddr_t paddr);