On Fri, 30 May 2025 16:00:19 +0100
Jonathan Cameron <jonathan.came...@huawei.com> wrote:
> 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.
there is some space for _OSC consolidation, but it's not realy related to pcihp,
so I'd rather do it as a separate series on top.
current PCI _DSM and _CXL one (build_cxl_dsm_method) are implementing
different namespaces, there is not much to consolidate there.
If later on CXL would need E5C937D0-3553-4D7A-9117-EA4D19C3434D,
then, I'd say do it at that time and use that moment as an opportunity
to consolidate.
> >
> > > > @@ -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.
yep, let's worry about merging _DSMs when that future comes.
>
> Thanks,
>
> Jonathan
>
> >
> > [...]
> >
> >
>
