xen/arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Linux requires UEFI support to be enabled on ARM64 devices. While many
ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
potentially taking over. Some common devices may need ACPI table
support.
Presently I think it is worth removing the dependancy on CONFIG_EXPERT.
I am rather tempted to add defaulting enabled, but I'm not yet confident
of going that far yet.
Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
I hope a popular ARM device capable of running Xen will soon be running
Xen on ACPI/UEFI, but it isn't quite there yet. As such I would like to
have "default y", but I don't think that is likely to be accepted yet.
---
xen/arch/arm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2777388265..f43d9074f9 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -32,7 +32,7 @@ menu "Architecture Features"
source "arch/Kconfig"
config ACPI
- bool "ACPI (Advanced Configuration and Power Interface) Support" if EXPERT
+ bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on ARM_64
---help---
--
2.20.1
--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Hi, On 22/10/2020 02:43, Elliott Mitchell wrote: > Linux requires UEFI support to be enabled on ARM64 devices. While many > ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI > potentially taking over. Some common devices may need ACPI table > support. > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT. The idea behind EXPERT is to gate any feature that is not considered to be stable/complete enough to be used in production. I don't consider the ACPI complete because the parsing of the IORT (used to discover SMMU and GICv3 ITS) is not there yet. I vaguely remember some issues on system using SMMU (e.g. Thunder-X) because Dom0 will try to use the IOMMU and this would break PV drivers. Therefore I think we at least want to consider to hide SMMUs from dom0 before removing EXPERT. Ideally, I would also like the feature to be tested in Osstest. The good news is Xen Project already has systems (e.g. Thunder-X, Softiron) that can supported ACPI. So it should hopefully be a matter to tell them to boot with ACPI rather than DT. Cheers, -- Julien Grall
On Thu, 22 Oct 2020, Julien Grall wrote: > On 22/10/2020 02:43, Elliott Mitchell wrote: > > Linux requires UEFI support to be enabled on ARM64 devices. While many > > ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI > > potentially taking over. Some common devices may need ACPI table > > support. > > > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT. > > The idea behind EXPERT is to gate any feature that is not considered to be > stable/complete enough to be used in production. Yes, and from that point of view I don't think we want to remove EXPERT from ACPI yet. However, the idea of hiding things behind EXPERT works very well for new esoteric features, something like memory introspection or memory overcommit. It does not work well for things that are actually required to boot on the platform. Typically ACPI systems don't come with device tree at all (RPi4 being an exception), so users don't really have much of a choice in the matter. From that point of view, it would be better to remove EXPERT from ACPI, maybe even build ACPI by default, *but* to add a warning at boot saying something like: "ACPI support is experimental. Boot using Device Tree if you can." That would better convey the risks of using ACPI, while at the same time making it a bit easier for users to boot on their ACPI-only platforms. > I don't consider the ACPI complete because the parsing of the IORT (used to > discover SMMU and GICv3 ITS) is not there yet. > > I vaguely remember some issues on system using SMMU (e.g. Thunder-X) because > Dom0 will try to use the IOMMU and this would break PV drivers. I am not sure why Dom0 using the IOMMU would break PV drivers? Is it because the pagetable is not properly updated when mapping foreign pages? > Therefore I think we at least want to consider to hide SMMUs from dom0 before > removing EXPERT. Ideally, I would also like the feature to be tested in > Osstest. > > The good news is Xen Project already has systems (e.g. Thunder-X, Softiron) > that can supported ACPI. So it should hopefully be a matter to tell them to > boot with ACPI rather than DT. I agree that we want to keep ACPI "expert/experimental" given its current state but maybe we can find a better way to carry that message than to set EXPERT in Kconfig. And yes, if we wanted to make ACPI less "expert/experimental" we definitely need some testing in OSSTest and any critical bugs (e.g. PV drivers not working) addressed.
On Thu, Oct 22, 2020 at 02:17:13PM -0700, Stefano Stabellini wrote: > On Thu, 22 Oct 2020, Julien Grall wrote: > > On 22/10/2020 02:43, Elliott Mitchell wrote: > > > Linux requires UEFI support to be enabled on ARM64 devices. While many > > > ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI > > > potentially taking over. Some common devices may need ACPI table > > > support. > > > > > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT. > > > > The idea behind EXPERT is to gate any feature that is not considered to be > > stable/complete enough to be used in production. > > Yes, and from that point of view I don't think we want to remove EXPERT > from ACPI yet. However, the idea of hiding things behind EXPERT works > very well for new esoteric features, something like memory introspection > or memory overcommit. It does not work well for things that are actually > required to boot on the platform. > > Typically ACPI systems don't come with device tree at all (RPi4 being an > exception), so users don't really have much of a choice in the matter. > > >From that point of view, it would be better to remove EXPERT from ACPI, > maybe even build ACPI by default, *but* to add a warning at boot saying > something like: > > "ACPI support is experimental. Boot using Device Tree if you can." > > > That would better convey the risks of using ACPI, while at the same time > making it a bit easier for users to boot on their ACPI-only platforms. This matches my view. I was thinking about including "default y", but I felt the chances of that getting through were lower. I concur with a warning on boot being a good approach. -- (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) \BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Hi Stefano,
On 22/10/2020 22:17, Stefano Stabellini wrote:
> On Thu, 22 Oct 2020, Julien Grall wrote:
>> On 22/10/2020 02:43, Elliott Mitchell wrote:
>>> Linux requires UEFI support to be enabled on ARM64 devices. While many
>>> ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
>>> potentially taking over. Some common devices may need ACPI table
>>> support.
>>>
>>> Presently I think it is worth removing the dependancy on CONFIG_EXPERT.
>>
>> The idea behind EXPERT is to gate any feature that is not considered to be
>> stable/complete enough to be used in production.
>
> Yes, and from that point of view I don't think we want to remove EXPERT
> from ACPI yet. However, the idea of hiding things behind EXPERT works
> very well for new esoteric features, something like memory introspection
> or memory overcommit.
Memaccess is not very new ;).
> It does not work well for things that are actually
> required to boot on the platform.
I am not sure where is the problem. It is easy to select EXPERT from the
menuconfig. It also hints the user that the feature may not fully work.
>
> Typically ACPI systems don't come with device tree at all (RPi4 being an
> exception), so users don't really have much of a choice in the matter.
And they typically have IOMMUs.
>
> From that point of view, it would be better to remove EXPERT from ACPI,
> maybe even build ACPI by default, *but* to add a warning at boot saying
> something like:
>
> "ACPI support is experimental. Boot using Device Tree if you can."
>
>
> That would better convey the risks of using ACPI, while at the same time
> making it a bit easier for users to boot on their ACPI-only platforms.
Right, I agree that this make easier for users to boot Xen on ACPI-only
platform. However, based on above, it is easy enough for a developper to
rebuild Xen with ACPI and EXPERT enabled.
So what sort of users are you targeting?
I am sort of okay to remove EXPERT. But I still think building ACPI by
default is still wrong because our default .config is meant to be
(security) supported. I don't think ACPI can earn this qualification today.
In order to remove EXPERT, there are a few things to needs to be done
(or checked):
1) SUPPORT.MD has a statement about ACPI on Arm
2) DT is favored over ACPI if the two firmware tables are present.
>
>
>> I don't consider the ACPI complete because the parsing of the IORT (used to
>> discover SMMU and GICv3 ITS) is not there yet.
>>
>> I vaguely remember some issues on system using SMMU (e.g. Thunder-X) because
>> Dom0 will try to use the IOMMU and this would break PV drivers.
>
> I am not sure why Dom0 using the IOMMU would break PV drivers? Is it
> because the pagetable is not properly updated when mapping foreign
> pages?
IIRC, yes. This would need to be tested again.
Cheers,
--
Julien Grall
On Fri, 23 Oct 2020, Julien Grall wrote:
> Hi Stefano,
>
> On 22/10/2020 22:17, Stefano Stabellini wrote:
> > On Thu, 22 Oct 2020, Julien Grall wrote:
> > > On 22/10/2020 02:43, Elliott Mitchell wrote:
> > > > Linux requires UEFI support to be enabled on ARM64 devices. While many
> > > > ARM64 devices lack ACPI, the writing seems to be on the wall of
> > > > UEFI/ACPI
> > > > potentially taking over. Some common devices may need ACPI table
> > > > support.
> > > >
> > > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT.
> > >
> > > The idea behind EXPERT is to gate any feature that is not considered to be
> > > stable/complete enough to be used in production.
> >
> > Yes, and from that point of view I don't think we want to remove EXPERT
> > from ACPI yet. However, the idea of hiding things behind EXPERT works
> > very well for new esoteric features, something like memory introspection
> > or memory overcommit.
>
> Memaccess is not very new ;).
>
> > It does not work well for things that are actually
> > required to boot on the platform.
>
> I am not sure where is the problem. It is easy to select EXPERT from the
> menuconfig. It also hints the user that the feature may not fully work.
>
> >
> > Typically ACPI systems don't come with device tree at all (RPi4 being an
> > exception), so users don't really have much of a choice in the matter.
>
> And they typically have IOMMUs.
>
> >
> > From that point of view, it would be better to remove EXPERT from ACPI,
> > maybe even build ACPI by default, *but* to add a warning at boot saying
> > something like:
> >
> > "ACPI support is experimental. Boot using Device Tree if you can."
> >
> >
> > That would better convey the risks of using ACPI, while at the same time
> > making it a bit easier for users to boot on their ACPI-only platforms.
>
> Right, I agree that this make easier for users to boot Xen on ACPI-only
> platform. However, based on above, it is easy enough for a developper to
> rebuild Xen with ACPI and EXPERT enabled.
>
> So what sort of users are you targeting?
Somebody trying Xen for the first time, they might know how to build it
but they might not know that ACPI is not available by default, and they
might not know that they need to enable EXPERT in order to get the ACPI
option in the menu. It is easy to do once you know it is there,
otherwise one might not know where to look in the menu.
> I am sort of okay to remove EXPERT.
OK. This would help (even without building it by default) because as you
go and look at the menu the first time, you'll find ACPI among the
options right away.
> But I still think building ACPI by default
> is still wrong because our default .config is meant to be (security)
> supported. I don't think ACPI can earn this qualification today.
Certainly we don't want to imply ACPI is security supported. I was
looking at SUPPORT.md and it is only says:
"""
EXPERT and DEBUG Kconfig options are not security supported. Other
Kconfig options are supported, if the related features are marked as
supported in this document.
"""
So technically we could enable ACPI in the build by default as ACPI for
ARM is marked as experimental. However, I can see that it is not a
great idea to enable by default an unsupported option in the kconfig, so
from that point of view it might be best to leave ACPI disabled by
default. Probably the best compromise at this time.
> In order to remove EXPERT, there are a few things to needs to be done (or
> checked):
> 1) SUPPORT.MD has a statement about ACPI on Arm
### Host ACPI (via Domain 0)
Status, x86 PV: Supported
Status, ARM: Experimental
> 2) DT is favored over ACPI if the two firmware tables are present.
Good idea. xen/arch/arm/acpi/boot.c:acpi_boot_table_init has:
/*
* Enable ACPI instead of device tree unless
* - ACPI has been disabled explicitly (acpi=off), or
* - the device tree is not empty (it has more than just a /chosen node)
* and ACPI has not been force enabled (acpi=force)
*/
if ( param_acpi_off)
goto disable;
if ( !param_acpi_force &&
device_tree_for_each_node(device_tree_flattened, 0,
dt_scan_depth1_nodes, NULL) )
goto disable;
We should be fine.
Hi Stefano, On 23/10/2020 17:57, Stefano Stabellini wrote: > On Fri, 23 Oct 2020, Julien Grall wrote: >> Hi Stefano, >> >> On 22/10/2020 22:17, Stefano Stabellini wrote: >>> On Thu, 22 Oct 2020, Julien Grall wrote: >>>> On 22/10/2020 02:43, Elliott Mitchell wrote: >>>>> Linux requires UEFI support to be enabled on ARM64 devices. While many >>>>> ARM64 devices lack ACPI, the writing seems to be on the wall of >>>>> UEFI/ACPI >>>>> potentially taking over. Some common devices may need ACPI table >>>>> support. >>>>> >>>>> Presently I think it is worth removing the dependancy on CONFIG_EXPERT. >>>> >>>> The idea behind EXPERT is to gate any feature that is not considered to be >>>> stable/complete enough to be used in production. >>> >>> Yes, and from that point of view I don't think we want to remove EXPERT >>> from ACPI yet. However, the idea of hiding things behind EXPERT works >>> very well for new esoteric features, something like memory introspection >>> or memory overcommit. >> >> Memaccess is not very new ;). >> >>> It does not work well for things that are actually >>> required to boot on the platform. >> >> I am not sure where is the problem. It is easy to select EXPERT from the >> menuconfig. It also hints the user that the feature may not fully work. >> >>> >>> Typically ACPI systems don't come with device tree at all (RPi4 being an >>> exception), so users don't really have much of a choice in the matter. >> >> And they typically have IOMMUs. >> >>> >>> From that point of view, it would be better to remove EXPERT from ACPI, >>> maybe even build ACPI by default, *but* to add a warning at boot saying >>> something like: >>> >>> "ACPI support is experimental. Boot using Device Tree if you can." >>> >>> >>> That would better convey the risks of using ACPI, while at the same time >>> making it a bit easier for users to boot on their ACPI-only platforms. >> >> Right, I agree that this make easier for users to boot Xen on ACPI-only >> platform. However, based on above, it is easy enough for a developper to >> rebuild Xen with ACPI and EXPERT enabled. >> >> So what sort of users are you targeting? > > Somebody trying Xen for the first time, they might know how to build it > but they might not know that ACPI is not available by default, and they > might not know that they need to enable EXPERT in order to get the ACPI > option in the menu. It is easy to do once you know it is there, > otherwise one might not know where to look in the menu. Right, EXPERT can now be enabled using Kconfig. So it is not very different from an option Foo has been hidden because the dependency Bar has not been selected. It should be easy enough (if it is not we should fix it) to figure out the dependency when searching the option via menuconfig. > > >> I am sort of okay to remove EXPERT. > > OK. This would help (even without building it by default) because as you > go and look at the menu the first time, you'll find ACPI among the > options right away. To be honest, this step is probably the easiest in the full process to get Xen build and booted on Arm. I briefly looked at Elliot's v2, and I can't keep thinking that we are trying to re-invent EXPERT for ACPI because we think the feature is *more* important than any other feature gated by EXPERT. In fact, all the features behind EXPERT are important. But they have been gated by EXPERT because they are not mature enough. We already moved EXPERT from a command line option to a menuconfig option. So it should be easy enough to enable it now. If it still not the case, then we should improve it. But I don't think ACPI is mature enough to deserve a different treatment. It would be more useful to get to the stage where ACPI can work without any crash/issue first. > > >> But I still think building ACPI by default >> is still wrong because our default .config is meant to be (security) >> supported. I don't think ACPI can earn this qualification today. > > Certainly we don't want to imply ACPI is security supported. I was > looking at SUPPORT.md and it is only says: > > """ > EXPERT and DEBUG Kconfig options are not security supported. Other > Kconfig options are supported, if the related features are marked as > supported in this document. > """ > > So technically we could enable ACPI in the build by default as ACPI for > ARM is marked as experimental. However, I can see that it is not a > great idea to enable by default an unsupported option in the kconfig, so > from that point of view it might be best to leave ACPI disabled by > default. Probably the best compromise at this time. From my understanding, the goal of EXPERT was to gate such features. With your suggestion, it is not clear to me what's the difference between "experimental" and option gated by "EXPERT". Do you mind clarifying? Cheers, -- Julien Grall
On Mon, Oct 26, 2020 at 09:19:49AM +0000, Julien Grall wrote: > On 23/10/2020 17:57, Stefano Stabellini wrote: > > On Fri, 23 Oct 2020, Julien Grall wrote: > >> I am sort of okay to remove EXPERT. > > > > OK. This would help (even without building it by default) because as you > > go and look at the menu the first time, you'll find ACPI among the > > options right away. > > To be honest, this step is probably the easiest in the full process to > get Xen build and booted on Arm. > > I briefly looked at Elliot's v2, and I can't keep thinking that we are > trying to re-invent EXPERT for ACPI because we think the feature is > *more* important than any other feature gated by EXPERT. Yet might that statement in fact be true? Most of the features currently controlled by CONFIG_EXPERT are relatively small tweaks which enable less often used features. Some of those are very high value in certain environments, but they're unimportant in common environments. Changing the scheduler might get you an extra 10-50% performance improvement in a special environment. ACPI support isn't like this. I'm unaware what Masami Hiramatsu's system does if a CONFIG_ACPI=n Xen build is tried. I haven't actually tried such a build on mine, but from the code it looks like Xen would panic if built that way. No output of any sort. Simply panic with the device appearing to go inert. > In fact, all the features behind EXPERT are important. But they have > been gated by EXPERT because they are not mature enough. > But I don't think ACPI is mature enough to deserve a different > treatment. It would be more useful to get to the stage where ACPI can > work without any crash/issue first. The difference is the severity of failure if the option is off, but needs to be on. Most CONFIG_EXPERT options will merely be performance reductions or security situations unacceptable to some users. CONFIG_ACPI=n on the wrong system could be a panic with *no* output. Tainting sounds reasonable. Messages in `dmesg` make sense. A message plus 10 second pause on boot seems reasonable. I think if CONFIG_ACPI is off, there should be code to try to detect ACPI and emit a warning if anything is detected. -- (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) \BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Hi Elliott, On 04/11/2020 19:03, Elliott Mitchell wrote: > On Mon, Oct 26, 2020 at 09:19:49AM +0000, Julien Grall wrote: >> On 23/10/2020 17:57, Stefano Stabellini wrote: >>> On Fri, 23 Oct 2020, Julien Grall wrote: > >>>> I am sort of okay to remove EXPERT. >>> >>> OK. This would help (even without building it by default) because as you >>> go and look at the menu the first time, you'll find ACPI among the >>> options right away. >> >> To be honest, this step is probably the easiest in the full process to >> get Xen build and booted on Arm. >> >> I briefly looked at Elliot's v2, and I can't keep thinking that we are >> trying to re-invent EXPERT for ACPI because we think the feature is >> *more* important than any other feature gated by EXPERT. > > Yet might that statement in fact be true? Everyone has a different opinion on what's important or not. I am sure we can spend hours bikeshedding on that... > > Most of the features currently controlled by CONFIG_EXPERT are relatively > small tweaks which enable less often used features. Some of those are > very high value in certain environments, but they're unimportant in > common environments. Changing the scheduler might get you an extra > 10-50% performance improvement in a special environment. > > ACPI support isn't like this. I'm unaware what Masami Hiramatsu's system > does if a CONFIG_ACPI=n Xen build is tried. I haven't actually tried > such a build on mine, but from the code it looks like Xen would panic if > built that way. No output of any sort. Simply panic with the device > appearing to go inert. There will always be cases where the console is not working: 1) There is no driver in Xen 2) There is no SPCR table present I think you are in the second situation and you had to enable earlyprintk. Is that correct? >> In fact, all the features behind EXPERT are important. But they have >> been gated by EXPERT because they are not mature enough. > >> But I don't think ACPI is mature enough to deserve a different >> treatment. It would be more useful to get to the stage where ACPI can >> work without any crash/issue first. > > The difference is the severity of failure if the option is off, but needs > to be on. Most CONFIG_EXPERT options will merely be performance > reductions or security situations unacceptable to some users. > CONFIG_ACPI=n on the wrong system could be a panic with *no* output. > > > Tainting sounds reasonable. Messages in `dmesg` make sense. A message > plus 10 second pause on boot seems reasonable. I think if CONFIG_ACPI is > off, there should be code to try to detect ACPI and emit a warning if > anything is detected. All of this would be moot if we focus on getting ACPI (or just a subset) working on a few platforms (e.g. RPI, Thunder-X). I don't think we are too far from this acheviement. This would be IMHO be enough to move ACPI one way up in the support "ladder". We might even be able to do this for 4.15. Cheers, -- Julien Grall
On Mon, 26 Oct 2020, Julien Grall wrote:
> Hi Stefano,
>
> On 23/10/2020 17:57, Stefano Stabellini wrote:
> > On Fri, 23 Oct 2020, Julien Grall wrote:
> > > Hi Stefano,
> > >
> > > On 22/10/2020 22:17, Stefano Stabellini wrote:
> > > > On Thu, 22 Oct 2020, Julien Grall wrote:
> > > > > On 22/10/2020 02:43, Elliott Mitchell wrote:
> > > > > > Linux requires UEFI support to be enabled on ARM64 devices. While
> > > > > > many
> > > > > > ARM64 devices lack ACPI, the writing seems to be on the wall of
> > > > > > UEFI/ACPI
> > > > > > potentially taking over. Some common devices may need ACPI table
> > > > > > support.
> > > > > >
> > > > > > Presently I think it is worth removing the dependancy on
> > > > > > CONFIG_EXPERT.
> > > > >
> > > > > The idea behind EXPERT is to gate any feature that is not considered
> > > > > to be
> > > > > stable/complete enough to be used in production.
> > > >
> > > > Yes, and from that point of view I don't think we want to remove EXPERT
> > > > from ACPI yet. However, the idea of hiding things behind EXPERT works
> > > > very well for new esoteric features, something like memory introspection
> > > > or memory overcommit.
> > >
> > > Memaccess is not very new ;).
> > >
> > > > It does not work well for things that are actually
> > > > required to boot on the platform.
> > >
> > > I am not sure where is the problem. It is easy to select EXPERT from the
> > > menuconfig. It also hints the user that the feature may not fully work.
> > >
> > > >
> > > > Typically ACPI systems don't come with device tree at all (RPi4 being an
> > > > exception), so users don't really have much of a choice in the matter.
> > >
> > > And they typically have IOMMUs.
> > >
> > > >
> > > > From that point of view, it would be better to remove EXPERT from
> > > > ACPI,
> > > > maybe even build ACPI by default, *but* to add a warning at boot saying
> > > > something like:
> > > >
> > > > "ACPI support is experimental. Boot using Device Tree if you can."
> > > >
> > > >
> > > > That would better convey the risks of using ACPI, while at the same time
> > > > making it a bit easier for users to boot on their ACPI-only platforms.
> > >
> > > Right, I agree that this make easier for users to boot Xen on ACPI-only
> > > platform. However, based on above, it is easy enough for a developper to
> > > rebuild Xen with ACPI and EXPERT enabled.
> > >
> > > So what sort of users are you targeting?
> >
> > Somebody trying Xen for the first time, they might know how to build it
> > but they might not know that ACPI is not available by default, and they
> > might not know that they need to enable EXPERT in order to get the ACPI
> > option in the menu. It is easy to do once you know it is there,
> > otherwise one might not know where to look in the menu.
>
> Right, EXPERT can now be enabled using Kconfig. So it is not very different
> from an option Foo has been hidden because the dependency Bar has not been
> selected.
>
> It should be easy enough (if it is not we should fix it) to figure out the
> dependency when searching the option via menuconfig.
I do `make menuconfig` and there is no ACPI option. How do I even know
that ACPI has a kconfig option to enable? I'd assume that ACPI is always
enabled in the kconfig unless told otherwise.
But let's say that you know that you need to look for ACPI. I'd use the
search function, and it tells me:
Symbol: ACPI [=n] │
Type : bool │
Prompt: ACPI (Advanced Configuration and Power Interface) Support │
Location: │
(1) -> Architecture Features │
Defined at arch/arm/Kconfig:34 │
Depends on: ARM_64 [=y]
I go and look "Architecture Features" as told, but it is not there. How
do I need that I need to enable "Configure standard Xen features (expert
users)" to get that option?
> > > I am sort of okay to remove EXPERT.
> >
> > OK. This would help (even without building it by default) because as you
> > go and look at the menu the first time, you'll find ACPI among the
> > options right away.
>
> To be honest, this step is probably the easiest in the full process to get Xen
> build and booted on Arm.
>
> I briefly looked at Elliot's v2, and I can't keep thinking that we are trying
> to re-invent EXPERT for ACPI because we think the feature is *more* important
> than any other feature gated by EXPERT.
>
> In fact, all the features behind EXPERT are important. But they have been
> gated by EXPERT because they are not mature enough.
It is not as much a matter of "importance" but a matter of required for
booting. I don't think that EXPERT is really a good tool for gating
things that are required for booting. If we had something else (not
ACPI) that is required for booting and marked as EXPERT, I'd say the
same thing. The only other thing that might qualify is ITS support.
> We already moved EXPERT from a command line option to a menuconfig option. So
> it should be easy enough to enable it now. If it still not the case, then we
> should improve it.
>
> But I don't think ACPI is mature enough to deserve a different treatment.
I fully agree ACPI is not mature.
> It would be more useful to get to the stage where ACPI can work
> without any crash/issue first.
Yes indeed. I don't have any stake in this, given that none of my
systems have ACPI, so I'd better shut up maybe :-)
> > > But I still think building ACPI by default
> > > is still wrong because our default .config is meant to be (security)
> > > supported. I don't think ACPI can earn this qualification today.
> >
> > Certainly we don't want to imply ACPI is security supported. I was
> > looking at SUPPORT.md and it is only says:
> >
> > """
> > EXPERT and DEBUG Kconfig options are not security supported. Other
> > Kconfig options are supported, if the related features are marked as
> > supported in this document.
> > """
> >
> > So technically we could enable ACPI in the build by default as ACPI for
> > ARM is marked as experimental. However, I can see that it is not a
> > great idea to enable by default an unsupported option in the kconfig, so
> > from that point of view it might be best to leave ACPI disabled by
> > default. Probably the best compromise at this time.
> From my understanding, the goal of EXPERT was to gate such features. With your
> suggestion, it is not clear to me what's the difference between "experimental"
> and option gated by "EXPERT".
>
> Do you mind clarifying?
Ah! That's a good question actually. Is the expectation and
"experimental" in SUPPORT.md and EXPERT in Kconfig are pretty much the
same thing? I didn't think so. Let's have a look. SUPPORT.md says:
### KCONFIG Expert
Status: Experimental
And EXPERT says:
config EXPERT
bool "Configure standard Xen features (expert users)"
help
This option allows certain base Xen options and settings
to be disabled or tweaked. This is for specialized environments
which can tolerate a "non-standard" Xen.
Only use this if you really know what you are doing.
Xen binaries built with this option enabled are not security
supported.
It seems that they are not the same: EXPERT is a *subset* of
Experimental for certain Xen options "for specialized environments" and
"expert users, which I think makes sense. ACPI is a good example of
something "experimental" but not for specialized environments.
Linux requires UEFI support to be enabled on ARM64 devices. While many
ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
potentially taking over. Some common devices may require ACPI table
support to boot.
For devices which can boot in either mode, continue defaulting to
device-tree. Add warnings about using ACPI advising users of present
situation.
Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
Okay, hopefully this is okay. Warning in Kconfig, warning on boot.
Perhaps "default y if ARM_64" is redundant, yet if someone tries to make
it possible to boot aarch32 on a ACPI machine...
I also want a date in the message. Theory is this won't be there
forever, so a date is essential.
---
xen/arch/arm/Kconfig | 7 ++++++-
xen/arch/arm/acpi/boot.c | 9 +++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2777388265..29624d03fa 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -32,13 +32,18 @@ menu "Architecture Features"
source "arch/Kconfig"
config ACPI
- bool "ACPI (Advanced Configuration and Power Interface) Support" if EXPERT
+ bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on ARM_64
+ default y if ARM_64
---help---
Advanced Configuration and Power Interface (ACPI) support for Xen is
an alternative to device tree on ARM64.
+ Note this is presently EXPERIMENTAL. If a given device has both
+ device-tree and ACPI support, it is presently (October 2020)
+ recommended to boot using the device-tree.
+
config GICV3
bool "GICv3 driver"
depends on ARM_64 && !NEW_VGIC
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 30e4bd1bc5..c0e8f85325 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -254,6 +254,15 @@ int __init acpi_boot_table_init(void)
dt_scan_depth1_nodes, NULL) )
goto disable;
+ printk("\n"
+"*************************************************************************\n"
+"* WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING *\n"
+"* *\n"
+"* Xen-ARM ACPI support is EXPERIMENTAL. It is presently (October 2020) *\n"
+"* recommended you boot your system in device-tree mode if you can. *\n"
+"*************************************************************************\n"
+ "\n");
+
/*
* ACPI is disabled at this point. Enable it in order to parse
* the ACPI tables.
--
2.20.1
--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
On Thu, 22 Oct 2020, Elliott Mitchell wrote:
> Linux requires UEFI support to be enabled on ARM64 devices. While many
> ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
> potentially taking over. Some common devices may require ACPI table
> support to boot.
Let's not make guesses on the direction of the industry in a commit
message :-)
The following would suffice:
Some common ARM64 devices require ACPI to boot (no device tree is
available).
> For devices which can boot in either mode, continue defaulting to
> device-tree. Add warnings about using ACPI advising users of present
> situation.
>
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> ---
> Okay, hopefully this is okay. Warning in Kconfig, warning on boot.
> Perhaps "default y if ARM_64" is redundant, yet if someone tries to make
> it possible to boot aarch32 on a ACPI machine...
>
> I also want a date in the message. Theory is this won't be there
> forever, so a date is essential.
> ---
> xen/arch/arm/Kconfig | 7 ++++++-
> xen/arch/arm/acpi/boot.c | 9 +++++++++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2777388265..29624d03fa 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -32,13 +32,18 @@ menu "Architecture Features"
> source "arch/Kconfig"
>
> config ACPI
> - bool "ACPI (Advanced Configuration and Power Interface) Support" if EXPERT
> + bool "ACPI (Advanced Configuration and Power Interface) Support"
> depends on ARM_64
> + default y if ARM_64
I am not so sure about the "default y" for the reason that the option is
not technically "supported", so it is probably best to take the default
line out. Otherwise we end up with a default "unsupported" kconfig which
is not great.
> ---help---
>
> Advanced Configuration and Power Interface (ACPI) support for Xen is
> an alternative to device tree on ARM64.
>
> + Note this is presently EXPERIMENTAL. If a given device has both
> + device-tree and ACPI support, it is presently (October 2020)
> + recommended to boot using the device-tree.
Please remove the date from the message. We'll update as needed in the
future. The following works:
Note this is presently EXPERIMENTAL. If a given device has both
device-tree and ACPI support, it is recommended to boot using the
device-tree.
> config GICV3
> bool "GICv3 driver"
> depends on ARM_64 && !NEW_VGIC
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 30e4bd1bc5..c0e8f85325 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -254,6 +254,15 @@ int __init acpi_boot_table_init(void)
> dt_scan_depth1_nodes, NULL) )
> goto disable;
>
> + printk("\n"
> +"*************************************************************************\n"
> +"* WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING *\n"
> +"* *\n"
> +"* Xen-ARM ACPI support is EXPERIMENTAL. It is presently (October 2020) *\n"
> +"* recommended you boot your system in device-tree mode if you can. *\n"
> +"*************************************************************************\n"
> + "\n");
Please use warning_add and remove the date from the message.
> /*
> * ACPI is disabled at this point. Enable it in order to parse
> * the ACPI tables.
> --
> 2.20.1
>
>
> --
> (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
> \BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) /
> \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
> 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
>
>
On 23.10.2020 05:35, Elliott Mitchell wrote:
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -32,13 +32,18 @@ menu "Architecture Features"
> source "arch/Kconfig"
>
> config ACPI
> - bool "ACPI (Advanced Configuration and Power Interface) Support" if EXPERT
> + bool "ACPI (Advanced Configuration and Power Interface) Support"
> depends on ARM_64
> + default y if ARM_64
The "if" is pointless with the "depends on".
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -254,6 +254,15 @@ int __init acpi_boot_table_init(void)
> dt_scan_depth1_nodes, NULL) )
> goto disable;
>
> + printk("\n"
> +"*************************************************************************\n"
> +"* WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING *\n"
> +"* *\n"
> +"* Xen-ARM ACPI support is EXPERIMENTAL. It is presently (October 2020) *\n"
> +"* recommended you boot your system in device-tree mode if you can. *\n"
> +"*************************************************************************\n"
> + "\n");
> +
We have an abstraction for such warnings, causing them to appear
later in the boot process and then consistently all in one place
(both increasing, as we believe, the chances of being noticed):
warning_add(). There's a delay accompanied with this, so I think
you will want to also have a command line option allowing to
silence this warning. "acpi=on" or "acpi=force", as available on
x86 and (possibly wrongly right now) not documented as
x86-specific, may be (re-)usable, i.e. to avoid having to
introduce some entirely new option.
Also a few formal nits: The subject tag should have been [PATCH v2],
there should have been a short revision log outside of the commit
message area, and new patch versione would better start their own
new threads than being in-reply-to the earlier version's one.
Jan
© 2016 - 2024 Red Hat, Inc.