| Message ID | 54BE1FEA.5040109@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 20, 2015 at 09:29:14AM +0000, Hanjun Guo wrote: > On 2015?01?20? 02:01, Mark Rutland wrote: > > On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote: > >> On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote: > >>> On 01/19/2015 10:13 AM, Grant Likely wrote: > >>>> On Mon, 19 Jan 2015 13:51:45 +0000 > >>>> , Catalin Marinas <catalin.marinas@arm.com> > >>>> wrote: > >>>>> On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote: > >>>>>> On 19 January 2015 at 11:42, Catalin Marinas <catalin.marinas@arm.com> wrote: > >>>>>>> On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote: > >>>>>>>> From: Al Stone <al.stone@linaro.org> > >>>>>>>> > >>>>>>>> Introduce one early parameters "off" and "force" for "acpi", acpi=off > >>>>>>>> will be the default behavior for ARM64, so introduce acpi=force to > >>>>>>>> enable ACPI on ARM64. > >>>>>>>> > >>>>>>>> Disable ACPI before early parameters parsed, and enable it to pass > >>>>>>>> "acpi=force" if people want use ACPI on ARM64. This ensures DT be > >>>>>>>> the prefer one if ACPI table and DT both are provided at this moment. > >>>>>>> [...] > >>>>>>>> --- a/arch/arm64/kernel/setup.c > >>>>>>>> +++ b/arch/arm64/kernel/setup.c > >>>>>>>> @@ -62,6 +62,7 @@ > >>>>>>>> #include <asm/memblock.h> > >>>>>>>> #include <asm/psci.h> > >>>>>>>> #include <asm/efi.h> > >>>>>>>> +#include <asm/acpi.h> > >>>>>>>> > >>>>>>>> unsigned int processor_id; > >>>>>>>> EXPORT_SYMBOL(processor_id); > >>>>>>>> @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p) > >>>>>>>> early_fixmap_init(); > >>>>>>>> early_ioremap_init(); > >>>>>>>> > >>>>>>>> + disable_acpi(); > >>>>>>>> + > >>>>>>>> parse_early_param(); > >>>>>>>> > >>>>>>>> /* > >>>>>>> > >>>>>>> Did we get to any conclusion here? DT being the preferred one is fine > >>>>>>> when both DT and ACPI are present but do we still want the kernel to > >>>>>>> ignore ACPI altogether if DT is not present? It's a bit harder to detect > >>>>>>> the presence of DT at this point since the EFI_STUB added one already. I > >>>>>>> guess we could move the "acpi=force" argument passing to EFI_STUB if no > >>>>>>> DT is present at boot. > >>>>>> > >>>>>> Since the EFI stub populates the /chosen node in DT, I would prefer > >>>>>> for it to add a property there to indicate whether it created the DT > >>>>>> from scratch rather than adding ACPI specific stuff in there (even if > >>>>>> it is just a string to concatenate) > >>>>> > >>>>> This works for me. So we could pass "acpi=force" in EFI stub if it > >>>>> created the DT from scratch *and* ACPI tables are present (can it detect > >>>>> the latter? And maybe it could print something if none are available). > >>>>> If that works, the actual kernel can assume that ACPI needs to be > >>>>> explicitly enabled via acpi=force, irrespective of how much information > >>>>> it has in DT. > >>>> > >>>> Ditto for me. I think this is a fine solution. And, yes, the stub can > >>>> easily detect the presence of ACPI by looking in the UEFI config table. > >>> > >>> I get the point behind doing this, but could we not have it pass in a > >>> different parameter than =force? Perhaps something new? I'd like to > >>> separate out the case that it was enabled automatically vs explicitly > >>> forced on by a user wanting to use ACPI on a system with both tables. > >> > >> Ard had a point, so we should probably not pass acpi=force from EFI stub > >> (especially since a user may explicitly pass acpi=off irrespective of DT > >> presence). Some other property in the chosen node? It's not even an ABI > >> since that's a contract between EFI stub and the rest of the kernel, so > >> an in-kernel only interface. > > > > Not strictly true once kexec is in place. Then it becomes a stub -> > > kernel -> kernel -> kernel -> ... interface, alnog with the rest of the > > properties the stub puts in the DTB. > > > > Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane > > regardless. > > How about the patch (just RFC, maybe it is horrible :) ) below: > > When system supporting both DT and ACPI but firmware providing > no dtb, we can use this linux,uefi-stub-generated-dtb property > to let kernel know that we can try ACPI configuration data. > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> I'm ok with the idea but I'll let Mark comment on the DT aspects.
On Tue, Jan 20, 2015 at 09:29:14AM +0000, Hanjun Guo wrote: > On 2015?01?20? 02:01, Mark Rutland wrote: > > On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote: > >> On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote: > >>> On 01/19/2015 10:13 AM, Grant Likely wrote: > >>>> On Mon, 19 Jan 2015 13:51:45 +0000 > >>>> , Catalin Marinas <catalin.marinas@arm.com> > >>>> wrote: > >>>>> On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote: > >>>>>> On 19 January 2015 at 11:42, Catalin Marinas <catalin.marinas@arm.com> wrote: > >>>>>>> On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote: > >>>>>>>> From: Al Stone <al.stone@linaro.org> > >>>>>>>> > >>>>>>>> Introduce one early parameters "off" and "force" for "acpi", acpi=off > >>>>>>>> will be the default behavior for ARM64, so introduce acpi=force to > >>>>>>>> enable ACPI on ARM64. > >>>>>>>> > >>>>>>>> Disable ACPI before early parameters parsed, and enable it to pass > >>>>>>>> "acpi=force" if people want use ACPI on ARM64. This ensures DT be > >>>>>>>> the prefer one if ACPI table and DT both are provided at this moment. > >>>>>>> [...] > >>>>>>>> --- a/arch/arm64/kernel/setup.c > >>>>>>>> +++ b/arch/arm64/kernel/setup.c > >>>>>>>> @@ -62,6 +62,7 @@ > >>>>>>>> #include <asm/memblock.h> > >>>>>>>> #include <asm/psci.h> > >>>>>>>> #include <asm/efi.h> > >>>>>>>> +#include <asm/acpi.h> > >>>>>>>> > >>>>>>>> unsigned int processor_id; > >>>>>>>> EXPORT_SYMBOL(processor_id); > >>>>>>>> @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p) > >>>>>>>> early_fixmap_init(); > >>>>>>>> early_ioremap_init(); > >>>>>>>> > >>>>>>>> + disable_acpi(); > >>>>>>>> + > >>>>>>>> parse_early_param(); > >>>>>>>> > >>>>>>>> /* > >>>>>>> > >>>>>>> Did we get to any conclusion here? DT being the preferred one is fine > >>>>>>> when both DT and ACPI are present but do we still want the kernel to > >>>>>>> ignore ACPI altogether if DT is not present? It's a bit harder to detect > >>>>>>> the presence of DT at this point since the EFI_STUB added one already. I > >>>>>>> guess we could move the "acpi=force" argument passing to EFI_STUB if no > >>>>>>> DT is present at boot. > >>>>>> > >>>>>> Since the EFI stub populates the /chosen node in DT, I would prefer > >>>>>> for it to add a property there to indicate whether it created the DT > >>>>>> from scratch rather than adding ACPI specific stuff in there (even if > >>>>>> it is just a string to concatenate) > >>>>> > >>>>> This works for me. So we could pass "acpi=force" in EFI stub if it > >>>>> created the DT from scratch *and* ACPI tables are present (can it detect > >>>>> the latter? And maybe it could print something if none are available). > >>>>> If that works, the actual kernel can assume that ACPI needs to be > >>>>> explicitly enabled via acpi=force, irrespective of how much information > >>>>> it has in DT. > >>>> > >>>> Ditto for me. I think this is a fine solution. And, yes, the stub can > >>>> easily detect the presence of ACPI by looking in the UEFI config table. > >>> > >>> I get the point behind doing this, but could we not have it pass in a > >>> different parameter than =force? Perhaps something new? I'd like to > >>> separate out the case that it was enabled automatically vs explicitly > >>> forced on by a user wanting to use ACPI on a system with both tables. > >> > >> Ard had a point, so we should probably not pass acpi=force from EFI stub > >> (especially since a user may explicitly pass acpi=off irrespective of DT > >> presence). Some other property in the chosen node? It's not even an ABI > >> since that's a contract between EFI stub and the rest of the kernel, so > >> an in-kernel only interface. > > > > Not strictly true once kexec is in place. Then it becomes a stub -> > > kernel -> kernel -> kernel -> ... interface, alnog with the rest of the > > properties the stub puts in the DTB. > > > > Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane > > regardless. > > How about the patch (just RFC, maybe it is horrible :) ) below: > > When system supporting both DT and ACPI but firmware providing > no dtb, we can use this linux,uefi-stub-generated-dtb property > to let kernel know that we can try ACPI configuration data. > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ > arch/arm64/kernel/setup.c | 34 > +++++++++++++++++++++++++++- > drivers/firmware/efi/libstub/fdt.c | 6 +++++ > 3 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/chosen.txt > b/Documentation/devicetree/bindings/chosen.txt > index ed838f4..18776b9 100644 > --- a/Documentation/devicetree/bindings/chosen.txt > +++ b/Documentation/devicetree/bindings/chosen.txt > @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property > "linux,stdout-path" or > on PowerPC "stdout" if "stdout-path" is not found. However, the > "linux,stdout-path" and "stdout" properties are deprecated. New platforms > should only use the "stdout-path" property. > + > + > +linux,uefi-stub-generated-dtb property > +-------------------------------------- > + > +UEFI stub will generate this property in the chosen node to let linux > kernel > +know that there is no DTB provided by firmware. > + > +There is a use case for system supporting both DT and ACPI, when firmware > +doesn't provide DT, we can try ACPI configration data to boot the system. I don't think we need to list potential use cases here, this can be useful regardless of UEFI. The other UEFI stub properties currently live under Documentation/arm/uefi.txt. This should live with them. > + > +Usage: > + > +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb > +is generated by uefi stub > + > +or > + > +linux,uefi-stub-generated-dtb = "false" is the reverse. I imagined this would be an empty property. It would only be present if the stub generated the DTB, and has no value: /chosen { linux,uefi-stub-generated-dtb; }; > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 54e39e3..8268c7b 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -371,6 +371,31 @@ static void __init request_standard_resources(void) > } > } > > +int __init dt_scan_chosen(unsigned long node, const char *uname, > + int depth, void *data) > +{ > + const char *p; > + > + if (depth != 1 || !data || > + (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0)) > + return 0; Do we ever generate chosen@0, and do we even accept that? > + > + p = of_get_flat_dt_prop(node, "linux,uefi-stub-generated-dtb", NULL); > + if (!p && !strcmp(p, "true")) > + *data = true; > + > + return 1; > +} > + > +static bool __init is_uefi_stub_generated_dtb(void) > +{ > + bool flag = false; > + > + of_scan_flat_dt(dt_scan_chosen, &flag); > + > + return flag; > +} > + > u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID }; > > void __init setup_arch(char **cmdline_p) > @@ -389,7 +414,14 @@ void __init setup_arch(char **cmdline_p) > early_fixmap_init(); > early_ioremap_init(); > > - disable_acpi(); > + /* > + * If no dtb provided by firmware, enable ACPI > + * and try to boot with ACPI configuration data > + */ > + if (is_uefi_stub_generated_dtb()) > + enable_acpi(); > + else > + disable_acpi(); > > parse_early_param(); > > diff --git a/drivers/firmware/efi/libstub/fdt.c > b/drivers/firmware/efi/libstub/fdt.c > index c846a96..9e2084b 100644 > --- a/drivers/firmware/efi/libstub/fdt.c > +++ b/drivers/firmware/efi/libstub/fdt.c > @@ -154,6 +154,12 @@ efi_status_t update_fdt(efi_system_table_t > *sys_table, void *orig_fdt, > if (status) > goto fdt_set_fail; > > + /* Add a property to show the dtb is generated by uefi stub or not */ > + status = fdt_setprop_string(fdt, node, "linux,uefi-stub-generated-dtb", > + orig_fdt ? "false" : "true"); > + if (status) > + goto fdt_set_fail; > + This should create an empty property, and only when generated by the stub. Thanks, Mark.
On 2015?01?20? 19:10, Mark Rutland wrote: > On Tue, Jan 20, 2015 at 09:29:14AM +0000, Hanjun Guo wrote: >> On 2015?01?20? 02:01, Mark Rutland wrote: >>> On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote: >>>> On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote: >>>>> On 01/19/2015 10:13 AM, Grant Likely wrote: >>>>>> On Mon, 19 Jan 2015 13:51:45 +0000 >>>>>> , Catalin Marinas <catalin.marinas@arm.com> >>>>>> wrote: >>>>>>> On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote: >>>>>>>> On 19 January 2015 at 11:42, Catalin Marinas <catalin.marinas@arm.com> wrote: >>>>>>>>> On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote: >>>>>>>>>> From: Al Stone <al.stone@linaro.org> >>>>>>>>>> >>>>>>>>>> Introduce one early parameters "off" and "force" for "acpi", acpi=off >>>>>>>>>> will be the default behavior for ARM64, so introduce acpi=force to >>>>>>>>>> enable ACPI on ARM64. >>>>>>>>>> >>>>>>>>>> Disable ACPI before early parameters parsed, and enable it to pass >>>>>>>>>> "acpi=force" if people want use ACPI on ARM64. This ensures DT be >>>>>>>>>> the prefer one if ACPI table and DT both are provided at this moment. >>>>>>>>> [...] >>>>>>>>>> --- a/arch/arm64/kernel/setup.c >>>>>>>>>> +++ b/arch/arm64/kernel/setup.c >>>>>>>>>> @@ -62,6 +62,7 @@ >>>>>>>>>> #include <asm/memblock.h> >>>>>>>>>> #include <asm/psci.h> >>>>>>>>>> #include <asm/efi.h> >>>>>>>>>> +#include <asm/acpi.h> >>>>>>>>>> >>>>>>>>>> unsigned int processor_id; >>>>>>>>>> EXPORT_SYMBOL(processor_id); >>>>>>>>>> @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p) >>>>>>>>>> early_fixmap_init(); >>>>>>>>>> early_ioremap_init(); >>>>>>>>>> >>>>>>>>>> + disable_acpi(); >>>>>>>>>> + >>>>>>>>>> parse_early_param(); >>>>>>>>>> >>>>>>>>>> /* >>>>>>>>> >>>>>>>>> Did we get to any conclusion here? DT being the preferred one is fine >>>>>>>>> when both DT and ACPI are present but do we still want the kernel to >>>>>>>>> ignore ACPI altogether if DT is not present? It's a bit harder to detect >>>>>>>>> the presence of DT at this point since the EFI_STUB added one already. I >>>>>>>>> guess we could move the "acpi=force" argument passing to EFI_STUB if no >>>>>>>>> DT is present at boot. >>>>>>>> >>>>>>>> Since the EFI stub populates the /chosen node in DT, I would prefer >>>>>>>> for it to add a property there to indicate whether it created the DT >>>>>>>> from scratch rather than adding ACPI specific stuff in there (even if >>>>>>>> it is just a string to concatenate) >>>>>>> >>>>>>> This works for me. So we could pass "acpi=force" in EFI stub if it >>>>>>> created the DT from scratch *and* ACPI tables are present (can it detect >>>>>>> the latter? And maybe it could print something if none are available). >>>>>>> If that works, the actual kernel can assume that ACPI needs to be >>>>>>> explicitly enabled via acpi=force, irrespective of how much information >>>>>>> it has in DT. >>>>>> >>>>>> Ditto for me. I think this is a fine solution. And, yes, the stub can >>>>>> easily detect the presence of ACPI by looking in the UEFI config table. >>>>> >>>>> I get the point behind doing this, but could we not have it pass in a >>>>> different parameter than =force? Perhaps something new? I'd like to >>>>> separate out the case that it was enabled automatically vs explicitly >>>>> forced on by a user wanting to use ACPI on a system with both tables. >>>> >>>> Ard had a point, so we should probably not pass acpi=force from EFI stub >>>> (especially since a user may explicitly pass acpi=off irrespective of DT >>>> presence). Some other property in the chosen node? It's not even an ABI >>>> since that's a contract between EFI stub and the rest of the kernel, so >>>> an in-kernel only interface. >>> >>> Not strictly true once kexec is in place. Then it becomes a stub -> >>> kernel -> kernel -> kernel -> ... interface, alnog with the rest of the >>> properties the stub puts in the DTB. >>> >>> Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane >>> regardless. >> >> How about the patch (just RFC, maybe it is horrible :) ) below: >> >> When system supporting both DT and ACPI but firmware providing >> no dtb, we can use this linux,uefi-stub-generated-dtb property >> to let kernel know that we can try ACPI configuration data. >> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> >> --- >> Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ >> arch/arm64/kernel/setup.c | 34 >> +++++++++++++++++++++++++++- >> drivers/firmware/efi/libstub/fdt.c | 6 +++++ >> 3 files changed, 58 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/chosen.txt >> b/Documentation/devicetree/bindings/chosen.txt >> index ed838f4..18776b9 100644 >> --- a/Documentation/devicetree/bindings/chosen.txt >> +++ b/Documentation/devicetree/bindings/chosen.txt >> @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property >> "linux,stdout-path" or >> on PowerPC "stdout" if "stdout-path" is not found. However, the >> "linux,stdout-path" and "stdout" properties are deprecated. New platforms >> should only use the "stdout-path" property. >> + >> + >> +linux,uefi-stub-generated-dtb property >> +-------------------------------------- >> + >> +UEFI stub will generate this property in the chosen node to let linux >> kernel >> +know that there is no DTB provided by firmware. >> + >> +There is a use case for system supporting both DT and ACPI, when firmware >> +doesn't provide DT, we can try ACPI configration data to boot the system. > > I don't think we need to list potential use cases here, this can be > useful regardless of UEFI. OK. > > The other UEFI stub properties currently live under > Documentation/arm/uefi.txt. This should live with them. OK, will update in next version. > >> + >> +Usage: >> + >> +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb >> +is generated by uefi stub >> + >> +or >> + >> +linux,uefi-stub-generated-dtb = "false" is the reverse. > > I imagined this would be an empty property. It would only be present if > the stub generated the DTB, and has no value: > > /chosen { > linux,uefi-stub-generated-dtb; > }; > >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c >> index 54e39e3..8268c7b 100644 >> --- a/arch/arm64/kernel/setup.c >> +++ b/arch/arm64/kernel/setup.c >> @@ -371,6 +371,31 @@ static void __init request_standard_resources(void) >> } >> } >> >> +int __init dt_scan_chosen(unsigned long node, const char *uname, >> + int depth, void *data) >> +{ >> + const char *p; >> + >> + if (depth != 1 || !data || >> + (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0)) >> + return 0; > > Do we ever generate chosen@0, and do we even accept that? Sorry, I have limited knowledge about the history of DT, so I think you meant that I just need to check strcmp(uname, "chosen") here, right? > >> + >> + p = of_get_flat_dt_prop(node, "linux,uefi-stub-generated-dtb", NULL); >> + if (!p && !strcmp(p, "true")) >> + *data = true; >> + >> + return 1; >> +} >> + >> +static bool __init is_uefi_stub_generated_dtb(void) >> +{ >> + bool flag = false; >> + >> + of_scan_flat_dt(dt_scan_chosen, &flag); >> + >> + return flag; >> +} >> + >> u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID }; >> >> void __init setup_arch(char **cmdline_p) >> @@ -389,7 +414,14 @@ void __init setup_arch(char **cmdline_p) >> early_fixmap_init(); >> early_ioremap_init(); >> >> - disable_acpi(); >> + /* >> + * If no dtb provided by firmware, enable ACPI >> + * and try to boot with ACPI configuration data >> + */ >> + if (is_uefi_stub_generated_dtb()) >> + enable_acpi(); >> + else >> + disable_acpi(); >> >> parse_early_param(); >> >> diff --git a/drivers/firmware/efi/libstub/fdt.c >> b/drivers/firmware/efi/libstub/fdt.c >> index c846a96..9e2084b 100644 >> --- a/drivers/firmware/efi/libstub/fdt.c >> +++ b/drivers/firmware/efi/libstub/fdt.c >> @@ -154,6 +154,12 @@ efi_status_t update_fdt(efi_system_table_t >> *sys_table, void *orig_fdt, >> if (status) >> goto fdt_set_fail; >> >> + /* Add a property to show the dtb is generated by uefi stub or not */ >> + status = fdt_setprop_string(fdt, node, "linux,uefi-stub-generated-dtb", >> + orig_fdt ? "false" : "true"); >> + if (status) >> + goto fdt_set_fail; >> + > > This should create an empty property, and only when generated by the > stub. OK. could you give me some guidance to use which API to create an empty property? I try to find but failed. Thanks for the review! Hanjun
On Tue, Jan 20, 2015 at 11:10:32AM +0000, Mark Rutland wrote: > > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > > index 54e39e3..8268c7b 100644 > > --- a/arch/arm64/kernel/setup.c > > +++ b/arch/arm64/kernel/setup.c > > @@ -371,6 +371,31 @@ static void __init request_standard_resources(void) > > } > > } > > > > +int __init dt_scan_chosen(unsigned long node, const char *uname, > > + int depth, void *data) > > +{ > > + const char *p; > > + > > + if (depth != 1 || !data || > > + (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0)) > > + return 0; > > Do we ever generate chosen@0, and do we even accept that? This probably originates from some stupid cargo-culting on my side, based on some of the PPC-specific workarounds for old machines remaining in drivers/of/*. It should go. / Leif
On Tue, 20 Jan 2015, Hanjun Guo wrote: > On 2015?01?20? 02:01, Mark Rutland wrote: > > On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote: > > > On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote: > > > > On 01/19/2015 10:13 AM, Grant Likely wrote: > > > > > On Mon, 19 Jan 2015 13:51:45 +0000 > > > > > , Catalin Marinas <catalin.marinas@arm.com> > > > > > wrote: > > > > > > On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote: > > > > > > > On 19 January 2015 at 11:42, Catalin Marinas > > > > > > > <catalin.marinas@arm.com> wrote: > > > > > > > > On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote: > > > > > > > > > From: Al Stone <al.stone@linaro.org> > > > > > > > > > > > > > > > > > > Introduce one early parameters "off" and "force" for "acpi", > > > > > > > > > acpi=off > > > > > > > > > will be the default behavior for ARM64, so introduce > > > > > > > > > acpi=force to > > > > > > > > > enable ACPI on ARM64. > > > > > > > > > > > > > > > > > > Disable ACPI before early parameters parsed, and enable it to > > > > > > > > > pass > > > > > > > > > "acpi=force" if people want use ACPI on ARM64. This ensures DT > > > > > > > > > be > > > > > > > > > the prefer one if ACPI table and DT both are provided at this > > > > > > > > > moment. > > > > > > > > [...] > > > > > > > > > --- a/arch/arm64/kernel/setup.c > > > > > > > > > +++ b/arch/arm64/kernel/setup.c > > > > > > > > > @@ -62,6 +62,7 @@ > > > > > > > > > #include <asm/memblock.h> > > > > > > > > > #include <asm/psci.h> > > > > > > > > > #include <asm/efi.h> > > > > > > > > > +#include <asm/acpi.h> > > > > > > > > > > > > > > > > > > unsigned int processor_id; > > > > > > > > > EXPORT_SYMBOL(processor_id); > > > > > > > > > @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p) > > > > > > > > > early_fixmap_init(); > > > > > > > > > early_ioremap_init(); > > > > > > > > > > > > > > > > > > + disable_acpi(); > > > > > > > > > + > > > > > > > > > parse_early_param(); > > > > > > > > > > > > > > > > > > /* > > > > > > > > > > > > > > > > Did we get to any conclusion here? DT being the preferred one is > > > > > > > > fine > > > > > > > > when both DT and ACPI are present but do we still want the > > > > > > > > kernel to > > > > > > > > ignore ACPI altogether if DT is not present? It's a bit harder > > > > > > > > to detect > > > > > > > > the presence of DT at this point since the EFI_STUB added one > > > > > > > > already. I > > > > > > > > guess we could move the "acpi=force" argument passing to > > > > > > > > EFI_STUB if no > > > > > > > > DT is present at boot. > > > > > > > > > > > > > > Since the EFI stub populates the /chosen node in DT, I would > > > > > > > prefer > > > > > > > for it to add a property there to indicate whether it created the > > > > > > > DT > > > > > > > from scratch rather than adding ACPI specific stuff in there (even > > > > > > > if > > > > > > > it is just a string to concatenate) > > > > > > > > > > > > This works for me. So we could pass "acpi=force" in EFI stub if it > > > > > > created the DT from scratch *and* ACPI tables are present (can it > > > > > > detect > > > > > > the latter? And maybe it could print something if none are > > > > > > available). > > > > > > If that works, the actual kernel can assume that ACPI needs to be > > > > > > explicitly enabled via acpi=force, irrespective of how much > > > > > > information > > > > > > it has in DT. > > > > > > > > > > Ditto for me. I think this is a fine solution. And, yes, the stub can > > > > > easily detect the presence of ACPI by looking in the UEFI config > > > > > table. > > > > > > > > I get the point behind doing this, but could we not have it pass in a > > > > different parameter than =force? Perhaps something new? I'd like to > > > > separate out the case that it was enabled automatically vs explicitly > > > > forced on by a user wanting to use ACPI on a system with both tables. > > > > > > Ard had a point, so we should probably not pass acpi=force from EFI stub > > > (especially since a user may explicitly pass acpi=off irrespective of DT > > > presence). Some other property in the chosen node? It's not even an ABI > > > since that's a contract between EFI stub and the rest of the kernel, so > > > an in-kernel only interface. > > > > Not strictly true once kexec is in place. Then it becomes a stub -> > > kernel -> kernel -> kernel -> ... interface, alnog with the rest of the > > properties the stub puts in the DTB. > > > > Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane > > regardless. > > How about the patch (just RFC, maybe it is horrible :) ) below: > > When system supporting both DT and ACPI but firmware providing > no dtb, we can use this linux,uefi-stub-generated-dtb property > to let kernel know that we can try ACPI configuration data. > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ > arch/arm64/kernel/setup.c | 34 > +++++++++++++++++++++++++++- > drivers/firmware/efi/libstub/fdt.c | 6 +++++ > 3 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/chosen.txt > b/Documentation/devicetree/bindings/chosen.txt > index ed838f4..18776b9 100644 > --- a/Documentation/devicetree/bindings/chosen.txt > +++ b/Documentation/devicetree/bindings/chosen.txt > @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property > "linux,stdout-path" or > on PowerPC "stdout" if "stdout-path" is not found. However, the > "linux,stdout-path" and "stdout" properties are deprecated. New platforms > should only use the "stdout-path" property. > + > + > +linux,uefi-stub-generated-dtb property > +-------------------------------------- > + > +UEFI stub will generate this property in the chosen node to let linux kernel > +know that there is no DTB provided by firmware. > + > +There is a use case for system supporting both DT and ACPI, when firmware > +doesn't provide DT, we can try ACPI configration data to boot the system. > + > +Usage: > + > +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb > +is generated by uefi stub > + > +or > + > +linux,uefi-stub-generated-dtb = "false" is the reverse. I am sorry to have to make the discussion even more complex than already is, however we have one more use case to consider: Linux booting on Xen as Dom0. When booting as Dom0 on ACPI hardware, Linux doesn't have access to the UEFI firmware (no EFI stub). Xen passes a small device tree blob with a chosen node, memory information and a pointer to the ACPI tables. It looks similar to the DTB passed to Linux by the EFI stub but it is generated by Xen instead. See also Christoffer's explanation: http://marc.info/?l=linux-arm-kernel&m=142169977401658&w=2 The actual patches haven't been posted to the list yet, but Linaro seems to have an internal prototype working already. We really need to document this interface properly soon if we expect future Linux and Xen to adhere to it (hint hint). In the Dom0 case Linux should parse the small device tree binary, then enable acpi. Maybe we could generalize the concept of this uefi-stub-generated-dtb variable and call it instead "enable_acpi", optionally exporting the RDSP, like Parth's patches to Xen are doing (I haven't seen the code yet so I don't know the details). Do you think that would work for you?
On 21 January 2015 at 00:50, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Tue, 20 Jan 2015, Hanjun Guo wrote: >> On 2015?01?20? 02:01, Mark Rutland wrote: >> > On Mon, Jan 19, 2015 at 05:52:33PM +0000, Catalin Marinas wrote: >> > > On Mon, Jan 19, 2015 at 04:59:47PM +0000, Jon Masters wrote: >> > > > On 01/19/2015 10:13 AM, Grant Likely wrote: >> > > > > On Mon, 19 Jan 2015 13:51:45 +0000 >> > > > > , Catalin Marinas <catalin.marinas@arm.com> >> > > > > wrote: >> > > > > > On Mon, Jan 19, 2015 at 11:55:32AM +0000, Ard Biesheuvel wrote: >> > > > > > > On 19 January 2015 at 11:42, Catalin Marinas >> > > > > > > <catalin.marinas@arm.com> wrote: >> > > > > > > > On Wed, Jan 14, 2015 at 03:04:52PM +0000, Hanjun Guo wrote: >> > > > > > > > > From: Al Stone <al.stone@linaro.org> >> > > > > > > > > >> > > > > > > > > Introduce one early parameters "off" and "force" for "acpi", >> > > > > > > > > acpi=off >> > > > > > > > > will be the default behavior for ARM64, so introduce >> > > > > > > > > acpi=force to >> > > > > > > > > enable ACPI on ARM64. >> > > > > > > > > >> > > > > > > > > Disable ACPI before early parameters parsed, and enable it to >> > > > > > > > > pass >> > > > > > > > > "acpi=force" if people want use ACPI on ARM64. This ensures DT >> > > > > > > > > be >> > > > > > > > > the prefer one if ACPI table and DT both are provided at this >> > > > > > > > > moment. >> > > > > > > > [...] >> > > > > > > > > --- a/arch/arm64/kernel/setup.c >> > > > > > > > > +++ b/arch/arm64/kernel/setup.c >> > > > > > > > > @@ -62,6 +62,7 @@ >> > > > > > > > > #include <asm/memblock.h> >> > > > > > > > > #include <asm/psci.h> >> > > > > > > > > #include <asm/efi.h> >> > > > > > > > > +#include <asm/acpi.h> >> > > > > > > > > >> > > > > > > > > unsigned int processor_id; >> > > > > > > > > EXPORT_SYMBOL(processor_id); >> > > > > > > > > @@ -388,6 +389,8 @@ void __init setup_arch(char **cmdline_p) >> > > > > > > > > early_fixmap_init(); >> > > > > > > > > early_ioremap_init(); >> > > > > > > > > >> > > > > > > > > + disable_acpi(); >> > > > > > > > > + >> > > > > > > > > parse_early_param(); >> > > > > > > > > >> > > > > > > > > /* >> > > > > > > > >> > > > > > > > Did we get to any conclusion here? DT being the preferred one is >> > > > > > > > fine >> > > > > > > > when both DT and ACPI are present but do we still want the >> > > > > > > > kernel to >> > > > > > > > ignore ACPI altogether if DT is not present? It's a bit harder >> > > > > > > > to detect >> > > > > > > > the presence of DT at this point since the EFI_STUB added one >> > > > > > > > already. I >> > > > > > > > guess we could move the "acpi=force" argument passing to >> > > > > > > > EFI_STUB if no >> > > > > > > > DT is present at boot. >> > > > > > > >> > > > > > > Since the EFI stub populates the /chosen node in DT, I would >> > > > > > > prefer >> > > > > > > for it to add a property there to indicate whether it created the >> > > > > > > DT >> > > > > > > from scratch rather than adding ACPI specific stuff in there (even >> > > > > > > if >> > > > > > > it is just a string to concatenate) >> > > > > > >> > > > > > This works for me. So we could pass "acpi=force" in EFI stub if it >> > > > > > created the DT from scratch *and* ACPI tables are present (can it >> > > > > > detect >> > > > > > the latter? And maybe it could print something if none are >> > > > > > available). >> > > > > > If that works, the actual kernel can assume that ACPI needs to be >> > > > > > explicitly enabled via acpi=force, irrespective of how much >> > > > > > information >> > > > > > it has in DT. >> > > > > >> > > > > Ditto for me. I think this is a fine solution. And, yes, the stub can >> > > > > easily detect the presence of ACPI by looking in the UEFI config >> > > > > table. >> > > > >> > > > I get the point behind doing this, but could we not have it pass in a >> > > > different parameter than =force? Perhaps something new? I'd like to >> > > > separate out the case that it was enabled automatically vs explicitly >> > > > forced on by a user wanting to use ACPI on a system with both tables. >> > > >> > > Ard had a point, so we should probably not pass acpi=force from EFI stub >> > > (especially since a user may explicitly pass acpi=off irrespective of DT >> > > presence). Some other property in the chosen node? It's not even an ABI >> > > since that's a contract between EFI stub and the rest of the kernel, so >> > > an in-kernel only interface. >> > >> > Not strictly true once kexec is in place. Then it becomes a stub -> >> > kernel -> kernel -> kernel -> ... interface, alnog with the rest of the >> > properties the stub puts in the DTB. >> > >> > Having something like /chosen/linux,uefi-stub-generated-dtb sounds sane >> > regardless. >> >> How about the patch (just RFC, maybe it is horrible :) ) below: >> >> When system supporting both DT and ACPI but firmware providing >> no dtb, we can use this linux,uefi-stub-generated-dtb property >> to let kernel know that we can try ACPI configuration data. >> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> >> --- >> Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ >> arch/arm64/kernel/setup.c | 34 >> +++++++++++++++++++++++++++- >> drivers/firmware/efi/libstub/fdt.c | 6 +++++ >> 3 files changed, 58 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/chosen.txt >> b/Documentation/devicetree/bindings/chosen.txt >> index ed838f4..18776b9 100644 >> --- a/Documentation/devicetree/bindings/chosen.txt >> +++ b/Documentation/devicetree/bindings/chosen.txt >> @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property >> "linux,stdout-path" or >> on PowerPC "stdout" if "stdout-path" is not found. However, the >> "linux,stdout-path" and "stdout" properties are deprecated. New platforms >> should only use the "stdout-path" property. >> + >> + >> +linux,uefi-stub-generated-dtb property >> +-------------------------------------- >> + >> +UEFI stub will generate this property in the chosen node to let linux kernel >> +know that there is no DTB provided by firmware. >> + >> +There is a use case for system supporting both DT and ACPI, when firmware >> +doesn't provide DT, we can try ACPI configration data to boot the system. >> + >> +Usage: >> + >> +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb >> +is generated by uefi stub >> + >> +or >> + >> +linux,uefi-stub-generated-dtb = "false" is the reverse. > > I am sorry to have to make the discussion even more complex than already > is, however we have one more use case to consider: Linux booting on Xen > as Dom0. > > When booting as Dom0 on ACPI hardware, Linux doesn't have access to the > UEFI firmware (no EFI stub). Xen passes a small device tree blob with > a chosen node, memory information and a pointer to the ACPI tables. > It looks similar to the DTB passed to Linux by the EFI stub but it is > generated by Xen instead. > > See also Christoffer's explanation: > > http://marc.info/?l=linux-arm-kernel&m=142169977401658&w=2 > > The actual patches haven't been posted to the list yet, but Linaro seems > to have an internal prototype working already. We really need to > document this interface properly soon if we expect future Linux and Xen > to adhere to it (hint hint). > > In the Dom0 case Linux should parse the small device tree binary, then > enable acpi. > > Maybe we could generalize the concept of this uefi-stub-generated-dtb > variable and call it instead "enable_acpi", optionally exporting the > RDSP, like Parth's patches to Xen are doing (I haven't seen the code yet > so I don't know the details). > > Do you think that would work for you? Here is the actual patch for booting linux on xen without uefi, https://git.linaro.org/people/parth.dixit/xen-acpi-support/leg-kernel.git/commitdiff/0b9ca47a801d6f63e6fbf1885962b7d6b9910d1d?hp=380ad5d8817a03204e6a92d0ad1450ec32886d80 it is working and we are able to boot to linux from xen. We are doing this by passing RSDP pointer in skeleton device tree to linux and reading it as part of early boot initialization. As explained by stefano and christoffer we are not exposing UEFI so the EFI stub based approach will not work for us. Also we need to describe the memory in device tree as ACPI does not contain that information. Is it possible that we don't tie up EFI with ACPI ?
On Tue, Jan 20, 2015 at 07:20:06PM +0000, Stefano Stabellini wrote: > On Tue, 20 Jan 2015, Hanjun Guo wrote: > > How about the patch (just RFC, maybe it is horrible :) ) below: > > > > When system supporting both DT and ACPI but firmware providing > > no dtb, we can use this linux,uefi-stub-generated-dtb property > > to let kernel know that we can try ACPI configuration data. > > > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > > --- > > Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ > > arch/arm64/kernel/setup.c | 34 > > +++++++++++++++++++++++++++- > > drivers/firmware/efi/libstub/fdt.c | 6 +++++ > > 3 files changed, 58 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/chosen.txt > > b/Documentation/devicetree/bindings/chosen.txt > > index ed838f4..18776b9 100644 > > --- a/Documentation/devicetree/bindings/chosen.txt > > +++ b/Documentation/devicetree/bindings/chosen.txt > > @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property > > "linux,stdout-path" or > > on PowerPC "stdout" if "stdout-path" is not found. However, the > > "linux,stdout-path" and "stdout" properties are deprecated. New platforms > > should only use the "stdout-path" property. > > + > > + > > +linux,uefi-stub-generated-dtb property > > +-------------------------------------- > > + > > +UEFI stub will generate this property in the chosen node to let linux kernel > > +know that there is no DTB provided by firmware. > > + > > +There is a use case for system supporting both DT and ACPI, when firmware > > +doesn't provide DT, we can try ACPI configration data to boot the system. > > + > > +Usage: > > + > > +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb > > +is generated by uefi stub > > + > > +or > > + > > +linux,uefi-stub-generated-dtb = "false" is the reverse. > > I am sorry to have to make the discussion even more complex than already > is, however we have one more use case to consider: Linux booting on Xen > as Dom0. > > When booting as Dom0 on ACPI hardware, Linux doesn't have access to the > UEFI firmware (no EFI stub). Xen passes a small device tree blob with > a chosen node, memory information and a pointer to the ACPI tables. > It looks similar to the DTB passed to Linux by the EFI stub but it is > generated by Xen instead. We have two (or even three) different use cases here. One of them is a way to tell the kernel that there is no point in trying DT since it has been generated by the EFI stub and it doesn't have any SoC information. The kernel can bet on ACPI tables being present or just fail to boot. What I want to avoid is "acpi=force" parameter being part of the kernel/firmware ABI for using ACPI (whether DT is present or not). We could call this something else ("linux,bare-dtb"?) if we want to use it in other situations but not "enable_acpi" as we don't want to move the ACPI enabling policy in the EFI stub or boot loader (or whatever generates the chosen node). The Xen case is close to the kexec one. For the latter (I haven't looked at the current arm64 patches yet), I can see that you can pass an "acpi_rsdp" argument to the kernel with the ACPI RSDP address. I don't think that's ideal, we better have some defined DT bindings for such information in the absence of EFI. But it basically means that we'll allow ACPI on arm64 without EFI. I have some questions for the ACPI and EFI folk: 1. When booting with ACPI, are the EFI run-time services required for anything? If yes, Xen may have a bigger problem 2. Could a boot loader (either kernel doing kexec or Xen) emulate the EFI system/config tables and still make them useful to the kernel but without EFI_BOOT or EFI_RUNTIME_SERVICES? This way Xen could emulate the EFI stub generated DT but without additional bindings for the RSDB address
On 01/21/2015 10:23 AM, Catalin Marinas wrote: > I have some questions for the ACPI and EFI folk: > > 1. When booting with ACPI, are the EFI run-time services required for > anything? If yes, Xen may have a bigger problem Yes. At least for some things. For example, installing an Operating System would require that you make runtime services calls to set the BootOrder/BootNext variables, and so on. Further, we use the GetTime service and EFI based reboot to avoid having special drivers. I had those added to SBBR as requirements for that reason. > 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > EFI system/config tables and still make them useful to the kernel but > without EFI_BOOT or EFI_RUNTIME_SERVICES? Yes. But again, without the other required pieces (including the services function pointers in the systab which are required) you'd crash soon after boot trying to make those calls. We saw this with e.g. RTC in early firmwares. One of the reasons we've been hitting every service and establishing a reliance upon them immediately is to allow third party EFI vendors to notice when they've got any firmware bugs. Jon.
On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: > On 01/21/2015 10:23 AM, Catalin Marinas wrote: > > I have some questions for the ACPI and EFI folk: > > > > 1. When booting with ACPI, are the EFI run-time services required for > > anything? If yes, Xen may have a bigger problem > > Yes. At least for some things. For example, installing an Operating > System would require that you make runtime services calls to set the > BootOrder/BootNext variables, and so on. Further, we use the GetTime > service and EFI based reboot to avoid having special drivers. I had > those added to SBBR as requirements for that reason. So what would a kexec'ed kernel do here? Or we usually expect it to be short lived and doesn't need reboot, nor GetTime. Xen is slightly more problematic but I wonder whether it could run a (paravirtualised) UEFI. > > 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > > EFI system/config tables and still make them useful to the kernel but > > without EFI_BOOT or EFI_RUNTIME_SERVICES? > > Yes. But again, without the other required pieces (including the > services function pointers in the systab which are required) you'd crash > soon after boot trying to make those calls. My point was whether you can still pass information like RSDP address via EFI tables but explicitly disable runtime services so that the kernel won't try to make such calls (and crash).
On Wed, Jan 21, 2015 at 03:42:43PM +0000, Catalin Marinas wrote: > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: > > On 01/21/2015 10:23 AM, Catalin Marinas wrote: > > > I have some questions for the ACPI and EFI folk: > > > > > > 1. When booting with ACPI, are the EFI run-time services required for > > > anything? If yes, Xen may have a bigger problem > > > > Yes. At least for some things. For example, installing an Operating > > System would require that you make runtime services calls to set the > > BootOrder/BootNext variables, and so on. Further, we use the GetTime > > service and EFI based reboot to avoid having special drivers. I had > > those added to SBBR as requirements for that reason. > > So what would a kexec'ed kernel do here? Or we usually expect it to be > short lived and doesn't need reboot, nor GetTime. > > Xen is slightly more problematic but I wonder whether it could run a > (paravirtualised) UEFI. > > > > 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > > > EFI system/config tables and still make them useful to the kernel but > > > without EFI_BOOT or EFI_RUNTIME_SERVICES? > > > > Yes. But again, without the other required pieces (including the > > services function pointers in the systab which are required) you'd crash > > soon after boot trying to make those calls. > > My point was whether you can still pass information like RSDP address > via EFI tables but explicitly disable runtime services so that the > kernel won't try to make such calls (and crash). > There is no specific dependency from ACPI->EFI its just the only current method defind to get the RSDP pointer. It would work just as well getting the pointer from /chosen/ if we just pick a node and document it for Xen/kexec/other usage. We were running ACPI on machine from u-boot doing exactly this for a long time. Graeme
On 01/21/2015 10:42 AM, Catalin Marinas wrote: > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: >> On 01/21/2015 10:23 AM, Catalin Marinas wrote: >>> I have some questions for the ACPI and EFI folk: >>> >>> 1. When booting with ACPI, are the EFI run-time services required for >>> anything? If yes, Xen may have a bigger problem >> >> Yes. At least for some things. For example, installing an Operating >> System would require that you make runtime services calls to set the >> BootOrder/BootNext variables, and so on. Further, we use the GetTime >> service and EFI based reboot to avoid having special drivers. I had >> those added to SBBR as requirements for that reason. > > So what would a kexec'ed kernel do here? Or we usually expect it to be > short lived and doesn't need reboot, nor GetTime. In the use case that I have, it'll use EFI Runtime Servies to handle both the time of day (which it will need) and to subsequently reboot. This is currently being worked on (integration into kdump). > Xen is slightly more problematic but I wonder whether it could run a > (paravirtualised) UEFI. > >>> 2. Could a boot loader (either kernel doing kexec or Xen) emulate the >>> EFI system/config tables and still make them useful to the kernel but >>> without EFI_BOOT or EFI_RUNTIME_SERVICES? >> >> Yes. But again, without the other required pieces (including the >> services function pointers in the systab which are required) you'd crash >> soon after boot trying to make those calls. > > My point was whether you can still pass information like RSDP address > via EFI tables but explicitly disable runtime services so that the > kernel won't try to make such calls (and crash). Yes. As Graeme says, it works just to pass in the ACPI information and turn off EFI *BUT* it does not work to say you have EFI and then not provide the correct EFI services. To do so is out of spec, and in fact it's one reason we weren't able to turn the GetTime service on generally for x86 - some older x86 boxes didn't implement it originally (another reason on our end we're requiring all of these services on day one so that there won't be time for someone to miss them in firmware). Jon.
On Wed, 21 Jan 2015, Catalin Marinas wrote: > On Tue, Jan 20, 2015 at 07:20:06PM +0000, Stefano Stabellini wrote: > > On Tue, 20 Jan 2015, Hanjun Guo wrote: > > > How about the patch (just RFC, maybe it is horrible :) ) below: > > > > > > When system supporting both DT and ACPI but firmware providing > > > no dtb, we can use this linux,uefi-stub-generated-dtb property > > > to let kernel know that we can try ACPI configuration data. > > > > > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > > > --- > > > Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ > > > arch/arm64/kernel/setup.c | 34 > > > +++++++++++++++++++++++++++- > > > drivers/firmware/efi/libstub/fdt.c | 6 +++++ > > > 3 files changed, 58 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/chosen.txt > > > b/Documentation/devicetree/bindings/chosen.txt > > > index ed838f4..18776b9 100644 > > > --- a/Documentation/devicetree/bindings/chosen.txt > > > +++ b/Documentation/devicetree/bindings/chosen.txt > > > @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property > > > "linux,stdout-path" or > > > on PowerPC "stdout" if "stdout-path" is not found. However, the > > > "linux,stdout-path" and "stdout" properties are deprecated. New platforms > > > should only use the "stdout-path" property. > > > + > > > + > > > +linux,uefi-stub-generated-dtb property > > > +-------------------------------------- > > > + > > > +UEFI stub will generate this property in the chosen node to let linux kernel > > > +know that there is no DTB provided by firmware. > > > + > > > +There is a use case for system supporting both DT and ACPI, when firmware > > > +doesn't provide DT, we can try ACPI configration data to boot the system. > > > + > > > +Usage: > > > + > > > +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb > > > +is generated by uefi stub > > > + > > > +or > > > + > > > +linux,uefi-stub-generated-dtb = "false" is the reverse. > > > > I am sorry to have to make the discussion even more complex than already > > is, however we have one more use case to consider: Linux booting on Xen > > as Dom0. > > > > When booting as Dom0 on ACPI hardware, Linux doesn't have access to the > > UEFI firmware (no EFI stub). Xen passes a small device tree blob with > > a chosen node, memory information and a pointer to the ACPI tables. > > It looks similar to the DTB passed to Linux by the EFI stub but it is > > generated by Xen instead. > > We have two (or even three) different use cases here. One of them is a > way to tell the kernel that there is no point in trying DT since it has > been generated by the EFI stub and it doesn't have any SoC information. > The kernel can bet on ACPI tables being present or just fail to boot. > What I want to avoid is "acpi=force" parameter being part of the > kernel/firmware ABI for using ACPI (whether DT is present or not). > > We could call this something else ("linux,bare-dtb"?) if we want to use > it in other situations but not "enable_acpi" as we don't want to move > the ACPI enabling policy in the EFI stub or boot loader (or whatever > generates the chosen node). > > The Xen case is close to the kexec one. For the latter (I haven't looked > at the current arm64 patches yet), I can see that you can pass an > "acpi_rsdp" argument to the kernel with the ACPI RSDP address. I don't > think that's ideal, we better have some defined DT bindings for such > information in the absence of EFI. But it basically means that we'll > allow ACPI on arm64 without EFI. Good, I agree. On Wed, 21 Jan 2015, Catalin Marinas wrote: > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: > > On 01/21/2015 10:23 AM, Catalin Marinas wrote: > > > I have some questions for the ACPI and EFI folk: > > > > > > 1. When booting with ACPI, are the EFI run-time services required for > > > anything? If yes, Xen may have a bigger problem > > > > Yes. At least for some things. For example, installing an Operating > > System would require that you make runtime services calls to set the > > BootOrder/BootNext variables, and so on. Further, we use the GetTime > > service and EFI based reboot to avoid having special drivers. I had > > those added to SBBR as requirements for that reason. > > So what would a kexec'ed kernel do here? Or we usually expect it to be > short lived and doesn't need reboot, nor GetTime. > > Xen is slightly more problematic but I wonder whether it could run a > (paravirtualised) UEFI. As a matter of fact that's how it is done on x86: Xen exports a paravirtualized UEFI run-time services interface (drivers/xen/efi.c). Daniel Kiper (CC'ed) wrote the code that went upstream last August. There is no reason why we could not do the same on ARM. > > > 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > > > EFI system/config tables and still make them useful to the kernel but > > > without EFI_BOOT or EFI_RUNTIME_SERVICES? > > > > Yes. But again, without the other required pieces (including the > > services function pointers in the systab which are required) you'd crash > > soon after boot trying to make those calls. > > My point was whether you can still pass information like RSDP address > via EFI tables but explicitly disable runtime services so that the > kernel won't try to make such calls (and crash). I don't think that run-time services are going to be a problem for Xen, but I agree that it would be nice not to depend on them to have ACPI.
On Wed, Jan 21, 2015 at 04:05:33PM +0000, Jon Masters wrote: > On 01/21/2015 10:42 AM, Catalin Marinas wrote: > > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: > >> On 01/21/2015 10:23 AM, Catalin Marinas wrote: > >>> I have some questions for the ACPI and EFI folk: > >>> > >>> 1. When booting with ACPI, are the EFI run-time services required for > >>> anything? If yes, Xen may have a bigger problem > >> > >> Yes. At least for some things. For example, installing an Operating > >> System would require that you make runtime services calls to set the > >> BootOrder/BootNext variables, and so on. Further, we use the GetTime > >> service and EFI based reboot to avoid having special drivers. I had > >> those added to SBBR as requirements for that reason. > > > > So what would a kexec'ed kernel do here? Or we usually expect it to be > > short lived and doesn't need reboot, nor GetTime. > > In the use case that I have, it'll use EFI Runtime Servies to handle > both the time of day (which it will need) and to subsequently reboot. > This is currently being worked on (integration into kdump). So the EFI run-time services (and EFI tables) will be preserved across kexec? Could Xen not to something similar? > >>> 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > >>> EFI system/config tables and still make them useful to the kernel but > >>> without EFI_BOOT or EFI_RUNTIME_SERVICES? > >> > >> Yes. But again, without the other required pieces (including the > >> services function pointers in the systab which are required) you'd crash > >> soon after boot trying to make those calls. > > > > My point was whether you can still pass information like RSDP address > > via EFI tables but explicitly disable runtime services so that the > > kernel won't try to make such calls (and crash). > > Yes. As Graeme says, it works just to pass in the ACPI information and > turn off EFI *BUT* it does not work to say you have EFI and then not > provide the correct EFI services. To do so is out of spec, and in fact > it's one reason we weren't able to turn the GetTime service on generally > for x86 - some older x86 boxes didn't implement it originally (another > reason on our end we're requiring all of these services on day one so > that there won't be time for someone to miss them in firmware). OK, thanks for confirming this. So the answer to my second question is "not really".
On 21 January 2015 at 21:46, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Wed, Jan 21, 2015 at 04:05:33PM +0000, Jon Masters wrote: >> On 01/21/2015 10:42 AM, Catalin Marinas wrote: >> > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: >> >> On 01/21/2015 10:23 AM, Catalin Marinas wrote: >> >>> I have some questions for the ACPI and EFI folk: >> >>> >> >>> 1. When booting with ACPI, are the EFI run-time services required for >> >>> anything? If yes, Xen may have a bigger problem >> >> >> >> Yes. At least for some things. For example, installing an Operating >> >> System would require that you make runtime services calls to set the >> >> BootOrder/BootNext variables, and so on. Further, we use the GetTime >> >> service and EFI based reboot to avoid having special drivers. I had >> >> those added to SBBR as requirements for that reason. >> > >> > So what would a kexec'ed kernel do here? Or we usually expect it to be >> > short lived and doesn't need reboot, nor GetTime. >> >> In the use case that I have, it'll use EFI Runtime Servies to handle >> both the time of day (which it will need) and to subsequently reboot. >> This is currently being worked on (integration into kdump). > > So the EFI run-time services (and EFI tables) will be preserved across > kexec? Could Xen not to something similar? > >> >>> 2. Could a boot loader (either kernel doing kexec or Xen) emulate the >> >>> EFI system/config tables and still make them useful to the kernel but >> >>> without EFI_BOOT or EFI_RUNTIME_SERVICES? >> >> >> >> Yes. But again, without the other required pieces (including the >> >> services function pointers in the systab which are required) you'd crash >> >> soon after boot trying to make those calls. >> > >> > My point was whether you can still pass information like RSDP address >> > via EFI tables but explicitly disable runtime services so that the >> > kernel won't try to make such calls (and crash). >> >> Yes. As Graeme says, it works just to pass in the ACPI information and >> turn off EFI *BUT* it does not work to say you have EFI and then not >> provide the correct EFI services. To do so is out of spec, and in fact >> it's one reason we weren't able to turn the GetTime service on generally >> for x86 - some older x86 boxes didn't implement it originally (another >> reason on our end we're requiring all of these services on day one so >> that there won't be time for someone to miss them in firmware). This is the use case i am talking about, we have a wroking setup with efi disabled and rsdp passed via dtb, right now its done by adding a "rsdp" field in the chosen node. Do we have a formal way to pass RSDP without EFI? if not, it would be good to have dtb binding which we can use to pass RSDP address to kernel for ACPI. > OK, thanks for confirming this. So the answer to my second question is > "not really". > > -- > Catalin
On Wed, Jan 21, 2015 at 04:10:00PM +0000, Stefano Stabellini wrote: > On Wed, 21 Jan 2015, Catalin Marinas wrote: > > On Tue, Jan 20, 2015 at 07:20:06PM +0000, Stefano Stabellini wrote: > > > On Tue, 20 Jan 2015, Hanjun Guo wrote: > > > > How about the patch (just RFC, maybe it is horrible :) ) below: > > > > > > > > When system supporting both DT and ACPI but firmware providing > > > > no dtb, we can use this linux,uefi-stub-generated-dtb property > > > > to let kernel know that we can try ACPI configuration data. > > > > > > > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > > > > --- > > > > Documentation/devicetree/bindings/chosen.txt | 19 ++++++++++++++++ > > > > arch/arm64/kernel/setup.c | 34 > > > > +++++++++++++++++++++++++++- > > > > drivers/firmware/efi/libstub/fdt.c | 6 +++++ > > > > 3 files changed, 58 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/chosen.txt > > > > b/Documentation/devicetree/bindings/chosen.txt > > > > index ed838f4..18776b9 100644 > > > > --- a/Documentation/devicetree/bindings/chosen.txt > > > > +++ b/Documentation/devicetree/bindings/chosen.txt > > > > @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property > > > > "linux,stdout-path" or > > > > on PowerPC "stdout" if "stdout-path" is not found. However, the > > > > "linux,stdout-path" and "stdout" properties are deprecated. New platforms > > > > should only use the "stdout-path" property. > > > > + > > > > + > > > > +linux,uefi-stub-generated-dtb property > > > > +-------------------------------------- > > > > + > > > > +UEFI stub will generate this property in the chosen node to let linux kernel > > > > +know that there is no DTB provided by firmware. > > > > + > > > > +There is a use case for system supporting both DT and ACPI, when firmware > > > > +doesn't provide DT, we can try ACPI configration data to boot the system. > > > > + > > > > +Usage: > > > > + > > > > +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb > > > > +is generated by uefi stub > > > > + > > > > +or > > > > + > > > > +linux,uefi-stub-generated-dtb = "false" is the reverse. > > > > > > I am sorry to have to make the discussion even more complex than already > > > is, however we have one more use case to consider: Linux booting on Xen > > > as Dom0. > > > > > > When booting as Dom0 on ACPI hardware, Linux doesn't have access to the > > > UEFI firmware (no EFI stub). Xen passes a small device tree blob with > > > a chosen node, memory information and a pointer to the ACPI tables. > > > It looks similar to the DTB passed to Linux by the EFI stub but it is > > > generated by Xen instead. > > > > We have two (or even three) different use cases here. One of them is a > > way to tell the kernel that there is no point in trying DT since it has > > been generated by the EFI stub and it doesn't have any SoC information. > > The kernel can bet on ACPI tables being present or just fail to boot. > > What I want to avoid is "acpi=force" parameter being part of the > > kernel/firmware ABI for using ACPI (whether DT is present or not). > > > > We could call this something else ("linux,bare-dtb"?) if we want to use > > it in other situations but not "enable_acpi" as we don't want to move > > the ACPI enabling policy in the EFI stub or boot loader (or whatever > > generates the chosen node). > > > > The Xen case is close to the kexec one. For the latter (I haven't looked > > at the current arm64 patches yet), I can see that you can pass an > > "acpi_rsdp" argument to the kernel with the ACPI RSDP address. I don't > > think that's ideal, we better have some defined DT bindings for such > > information in the absence of EFI. But it basically means that we'll > > allow ACPI on arm64 without EFI. > > Good, I agree. > > > On Wed, 21 Jan 2015, Catalin Marinas wrote: > > On Wed, Jan 21, 2015 at 03:29:52PM +0000, Jon Masters wrote: > > > On 01/21/2015 10:23 AM, Catalin Marinas wrote: > > > > I have some questions for the ACPI and EFI folk: > > > > > > > > 1. When booting with ACPI, are the EFI run-time services required for > > > > anything? If yes, Xen may have a bigger problem > > > > > > Yes. At least for some things. For example, installing an Operating > > > System would require that you make runtime services calls to set the > > > BootOrder/BootNext variables, and so on. Further, we use the GetTime > > > service and EFI based reboot to avoid having special drivers. I had > > > those added to SBBR as requirements for that reason. > > > > So what would a kexec'ed kernel do here? Or we usually expect it to be > > short lived and doesn't need reboot, nor GetTime. > > > > Xen is slightly more problematic but I wonder whether it could run a > > (paravirtualised) UEFI. > > As a matter of fact that's how it is done on x86: Xen exports a > paravirtualized UEFI run-time services interface (drivers/xen/efi.c). > Daniel Kiper (CC'ed) wrote the code that went upstream last August. > There is no reason why we could not do the same on ARM. I have done it with ARM in my mind. So, I think that you can reuse that code fairly easy. Just take look at drivers/xen/efi.c (I think that this should work on ARM without major changes) and assume arch/x86/xen/efi.c and arch/x86/xen/enlighten.c as an example of starting point. > > > > 2. Could a boot loader (either kernel doing kexec or Xen) emulate the > > > > EFI system/config tables and still make them useful to the kernel but > > > > without EFI_BOOT or EFI_RUNTIME_SERVICES? > > > > > > Yes. But again, without the other required pieces (including the > > > services function pointers in the systab which are required) you'd crash > > > soon after boot trying to make those calls. > > > > My point was whether you can still pass information like RSDP address > > via EFI tables but explicitly disable runtime services so that the > > kernel won't try to make such calls (and crash). > > I don't think that run-time services are going to be a problem for Xen, > but I agree that it would be nice not to depend on them to have ACPI. IIRC, there are relevant options in Linux Kernel and Xen to disable runtime services, however, I have not tested them. Daniel
diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt index ed838f4..18776b9 100644 --- a/Documentation/devicetree/bindings/chosen.txt +++ b/Documentation/devicetree/bindings/chosen.txt @@ -44,3 +44,22 @@ Implementation note: Linux will look for the property "linux,stdout-path" or on PowerPC "stdout" if "stdout-path" is not found. However, the "linux,stdout-path" and "stdout" properties are deprecated. New platforms should only use the "stdout-path" property. + + +linux,uefi-stub-generated-dtb property +-------------------------------------- + +UEFI stub will generate this property in the chosen node to let linux kernel +know that there is no DTB provided by firmware. + +There is a use case for system supporting both DT and ACPI, when firmware +doesn't provide DT, we can try ACPI configration data to boot the system. + +Usage: + +linux,uefi-stub-generated-dtb = "true" means that it is true that the dtb +is generated by uefi stub + +or + +linux,uefi-stub-generated-dtb = "false" is the reverse. diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 54e39e3..8268c7b 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -371,6 +371,31 @@ static void __init request_standard_resources(void) } } +int __init dt_scan_chosen(unsigned long node, const char *uname, + int depth, void *data) +{ + const char *p; + + if (depth != 1 || !data || + (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0)) + return 0; + + p = of_get_flat_dt_prop(node, "linux,uefi-stub-generated-dtb", NULL); + if (!p && !strcmp(p, "true")) + *data = true; + + return 1; +} + +static bool __init is_uefi_stub_generated_dtb(void) +{ + bool flag = false; + + of_scan_flat_dt(dt_scan_chosen, &flag); + + return flag; +} + u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID }; void __init setup_arch(char **cmdline_p) @@ -389,7 +414,14 @@ void __init setup_arch(char **cmdline_p) early_fixmap_init(); early_ioremap_init(); - disable_acpi(); + /* + * If no dtb provided by firmware, enable ACPI + * and try to boot with ACPI configuration data + */ + if (is_uefi_stub_generated_dtb()) + enable_acpi(); + else + disable_acpi(); parse_early_param(); diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c index c846a96..9e2084b 100644 --- a/drivers/firmware/efi/libstub/fdt.c +++ b/drivers/firmware/efi/libstub/fdt.c @@ -154,6 +154,12 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, if (status) goto fdt_set_fail; + /* Add a property to show the dtb is generated by uefi stub or not */ + status = fdt_setprop_string(fdt, node, "linux,uefi-stub-generated-dtb", + orig_fdt ? "false" : "true"); + if (status) + goto fdt_set_fail; + return EFI_SUCCESS;