Re: [Qemu-devel] [PATCH V5] hw/pxb: add chassis_nr property

[Michael S. Tsirkin]

On Tue, Mar 15, 2016 at 05:44:29PM +0200, Marcel Apfelbaum wrote:
> On 03/15/2016 05:03 PM, Michael S. Tsirkin wrote:
> >On Tue, Mar 15, 2016 at 01:53:08PM +0200, Marcel Apfelbaum wrote:
> >>Add a chassis_nr property instead of using PXB bus number
> >>as internal bridge's chassis nr.
> >>
> >>Suggested-by: Michael S. Tsirkin <address@hidden>
> >>Signed-off-by: Marcel Apfelbaum <address@hidden>
> >
> >Does not seem to apply.
> >Is my branch outdated?
> 
> Is compiled against today master. But maybe there is a patch on your tree.
> Do you want me to rebase on one of your trees?
> 
> Thanks,
> Marcel


Pls do.

> 
> >
> >>---
> >>v4->v5:
> >>  - fixed comments (Cao jin)
> >>
> >>v3->v4:
> >>  - re-coded to fit current codebase
> >>v2->v3:
> >>  - use bus nr if chassis nr is 0 (Micahel S. Tsirkin)
> >>v1->v2:
> >>  - Rebased on master
> >>
> >>  docs/pci_expander_bridge.txt        |  7 +++----
> >>  hw/pci-bridge/pci_expander_bridge.c | 23 ++++++++++++++++++++---
> >>  2 files changed, 23 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/docs/pci_expander_bridge.txt b/docs/pci_expander_bridge.txt
> >>index e7c8fe9..fe058a6 100644
> >>--- a/docs/pci_expander_bridge.txt
> >>+++ b/docs/pci_expander_bridge.txt
> >>@@ -23,9 +23,9 @@ A detailed command line would be:
> >>  -m 2G
> >>  -object 
> >> memory-backend-ram,size=1024M,policy=bind,host-nodes=0,id=ram-node0 -numa 
> >> node,nodeid=0,cpus=0,memdev=ram-node0
> >>  -object 
> >> memory-backend-ram,size=1024M,policy=bind,host-nodes=1,id=ram-node1 -numa 
> >> node,nodeid=1,cpus=1,memdev=ram-node1
> >>--device pxb,id=bridge1,bus=pci.0,numa_node=1,bus_nr=4 -netdev user,id=nd 
> >>-device e1000,bus=bridge1,addr=0x4,netdev=nd
> >>--device pxb,id=bridge2,bus=pci.0,numa_node=0,bus_nr=8, -device 
> >>e1000,bus=bridge2,addr=0x3
> >>--device pxb,id=bridge3,bus=pci.0,bus_nr=40, -drive 
> >>if=none,id=drive0,file=[img] -device 
> >>virtio-blk-pci,drive=drive0,scsi=off,bus=bridge3,addr=1
> >>+-device pxb,id=bridge1,bus=pci.0,numa_node=1,bus_nr=4,chassis_nr=1 -netdev 
> >>user,id=nd -device e1000,bus=bridge1,addr=0x4,netdev=nd
> >>+-device pxb,id=bridge2,bus=pci.0,numa_node=0,bus_nr=8,chassis_nr=2 -device 
> >>e1000,bus=bridge2,addr=0x3
> >>+-device pxb,id=bridge3,bus=pci.0,bus_nr=40,chassis_nr=3 -drive 
> >>if=none,id=drive0,file=[img] -device 
> >>virtio-blk-pci,drive=drive0,scsi=off,bus=bridge3,addr=1
> >>
> >>  Here you have:
> >>   - 2 NUMA nodes for the guest, 0 and 1. (both mapped to the same NUMA 
> >> node in host, but you can and should put it in different host NUMA nodes)
> >>@@ -55,4 +55,3 @@ The PXB is composed by:
> >>    - Using the bridge will enable hotplug support
> >>    - All the devices behind the bridge will use bridge's IO/MEM windows 
> >> compacting
> >>      the PCI address space.
> >>-
> >>diff --git a/hw/pci-bridge/pci_expander_bridge.c 
> >>b/hw/pci-bridge/pci_expander_bridge.c
> >>index d23b8da..124261f 100644
> >>--- a/hw/pci-bridge/pci_expander_bridge.c
> >>+++ b/hw/pci-bridge/pci_expander_bridge.c
> >>@@ -46,6 +46,7 @@ typedef struct PXBDev {
> >>      PCIDevice parent_obj;
> >>      /*< public >*/
> >>
> >>+    uint8_t chassis_nr;
> >>      uint8_t bus_nr;
> >>      uint16_t numa_node;
> >>  } PXBDev;
> >>@@ -237,7 +238,8 @@ static int pxb_dev_init_common(PCIDevice *dev, bool 
> >>pcie)
> >>          bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, 
> >> TYPE_PXB_BUS);
> >>          bds = qdev_create(BUS(bus), "pci-bridge");
> >>          bds->id = dev_name;
> >>-        qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, 
> >>pxb->bus_nr);
> >>+        qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR,
> >>+                            pxb->chassis_nr);
> >>          qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, false);
> >>      }
> >>
> >>@@ -267,11 +269,17 @@ static int pxb_dev_init_common(PCIDevice *dev, bool 
> >>pcie)
> >>
> >>  static int pxb_dev_initfn(PCIDevice *dev)
> >>  {
> >>+    PXBDev *pxb = convert_to_pxb(dev);
> >>+
> >>      if (pci_bus_is_express(dev->bus)) {
> >>          error_report("pxb devices cannot reside on a PCIe bus!");
> >>          return -EINVAL;
> >>      }
> >>
> >>+    if (!pxb->chassis_nr) {
> >>+        pxb->chassis_nr = pxb->bus_nr;
> >>+    }
> >>+
> >>      return pxb_dev_init_common(dev, false);
> >>  }
> >>
> >>@@ -283,9 +291,11 @@ static void pxb_dev_exitfn(PCIDevice *pci_dev)
> >>  }
> >>
> >>  static Property pxb_dev_properties[] = {
> >>-    /* Note: 0 is not a legal a PXB bus number. */
> >>+    /* Note: 0 is not a legal PXB bus number. */
> >>      DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
> >>      DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, 
> >> NUMA_NODE_UNASSIGNED),
> >>+    /* Note: 0 is not a legal chassis number. */
> >>+    DEFINE_PROP_UINT8("chassis_nr", PXBDev, chassis_nr, 0),
> >>      DEFINE_PROP_END_OF_LIST(),
> >>  };
> >>
> >>@@ -322,6 +332,13 @@ static int pxb_pcie_dev_initfn(PCIDevice *dev)
> >>      return pxb_dev_init_common(dev, true);
> >>  }
> >>
> >>+static Property pxb_pcie_dev_properties[] = {
> >>+    /* Note: 0 is not a legal PXB bus number. */
> >>+    DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
> >>+    DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, 
> >>NUMA_NODE_UNASSIGNED),
> >>+    DEFINE_PROP_END_OF_LIST(),
> >>+};
> >>+
> >>  static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)
> >>  {
> >>      DeviceClass *dc = DEVICE_CLASS(klass);
> >>@@ -334,7 +351,7 @@ static void pxb_pcie_dev_class_init(ObjectClass *klass, 
> >>void *data)
> >>      k->class_id = PCI_CLASS_BRIDGE_HOST;
> >>
> >>      dc->desc = "PCI Express Expander Bridge";
> >>-    dc->props = pxb_dev_properties;
> >>+    dc->props = pxb_pcie_dev_properties;
> >>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >>  }
> >>
> >>--
> >>2.4.3