qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] pc: support user provided NIC naming/indexing
@ 2021-03-15 18:00 Igor Mammedov
  2021-03-15 18:00 ` [PATCH 1/6] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst


Changes since rfc:
  * make sure that acpi-index unique
  * use dedicated MMIO register for it
  * drop syntetic acpi-label field in _DSM
  * add more comments

Series implements support for 'onboard' naming scheme for network
interfaces (1), which is based on PCI firmware spec and lets user
to explicitly specify index that will be used by guest to name
network interface, ex:
    -device e1000,acpi-index=33
should make guest rename NIC name to 'eno33' where 'eno' is default
prefix for this scheme.

Hope is that it will allow to simplify launching VMs from
template disk images with different set VM configurations
without need to reconfigure guest network intrfaces or
risk of loosing network connectivity.

For more detailed description/examples see patches [3-4/5]

1)
 https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ 

Git repo for testing:
   https://github.com/imammedo/qemu/branches acpi-index-v1


CC: laine@redhat.com
CC: mst@redhat.com
CC: jusual@redhat.com

Igor Mammedov (6):
  tests: acpi: temporary whitelist DSDT changes
  pci: introduce apci-index property for PCI device
  pci: acpi: ensure that acpi-index is unique
  acpi: add aml_to_decimalstring() and aml_call6() helpers
  pci: acpi: add _DSM method to PCI devices
  tests: acpi: update expected blobs

 include/hw/acpi/aml-build.h      |   3 +
 include/hw/acpi/pci.h            |   1 +
 include/hw/acpi/pcihp.h          |   9 ++-
 include/hw/pci/pci.h             |   1 +
 hw/acpi/aml-build.c              |  28 ++++++++
 hw/acpi/pci.c                    |   1 -
 hw/acpi/pcihp.c                  | 104 ++++++++++++++++++++++++++-
 hw/acpi/piix4.c                  |   3 +-
 hw/acpi/trace-events             |   2 +
 hw/i386/acpi-build.c             | 118 +++++++++++++++++++++++++++++--
 hw/pci/pci.c                     |   1 +
 tests/data/acpi/pc/DSDT          | Bin 5065 -> 6002 bytes
 tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7327 bytes
 tests/data/acpi/pc/DSDT.bridge   | Bin 6924 -> 8668 bytes
 tests/data/acpi/pc/DSDT.cphp     | Bin 5529 -> 6466 bytes
 tests/data/acpi/pc/DSDT.dimmpxm  | Bin 6719 -> 7656 bytes
 tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5969 bytes
 tests/data/acpi/pc/DSDT.ipmikcs  | Bin 5137 -> 6074 bytes
 tests/data/acpi/pc/DSDT.memhp    | Bin 6424 -> 7361 bytes
 tests/data/acpi/pc/DSDT.nohpet   | Bin 4923 -> 5860 bytes
 tests/data/acpi/pc/DSDT.numamem  | Bin 5071 -> 6008 bytes
 tests/data/acpi/pc/DSDT.roothp   | Bin 5261 -> 6210 bytes
 22 files changed, 261 insertions(+), 10 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] tests: acpi: temporary whitelist DSDT changes
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
@ 2021-03-15 18:00 ` Igor Mammedov
  2021-03-15 18:00 ` [PATCH 2/6] pci: introduce apci-index property for PCI device Igor Mammedov
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..fddcfc061f 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,12 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT",
+"tests/data/acpi/pc/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.bridge",
+"tests/data/acpi/pc/DSDT.cphp",
+"tests/data/acpi/pc/DSDT.dimmpxm",
+"tests/data/acpi/pc/DSDT.hpbridge",
+"tests/data/acpi/pc/DSDT.hpbrroot",
+"tests/data/acpi/pc/DSDT.ipmikcs",
+"tests/data/acpi/pc/DSDT.memhp",
+"tests/data/acpi/pc/DSDT.numamem",
+"tests/data/acpi/pc/DSDT.roothp",
-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/6] pci: introduce apci-index property for PCI device
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
  2021-03-15 18:00 ` [PATCH 1/6] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
@ 2021-03-15 18:00 ` Igor Mammedov
  2021-04-06  9:47   ` Daniel P. Berrangé
  2021-05-14 13:37   ` Michael S. Tsirkin
  2021-03-15 18:00 ` [PATCH 3/6] pci: acpi: ensure that acpi-index is unique Igor Mammedov
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

In x86/ACPI world, linux distros are using predictable
network interface naming since systemd v197. Which on
QEMU based VMs results into path based naming scheme,
that names network interfaces based on PCI topology.

With itm on has to plug NIC in exactly the same bus/slot,
which was used when disk image was first provisioned/configured
or one risks to loose network configuration due to NIC being
renamed to actually used topology.
That also restricts freedom to reshape PCI configuration of
VM without need to reconfigure used guest image.

systemd also offers "onboard" naming scheme which is
preferred over PCI slot/topology one, provided that
firmware implements:
    "
    PCI Firmware Specification 3.1
    4.6.7.  DSM for Naming a PCI or PCI Express Device Under
            Operating Systems
    "
that allows to assign user defined index to PCI device,
which systemd will use to name NIC. For example, using
  -device e1000,acpi-index=100
guest will rename NIC to 'eno100', where 'eno' is default
prefix for "onboard" naming scheme. This doesn't require
any advance configuration on guest side to com in effect
at 'onboard' scheme takes priority over path based naming.

Hope is that 'acpi-index' it will be easier to consume by
management layer, compared to forcing specific PCI topology
and/or having several disk image templates for different
topologies and will help to simplify process of spawning
VM from the same template without need to reconfigure
guest NIC.

This patch adds, 'acpi-index'* property and wires up
a 32bit register on top of pci hotplug register block
to pass index value to AML code at runtime.
Following patch will add corresponding _DSM code and
wire it up to PCI devices described in ACPI.

*) name comes from linux kernel terminology

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v1:
 *  fix build failure of mips target
 *  move out handling acpi-index into separate MMIO register,
    instead of abusing unused PCI hotplug registers (mst)
 *  default acpi-index property value to 0
 *  set and check for max acpi-index value (systemd limitation)
---
 include/hw/acpi/pcihp.h |  9 +++++--
 include/hw/pci/pci.h    |  1 +
 hw/acpi/pci.c           |  1 -
 hw/acpi/pcihp.c         | 58 +++++++++++++++++++++++++++++++++++++++--
 hw/acpi/piix4.c         |  3 ++-
 hw/acpi/trace-events    |  2 ++
 hw/i386/acpi-build.c    | 13 ++++++++-
 hw/pci/pci.c            |  1 +
 8 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index dfd375820f..2dd90aea30 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -46,6 +46,7 @@ typedef struct AcpiPciHpPciStatus {
 typedef struct AcpiPciHpState {
     AcpiPciHpPciStatus acpi_pcihp_pci_status[ACPI_PCIHP_MAX_HOTPLUG_BUS];
     uint32_t hotplug_select;
+    uint32_t acpi_index;
     PCIBus *root;
     MemoryRegion io;
     bool legacy_piix;
@@ -71,13 +72,17 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off);
 
 extern const VMStateDescription vmstate_acpi_pcihp_pci_status;
 
-#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp) \
+bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id);
+
+#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
         VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
                             test_pcihp), \
         VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
                                   ACPI_PCIHP_MAX_HOTPLUG_BUS, \
                                   test_pcihp, 1, \
                                   vmstate_acpi_pcihp_pci_status, \
-                                  AcpiPciHpPciStatus)
+                                  AcpiPciHpPciStatus), \
+        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
+                            test_acpi_index)
 
 #endif
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 1bc231480f..6be4e0c460 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -359,6 +359,7 @@ struct PCIDevice {
 
     /* ID of standby device in net_failover pair */
     char *failover_pair_id;
+    uint32_t acpi_index;
 };
 
 void pci_register_bar(PCIDevice *pci_dev, int region_num,
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index ec455c3b25..75b1103ec4 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -59,4 +59,3 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
     build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
                  "MCFG", table_data->len - mcfg_start, 1, oem_id, oem_table_id);
 }
-
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 9dc4d3e2db..ceab287bd3 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -39,12 +39,13 @@
 #include "trace.h"
 
 #define ACPI_PCIHP_ADDR 0xae00
-#define ACPI_PCIHP_SIZE 0x0014
+#define ACPI_PCIHP_SIZE 0x0018
 #define PCI_UP_BASE 0x0000
 #define PCI_DOWN_BASE 0x0004
 #define PCI_EJ_BASE 0x0008
 #define PCI_RMV_BASE 0x000c
 #define PCI_SEL_BASE 0x0010
+#define PCI_AIDX_BASE 0x0014
 
 typedef struct AcpiPciHpFind {
     int bsel;
@@ -251,9 +252,13 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off)
     acpi_pcihp_update(s);
 }
 
+#define ONBOARD_INDEX_MAX (16 * 1024 - 1)
+
 void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                    DeviceState *dev, Error **errp)
 {
+    PCIDevice *pdev = PCI_DEVICE(dev);
+
     /* Only hotplugged devices need the hotplug capability. */
     if (dev->hotplugged &&
         acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))) < 0) {
@@ -261,6 +266,17 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                    ACPI_PCIHP_PROP_BSEL "' set");
         return;
     }
+
+    /*
+     * capped by systemd (see: udev-builtin-net_id.c)
+     * as it's the only known user honor it to avoid users
+     * misconfigure QEMU and then wonder why acpi-index doesn't work
+     */
+    if (pdev->acpi_index > ONBOARD_INDEX_MAX) {
+        error_setg(errp, "acpi-index should be less or equal to %u",
+                   ONBOARD_INDEX_MAX);
+        return;
+    }
 }
 
 void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
@@ -347,7 +363,6 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
         trace_acpi_pci_down_read(val);
         break;
     case PCI_EJ_BASE:
-        /* No feature defined yet */
         trace_acpi_pci_features_read(val);
         break;
     case PCI_RMV_BASE:
@@ -357,6 +372,12 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
     case PCI_SEL_BASE:
         val = s->hotplug_select;
         trace_acpi_pci_sel_read(val);
+        break;
+    case PCI_AIDX_BASE:
+        val = s->acpi_index;
+        s->acpi_index = 0;
+        trace_acpi_pci_acpi_index_read(val);
+        break;
     default:
         break;
     }
@@ -367,8 +388,35 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
 static void pci_write(void *opaque, hwaddr addr, uint64_t data,
                       unsigned int size)
 {
+    int slot;
+    PCIBus *bus;
+    BusChild *kid, *next;
     AcpiPciHpState *s = opaque;
+
+    s->acpi_index = 0;
     switch (addr) {
+    case PCI_AIDX_BASE:
+        /*
+         * fetch acpi-index for specified slot so that follow up read from
+         * PCI_AIDX_BASE can return it to guest
+         */
+        slot = ctz32(data);
+
+        if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
+            break;
+        }
+
+        bus = acpi_pcihp_find_hotplug_bus(s, s->hotplug_select);
+        QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
+            Object *o = OBJECT(kid->child);
+            PCIDevice *dev = PCI_DEVICE(o);
+            if (PCI_SLOT(dev->devfn) == slot) {
+                s->acpi_index = object_property_get_uint(o, "acpi-index", NULL);
+                break;
+            }
+        }
+        trace_acpi_pci_acpi_index_write(s->hotplug_select, slot, s->acpi_index);
+        break;
     case PCI_EJ_BASE:
         if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
             break;
@@ -413,6 +461,12 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
                                    OBJ_PROP_FLAG_READ);
 }
 
+bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
+{
+     AcpiPciHpState *s = opaque;
+     return s->acpi_index;
+}
+
 const VMStateDescription vmstate_acpi_pcihp_pci_status = {
     .name = "acpi_pcihp_pci_status",
     .version_id = 1,
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 1efc0ded9f..6056d51667 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -297,7 +297,8 @@ static const VMStateDescription vmstate_acpi = {
             2, vmstate_pci_status,
             struct AcpiPciHpPciStatus),
         VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
-                            vmstate_test_use_acpi_hotplug_bridge),
+                            vmstate_test_use_acpi_hotplug_bridge,
+                            vmstate_acpi_pcihp_use_acpi_index),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription*[]) {
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index f91ced477d..dcc1438f3a 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -41,6 +41,8 @@ acpi_pci_unplug_request(int bsel, int slot) "bsel: %d slot: %d"
 acpi_pci_up_read(uint32_t val) "%" PRIu32
 acpi_pci_down_read(uint32_t val) "%" PRIu32
 acpi_pci_features_read(uint32_t val) "%" PRIu32
+acpi_pci_acpi_index_read(uint32_t val) "%" PRIu32
+acpi_pci_acpi_index_write(unsigned bsel, unsigned slot, uint32_t aidx) "bsel: %u slot: %u aidx: %" PRIu32
 acpi_pci_rmv_read(uint32_t val) "%" PRIu32
 acpi_pci_sel_read(uint32_t val) "%" PRIu32
 acpi_pci_ej_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 442b4629a9..e49fae2bfd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1168,9 +1168,10 @@ static void build_piix4_pci_hotplug(Aml *table)
     aml_append(scope, field);
 
     aml_append(scope,
-        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
+        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x08));
     field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
     aml_append(field, aml_named_field("BNUM", 32));
+    aml_append(field, aml_named_field("PIDX", 32));
     aml_append(scope, field);
 
     aml_append(scope, aml_mutex("BLCK", 0));
@@ -1184,6 +1185,16 @@ static void build_piix4_pci_hotplug(Aml *table)
     aml_append(method, aml_return(aml_int(0)));
     aml_append(scope, method);
 
+    method = aml_method("AIDX", 2, AML_NOTSERIALIZED);
+    aml_append(method, aml_acquire(aml_name("BLCK"), 0xFFFF));
+    aml_append(method, aml_store(aml_arg(0), aml_name("BNUM")));
+    aml_append(method,
+        aml_store(aml_shiftleft(aml_int(1), aml_arg(1)), aml_name("PIDX")));
+    aml_append(method, aml_store(aml_name("PIDX"), aml_local(0)));
+    aml_append(method, aml_release(aml_name("BLCK")));
+    aml_append(method, aml_return(aml_local(0)));
+    aml_append(scope, method);
+
     aml_append(table, scope);
 }
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 0eadcdbc9e..ac9a24889c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -79,6 +79,7 @@ static Property pci_props[] = {
                     QEMU_PCIE_EXTCAP_INIT_BITNR, true),
     DEFINE_PROP_STRING("failover_pair_id", PCIDevice,
                        failover_pair_id),
+    DEFINE_PROP_UINT32("acpi-index",  PCIDevice, acpi_index, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/6] pci: acpi: ensure that acpi-index is unique
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
  2021-03-15 18:00 ` [PATCH 1/6] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
  2021-03-15 18:00 ` [PATCH 2/6] pci: introduce apci-index property for PCI device Igor Mammedov
@ 2021-03-15 18:00 ` Igor Mammedov
  2021-03-15 18:01 ` [PATCH 4/6] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

it helps to avoid device naming conflicts when guest OS is
configured to use acpi-index for naming.
Spec ialso says so:

PCI Firmware Specification Revision 3.2
4.6.7.  _DSM for Naming a PCI or PCI Express Device Under Operating Systems
"
Instance number must be unique under \_SB scope. This instance number does not have to
be sequential in a given system configuration.
"

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/pcihp.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index ceab287bd3..f4cb3c979d 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -52,6 +52,21 @@ typedef struct AcpiPciHpFind {
     PCIBus *bus;
 } AcpiPciHpFind;
 
+static gint g_cmp_uint32(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+    return a - b;
+}
+
+static GSequence *pci_acpi_index_list(void)
+{
+    static GSequence *used_acpi_index_list;
+
+    if (!used_acpi_index_list) {
+        used_acpi_index_list = g_sequence_new(NULL);
+    }
+    return used_acpi_index_list;
+}
+
 static int acpi_pcihp_get_bsel(PCIBus *bus)
 {
     Error *local_err = NULL;
@@ -277,6 +292,23 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                    ONBOARD_INDEX_MAX);
         return;
     }
+
+    /*
+     * make sure that acpi-index is unique across all present PCI devices
+     */
+    if (pdev->acpi_index) {
+        GSequence *used_indexes = pci_acpi_index_list();
+
+        if (g_sequence_lookup(used_indexes, GINT_TO_POINTER(pdev->acpi_index),
+                              g_cmp_uint32, NULL)) {
+            error_setg(errp, "a PCI device with acpi-index = %" PRIu32
+                       " already exist", pdev->acpi_index);
+            return;
+        }
+        g_sequence_insert_sorted(used_indexes,
+                                 GINT_TO_POINTER(pdev->acpi_index),
+                                 g_cmp_uint32, NULL);
+    }
 }
 
 void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
@@ -315,8 +347,22 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
 void acpi_pcihp_device_unplug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
                                  DeviceState *dev, Error **errp)
 {
+    PCIDevice *pdev = PCI_DEVICE(dev);
+
     trace_acpi_pci_unplug(PCI_SLOT(PCI_DEVICE(dev)->devfn),
                           acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))));
+
+    /*
+     * clean up acpi-index so it could reused by another device
+     */
+    if (pdev->acpi_index) {
+        GSequence *used_indexes = pci_acpi_index_list();
+
+        g_sequence_remove(g_sequence_lookup(used_indexes,
+                          GINT_TO_POINTER(pdev->acpi_index),
+                          g_cmp_uint32, NULL));
+    }
+
     qdev_unrealize(dev);
 }
 
-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/6] acpi: add aml_to_decimalstring() and aml_call6() helpers
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
                   ` (2 preceding siblings ...)
  2021-03-15 18:00 ` [PATCH 3/6] pci: acpi: ensure that acpi-index is unique Igor Mammedov
@ 2021-03-15 18:01 ` Igor Mammedov
  2021-03-15 18:01 ` [PATCH 5/6] pci: acpi: add _DSM method to PCI devices Igor Mammedov
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

it will be used by follow up patches

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/acpi/aml-build.h |  3 +++
 hw/acpi/aml-build.c         | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 380d3e3924..e652106e26 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -301,6 +301,7 @@ Aml *aml_arg(int pos);
 Aml *aml_to_integer(Aml *arg);
 Aml *aml_to_hexstring(Aml *src, Aml *dst);
 Aml *aml_to_buffer(Aml *src, Aml *dst);
+Aml *aml_to_decimalstring(Aml *src, Aml *dst);
 Aml *aml_store(Aml *val, Aml *target);
 Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst);
 Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst);
@@ -323,6 +324,8 @@ Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
 Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
 Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
                Aml *arg5);
+Aml *aml_call6(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5, Aml *arg6);
 Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
                   AmlLevelAndEdge edge_level,
                   AmlActiveHighAndLow active_level, AmlShared shared,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index a2cd7a5830..d33ce8954a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -634,6 +634,19 @@ Aml *aml_to_buffer(Aml *src, Aml *dst)
     return var;
 }
 
+/* ACPI 2.0a: 17.2.4.4 Type 2 Opcodes Encoding: DefToDecimalString */
+Aml *aml_to_decimalstring(Aml *src, Aml *dst)
+{
+    Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);
+    aml_append(var, src);
+    if (dst) {
+        aml_append(var, dst);
+    } else {
+        build_append_byte(var->buf, 0x00 /* NullNameOp */);
+    }
+    return var;
+}
+
 /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefStore */
 Aml *aml_store(Aml *val, Aml *target)
 {
@@ -835,6 +848,21 @@ Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
     return var;
 }
 
+/* helper to call method with 5 arguments */
+Aml *aml_call6(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5, Aml *arg6)
+{
+    Aml *var = aml_alloc();
+    build_append_namestring(var->buf, "%s", method);
+    aml_append(var, arg1);
+    aml_append(var, arg2);
+    aml_append(var, arg3);
+    aml_append(var, arg4);
+    aml_append(var, arg5);
+    aml_append(var, arg6);
+    return var;
+}
+
 /*
  * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor
  * Type 1, Large Item Name 0xC
-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/6] pci: acpi: add _DSM method to PCI devices
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
                   ` (3 preceding siblings ...)
  2021-03-15 18:01 ` [PATCH 4/6] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
@ 2021-03-15 18:01 ` Igor Mammedov
  2021-03-15 18:01 ` [PATCH 6/6] tests: acpi: update expected blobs Igor Mammedov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

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 -device e1000,acpi-index=100
         => 'eno100'
  - hot-plugged
      (monitor) device_add e1000,acpi-index=200,id=remove_me
         => 'eno200'
  - re-plugged
      (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>
---
v1:
   * drop optional text label
   * do not report capability if PIDX is not supported (i.e. old QEMU)
---
 include/hw/acpi/pci.h |   1 +
 hw/i386/acpi-build.c  | 105 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index e514f179d8..b5deee0a9d 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -35,4 +35,5 @@ typedef struct AcpiMcfgInfo {
 
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
                 const char *oem_id, const char *oem_table_id);
+Aml *aml_pci_device_dsm(void);
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e49fae2bfd..d14a5fab62 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -397,6 +397,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);
@@ -424,6 +431,16 @@ 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)));
 
+        if (bsel) {
+            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;
@@ -446,9 +463,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"))
@@ -511,6 +526,88 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
     qobject_unref(bsel);
 }
 
+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 *func = aml_arg(2);
+    Aml *rev = aml_arg(1);
+    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(func, zero));
+        {
+            uint8_t byte_list[1];
+
+            ifctx2 = aml_if(aml_equal(rev, aml_int(2)));
+            {
+                /*
+                 * advertise function 7 if device has acpi-index
+                 * acpi_index values:
+                 *            0: not present (default value)
+                 *     FFFFFFFF: not supported (old QEMU without PIDX reg)
+                 *        other: device's acpi-index
+                 */
+                ifctx3 = aml_if(aml_lnot(
+                    aml_or(aml_equal(acpi_index, zero),
+                           aml_equal(acpi_index, aml_int(0xFFFFFFFF)), NULL)
+                ));
+                {
+                    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(func, aml_int(7)));
+         {
+             Aml *pkg = aml_package(2);
+             Aml *ret = aml_local(1);
+
+             aml_append(pkg, zero);
+             /*
+              * optional, if not impl. should return null string
+              */
+             aml_append(pkg, aml_string("%s", ""));
+             aml_append(ifctx1, aml_store(pkg, ret));
+             /*
+              * update apci-index to actual value
+              */
+             aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
+             aml_append(ifctx1, aml_return(ret));
+         }
+         aml_append(elsectx, ifctx1);
+         aml_append(ifctx, elsectx);
+    }
+    aml_append(method, ifctx);
+    return method;
+}
+
 /**
  * build_prt_entry:
  * @link_name: link name for PCI route entry
@@ -1195,6 +1292,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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/6] tests: acpi: update expected blobs
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
                   ` (4 preceding siblings ...)
  2021-03-15 18:01 ` [PATCH 5/6] pci: acpi: add _DSM method to PCI devices Igor Mammedov
@ 2021-03-15 18:01 ` Igor Mammedov
  2021-03-15 18:36 ` [PATCH 0/6] pc: support user provided NIC naming/indexing no-reply
  2021-05-14 13:32 ` Michael S. Tsirkin
  7 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 18:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: jusual, laine, mst

expected changes are:
 * larger BNMR operation region
 * new PIDX field and method to fetch acpi-index
 * PDSM method that implements PCI device _DSM +
   per device _DSM that calls PDSM

@@ -221,10 +221,11 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
             B0EJ,   32
         }

-        OperationRegion (BNMR, SystemIO, 0xAE10, 0x04)
+        OperationRegion (BNMR, SystemIO, 0xAE10, 0x08)
         Field (BNMR, DWordAcc, NoLock, WriteAsZeros)
         {
-            BNUM,   32
+            BNUM,   32,
+            PIDX,   32
         }

         Mutex (BLCK, 0x00)
@@ -236,6 +237,52 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
             Release (BLCK)
             Return (Zero)
         }
+
+        Method (AIDX, 2, NotSerialized)
+        {
+            Acquire (BLCK, 0xFFFF)
+            BNUM = Arg0
+            PIDX = (One << Arg1)
+            Local0 = PIDX /* \_SB_.PCI0.PIDX */
+            Release (BLCK)
+            Return (Local0)
+        }
+
+        Method (PDSM, 6, Serialized)
+        {
+            If ((Arg0 == ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d") /* Device Labeling Interface */))
+            {
+                Local0 = AIDX (Arg4, Arg5)
+                If ((Arg2 == Zero))
+                {
+                    If ((Arg1 == 0x02))
+                    {
+                        If (!((Local0 == Zero) | (Local0 == 0xFFFFFFFF)))
+                        {
+                            Return (Buffer (One)
+                            {
+                                 0x81                                             // .
+                            })
+                        }
+                    }
+
+                    Return (Buffer (One)
+                    {
+                         0x00                                             // .
+                    })
+                }
+                ElseIf ((Arg2 == 0x07))
+                {
+                    Local1 = Package (0x02)
+                        {
+                            Zero,
+                            ""
+                        }
+                    Local1 [Zero] = Local0
+                    Return (Local1)
+                }
+            }
+        }
     }

     Scope (_SB)
@@ -785,7 +832,7 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
                     0xAE00,             // Range Minimum
                     0xAE00,             // Range Maximum
                     0x01,               // Alignment
-                    0x14,               // Length
+                    0x18,               // Length
                     )
             })
         }
@@ -842,11 +889,22 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
             Device (S00)
             {
                 Name (_ADR, Zero)  // _ADR: Address
+                Name (_SUN, Zero)  // _SUN: Slot User Number
+                Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                {
+                    Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                }
             }

             Device (S10)
             {
                 Name (_ADR, 0x00020000)  // _ADR: Address
+                Name (_SUN, 0x02)  // _SUN: Slot User Number
+                Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
+                {
+                    Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+                }
+
                 Method (_S1D, 0, NotSerialized)  // _S1D: S1 Device State
                 {
                     Return (Zero)
[...]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h |  11 -----------
 tests/data/acpi/pc/DSDT                     | Bin 5065 -> 6002 bytes
 tests/data/acpi/pc/DSDT.acpihmat            | Bin 6390 -> 7327 bytes
 tests/data/acpi/pc/DSDT.bridge              | Bin 6924 -> 8668 bytes
 tests/data/acpi/pc/DSDT.cphp                | Bin 5529 -> 6466 bytes
 tests/data/acpi/pc/DSDT.dimmpxm             | Bin 6719 -> 7656 bytes
 tests/data/acpi/pc/DSDT.hpbridge            | Bin 5026 -> 5969 bytes
 tests/data/acpi/pc/DSDT.ipmikcs             | Bin 5137 -> 6074 bytes
 tests/data/acpi/pc/DSDT.memhp               | Bin 6424 -> 7361 bytes
 tests/data/acpi/pc/DSDT.nohpet              | Bin 4923 -> 5860 bytes
 tests/data/acpi/pc/DSDT.numamem             | Bin 5071 -> 6008 bytes
 tests/data/acpi/pc/DSDT.roothp              | Bin 5261 -> 6210 bytes
 12 files changed, 11 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index fddcfc061f..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,12 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/DSDT",
-"tests/data/acpi/pc/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.bridge",
-"tests/data/acpi/pc/DSDT.cphp",
-"tests/data/acpi/pc/DSDT.dimmpxm",
-"tests/data/acpi/pc/DSDT.hpbridge",
-"tests/data/acpi/pc/DSDT.hpbrroot",
-"tests/data/acpi/pc/DSDT.ipmikcs",
-"tests/data/acpi/pc/DSDT.memhp",
-"tests/data/acpi/pc/DSDT.numamem",
-"tests/data/acpi/pc/DSDT.roothp",
diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT
index 11ef89bd322271ee30f3971b880dadfda565d413..b9dd9b38e4ef720636ba19ccbdf262de8a6439d5 100644
GIT binary patch
delta 1301
zcmajfKTE?<5CHH?>Yo~-P1<Nx90b8ygwiVFs7)eTZ9*{?ga}EuwpIz<6r|Y6RUaZE
zLJIi;($zs!q;7r%2SF4O{Q|X5I|=39a2)S`@3>puW8^!=sfM~y4g)ZgSKKwr>LO{d
zYeBzxD9F0DuE=^;8_%TFO)~C_Rix=;D`m|IIjyWUn@*mVojs-ilsGilW`q_!TT**6
zs-X?I>28u2L!9t5|M+6e?Q6&3H*Mrj(Hz>Vv}3yyqzLW^DR8VCIyoRV5Swqd4tS!E
zIivw<VRw)egIuo@>}H`Ka*lnJ1MqTn-N*Ma!^GqZm6h6-WyqGWSj#A>xL-Dw(qLAj
zMm%o$t)#jRe#^+}Acd{gT)ao8%NL5<)X{=jUF|XwZOa3&f1YDIJ&;=c5-NB=lNnWL
zXS4Lmtjb+P>Yp0x^!OJnV4#SBQw-c-;2i@)1IQLh43sc%hJjlQd|+TWfNY^)U>5`D
v7`Vs4Ck93ZWQ!#XI2gFVzyk)pFfb+}TP$OsjDbrGG{k7&-$zh?6`z?OB0Yu?

delta 345
zcmeyQcT%0pCD<k8q%Z>mqyI*(Jxoj<9FtEldGoSFH*!1q`35;V`GxvUR%O=P{Dhf@
zlTl>yN1jY0*MgY%V5fKi-~0e)PXi98U{@c8=qB!91A};uct@8YAV(|M5X#|UU|?dH
ze2Q0=QFHQ3UJd5xCe6vZd=R=2N^gbIKcTcOKSaI}N*{&NoB|MWUno5hN?(Q2l7bNV
zSSY;^N<W3tnnDoyTqwN}N`HmYmckJES}1)GO0$YU#66*OFO<FrrA0*{@{yvNn`epM
GU<3fvFk-6!

diff --git a/tests/data/acpi/pc/DSDT.acpihmat b/tests/data/acpi/pc/DSDT.acpihmat
index c561e91be79f80dbea1cf4eb0f8658541a3aab2f..cba5a1dcb0464e56753bc0b931a4dd2e3b209787 100644
GIT binary patch
delta 1301
zcmajfPfG$(5CHJmHA@yV|Ey^t5qKyGLW_uQxw}%PyGWZ5Y_RK1&0>iz1tEIsTJI1M
z;R^c&qHCv$uugr2h%P}yL|x5So&uS77=|~$H_Tk#ccQVtNvgEghyXCp7u*%gDn8O;
zGk%v=^9qW*Dhf(o9?zwvbu#XluJTjKPReRgAuDFuZL>(r<{nc5N=zCIZGskUgO~bq
zk}7+wtdkUd0nYn4fB&M+?aN1@S9#<{lug>JvSnCNQU+^;lsMK393Kz^h~aK`4%j{i
zQ=<WzAuCAAer~|@J6RZnf@vHU0PMo0;#lx7VRGmzRhOF^x+>`2#b!=S$6bP&%7Rvp
zsqwh((v#9ExO6qI03XsjbGdCgU9nW&q>dJ}`bwXg=vp@b`{y~f*8{%OFCl>qG?|gS
z-E5X#X?3BGB>j_-UXOo41p_A-xW>R62EqYkND%|u7&yhiEe75(Fv=raq%dG&;0yzI
w82G?IB#3OWgn=CloMWJcflmyK4I^8mF|dn)1_mB5@HHG8`u7oB218HG4_Q%&Gynhq

delta 345
zcmbPl`OT2aCD<k8n*;*`BhN;zJxoj<9FtEldGoSFH*!1q`35;V`GxvUR%O=P{Dj$(
zk5Oc@oKU8bYe7tWuv5H%Z+?KYrvZmku&WP4bQ5>5fk8Y+yrWAHkfRlB2<7lFFfcJp
z{w^fTs5x0oSc5sbNpo@(l%5BrA3<p~5r}*ilwJp=KS60TQHXpMl->uWnZzLCZcw@l
zN}q$$LgEnlFep6>O5cOhN)izHG$_3aO232BMv@TuGAO+ZO8<k>PErv0HYv@`$E9vC
F0szp9V*vmF

diff --git a/tests/data/acpi/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
index a234075518fa8e187349d64c313779cc25db8299..a9b4d5659457f6de30b993962bce673c9413d81d 100644
GIT binary patch
delta 2375
zcmeA%yW`B|66_LkN0EVn@y153Jxok~f|E}$c?)qwHwrlU`35;V`Gxu_1bDhcOjcml
zs~6FB1TvYTm7RQ?z5o9&$N)(fR5E5VfRq$~h=gc$kmQmC5f`=qmtbGM1wO2kGX#aX
z1TL7Ld>U+8<vUUQm9OMsXWs&lSvk203zR2kF)WaroXN$sKzLH^<OGJv2|WLSfMJOs
zGh-u&U|1-=Ky-4J5Epxa5GxY{FGFHMLPsJ4!;-|!e#|_aj1rThc{1ypJQ!o*gPr09
zI^7uqoIMRVoPu3_7^0grgAEMgIpQ5%f*3gBgG2ooM8xBPF5_7Oa$iPfRyIf-NFut)
zizV0)s+Na=feE6XiwTzo5uka7E(}W;KnxEf7|R&Mh;H&-6>Mk$u^4C=Gt@9<T!w+2
z%NX5cGg*gMSq7J?=q5*&$w9p8coaCXOs?V8z^`CAAqA}>1WedKNWpDF3fM#mSYb#=
zK@1@U?SvHk=Of^TyM#>O5F=nh13v);afD3hB&2{*fPfYE37NnpPQV0HLJATHDd;Ao
zU>hL?4+$yYks#m;b3zK%3lcD)myiiN2q`cSB47odq$WRF3AMReh?9v4n6XE*C9sqR
XhQnyKgoMLrwuFSk0A))($$N|dt?<SV

delta 619
zcmccP++)V&66_MfBhA3TIDI469wsIaj>#vOym?uo8@ZkQe1n{w{6c*vt1|0te!|Sd
z$tW`UBTuH0$CH@&V5fKihbIBfo(3FF!LB|G(M{aJ1_tpQ@s2J*K#o?hA(X?zz`(>X
z`3$eDq<>kkp#?+*P>`7ctdTLgNoBGkpE7fFlg8u(K6N0iIe9Uk2AF;hrM3AX{Cp_A
z8A^YL($)eH`FbdQ7)rAXLd3nHbU&2745h_|Ao9^rdOnnX45ihDA@bQ!dOei>45iIQ
uAoA5vdOwtA7KMnrL+NfPeI7~+i$UbW#WWYs5MyH0oIH$uxS3tyDkA`KW3MOx

diff --git a/tests/data/acpi/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
index 6ac47a7d1001c711b957f8e28cab1a143d2cf65f..8d86155e275aa688f8767dd92c4b9df08b4a18ad 100644
GIT binary patch
delta 1301
zcmajfPfG$(5CHJmRZAArT~{%oL(ri_2sI*z2wOMkkGqJQ5NxpPA#$x?uLU7`icZyw
zsM`wr1;R^E5!Iz`eSj`OMM&SleB~*Kd52+m^LxY0<(((*Gn}l+D^eVQIk@1iSymOu
zBfA{->vdjIm10g(OUiIzQC=m(p7B~FIoC~DeMHLVmOCB0Ld*6JV*w>L4W>Ryi>?`w
z`wFtA1f9H(R0NUZpQoPQwaH!ODArcu4>`rAy|%VZXM|M2i4%)s9r1XNm_W=xr+dH^
zJWQPiXvUl<sfM|JJM3d&0Lr$xUj}f?E!FeFF@q!-v}y~@HA9mOKHDth7E^vno6m#Z
zNNA~);Wy^wBKQrhq(UTSbmt1Vbh?UF-K35d^v04`OZRL5fc^Cx``?2|w_jQY7if}^
z`Cc|buk?oGC2zs8q5mHLfKUhl(->ICz##^%Fz|r^fk(EO!N3Lvjxlh9flmyG0<y&{
x1}qGmVBi)5Z45-i$QD@)Y+~RP19uqs!az(!wvaHeje#>U5&H8Hv@XS0<{JoRhA995

delta 345
zcmX?PG*g?)CD<iorYHjg<K~TAdzhF!I3}N9^5$iUZsd0I^9^!#@(cBytjesn`3dtK
zE=G~b2l+CMTnl32gPr09eDedGJq<XVf?a(WqMNvb4GiKr;vHRrfE=x0Lnw!bfq{u(
z@+^K?M$O4P`8AlMn=~i?h0=}!5PmC^J_)6H1tH>rP<kqqz6qseg&^{YP<knpehH;@
zg(32VP<ktr{t2aRMIiExQ2Hp8<`jj9`$Fl7Q2HvAmK1}?$3p3aQ2Hs9))d#=oF#sP
F5de9{W0U{@

diff --git a/tests/data/acpi/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
index d24377279c307adeaba2b98aba20677872dfbbda..e00a447f92b27f9a91be802eb11fe89dc0457e20 100644
GIT binary patch
delta 1291
zcmaje-75r96aerucYSPQ?bi&uic<T)rnKZMBxXnX*qOE+N{!5HlHEnkgBK*(CrQHn
z2WD&j0Z9__;DtB-1B5pJ#F`sVYCHGgbnE=ix#v9G`@ZrBFKhB#xeb8ZJm+r-QA6~R
z%!a-Ceo#`COj1(kmG<<syg=JM-Fsr!gnK1=OiCqZD-|op($*H22SzLwOud689aEI+
z(z2%b?Nlwzg#<qMJoxmf^=~Vuk#D8tA*oob*ZPKO$7liU7MkaYEu8IA6R7E{xCfk&
zhudcXN)fw>7Q%eJ6|N=F2wBTK$O1Uo3)NE$a8dda&hJf?77a}@g5#xhaysFaw8<3c
z#kiJ87+zyS&Vbj@=2Z|QhC7$fVbkUEg*E0_LNCtLXgz<{2S9#3PpTD&ZoeKG9AKAT
zneGUFBGu4Cp|iDGiC%QUB?ewG&>$jD3}7IOfg=oDVc;DDjZMfFLl{`bz%d4{G4O$b
yKr^z%Fa}mIaDssw418fA6hXEa#lR{C&M<I?fieb!D6+*^G#>c({V$fHZ`=<Gs)Z*2

delta 345
zcmaE1z2Ah(CD<jzUW$Q%an44rJxoj<9FtEldGoSFH*!1q`35;V`GxvUR%O=P{DirQ
zpHXCTx^SkEYe7tWuv5H%Z+?KYrvZmku&WP4bQ5>5fk8Y+yrWAHkfRlB2<7lFFfcJp
zb`X(e)SMhIqQM;9q&ayplztASwM8M~`A~W@l>QE-t;Hbn^-%gSlx7!)h<iinekgqz
zN{dTC<fEbVd?@`GN~=pk<g=mldMN!FN}EeT<g20dekjc>4H0*T(%n${ytL+K0ht?&
E09A8hhX4Qo

diff --git a/tests/data/acpi/pc/DSDT.hpbridge b/tests/data/acpi/pc/DSDT.hpbridge
index 9dfac45eab12b680bc963d0528553a7149a378cc..5d8ba195055f2eda74223323baeb88390ea36739 100644
GIT binary patch
delta 1315
zcmajf&o9GZ7zgmDZ~d62SZ6X42Z_imi)ACik**?GYxW|NO4{AFA?X#7Wpi=my;IVN
zh)BfM&4r5)#M!~c-yo)M>?Gmo;mwmipMKxxx%4>tn2<E8EzCy%gttrSnh--_w`4XH
zFe^dT&~s_kSk!y7)7moY6%*xfe8Mk@*{5dGv$dL2;B99^C;}%AKWsC~o1Puknz9<z
z+uTeeD<~aO@L}}sosMkkdyx;l`zEbBJZo*;cKcWf+-_Esh}*Hh&1_({T=O6B6j7+~
z1CAnYCo6@dW+&80pcV3ty_*N{@>hl^w+LO#l#Au5!xf9FR&eeho1RVvR63afvl^qx
zq!q9xv>XI1x@bT+V)=8)9-ppIEUj|K8)kK;fu`zK1|UCPB7c1l_VZF2c)*JxZM<F$
z^H*jyK~rNt_xf34Jxly6%dDymtiN@j=U<S;z!nBhFz|?hPYguckS%f;*v7yq2A(kR
zg@IT*vc&=h${0AqzzYKQgUF0=3@l-wf`M}kykdYT$QFYbSjNB(1}>CX`>)@^)xGj8
Fd;_9>h+F^w

delta 354
zcmcbpw@97KCD<iokuU=TW5-6WJxoj<9FtEldGoSFH*!1q`35;V`GxvUR%O=P{Dhf@
zlTl>yN1jY0zucJkV5fKix7+|{PXi98U{@c8=qB!91A};uct@8YAV(|M5X#|UU|?dH
ze2Q0Ah(FlS0-^vY#tc-XIa!iVgBe7}Lg|H2`YDvw<cG-TLg|fA`YV*S6oAOrLg|B0
znpF@Y?g^!Pq4Y&4Eh+?&kA%{5q4Yy2ttt$W&xF!zq4Y;6Z7KqhuY}Tjp){i?MBG(W
Kb90;MX+{7Q6k^H%

diff --git a/tests/data/acpi/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
index 1814f291b704d737d3578b83fbcc6e090384943a..01e53bd436698db6f6adfff584ec56cb99074a5f 100644
GIT binary patch
delta 1291
zcmaje!Artm6aetI&1#Klb1h8h5Ol~0$_kNgITvI)k(Lll+Kdir1-ll6=%pee?1Rpe
z_79X1;k8q@{(%UIF8vd>k2(eNzQg0e`@P3|hp!duE()5VZFeI8O!r<0aBMzIBrYrS
zW<yf*`c_)a7xn2vM%y73ZlSKsr~H&PXVhFe+wVIST6T_^Dp2CkV4JhF=-G-kT+j?X
z<mQG*B|I)jZ^_p$BXOd)BHw!SC9OMj*5Q%u&X5|oQBoCHcl_p**g$Nt??3Ru9Mhly
z+7Wkx)MR1Ak%w3aLfNs;%K%=vbIXaN%p|!JtM!d`$ud++T5A{5nK-W+>p3u+b4EOF
z@m5OP0^Tx;c~ByjznAFI?JCvU0d=%sHa7#t(w`Lp*k3QOgBlcn-jW6$&}3O7dN9PQ
z0a6OarUtc`O&^?L-~j{g7~n(54l5Yg#lQsy9x?EN0bvZeB7p%L1D6<h!axrLqJ&(L
q#K0Z~t}yV7flmyC!pIe?7}&?aH3qsE_`-lB&yD^2{++noV}1aHiGk(-

delta 369
zcmdm`KT(6rCD<iIP=tYj@zF*u7G@?7j>!Vd-n=Z)joeOtzCq4TexbgTbD8xv>#*=}
zGKx&L<jo9lEr^K^c8VA9%@1()G~jRwcJ*P1ZsHC$Fo@@fcXSB?a<qaCp&T9t1||j(
zj`(0h7ltKZh7pWm3}Qq#X-+oe(_jYCrBHe&l>Q5)9r+>htx)<Tl;#zHhzCOHsZjbR
zl$I5Q$R|SSrBM1Ml-3o3$QMHCtx)<Wl(rRy$TvdiqfnYt1S0MWr6)q^t58}}6e1rB
Or5B27Zhj<sgAo8X?`0?e

diff --git a/tests/data/acpi/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
index 3c81339d397969e954c94eb03f2654c57e024a6e..b8103799b45224c08344369931b87cf3b7797d7e 100644
GIT binary patch
delta 1301
zcmajfKTE?<5CHH?S~Y}fn>IBqI0z0Zg0vMWiijo={L_SD6od##5wU3%vMNZilar`V
z#4&|TZqi9qq`2tjR}fT0{0_BGI|*`cIF5I}cib)SBGZ`QRYP5A^Z_u5EB=;cb%8vx
zOHs}|h{(FOtjPMR)}LQc*GPY0v?7kowo=v{kaNmXv*{LT+1+Ivpv0xYHV0|Zw?(xx
zuNqp9m+K%!A;w3Zvrliv_?~tmeQL=EMRRGd?H$`2ASLjU#Nk;lcDhe&Aa<zPI^YWd
z=70vMNnV_kqI{Pd?O>rB3a))v0PqWSJz(}Q334eomHFDbWyn@!rj}P0Qk-ne<-n{C
z8>y7VS+nXgaF(&EgD6?8xk5gjuIQA?)X{=jT?`uOwhaNWzn*9Rdmy&@rB(2OCK)x-
z&c^7KS(SriBRbUg-{T)}gn?@eyka06LuQ!7z$ONcF>r%{cMJ$3vc(hzwlHvtfjbO*
vU?3JpwwT6%gMl*)++*Mq1MyyDix~`*F>sE71_r(`AW6gFKOaFoE4?t^B&CLx

delta 344
zcmX?TIm3v{CD<iILXv@j(P1Ok9wsIaj>#vOym?uo8@ZkQe1n{w{6c*vt1|0te!?8h
z$0#z{K`7J6wIC)w*ePDXH$TAH(}2S%*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>C)
zOA5;}YEHHg)?kiq(wtlar4K-977>WJ2bAuC(ifn#h$uup0!q(;(hs1tiWo#b14^%f
z(jTC-i8w^Q0!r_J(32z}!Y)v{14^HP(gKnY`4A{Q14`e4(h5=#`4lL<LP~S<OQ{=-
E0EOXTga7~l

diff --git a/tests/data/acpi/pc/DSDT.nohpet b/tests/data/acpi/pc/DSDT.nohpet
index d7d21be070c3b879e558193cbf8ded5a64c15eda..d4f0050533f970128774f825274177096a46c3b8 100644
GIT binary patch
delta 1283
zcmdn3_C%M<CD<k8i5LR|<A;r0dQ41yf|IS7yoETT8wH&Fe1n{w{6c*d0z6$JCQoA0
zV-wMK1ThydiPVF{3P40cw7Qdzv-gq&5f`=qmtbGM1wO2kGX#aX1TL7Ld>U+8<vUUQ
zm9OMsXWs&l=A7Jw1<I4N7#2uQ&g5cRAUvsdastET1fKsuz_3J+nXwT>Ff0^bAUZjV
zi@iXIm5G6uA+aE#Bawk&Nuq$CS$wclyujuYOe~y?5|htzXV!Zxi~)-|EDUh=G~jRw
zcJ*P1Zqf`kFo@@fcXSD2;D`?n^<xkbj|V!BX9>uW8JStxAax*#=q4|gU_+={9tH*`
zh<Yw2TpC1x<{7#$EMWjKj9ey{^YAe_u}p5`(U8HVCAtZyU?U*~cL*t9=Oti;5g`S!
zgcNiTQm~njf_sD%aPkrGg$W@A@q`p~5mK;~kb(z<6mats@P!#61&M?d^bk_8osfb@
sgcR@!5b%WsAqB~V6!Z~Nu#=F2CxjI63li{!m7pd+TK3-@CHRmL0O=5an*aa+

delta 345
zcmaE&yIYORCD<jzT9|=>QDP&P9ut!X$7CxeZ(f$@Ms6oR-ymlvzfj-F)0y-(hcWYT
zGKx%2=E*d2Er^K^c8VA9%@1()G~jRwcJ*P1ZsHC$Fo@@fcXSB?a<qaCp&T9t1}28d
zHoUTonv<h>HJGEDG$+r8(vP9EIv+$l8%nQ-(x0KUIX^_c8cOen(#!%7ad#-)4W-XR
zX<<Q#d^nVz4W;ixX=NdZd^(g~4W-{hX=7oCd^wcf4W<7>X=f3Ld^?mrEuy)ZNAw0G
E0LKMiz5oCK

diff --git a/tests/data/acpi/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
index 195f8da900c5fc56c504adfef756af8f74f5823d..8632dfe8a8bdd991871a1e633162eb9a2e1497ea 100644
GIT binary patch
delta 1291
zcmajePfG$(5CHJmXO?aLTQjVXkUE$IqCs@6b)!so5jP>&VAq>k!4eS`gygAfy<<cx
z?H7oy-GZoy@+)-c(jkb5y4gPJ6v({8FfhLvW)ANu{+;9{Ra&V>0J!!Ae?^EQ&|8uV
zd9;I|sK~{PsFdVbAuFxYm@C-`Pt7|i(I&)vCfDhh6;?KPxhgPXvS4U&R<w<<)LW2L
z*>C0Dv?2`h!S?Lki#oF>A4Oi}k?V|XvRNCOhBZNJV2#i!PprY?eQE$TydCF(Ex5P?
z7N8ZehG;Fs_n9F#0Uwl2<FE{1moF7psGl3AWpA~y)LPS3Q4h|w3Yl!mBdQB|(3%M~
zmC`->yi^2_u9g%CM|5W|ug$irRBP+Zv4YlI?orc!)(b#>Jx{tdgq?Y532b1=v_uDd
zej@hJQQu^=TZ`6ozybzL44h!#1_SRHi29K$7BR4efm00JV&DS<V*%ufEC#kQaE5_<
t418iB7DTSdVPFRXEet$h;0pue0&<0nfjS1xG4LoP0{_1M<(u%#{QxfMgw6l}

delta 345
zcmeyNcV3;#CD<k8yf6a;<Ijy;dzhF!I3}N9^5$iUZsd0I^9^!#@(cBytjesn`3bWS
zC!@&Z?>w1Ct_3mi!A|i4zWD*po(3FF!LB|G(M{aJ1_tpQ@s2J*K#o?hA(X?zz`(>X
z`2w#jqvquIyc*2WO`4O9`5<&Tl->=c|3hhKeu#WKls*lm`2`^2!BBcSl)eq6<pm+~
z$xwPZlzt7R^@Skv#ZY=Xl>QB+?S&!o%~1L{l;#$Ji2Fn7$x!+_l$I8S$j6InZeAdI
GgAo8NGGr|P

diff --git a/tests/data/acpi/pc/DSDT.roothp b/tests/data/acpi/pc/DSDT.roothp
index 1d0a2c2f3cc4bfac75948d2ed6a69403cd18379b..cee3b4d80b51ad30153953ace46127923ce8b271 100644
GIT binary patch
delta 1279
zcmeCxJY>M-66_M<B*DPI=(&+=4-=E0;N%lb-a;JFjRH=7zCq4Texbe!0iG@qlNFft
z>P562flQ`oWhWnJ@BjY`GC<M=m5iATASDGLA|YBGB)KF(#Dy)uCD@m5fe-8C3_)Qo
zfeYp*p9Y&&`A!sn<tusE*|z{>R!(lh0_DkB3=1SDXL2zu5S~;!Ie}qv0?&USU|1r^
z%-9Gb7#4~z5S^SQ#Km49#LC3L%aB-*(2>Z%uq1J_A2Sapqr~KBo=h%}busb5PVoW`
z>n5M$v1jsIF`1uNjL~MY8n3d9h<H5EF+58^PRhv4%61BN^@$G-^^0zDWSQ*4tByy3
z6U*c>UJd*T77|i$hLD2qgcK<85paVqAq5qL6f7pB;2a?ZKM5&N;V0k=e?khX2q{=f
zNWld{3Vst(pe8`T7lDKn)DTjzoRES`gcSTGq(DQEfG>gxDX1f)U?m|1R|qNiPe_55
T5CLC=3Tg7A<^Ij3LJ~{>th0qv

delta 353
zcmX?P(5uPi66_MvE5g9QSh|sG4-=CI$K(@C-n=Z)joeOtzCq4TexbgTRhjiRKVjzK
zWE7eFktdVOvnD1!*ePDXv1W1#uf3#yS+Jo+JV(5vOArqO12Y4MQ?RQKLv)kM<UPE~
z%+XC6lYjH718L34c6=IOx(P}jgVJ355OF^!Jqb!*gVIt05cxPLy$DJ_gVI`p5cxbP
zy$MQxgVI()5cxVNeF#dk2}8uapmZOUz67PkL?H4}P<kGeegvh}L?QB7P<kDd{v@io
J*;tI92>|#jWZ(b*

-- 
2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
                   ` (5 preceding siblings ...)
  2021-03-15 18:01 ` [PATCH 6/6] tests: acpi: update expected blobs Igor Mammedov
@ 2021-03-15 18:36 ` no-reply
  2021-03-15 20:21   ` Igor Mammedov
  2021-05-14 13:32 ` Michael S. Tsirkin
  7 siblings, 1 reply; 13+ messages in thread
From: no-reply @ 2021-03-15 18:36 UTC (permalink / raw)
  To: imammedo; +Cc: jusual, qemu-devel, laine, mst

Patchew URL: https://patchew.org/QEMU/20210315180102.3008391-1-imammedo@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210315180102.3008391-1-imammedo@redhat.com
Subject: [PATCH 0/6] pc: support user provided NIC naming/indexing

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210315180102.3008391-1-imammedo@redhat.com -> patchew/20210315180102.3008391-1-imammedo@redhat.com
Switched to a new branch 'test'
c0692b4 tests: acpi: update expected blobs
1a843d9 pci: acpi: add _DSM method to PCI devices
6b55586 acpi: add aml_to_decimalstring() and aml_call6() helpers
7b887f4 pci: acpi: ensure that acpi-index is unique
4056291 pci: introduce apci-index property for PCI device
9bd3a1b tests: acpi: temporary whitelist DSDT changes

=== OUTPUT BEGIN ===
1/6 Checking commit 9bd3a1b94c0f (tests: acpi: temporary whitelist DSDT changes)
2/6 Checking commit 40562915acde (pci: introduce apci-index property for PCI device)
ERROR: Macros with complex values should be enclosed in parenthesis
#284: FILE: include/hw/acpi/pcihp.h:77:
+#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
         VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
                             test_pcihp), \
         VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
                                   ACPI_PCIHP_MAX_HOTPLUG_BUS, \
                                   test_pcihp, 1, \
                                   vmstate_acpi_pcihp_pci_status, \
-                                  AcpiPciHpPciStatus)
+                                  AcpiPciHpPciStatus), \
+        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
+                            test_acpi_index)

total: 1 errors, 0 warnings, 197 lines checked

Patch 2/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/6 Checking commit 7b887f42419a (pci: acpi: ensure that acpi-index is unique)
4/6 Checking commit 6b555869ad66 (acpi: add aml_to_decimalstring() and aml_call6() helpers)
WARNING: Block comments use a leading /* on a separate line
#27: FILE: hw/acpi/aml-build.c:640:
+    Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);

WARNING: Block comments use a leading /* on a separate line
#32: FILE: hw/acpi/aml-build.c:645:
+        build_append_byte(var->buf, 0x00 /* NullNameOp */);

total: 0 errors, 2 warnings, 55 lines checked

Patch 4/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/6 Checking commit 1a843d9de55e (pci: acpi: add _DSM method to PCI devices)
WARNING: Block comments use a leading /* on a separate line
#127: FILE: hw/i386/acpi-build.c:568:
+                        1 /* have supported functions */ |

total: 0 errors, 1 warnings, 140 lines checked

Patch 5/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/6 Checking commit c0692b4b7df0 (tests: acpi: update expected blobs)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210315180102.3008391-1-imammedo@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
  2021-03-15 18:36 ` [PATCH 0/6] pc: support user provided NIC naming/indexing no-reply
@ 2021-03-15 20:21   ` Igor Mammedov
  0 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-03-15 20:21 UTC (permalink / raw)
  To: no-reply; +Cc: jusual, qemu-devel, laine, mst

On Mon, 15 Mar 2021 11:36:49 -0700 (PDT)
no-reply@patchew.org wrote:

> Patchew URL: https://patchew.org/QEMU/20210315180102.3008391-1-imammedo@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20210315180102.3008391-1-imammedo@redhat.com
> Subject: [PATCH 0/6] pc: support user provided NIC naming/indexing
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]         patchew/20210315180102.3008391-1-imammedo@redhat.com -> patchew/20210315180102.3008391-1-imammedo@redhat.com
> Switched to a new branch 'test'
> c0692b4 tests: acpi: update expected blobs
> 1a843d9 pci: acpi: add _DSM method to PCI devices
> 6b55586 acpi: add aml_to_decimalstring() and aml_call6() helpers
> 7b887f4 pci: acpi: ensure that acpi-index is unique
> 4056291 pci: introduce apci-index property for PCI device
> 9bd3a1b tests: acpi: temporary whitelist DSDT changes
> 
> === OUTPUT BEGIN ===
> 1/6 Checking commit 9bd3a1b94c0f (tests: acpi: temporary whitelist DSDT changes)
> 2/6 Checking commit 40562915acde (pci: introduce apci-index property for PCI device)
> ERROR: Macros with complex values should be enclosed in parenthesis
> #284: FILE: include/hw/acpi/pcihp.h:77:
> +#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
>          VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
>                              test_pcihp), \
>          VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
>                                    ACPI_PCIHP_MAX_HOTPLUG_BUS, \
>                                    test_pcihp, 1, \
>                                    vmstate_acpi_pcihp_pci_status, \
> -                                  AcpiPciHpPciStatus)
> +                                  AcpiPciHpPciStatus), \
> +        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
> +                            test_acpi_index)
probably false positive
(I don't know what's wrong here and how to fix it)

> total: 1 errors, 0 warnings, 197 lines checked
> 
> Patch 2/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

below warnings are intentional to follow comments style we use within aml-build.c

> 3/6 Checking commit 7b887f42419a (pci: acpi: ensure that acpi-index is unique)
> 4/6 Checking commit 6b555869ad66 (acpi: add aml_to_decimalstring() and aml_call6() helpers)
> WARNING: Block comments use a leading /* on a separate line
> #27: FILE: hw/acpi/aml-build.c:640:
> +    Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);
> 
> WARNING: Block comments use a leading /* on a separate line
> #32: FILE: hw/acpi/aml-build.c:645:
> +        build_append_byte(var->buf, 0x00 /* NullNameOp */);
> 
> total: 0 errors, 2 warnings, 55 lines checked
> 
> Patch 4/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 5/6 Checking commit 1a843d9de55e (pci: acpi: add _DSM method to PCI devices)
> WARNING: Block comments use a leading /* on a separate line
> #127: FILE: hw/i386/acpi-build.c:568:
> +                        1 /* have supported functions */ |
> 
> total: 0 errors, 1 warnings, 140 lines checked
> 
> Patch 5/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 6/6 Checking commit c0692b4b7df0 (tests: acpi: update expected blobs)
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20210315180102.3008391-1-imammedo@redhat.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] pci: introduce apci-index property for PCI device
  2021-03-15 18:00 ` [PATCH 2/6] pci: introduce apci-index property for PCI device Igor Mammedov
@ 2021-04-06  9:47   ` Daniel P. Berrangé
  2021-04-06 17:38     ` Igor Mammedov
  2021-05-14 13:37   ` Michael S. Tsirkin
  1 sibling, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2021-04-06  9:47 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: jusual, qemu-devel, laine, mst

On Mon, Mar 15, 2021 at 02:00:58PM -0400, Igor Mammedov wrote:
> In x86/ACPI world, linux distros are using predictable
> network interface naming since systemd v197. Which on
> QEMU based VMs results into path based naming scheme,
> that names network interfaces based on PCI topology.
> 
> With itm on has to plug NIC in exactly the same bus/slot,
> which was used when disk image was first provisioned/configured
> or one risks to loose network configuration due to NIC being
> renamed to actually used topology.
> That also restricts freedom to reshape PCI configuration of
> VM without need to reconfigure used guest image.
> 
> systemd also offers "onboard" naming scheme which is
> preferred over PCI slot/topology one, provided that
> firmware implements:
>     "
>     PCI Firmware Specification 3.1
>     4.6.7.  DSM for Naming a PCI or PCI Express Device Under
>             Operating Systems
>     "
> that allows to assign user defined index to PCI device,
> which systemd will use to name NIC. For example, using
>   -device e1000,acpi-index=100
> guest will rename NIC to 'eno100', where 'eno' is default
> prefix for "onboard" naming scheme. This doesn't require
> any advance configuration on guest side to com in effect
> at 'onboard' scheme takes priority over path based naming.
> 
> Hope is that 'acpi-index' it will be easier to consume by
> management layer, compared to forcing specific PCI topology
> and/or having several disk image templates for different
> topologies and will help to simplify process of spawning
> VM from the same template without need to reconfigure
> guest NIC.
> 
> This patch adds, 'acpi-index'* property and wires up
> a 32bit register on top of pci hotplug register block
> to pass index value to AML code at runtime.
> Following patch will add corresponding _DSM code and
> wire it up to PCI devices described in ACPI.

You've illustrated usage & benefits with NICs, but IIUC, this
feature is wired up for any PCI device. Are you aware of any
usage of this feature for non-NIC devices ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] pci: introduce apci-index property for PCI device
  2021-04-06  9:47   ` Daniel P. Berrangé
@ 2021-04-06 17:38     ` Igor Mammedov
  0 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2021-04-06 17:38 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: jusual, qemu-devel, laine, mst

On Tue, 6 Apr 2021 10:47:12 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Mar 15, 2021 at 02:00:58PM -0400, Igor Mammedov wrote:
> > In x86/ACPI world, linux distros are using predictable
> > network interface naming since systemd v197. Which on
> > QEMU based VMs results into path based naming scheme,
> > that names network interfaces based on PCI topology.
> > 
> > With itm on has to plug NIC in exactly the same bus/slot,
> > which was used when disk image was first provisioned/configured
> > or one risks to loose network configuration due to NIC being
> > renamed to actually used topology.
> > That also restricts freedom to reshape PCI configuration of
> > VM without need to reconfigure used guest image.
> > 
> > systemd also offers "onboard" naming scheme which is
> > preferred over PCI slot/topology one, provided that
> > firmware implements:
> >     "
> >     PCI Firmware Specification 3.1
> >     4.6.7.  DSM for Naming a PCI or PCI Express Device Under
> >             Operating Systems
> >     "
> > that allows to assign user defined index to PCI device,
> > which systemd will use to name NIC. For example, using
> >   -device e1000,acpi-index=100
> > guest will rename NIC to 'eno100', where 'eno' is default
> > prefix for "onboard" naming scheme. This doesn't require
> > any advance configuration on guest side to com in effect
> > at 'onboard' scheme takes priority over path based naming.
> > 
> > Hope is that 'acpi-index' it will be easier to consume by
> > management layer, compared to forcing specific PCI topology
> > and/or having several disk image templates for different
> > topologies and will help to simplify process of spawning
> > VM from the same template without need to reconfigure
> > guest NIC.
> > 
> > This patch adds, 'acpi-index'* property and wires up
> > a 32bit register on top of pci hotplug register block
> > to pass index value to AML code at runtime.
> > Following patch will add corresponding _DSM code and
> > wire it up to PCI devices described in ACPI.  
> 
> You've illustrated usage & benefits with NICs, but IIUC, this
> feature is wired up for any PCI device. Are you aware of any
> usage of this feature for non-NIC devices ?

I'm not aware of anything else that might currently use it.

It might be useful for virtio-win (HDDs/NICs) (as we saw recently
breakage due to PCI root bus UID change, where devices installed
on old QEMU enumerated as new instances breaking network config or
causing boot issues)

As Lane mentioned, it would be really useful if we could add acpi-index
as some meta data to qcow2 image so that new guest could import it
at creation time.

> Regards,
> Daniel



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
  2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
                   ` (6 preceding siblings ...)
  2021-03-15 18:36 ` [PATCH 0/6] pc: support user provided NIC naming/indexing no-reply
@ 2021-05-14 13:32 ` Michael S. Tsirkin
  7 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2021-05-14 13:32 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: jusual, qemu-devel, laine

On Mon, Mar 15, 2021 at 02:00:56PM -0400, Igor Mammedov wrote:
> 
> Changes since rfc:
>   * make sure that acpi-index unique
>   * use dedicated MMIO register for it
>   * drop syntetic acpi-label field in _DSM
>   * add more comments


Could you rebase and repost please?
Some comments in commit log about use for non-NIC devices
would be helpful (even if it's just that "currently does not seem to be useful for
non-NICs").

> Series implements support for 'onboard' naming scheme for network
> interfaces (1), which is based on PCI firmware spec and lets user
> to explicitly specify index that will be used by guest to name
> network interface, ex:
>     -device e1000,acpi-index=33
> should make guest rename NIC name to 'eno33' where 'eno' is default
> prefix for this scheme.
> 
> Hope is that it will allow to simplify launching VMs from
> template disk images with different set VM configurations
> without need to reconfigure guest network intrfaces or
> risk of loosing network connectivity.
> 
> For more detailed description/examples see patches [3-4/5]
> 
> 1)
>  https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ 
> 
> Git repo for testing:
>    https://github.com/imammedo/qemu/branches acpi-index-v1
> 
> 
> CC: laine@redhat.com
> CC: mst@redhat.com
> CC: jusual@redhat.com
> 
> Igor Mammedov (6):
>   tests: acpi: temporary whitelist DSDT changes
>   pci: introduce apci-index property for PCI device
>   pci: acpi: ensure that acpi-index is unique
>   acpi: add aml_to_decimalstring() and aml_call6() helpers
>   pci: acpi: add _DSM method to PCI devices
>   tests: acpi: update expected blobs
> 
>  include/hw/acpi/aml-build.h      |   3 +
>  include/hw/acpi/pci.h            |   1 +
>  include/hw/acpi/pcihp.h          |   9 ++-
>  include/hw/pci/pci.h             |   1 +
>  hw/acpi/aml-build.c              |  28 ++++++++
>  hw/acpi/pci.c                    |   1 -
>  hw/acpi/pcihp.c                  | 104 ++++++++++++++++++++++++++-
>  hw/acpi/piix4.c                  |   3 +-
>  hw/acpi/trace-events             |   2 +
>  hw/i386/acpi-build.c             | 118 +++++++++++++++++++++++++++++--
>  hw/pci/pci.c                     |   1 +
>  tests/data/acpi/pc/DSDT          | Bin 5065 -> 6002 bytes
>  tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7327 bytes
>  tests/data/acpi/pc/DSDT.bridge   | Bin 6924 -> 8668 bytes
>  tests/data/acpi/pc/DSDT.cphp     | Bin 5529 -> 6466 bytes
>  tests/data/acpi/pc/DSDT.dimmpxm  | Bin 6719 -> 7656 bytes
>  tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5969 bytes
>  tests/data/acpi/pc/DSDT.ipmikcs  | Bin 5137 -> 6074 bytes
>  tests/data/acpi/pc/DSDT.memhp    | Bin 6424 -> 7361 bytes
>  tests/data/acpi/pc/DSDT.nohpet   | Bin 4923 -> 5860 bytes
>  tests/data/acpi/pc/DSDT.numamem  | Bin 5071 -> 6008 bytes
>  tests/data/acpi/pc/DSDT.roothp   | Bin 5261 -> 6210 bytes
>  22 files changed, 261 insertions(+), 10 deletions(-)
> 
> -- 
> 2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] pci: introduce apci-index property for PCI device
  2021-03-15 18:00 ` [PATCH 2/6] pci: introduce apci-index property for PCI device Igor Mammedov
  2021-04-06  9:47   ` Daniel P. Berrangé
@ 2021-05-14 13:37   ` Michael S. Tsirkin
  1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2021-05-14 13:37 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: jusual, qemu-devel, laine

On Mon, Mar 15, 2021 at 02:00:58PM -0400, Igor Mammedov wrote:
> In x86/ACPI world, linux distros are using predictable
> network interface naming since systemd v197. Which on
> QEMU based VMs results into path based naming scheme,
> that names network interfaces based on PCI topology.
> 
> With itm on has to plug NIC in exactly the same bus/slot,
> which was used when disk image was first provisioned/configured
> or one risks to loose network configuration due to NIC being
> renamed to actually used topology.
> That also restricts freedom to reshape PCI configuration of
> VM without need to reconfigure used guest image.
> 
> systemd also offers "onboard" naming scheme which is
> preferred over PCI slot/topology one, provided that
> firmware implements:
>     "
>     PCI Firmware Specification 3.1
>     4.6.7.  DSM for Naming a PCI or PCI Express Device Under
>             Operating Systems
>     "
> that allows to assign user defined index to PCI device,
> which systemd will use to name NIC. For example, using
>   -device e1000,acpi-index=100
> guest will rename NIC to 'eno100', where 'eno' is default
> prefix for "onboard" naming scheme. This doesn't require
> any advance configuration on guest side to com in effect
> at 'onboard' scheme takes priority over path based naming.
> 
> Hope is that 'acpi-index' it will be easier to consume by
> management layer, compared to forcing specific PCI topology
> and/or having several disk image templates for different
> topologies and will help to simplify process of spawning
> VM from the same template without need to reconfigure
> guest NIC.
> 
> This patch adds, 'acpi-index'* property and wires up
> a 32bit register on top of pci hotplug register block
> to pass index value to AML code at runtime.
> Following patch will add corresponding _DSM code and
> wire it up to PCI devices described in ACPI.
> 
> *) name comes from linux kernel terminology
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

So something to note here is that it's more or less linux centric even
though it's from the spec:

Assignment of specific device names to multi-function devices installed in expansion
slots, and/or PCI or PCI Express devices that are hot-added to expansion slots in operating system-
environment would be handled in operating system-specific manner, and is not specified via this
specification.

So I think commit log should note that point about hot add:
that even though spec excludes it, it seems to work
because the Linux guest re-evaluate the method each time
device is added.


Also, should not we exclude multi-function devices with this property?
Or at least document that it won't work ...




> ---
> v1:
>  *  fix build failure of mips target
>  *  move out handling acpi-index into separate MMIO register,
>     instead of abusing unused PCI hotplug registers (mst)
>  *  default acpi-index property value to 0
>  *  set and check for max acpi-index value (systemd limitation)
> ---
>  include/hw/acpi/pcihp.h |  9 +++++--
>  include/hw/pci/pci.h    |  1 +
>  hw/acpi/pci.c           |  1 -
>  hw/acpi/pcihp.c         | 58 +++++++++++++++++++++++++++++++++++++++--
>  hw/acpi/piix4.c         |  3 ++-
>  hw/acpi/trace-events    |  2 ++
>  hw/i386/acpi-build.c    | 13 ++++++++-
>  hw/pci/pci.c            |  1 +
>  8 files changed, 81 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index dfd375820f..2dd90aea30 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -46,6 +46,7 @@ typedef struct AcpiPciHpPciStatus {
>  typedef struct AcpiPciHpState {
>      AcpiPciHpPciStatus acpi_pcihp_pci_status[ACPI_PCIHP_MAX_HOTPLUG_BUS];
>      uint32_t hotplug_select;
> +    uint32_t acpi_index;
>      PCIBus *root;
>      MemoryRegion io;
>      bool legacy_piix;
> @@ -71,13 +72,17 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off);
>  
>  extern const VMStateDescription vmstate_acpi_pcihp_pci_status;
>  
> -#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp) \
> +bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id);
> +
> +#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
>          VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
>                              test_pcihp), \
>          VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
>                                    ACPI_PCIHP_MAX_HOTPLUG_BUS, \
>                                    test_pcihp, 1, \
>                                    vmstate_acpi_pcihp_pci_status, \
> -                                  AcpiPciHpPciStatus)
> +                                  AcpiPciHpPciStatus), \
> +        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
> +                            test_acpi_index)
>  
>  #endif
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 1bc231480f..6be4e0c460 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -359,6 +359,7 @@ struct PCIDevice {
>  
>      /* ID of standby device in net_failover pair */
>      char *failover_pair_id;
> +    uint32_t acpi_index;
>  };
>  
>  void pci_register_bar(PCIDevice *pci_dev, int region_num,
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index ec455c3b25..75b1103ec4 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -59,4 +59,3 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
>      build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
>                   "MCFG", table_data->len - mcfg_start, 1, oem_id, oem_table_id);
>  }
> -
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 9dc4d3e2db..ceab287bd3 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -39,12 +39,13 @@
>  #include "trace.h"
>  
>  #define ACPI_PCIHP_ADDR 0xae00
> -#define ACPI_PCIHP_SIZE 0x0014
> +#define ACPI_PCIHP_SIZE 0x0018
>  #define PCI_UP_BASE 0x0000
>  #define PCI_DOWN_BASE 0x0004
>  #define PCI_EJ_BASE 0x0008
>  #define PCI_RMV_BASE 0x000c
>  #define PCI_SEL_BASE 0x0010
> +#define PCI_AIDX_BASE 0x0014
>  
>  typedef struct AcpiPciHpFind {
>      int bsel;
> @@ -251,9 +252,13 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off)
>      acpi_pcihp_update(s);
>  }
>  
> +#define ONBOARD_INDEX_MAX (16 * 1024 - 1)
> +
>  void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>                                     DeviceState *dev, Error **errp)
>  {
> +    PCIDevice *pdev = PCI_DEVICE(dev);
> +
>      /* Only hotplugged devices need the hotplug capability. */
>      if (dev->hotplugged &&
>          acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))) < 0) {
> @@ -261,6 +266,17 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>                     ACPI_PCIHP_PROP_BSEL "' set");
>          return;
>      }
> +
> +    /*
> +     * capped by systemd (see: udev-builtin-net_id.c)
> +     * as it's the only known user honor it to avoid users
> +     * misconfigure QEMU and then wonder why acpi-index doesn't work
> +     */
> +    if (pdev->acpi_index > ONBOARD_INDEX_MAX) {
> +        error_setg(errp, "acpi-index should be less or equal to %u",
> +                   ONBOARD_INDEX_MAX);
> +        return;
> +    }
>  }
>  
>  void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
> @@ -347,7 +363,6 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
>          trace_acpi_pci_down_read(val);
>          break;
>      case PCI_EJ_BASE:
> -        /* No feature defined yet */
>          trace_acpi_pci_features_read(val);
>          break;
>      case PCI_RMV_BASE:
> @@ -357,6 +372,12 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
>      case PCI_SEL_BASE:
>          val = s->hotplug_select;
>          trace_acpi_pci_sel_read(val);
> +        break;
> +    case PCI_AIDX_BASE:
> +        val = s->acpi_index;
> +        s->acpi_index = 0;
> +        trace_acpi_pci_acpi_index_read(val);
> +        break;
>      default:
>          break;
>      }
> @@ -367,8 +388,35 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
>  static void pci_write(void *opaque, hwaddr addr, uint64_t data,
>                        unsigned int size)
>  {
> +    int slot;
> +    PCIBus *bus;
> +    BusChild *kid, *next;
>      AcpiPciHpState *s = opaque;
> +
> +    s->acpi_index = 0;
>      switch (addr) {
> +    case PCI_AIDX_BASE:
> +        /*
> +         * fetch acpi-index for specified slot so that follow up read from
> +         * PCI_AIDX_BASE can return it to guest
> +         */
> +        slot = ctz32(data);
> +
> +        if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> +            break;
> +        }
> +
> +        bus = acpi_pcihp_find_hotplug_bus(s, s->hotplug_select);
> +        QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
> +            Object *o = OBJECT(kid->child);
> +            PCIDevice *dev = PCI_DEVICE(o);
> +            if (PCI_SLOT(dev->devfn) == slot) {
> +                s->acpi_index = object_property_get_uint(o, "acpi-index", NULL);
> +                break;
> +            }
> +        }
> +        trace_acpi_pci_acpi_index_write(s->hotplug_select, slot, s->acpi_index);
> +        break;
>      case PCI_EJ_BASE:
>          if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
>              break;
> @@ -413,6 +461,12 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
>                                     OBJ_PROP_FLAG_READ);
>  }
>  
> +bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> +{
> +     AcpiPciHpState *s = opaque;
> +     return s->acpi_index;
> +}
> +
>  const VMStateDescription vmstate_acpi_pcihp_pci_status = {
>      .name = "acpi_pcihp_pci_status",
>      .version_id = 1,
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 1efc0ded9f..6056d51667 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -297,7 +297,8 @@ static const VMStateDescription vmstate_acpi = {
>              2, vmstate_pci_status,
>              struct AcpiPciHpPciStatus),
>          VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
> -                            vmstate_test_use_acpi_hotplug_bridge),
> +                            vmstate_test_use_acpi_hotplug_bridge,
> +                            vmstate_acpi_pcihp_use_acpi_index),
>          VMSTATE_END_OF_LIST()
>      },
>      .subsections = (const VMStateDescription*[]) {
> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> index f91ced477d..dcc1438f3a 100644
> --- a/hw/acpi/trace-events
> +++ b/hw/acpi/trace-events
> @@ -41,6 +41,8 @@ acpi_pci_unplug_request(int bsel, int slot) "bsel: %d slot: %d"
>  acpi_pci_up_read(uint32_t val) "%" PRIu32
>  acpi_pci_down_read(uint32_t val) "%" PRIu32
>  acpi_pci_features_read(uint32_t val) "%" PRIu32
> +acpi_pci_acpi_index_read(uint32_t val) "%" PRIu32
> +acpi_pci_acpi_index_write(unsigned bsel, unsigned slot, uint32_t aidx) "bsel: %u slot: %u aidx: %" PRIu32
>  acpi_pci_rmv_read(uint32_t val) "%" PRIu32
>  acpi_pci_sel_read(uint32_t val) "%" PRIu32
>  acpi_pci_ej_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 442b4629a9..e49fae2bfd 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1168,9 +1168,10 @@ static void build_piix4_pci_hotplug(Aml *table)
>      aml_append(scope, field);
>  
>      aml_append(scope,
> -        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
> +        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x08));
>      field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
>      aml_append(field, aml_named_field("BNUM", 32));
> +    aml_append(field, aml_named_field("PIDX", 32));
>      aml_append(scope, field);
>  
>      aml_append(scope, aml_mutex("BLCK", 0));
> @@ -1184,6 +1185,16 @@ static void build_piix4_pci_hotplug(Aml *table)
>      aml_append(method, aml_return(aml_int(0)));
>      aml_append(scope, method);
>  
> +    method = aml_method("AIDX", 2, AML_NOTSERIALIZED);
> +    aml_append(method, aml_acquire(aml_name("BLCK"), 0xFFFF));
> +    aml_append(method, aml_store(aml_arg(0), aml_name("BNUM")));
> +    aml_append(method,
> +        aml_store(aml_shiftleft(aml_int(1), aml_arg(1)), aml_name("PIDX")));
> +    aml_append(method, aml_store(aml_name("PIDX"), aml_local(0)));
> +    aml_append(method, aml_release(aml_name("BLCK")));
> +    aml_append(method, aml_return(aml_local(0)));
> +    aml_append(scope, method);
> +
>      aml_append(table, scope);
>  }
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 0eadcdbc9e..ac9a24889c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -79,6 +79,7 @@ static Property pci_props[] = {
>                      QEMU_PCIE_EXTCAP_INIT_BITNR, true),
>      DEFINE_PROP_STRING("failover_pair_id", PCIDevice,
>                         failover_pair_id),
> +    DEFINE_PROP_UINT32("acpi-index",  PCIDevice, acpi_index, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> -- 
> 2.27.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-05-14 13:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 18:00 [PATCH 0/6] pc: support user provided NIC naming/indexing Igor Mammedov
2021-03-15 18:00 ` [PATCH 1/6] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
2021-03-15 18:00 ` [PATCH 2/6] pci: introduce apci-index property for PCI device Igor Mammedov
2021-04-06  9:47   ` Daniel P. Berrangé
2021-04-06 17:38     ` Igor Mammedov
2021-05-14 13:37   ` Michael S. Tsirkin
2021-03-15 18:00 ` [PATCH 3/6] pci: acpi: ensure that acpi-index is unique Igor Mammedov
2021-03-15 18:01 ` [PATCH 4/6] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
2021-03-15 18:01 ` [PATCH 5/6] pci: acpi: add _DSM method to PCI devices Igor Mammedov
2021-03-15 18:01 ` [PATCH 6/6] tests: acpi: update expected blobs Igor Mammedov
2021-03-15 18:36 ` [PATCH 0/6] pc: support user provided NIC naming/indexing no-reply
2021-03-15 20:21   ` Igor Mammedov
2021-05-14 13:32 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).