| Message ID | 20201222233934.451578-5-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc: support user provided NIC naming/indexing | expand |
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; > + 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)); This code needs more comments to explain what it's doing. E.g. what is device label? Spec seems to say the string is optional. Is it relevant somehow? spec also says string must be unique. Do we need to enforce that? > + } > + 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
On Wed, 13 Jan 2021 07:13:04 -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; > > + 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)); > > This code needs more comments to explain what it's doing. > E.g. what is device label? > Spec seems to say the string is optional. Is it relevant somehow? potentially label may be used (with custom rules) to for interface naming or as human readable PCI device description. We probably can drop it, or provide and additional PCIDevice option to let user specify it on CLI along with acpi-index. > > spec also says string must be unique. Do we need to enforce that? I think we should enforce it (Though I intentionally dropped it from RFC) > > + } > > + 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 >
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? > + 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
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 >
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; + 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); }
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(-)