| Message ID | 1421247905-3749-5-git-send-email-hanjun.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/14/2015 09:04 AM, 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. > > Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > Tested-by: Yijing Wang <wangyijing@huawei.com> > Signed-off-by: Al Stone <al.stone@linaro.org> > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
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.
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)
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.
On 19 January 2015 at 13:51, 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. > Erm, that is not at all what I meant. What I meant was, that if it is interesting to the kernel proper to know whether the DT was created from scratch by the stub rather than received from the firmware, we can record that particular fact in the /chosen node, and nothing else. How this is interpreted is up to the kernel proper entirely. Note that the stub may outlive many subsequent kexec reboots, so dividing policy like this across the stub/kernel boundary is asking for trouble imo. For instance, booting with a DT via kexec would be impossible unless we add special handling for this case, which is exactly what I tried to avoid with my latest virtmap series.
On Mon, Jan 19, 2015 at 02:00:24PM +0000, Ard Biesheuvel wrote: > On 19 January 2015 at 13:51, 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. > > Erm, that is not at all what I meant. What I meant was, that if it is > interesting to the kernel proper to know whether the DT was created > from scratch by the stub rather than received from the firmware, we > can record that particular fact in the /chosen node, and nothing else. > How this is interpreted is up to the kernel proper entirely. That works as well and I agree with you that splitting the decision between EFI stub and the kernel could cause trouble. Basically what we need is to know whether DT has platform description or just the chosen node. There could be many ways to achieve this but EFI stub passing such information is a good place (and acpi=force went a bit too far ;)). > Note that the stub may outlive many subsequent kexec reboots, so > dividing policy like this across the stub/kernel boundary is asking > for trouble imo. For instance, booting with a DT via kexec would be > impossible unless we add special handling for this case, which is > exactly what I tried to avoid with my latest virtmap series. I haven't paid much attention to the kexec series yet but I assume a kexec'ed kernel doesn't go through EFI stub again.
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. g.
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. Jon.
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.
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. Mark.
On Wed, Jan 14, 2015 at 9:04 AM, Hanjun Guo <hanjun.guo@linaro.org> 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. What is the reason to assume that DT is preferred over ACPI? I would have thought that if ACPI is present, then it means we're on an ARM64 server platform, and therefore it should be used. It seems silly to require acpi=force on every ARM64 server platform.
On Wed, Jan 28, 2015 at 05:58:54PM +0000, Timur Tabi wrote: > On Wed, Jan 14, 2015 at 9:04 AM, Hanjun Guo <hanjun.guo@linaro.org> 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. > > What is the reason to assume that DT is preferred over ACPI? I would > have thought that if ACPI is present, then it means we're on an ARM64 > server platform, and therefore it should be used. It seems silly to > require acpi=force on every ARM64 server platform. I'm against requiring acpi=force when *only* ACPI tables are present (I don't like a command line argument to become firmware-kernel ABI), but otherwise DT takes precedence (it was the first supported booting method on arm64 and currently it is more mature and feature-rich than ACPI on arm64).
On Wed, Jan 28, 2015 at 11:58 AM, Timur Tabi <timur@codeaurora.org> wrote: > What is the reason to assume that DT is preferred over ACPI? I would > have thought that if ACPI is present, then it means we're on an ARM64 > server platform, and therefore it should be used. It seems silly to > require acpi=force on every ARM64 server platform. So it looks like there's a whole conversation about this already in this thread that I didn't notice. However, reading through all of it, I still don't understand sure why the presence of ACPI tables is insufficient to enable ACPI. In what situation would we want to ignore ACPI tables that are present? -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On Wed, Jan 28, 2015 at 06:08:24PM +0000, Timur Tabi wrote: > On Wed, Jan 28, 2015 at 11:58 AM, Timur Tabi <timur@codeaurora.org> wrote: > > What is the reason to assume that DT is preferred over ACPI? I would > > have thought that if ACPI is present, then it means we're on an ARM64 > > server platform, and therefore it should be used. It seems silly to > > require acpi=force on every ARM64 server platform. > > So it looks like there's a whole conversation about this already in > this thread that I didn't notice. However, reading through all of it, > I still don't understand sure why the presence of ACPI tables is > insufficient to enable ACPI. Because ACPI on arm64 is still experimental, no matter how many people claim that it is production ready in their private setups. > In what situation would we want to ignore ACPI tables that are > present? When DT tables are also present (and for the first platforms, that's highly recommended, though not easily enforceable at the kernel level).
On 01/28/2015 12:14 PM, Catalin Marinas wrote: >> >So it looks like there's a whole conversation about this already in >> >this thread that I didn't notice. However, reading through all of it, >> >I still don't understand sure why the presence of ACPI tables is >> >insufficient to enable ACPI. > Because ACPI on arm64 is still experimental, no matter how many people > claim that it is production ready in their private setups. Fair enough. Does this mean that passing "acpi=force" on the kernel command line is a requirement for ARM64 servers? >> >In what situation would we want to ignore ACPI tables that are >> >present? > When DT tables are also present (and for the first platforms, that's > highly recommended, though not easily enforceable at the kernel level). My understanding is that the EFI stub creates a device tree (and it contains some important information), so I don't understand how we can ever have an ACPI-only platform on ARM64 servers.
On Wed, Jan 28, 2015 at 06:18:44PM +0000, Timur Tabi wrote: > On 01/28/2015 12:14 PM, Catalin Marinas wrote: > >> >So it looks like there's a whole conversation about this already in > >> >this thread that I didn't notice. However, reading through all of it, > >> >I still don't understand sure why the presence of ACPI tables is > >> >insufficient to enable ACPI. > > > Because ACPI on arm64 is still experimental, no matter how many people > > claim that it is production ready in their private setups. > > Fair enough. Does this mean that passing "acpi=force" on the kernel > command line is a requirement for ARM64 servers? Please read my other email and ideally the whole sub-thread. The acpi=force should only be required if the SoC is described (from firmware) by both DT and ACPI. > >> >In what situation would we want to ignore ACPI tables that are > >> >present? > > > When DT tables are also present (and for the first platforms, that's > > highly recommended, though not easily enforceable at the kernel level). > > My understanding is that the EFI stub creates a device tree (and it > contains some important information), so I don't understand how we can > ever have an ACPI-only platform on ARM64 servers. If EFI stub creates the DT itself (not passed by firmware), it will write some information in the chosen node that the rest of the kernel can make use of and take the appropriate decision on whether to use ACPI or not. That's something that will be used by kexec and Xen as well which may want to boot with ACPI tables outside an EFI environment.
On 29 January 2015 at 15:19, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Wed, Jan 28, 2015 at 06:18:44PM +0000, Timur Tabi wrote: >> On 01/28/2015 12:14 PM, Catalin Marinas wrote: >> >> >So it looks like there's a whole conversation about this already in >> >> >this thread that I didn't notice. However, reading through all of it, >> >> >I still don't understand sure why the presence of ACPI tables is >> >> >insufficient to enable ACPI. >> >> > Because ACPI on arm64 is still experimental, no matter how many people >> > claim that it is production ready in their private setups. >> >> Fair enough. Does this mean that passing "acpi=force" on the kernel >> command line is a requirement for ARM64 servers? > > Please read my other email and ideally the whole sub-thread. The > acpi=force should only be required if the SoC is described (from > firmware) by both DT and ACPI. > >> >> >In what situation would we want to ignore ACPI tables that are >> >> >present? >> >> > When DT tables are also present (and for the first platforms, that's >> > highly recommended, though not easily enforceable at the kernel level). >> >> My understanding is that the EFI stub creates a device tree (and it >> contains some important information), so I don't understand how we can >> ever have an ACPI-only platform on ARM64 servers. > > If EFI stub creates the DT itself (not passed by firmware), it will > write some information in the chosen node that the rest of the kernel > can make use of and take the appropriate decision on whether to use ACPI > or not. That's something that will be used by kexec and Xen as well > which may want to boot with ACPI tables outside an EFI environment. > If we are going with this solution, we should also mandate that an ACPI enabled firmware should not supply a non-DT DTB, or we wouldn't spot the difference. Not sure how likely this is, but I could imagine a firmware setting up an initrd and hence populating the /chosen node in an otherwise empty DTB. In this case, the stub would not add its 'I-created-an-empty-dtb' property.
On 01/29/2015 12:20 PM, Ard Biesheuvel wrote: > If we are going with this solution, we should also mandate that an > ACPI enabled firmware should not supply a non-DT DTB What is a non-DT DTB? I thought the "DT" in "DTB" stood for device tree.
On 29 January 2015 at 18:21, Timur Tabi <timur@codeaurora.org> wrote: > On 01/29/2015 12:20 PM, Ard Biesheuvel wrote: >> >> If we are going with this solution, we should also mandate that an >> ACPI enabled firmware should not supply a non-DT DTB > > > What is a non-DT DTB? I thought the "DT" in "DTB" stood for device tree. > The UEFI stub in the kernel uses the DTB file format (FDT) to pass information about the UEFI memory map and system table to the kernel. It does so even if there is no device tree that describes the platform. In this case, the file only contains a /chosen DT node, and nothing else, and it is up to the kernel to figure out that it can ask UEFI for a set of ACPI tables that it can use instead to configure the system. Otherwise, the /chosen node properties are added to a device tree that contains the full platform description. The problem is that we have to decide how to distinguish a conventional device tree DTB from a DTB that only exists to communicate the UEFI entry points.
On 01/29/2015 12:28 PM, Ard Biesheuvel wrote: > The UEFI stub in the kernel uses the DTB file format (FDT) to pass > information about the UEFI memory map and system table to the kernel. > It does so even if there is no device tree that describes the > platform. In this case, the file only contains a /chosen DT node, and > nothing else, and it is up to the kernel to figure out that it can ask > UEFI for a set of ACPI tables that it can use instead to configure the > system. Otherwise, the /chosen node properties are added to a device > tree that contains the full platform description. > > The problem is that we have to decide how to distinguish a > conventional device tree DTB from a DTB that only exists to > communicate the UEFI entry points. Ah, that's exactly what I'm seeing. The UEFI stub in our kernel generates a DTB, and therefore I always need to put acpi=force on our kernel command line.
On 01/29/2015 01:34 PM, Timur Tabi wrote: > On 01/29/2015 12:28 PM, Ard Biesheuvel wrote: >> The UEFI stub in the kernel uses the DTB file format (FDT) to pass >> information about the UEFI memory map and system table to the kernel. >> It does so even if there is no device tree that describes the >> platform. In this case, the file only contains a /chosen DT node, and >> nothing else, and it is up to the kernel to figure out that it can ask >> UEFI for a set of ACPI tables that it can use instead to configure the >> system. Otherwise, the /chosen node properties are added to a device >> tree that contains the full platform description. >> >> The problem is that we have to decide how to distinguish a >> conventional device tree DTB from a DTB that only exists to >> communicate the UEFI entry points. > > Ah, that's exactly what I'm seeing. The UEFI stub in our kernel > generates a DTB, and therefore I always need to put acpi=force on our > kernel command line. I expect some of the distros to patch ACPI always enabled. So from my point of view this affects only those wanting to follow upstream. Jon.
On Thu, Jan 29, 2015 at 06:44:36PM +0000, Jon Masters wrote: > On 01/29/2015 01:34 PM, Timur Tabi wrote: > > On 01/29/2015 12:28 PM, Ard Biesheuvel wrote: > >> The UEFI stub in the kernel uses the DTB file format (FDT) to pass > >> information about the UEFI memory map and system table to the kernel. > >> It does so even if there is no device tree that describes the > >> platform. In this case, the file only contains a /chosen DT node, and > >> nothing else, and it is up to the kernel to figure out that it can ask > >> UEFI for a set of ACPI tables that it can use instead to configure the > >> system. Otherwise, the /chosen node properties are added to a device > >> tree that contains the full platform description. > >> > >> The problem is that we have to decide how to distinguish a > >> conventional device tree DTB from a DTB that only exists to > >> communicate the UEFI entry points. > > > > Ah, that's exactly what I'm seeing. The UEFI stub in our kernel > > generates a DTB, and therefore I always need to put acpi=force on our > > kernel command line. To Timur: that's because you use a set of patches that are still under development and not yet agreed as being upstream ready. There is a sub-thread on this topic and even a patch from Ard on how EFI stub can tell the kernel whether DT as any SoC description or not. This needs further discussion since similar feature is needed by kexec and Xen. > I expect some of the distros to patch ACPI always enabled. So from my > point of view this affects only those wanting to follow upstream. Sorry Jon but statements like this make me wonder whether we should simply let the whole ARM ACPI be an out of tree distro business. We spend a long time discussing OS-agnostic firmware implementation, planning mini-summits, just to get certain Linux distro representative stating that the kernel-firmware interface we discuss here only matters for those planning to follow upstream. Certain Linux distros will play by other rules. I'm trying to get some consensus here, coming with arguments why DT has priority over ACPI while still allowing ACPI-only firmware and you pretty much state that vendors picking a distro kernel rather than mainline don't need to bother. How does this work with Red Hat's stand on upstream first? Not having ACPI in mainline yet is not an excuse; there have been reasonable technical arguments and it's now a matter of time until they (well, part of them) are sorted, nothing political (what's political is distros patching the kernel to disable DT booting on purpose).
On 01/29/2015 06:11 PM, Catalin Marinas wrote: > Sorry Jon but statements like this make me wonder whether we should > simply let the whole ARM ACPI be an out of tree distro business. We > spend a long time discussing OS-agnostic firmware implementation, > planning mini-summits, just to get certain Linux distro representative > stating that the kernel-firmware interface we discuss here only matters > for those planning to follow upstream. Certain Linux distros will play > by other rules. Oh, don't take it that way - I just mean that if someone needs a different ACPI always on, they can do that separately. I support your position on upstream at this time! :) Jon.
On Thu, Jan 29, 2015 at 11:16:22PM +0000, Jon Masters wrote: > On 01/29/2015 06:11 PM, Catalin Marinas wrote: > > > Sorry Jon but statements like this make me wonder whether we should > > simply let the whole ARM ACPI be an out of tree distro business. We > > spend a long time discussing OS-agnostic firmware implementation, > > planning mini-summits, just to get certain Linux distro representative > > stating that the kernel-firmware interface we discuss here only matters > > for those planning to follow upstream. Certain Linux distros will play > > by other rules. > > Oh, don't take it that way - I just mean that if someone needs a > different ACPI always on, they can do that separately. And that's exactly what I'm trying to get consensus on. ACPI always on together with DT always on, subject to config options being enabled (not patching). It's up to firmware (and vendor) to provide only ACPI tables to the kernel if not interested in DT. In such case I don't want to see additional kernel parameters. But if the firmware provides both, then it is a user choice which one to use, defaulting to DT.
On Thu, Jan 29, 2015 at 06:20:06PM +0000, Ard Biesheuvel wrote: > On 29 January 2015 at 15:19, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Wed, Jan 28, 2015 at 06:18:44PM +0000, Timur Tabi wrote: > >> On 01/28/2015 12:14 PM, Catalin Marinas wrote: > >> >> >So it looks like there's a whole conversation about this already in > >> >> >this thread that I didn't notice. However, reading through all of it, > >> >> >I still don't understand sure why the presence of ACPI tables is > >> >> >insufficient to enable ACPI. > >> > >> > Because ACPI on arm64 is still experimental, no matter how many people > >> > claim that it is production ready in their private setups. > >> > >> Fair enough. Does this mean that passing "acpi=force" on the kernel > >> command line is a requirement for ARM64 servers? > > > > Please read my other email and ideally the whole sub-thread. The > > acpi=force should only be required if the SoC is described (from > > firmware) by both DT and ACPI. > > > >> >> >In what situation would we want to ignore ACPI tables that are > >> >> >present? > >> > >> > When DT tables are also present (and for the first platforms, that's > >> > highly recommended, though not easily enforceable at the kernel level). > >> > >> My understanding is that the EFI stub creates a device tree (and it > >> contains some important information), so I don't understand how we can > >> ever have an ACPI-only platform on ARM64 servers. > > > > If EFI stub creates the DT itself (not passed by firmware), it will > > write some information in the chosen node that the rest of the kernel > > can make use of and take the appropriate decision on whether to use ACPI > > or not. That's something that will be used by kexec and Xen as well > > which may want to boot with ACPI tables outside an EFI environment. For Timur's reference, here's Hanjun's proposal: http://article.gmane.org/gmane.linux.acpi.devel/73061 > If we are going with this solution, we should also mandate that an > ACPI enabled firmware should not supply a non-DT DTB, or we wouldn't > spot the difference. Not sure how likely this is, but I could imagine > a firmware setting up an initrd and hence populating the /chosen node > in an otherwise empty DTB. In this case, the stub would not add its > 'I-created-an-empty-dtb' property. I think that's a sane requirement. For the normal case of UEFI firmware booting Linux as an EFI application (stub), if the firmware passes a DTB it _must_ contain the full SoC description and be able to boot the kernel without acpi=force. Passing initrd is really not a feature of the SoC to be described in DT by firmware. We leave this to the EFI stub to transfer the command line arguments into the chosen DT node. Apart from UEFI, we have Xen and kexec and I think the plan is to emulate the EFI stub behaviour when starting the kernel and they'll state whether the dtb contains SoC description or not. All we need to do here is make sure that the EFI stub <-> kernel protocol via the /chosen node is well documented. We won't be able to prevent, for example, U-Boot booting Linux with ACPI but I don't see why we should care. Anyway, rather than a "I-created-an-empty-dtb" property, I would actually say something like "dtb-contains-no-hardware-description". Alternatively, we could avoid any such properties and look for signs of hardware description like more nodes than /chosen, CPU nodes, "model" property etc. and we won't need to change the stub code at all.
Catalin Marinas wrote: > Anyway, rather than a "I-created-an-empty-dtb" property, I would > actually say something like "dtb-contains-no-hardware-description". Why do we need a property for this? Wouldn't the absence of a hardware description be the best way to see if the dtb contains no hardware description? It's like putting a sign on an empty bookshelf that says, "there are no books here."
On 30 January 2015 at 14:48, Timur Tabi <timur@codeaurora.org> wrote: > Catalin Marinas wrote: >> >> Anyway, rather than a "I-created-an-empty-dtb" property, I would >> actually say something like "dtb-contains-no-hardware-description". > > > Why do we need a property for this? Wouldn't the absence of a hardware > description be the best way to see if the dtb contains no hardware > description? It's like putting a sign on an empty bookshelf that says, > "there are no books here." > So what constitutes a 'hardware description'? A /cpu node? A memory node? I don't think there is a mandated minimal set of nodes, even if booting without cpu and memory nodes doesn't get you very far. So those should go hand in hand: if we are going to implement logic that decides a DTB is considered empty if it has no /cpu node, we should update the boot protocol to mandate the presence of a /cpu node for DT boot, and not change the rules every couple of months if someone's use case requires it.
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 4df73da..4adfd50 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -165,7 +165,7 @@ multipliers 'Kilo', 'Mega', and 'Giga', equalling 2^10, 2^20, and 2^30 bytes respectively. Such letter suffixes can also be entirely omitted. - acpi= [HW,ACPI,X86] + acpi= [HW,ACPI,X86,ARM64] Advanced Configuration and Power Interface Format: { force | off | strict | noirq | rsdt } force -- enable ACPI if default was off @@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. strictly ACPI specification compliant. rsdt -- prefer RSDT over (default) XSDT copy_dsdt -- copy DSDT to memory + For ARM64, ONLY "acpi=off" or "acpi=force" are available See also Documentation/power/runtime_pm.txt, pci=noacpi diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 8b837ab..496c33b 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -26,6 +26,13 @@ static inline void disable_acpi(void) acpi_noirq = 1; } +static inline void enable_acpi(void) +{ + acpi_disabled = 0; + acpi_pci_disabled = 0; + acpi_noirq = 0; +} + /* * It's used from ACPI core in kdump to boot UP system with SMP kernel, * with this check the ACPI core will not override the CPU index @@ -40,6 +47,8 @@ static inline bool acpi_has_cpu_in_madt(void) static inline void arch_fix_phys_package_id(int num, u32 slot) { } +#else +static inline void disable_acpi(void) { } #endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 9252f72..39a1655 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -67,3 +67,20 @@ void __init acpi_boot_table_init(void) if (acpi_table_init()) disable_acpi(); } + +static int __init parse_acpi(char *arg) +{ + if (!arg) + return -EINVAL; + + /* "acpi=off" disables both ACPI table parsing and interpreter */ + if (strcmp(arg, "off") == 0) + disable_acpi(); + else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */ + enable_acpi(); + else + return -EINVAL; /* Core will print when we return error */ + + return 0; +} +early_param("acpi", parse_acpi); diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 726b019..4580ed3 100644 --- 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(); /*