On Tue, Oct 10, 2023 at 10:30:56AM +0100, Robin Murphy wrote:
> On 2023-10-10 09:50, Sudeep Holla wrote:
> > On Tue, Oct 10, 2023 at 04:21:23PM +0800, Hanjun Guo wrote:
> > > acpi_agdi_init() in acpi_arm_init() will register a SDEI event, so
> > > it needs the SDEI subsystem to be initialized (which is done in
> > > acpi_ghes_init()) before the AGDI driver probing.
> > >
> > > In commit fcea0ccf4fd7 ("ACPI: bus: Consolidate all arm specific
> > > initialisation into acpi_arm_init()"), the acpi_agdi_init() was
> > > called before acpi_ghes_init() and it causes following failure:
> > >
> > > | [ 0.515864] sdei: Failed to create event 1073741825: -5
> > > | [ 0.515866] agdi agdi.0: Failed to register for SDEI event 1073741825
> > > | [ 0.515867] agdi: probe of agdi.0 failed with error -5
> > > | ...
> > > | [ 0.516022] sdei: SDEIv1.0 (0x0) detected in firmware.
> > >
> > > Fix it by moving acpi_arm_init() to the place of after
> > > acpi_ghes_init().
> > >
> > > Fixes: fcea0ccf4fd7 ("ACPI: bus: Consolidate all arm specific initialisation into acpi_arm_init()")
> > > Reported-by: D Scott Phillips <scott@xxxxxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Hanjun Guo <guohanjun@xxxxxxxxxx>
> > > ---
> > >
> > > I did a test on a ARM server and I didn't see regressions, but
> > > I don't have a AGDI table firmware, so Scott please give a
> > > test to see if it fixes your issue.
> > >
> > > drivers/acpi/bus.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > index f41dda2d3493..a4aa53b7e2bb 100644
> > > --- a/drivers/acpi/bus.c
> > > +++ b/drivers/acpi/bus.c
> > > @@ -1410,10 +1410,10 @@ static int __init acpi_init(void)
> > > acpi_init_ffh();
> > >
> > > pci_mmcfg_late_init();
> > > - acpi_arm_init();
> > > acpi_viot_early_init();
> > > acpi_hest_init();
> > > acpi_ghes_init();
> > > + acpi_arm_init();
> >
> > I am fine with the change, but just wanted to check with Robin/Jean-Philippe
> > if there are any dependency on IORT initialisation for VIOT ? IIUC IORT was
> > always initialised before VIOT but that changes after this change.
>
> They should be independent, and typically we'd only expect to see one or the
> other anyway (although strictly a VMM *could* provide virtio-iommu for some
> devices while also emulating an SMMU for others if it really really wanted
> to).
IORT also describes the GIC ITS topology, so QEMU (and likely
cloud-hypervisor) do present both tables to a guest. But I don't think the
order matters in any case, and QEMU still boots fine with this patch.
Thanks,
Jean
