From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afrW2-0003gy-S0 for qemu-devel@nongnu.org; Tue, 15 Mar 2016 12:08:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afrVx-0003fj-Fh for qemu-devel@nongnu.org; Tue, 15 Mar 2016 12:08:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40223) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afrVx-0003fd-89 for qemu-devel@nongnu.org; Tue, 15 Mar 2016 12:08:09 -0400 Date: Tue, 15 Mar 2016 18:08:05 +0200 From: "Michael S. Tsirkin" Message-ID: <20160315180758-mutt-send-email-mst@redhat.com> References: <1458042788-14615-1-git-send-email-marcel@redhat.com> <20160315170257-mutt-send-email-mst@redhat.com> <56E82DDD.6030005@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E82DDD.6030005@redhat.com> Subject: Re: [Qemu-devel] [PATCH V5] hw/pxb: add chassis_nr property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: caoj.fnst@cn.fujitsu.com, qemu-devel@nongnu.org, laine@redhat.com, ehabkost@redhat.com 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 > >>Signed-off-by: Marcel Apfelbaum > > > >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