[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/pxb-pcie: fix PCI Express hotplug support
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] hw/pxb-pcie: fix PCI Express hotplug support |
Date: |
Fri, 3 Feb 2017 00:49:54 +0200 |
On Fri, Feb 03, 2017 at 12:44:17AM +0200, Marcel Apfelbaum wrote:
> On 02/03/2017 12:34 AM, Michael S. Tsirkin wrote:
> > On Fri, Feb 03, 2017 at 12:05:52AM +0200, Marcel Apfelbaum wrote:
> > > Add the missing osc method for pxb-pcie devices
> > >
> > > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > > ---
> > >
> > > Note: The patch is based on the fact that Q35's osc method is quite
> > > generic.
> > >
> > > Thanks,
> > > Marcel
> > >
> > > hw/i386/acpi-build.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 1c928ab..555aab3 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1964,6 +1964,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> > > aml_append(dev, aml_name_decl("_HID",
> > > aml_eisaid("PNP0A03")));
> > > aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> > > + if (pci_bus_is_express(bus)) {
> > > + aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> > > + aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> > > + aml_append(dev, build_q35_osc_method());
> > > + }
> >
>
> Hi Michael,
> Thanks for the review.
>
> > I think we want to move this stuff into build_q35_osc_method:
> > "SUPP" seems to be 0, CTRL should be a variable passed in.
> >
>
> Sure
>
> >
> > > if (numa_node != NUMA_NODE_UNASSIGNED) {
> > > aml_append(dev, aml_name_decl("_PXM",
> > > aml_int(numa_node)));
> >
> >
> > In fact build_q35_osc_method needs to be cleaned up.
> > It stores result in "CTRL" which should be documented.
> >
> > "SUPP" seems to be unused.
> >
> > A bunch of comments missing.
> >
> > More importantly, its an unserialized method but it creates a bunch of
> > fields so attempts to run two of these in parallel will crash.
>
> I don't see how this code will run in parallel. (the code that calls the _OSC
> method)
If the method is not serialized, it should be callable in parallel.
I'm guessing OSPMs do not do it for _OSC.
> > I see the same in ACPI spec which probably means it should be
> > revisited with a critical eye.
> >
>
> I am in PTO next week, then I'll revisit the build_q35_osc_method and (try
> to) clean it up.
>
> Thanks,
> Marcel
>
> >
> > > --
> > > 2.5.5