Message-ID: <CAL_JsqLAPp5cC_Usu32oCaLFu3c_7oxm6peQBvkLE-7xdqT4NA@mail.gmail.com>
Date: Fri, 3 Jan 2025 08:19:18 -0600
From: Rob Herring <robh@...nel.org>
To: Dmytro Maluka <dmaluka@...omium.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, 
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>, Saravana Kannan <saravanak@...gle.com>, 
	Saurabh Sengar <ssengar@...ux.microsoft.com>, Usama Arif <usamaarif642@...il.com>, 
	Stephen Boyd <sboyd@...nel.org>, Frank Rowand <frowand.list@...il.com>, 
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <linux-kernel@...r.kernel.org>, 
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE" <devicetree@...r.kernel.org>, Pierre-Clément Tosi <ptosi@...gle.com>, 
	Shikha Panwar <shikhapanwar@...gle.com>, Will Deacon <will@...nel.org>, 
	Keir Fraser <keirf@...gle.com>, Michal Mazurek <mazurekm@...gle.com>, 
	Bartłomiej Grzesik <bgrzesik@...gle.com>, 
	Tomasz Nowicki <tnowicki@...gle.com>, Grzegorz Jaszczyk <jaszczyk@...gle.com>
Subject: Re: [PATCH] x86/of: Restore possibility to use both ACPI and FDT from bootloader

On Thu, Jan 2, 2025 at 4:18 PM Dmytro Maluka <dmaluka@...omium.org> wrote:
>
> On Thu, Jan 02, 2025 at 12:08:07PM -0600, Rob Herring wrote:
> > On Mon, Dec 23, 2024 at 06:18:03PM +0000, Dmytro Maluka wrote:
> > > There are cases when the bootloader provides information to the kernel
> > > in both ACPI and DTB, not interchangeably. One such use case is virtual
> > > machines in Android. When running on x86, the Android Virtualization
> > > Framework (AVF) boots VMs with ACPI like it is usually done on x86 (i.e.
> > > the virtual LAPIC, IOAPIC, HPET, PCI MMCONFIG etc are described in ACPI)
> > > but also passes various AVF-specific boot parameters in DTB. This allows
> > > reusing the same implementations of various AVF components on both
> > > arm64 and x86.
> >
> > Anyone booting Arm ACPI based systems with AVF?
>
> No, on Arm side AVF is DT only.
>
> > Where's this AVF binding documented?
>
> The strictly AVF-specific properties are described in [1]. When it comes
> to Linux guests, actually AFAIK currently all those properties are
> consumed by the guest userspace, not the guest kernel, so the role of
> the kernel is in fact just to pass them over to the userspace via sysfs.
>
> Besides that, one important DT binding used by AVF's Linux guest
> kernels, not userspace, is google,open-dice documented in
> Documentation/devicetree/bindings/reserved-memory/google,open-dice.yaml.
> [2] describes how it is used by the protected VM firmware (pvmfw) to
> securely boot protected VMs.
>
> Cc'ing some AVF folks to keep me honest.
>
> (AVF used to support protected VMs on arm64 only, but now we are trying
> to make them work on x86. So, taking google,open-dice as an example, we
> could add an ACPI binding to the open-dice driver, however bloating
> pvmfw with AML support is a no go, so we want to keep passing it via DT
> on x86 as well.)
>
> > > Commit 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
> > > removed the possibility to do that, since among other things
> > > it introduced forcing emptying the bootloader-provided DTB if ACPI is
> > > enabled (probably assuming that if ACPI is available, a DTB can only be
> > > useful for applying overlays to it afterwards, for testing purposes).
> > >
> > > So restore this possibility. At the same time, since the aforementioned
> > > recently introduced restriction is actually useful for preventing
> > > conflicts between ACPI and DT for LAPIC/IOAPIC/HPET setup, don't remove
> > > this restriction completely but relax it: unflatten the bootloader
> > > supplied DTB but don't try to use it for SMP setup (i.e. don't override
> > > the .parse_smp_cfg callback) if ACPI is enabled. Precisely, right now
> > > this prevents at least:
> > >
> > > - incorrectly calling register_lapic_address(APIC_DEFAULT_PHYS_BASE)
> > >   after the LAPIC was already successfully enumerated via ACPI, causing
> > >   noisy kernel warnings and probably potential real issues as well
> > >
> > > - failed IOAPIC setup in the case when IOAPIC is enumerated via mptable
> > >   instead of ACPI (e.g. with acpi=noirq), due to
> > >   mpparse_parse_smp_config() overridden by x86_dtb_parse_smp_config()
> >
> > It would be better if we explicitly opt'ed into "things we want to get
> > from DT" rather than allowing anything except what we check for. There's
> > a strong desire at least for arm64 to prevent systems from using both
> > at the same time. There are growing usecases for doing just that, but I
> > think we need to have some control or restrictions in place to define
> > what we support in the kernel.
>
> When it comes to arm64, AFAICS it already enforces this mutual
> exclusion (in setup_arch()):
>
>         if (acpi_disabled)
>                 unflatten_device_tree();

That's right. The patch to remove the if here was rejected, but I
applied the rest of the series. The acpi_disabled check you are
removing from unflatten_device_tree() was there to handle the arm64
desire to not use the bootloader provided DT in case it contained
parallel h/w descriptions. So with removing it, we'll have to come up
with another way if we enable arm64. But I'm leaving fighting over
supporting the usecases on Arm to someone who needs it.

Note that there's also an interaction with kexec that has to be
considered since kexec needs the bootloader provided DT. Probably not
an issue in your use on x86 though.

Rob

Powered by blists - more mailing lists