On Fri, 30 May 2025 14:05:16 +0200
Igor Mammedov <imamm...@redhat.com> wrote:
> On Fri, 30 May 2025 11:02:27 +0100
> Jonathan Cameron <jonathan.came...@huawei.com> wrote:
>
> > On Tue, 27 May 2025 09:40:07 +0200
> > Eric Auger <eric.au...@redhat.com> wrote:
> >
> > > acpi_dsdt_add_pci_osc() name is confusing as it gives the impression
> > > it appends the _OSC method but in fact it also appends the _DSM method
> > > for the host bridge. Let's split the function into two separate ones
> > > and let them return the method Aml pointer instead. This matches the
> > > way it is done on x86 (build_q35_osc_method). In a subsequent patch
> > > we will replace the gpex method by the q35 implementation that will
> > > become shared between ARM and x86.
> > >
> > > acpi_dsdt_add_host_bridge_methods is a new top helper that generates
> > > both the _OSC and _DSM methods.
> > >
> > > Signed-off-by: Eric Auger <eric.au...@redhat.com>
> > > Reviewed-by: Gustavo Romero <gustavo.rom...@linaro.org>
> >
> > Makes complete sense. I've had local equivalent of this on the CXL
> > tree for a while as without it we don't register the _DSM for the
> > CXL path (and we should). However, can you modify it a little to
> > make that easier for me? Basically make sure the _DSM is registered
> > for the CXL path as well.
> >
> [...]
> unless CXL is root host bridge, current _DSM shouldn't be added to it.
> read on comment below.
I'm not clear how this is different from pxb-pcie where we do have
the _DSM. Both are pretending to be real host bridges.
>
> > > @@ -124,7 +125,17 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, bool
> > > enable_native_pcie_hotplug)
> > > byte_list[0] = 0;
> > > buf = aml_buffer(1, byte_list);
> > > aml_append(method, aml_return(buf));
> > > - aml_append(dev, method);
> > > + return method;
> > > +}
> > > +
> > > +static void acpi_dsdt_add_host_bridge_methods(Aml *dev,
> > > + bool
> > > enable_native_pcie_hotplug)
> > > +{
> > > + aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> > > + aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> >
> > These two declarations seem to be very much part of the _OSC build though
> > not
> > within the the method. I 'think' you get left with them later with no
> > users.
> > So move them into the osc build here and they will naturally go away when
> > you move to the generic code.
> >
> > They end up unused in the DSDT at the end of the series.
> >
> > I ran a quick GPEX + pxb-pcie test and we do get the odd mix that the OSC
> > for
> > the GPEX say no native hotplug but the OSC for the PXB allows it.
>
> It's fine for each PXB to have it's own _OSC.
> Also current incarnation of ACPI pcihp doesn't support PXBs at all,
> it would be wrong to enable the on PXBs.
>
> Thus I'd avoid touching CXL related code paths from this series.
>
> I'm working on extending ACPI pcihp to PXBs
> (for the same reason as Eric does for arm/virt, i.e. enable acpi-index
> support there).
> I can add CXL bits then if there is a need/demand for that in CXL land.
Ok. My original motivation for _DSM on CXL was function 5 to stop Linux
messing up
the reenumeration which I know has been rejected upstream for a bunch of
compatibility reasons. Anyhow, that's a future problem.
Thanks,
Jonathan
>
> [...]
>
>
