Re: [RFC 4/5] pci: acpi: add _DSM method to PCI devices

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 26 Jan 2021 06:16:50 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Dec 22, 2020 at 06:39:33PM -0500, Igor Mammedov wrote:
> > Implement _DSM according to:
> >     PCI Firmware Specification 3.1
> >     4.6.7.  DSM for Naming a PCI or PCI Express Device Under
> >             Operating Systems
> > and wire it up to cold and hot-plugged PCI devices.
> > Feature depends on ACPI hotplug being enabled (as that provides
> > PCI devices descriptions in ACPI and MMIO registers that are
> > reused to fetch acpi-index).
> > 
> > acpi-index should work for
> >   - cold plugged NICs:
> >       $QEMU -evice e1000,acpi-index=100  
> >          => 'eno100'  
> >   - hot-plugged
> >       (monitor) device_add e1000,acpi-index=200,id=remove_me  
> >          => 'eno200'  
> >   - replugged
> >       (monitor) device_del remove_me
> >       (monitor) device_add e1000,acpi-index=1  
> >          => 'eno1'  
> > 
> > Windows also sees index under "PCI Label Id" field in properties
> > dialog but otherwise it doesn't seem to have any effect.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/acpi/pci.h |  1 +
> >  hw/acpi/pci.c         | 78 +++++++++++++++++++++++++++++++++++++++++++
> >  hw/i386/acpi-build.c  | 21 ++++++++++--
> >  3 files changed, 97 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> > index bf2a3ed0ba..5e1eb2a96a 100644
> > --- a/include/hw/acpi/pci.h
> > +++ b/include/hw/acpi/pci.h
> > @@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
> >  } AcpiMcfgInfo;
> >  
> >  void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
> > +Aml *aml_pci_device_dsm(void);
> >  #endif
> > diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> > index 07d5101d83..6d49d515d3 100644
> > --- a/hw/acpi/pci.c
> > +++ b/hw/acpi/pci.c
> > @@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> >       AcpiPciHpState *s = opaque;
> >       return s->acpi_index;
> >  }
> > +
> > +Aml *aml_pci_device_dsm(void)
> > +{
> > +    Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;  
> 
> s/UUID/uuid/ I think ...
> 
> And can we move ifctx things to the correct scope?
I'll try to do it.

> 
> > +    Aml *acpi_index = aml_local(0);
> > +    Aml *zero = aml_int(0);
> > +    Aml *bnum = aml_arg(4);
> > +    Aml *sun = aml_arg(5);
> > +
> > +    method = aml_method("PDSM", 6, AML_SERIALIZED);
> > +
> > +    /*
> > +     * PCI Firmware Specification 3.1
> > +     * 4.6.  _DSM Definitions for PCI
> > +     */
> > +    UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> > +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > +    {
> > +        aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
> > +        ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
> > +        {
> > +            uint8_t byte_list[1];
> > +
> > +            ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
> > +            {
> > +                /*
> > +                 * advertise function 7 if device has acpi-index
> > +                 */
> > +                ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
> > +                {
> > +                    byte_list[0] =
> > +                        1 /* have supported functions */ |
> > +                        1 << 7 /* support for function 7 */
> > +                    ;
> > +                    aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +             }
> > +             aml_append(ifctx1, ifctx2);
> > +
> > +             byte_list[0] = 0; /* nothing supported */
> > +             aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
> > +         }
> > +         aml_append(ifctx, ifctx1);
> > +         elsectx = aml_else();
> > +         /*
> > +          * PCI Firmware Specification 3.1
> > +          * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> > +          *        Operating Systems
> > +          */
> > +         ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
> > +         {
> > +             Aml *pkg = aml_package(2);
> > +             Aml *label = aml_local(2);
> > +             Aml *ret = aml_local(1);
> > +
> > +             aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
> > +                 aml_to_decimalstring(acpi_index, NULL), label));
> > +
> > +             aml_append(pkg, zero);
> > +             aml_append(pkg, aml_string("placeholder"));
> > +             aml_append(ifctx1, aml_store(pkg, ret));
> > +             /*
> > +              * update apci-index to actual value
> > +              */
> > +             aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> > +             /*
> > +              * update device label to actual value
> > +              */
> > +             aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
> > +             aml_append(ifctx1, aml_return(ret));
> > +         }
> > +         aml_append(elsectx, ifctx1);
> > +         aml_append(ifctx, elsectx);
> > +    }
> > +    aml_append(method, ifctx);
> > +    return method;
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 27d2958e25..447ad39c35 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> >                      aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> >                  );
> >                  aml_append(dev, method);
> > +                method = aml_method("_DSM", 4, AML_SERIALIZED);
> > +                aml_append(method,
> > +                    aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
> > +                                         aml_arg(2), aml_arg(3),
> > +                                         aml_name("BSEL"), aml_name("_SUN")))
> > +                );
> > +                aml_append(dev, method);
> >                  aml_append(parent_scope, dev);
> >  
> >                  build_append_pcihp_notify_entry(notify_method, slot);
> > @@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> >          dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> >          aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
> >  
> > +        aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > +        method = aml_method("_DSM", 4, AML_SERIALIZED);
> > +        aml_append(method,
> > +           aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
> > +                                aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
> > +        );
> > +        aml_append(dev, method);
> > +
> >          if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
> >              /* add VGA specific AML methods */
> >              int s3d;
> > @@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> >              aml_append(method, aml_return(aml_int(s3d)));
> >              aml_append(dev, method);
> >          } else if (hotplug_enabled_dev) {
> > -            /* add _SUN/_EJ0 to make slot hotpluggable  */
> > -            aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > -
> > +            /* add _EJ0 to make slot hotpluggable  */
> >              method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> >              aml_append(method,
> >                  aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> > @@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
> >      aml_append(method, aml_return(aml_local(0)));
> >      aml_append(scope, method);
> >  
> > +    aml_append(scope, aml_pci_device_dsm());
> > +
> >      aml_append(table, scope);
> >  }
> >  
> > -- 
> > 2.27.0  
>