From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:34275) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtqwS-000242-EN for qemu-devel@nongnu.org; Wed, 13 Feb 2019 04:34:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gtqwJ-0007Qn-Tl for qemu-devel@nongnu.org; Wed, 13 Feb 2019 04:34:51 -0500 Received: from aserp2130.oracle.com ([141.146.126.79]:56584) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gtqwI-0007Nm-6D for qemu-devel@nongnu.org; Wed, 13 Feb 2019 04:34:46 -0500 Message-ID: From: Knut Omang Date: Wed, 13 Feb 2019 10:34:30 +0100 In-Reply-To: <4ad3f098-ded6-8ddb-4ae5-c2975a286245@gmail.com> References: <04c99e3c0b56dcbfe494f668bbe11777adbadba9.1549779509.git-series.knut.omang@oracle.com> <4ad3f098-ded6-8ddb-4ae5-c2975a286245@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , qemu-devel@nongnu.org Cc: "Michael S . Tsirkin" , Alex Williamson , Stefan Hajnoczi , Elijah Shakkour , Tal Attaly On Mon, 2019-02-11 at 10:07 +0200, Marcel Apfelbaum wrote: > > On 2/10/19 8:53 AM, Knut Omang wrote: > > Claim ACS support in the generic PCIe root port to allow > > passthrough of individual functions of a device to different > > guests (in a nested virt.setting) with VFIO. > > Without this patch, all functions of a device, such as all VFs of > > an SR/IOV device, will end up in the same IOMMU group. > > A similar situation occurs on Windows with Hyper-V. > > > > In the single function device case, it also has a small cosmetic > > benefit in that the root port itself is not grouped with > > the device. VFIO handles that situation in that binding rules > > only apply to endpoints, so it does not limit passthrough in > > those cases. > > > > Signed-off-by: Knut Omang > > --- > > hw/pci-bridge/gen_pcie_root_port.c | 4 ++++ > > hw/pci-bridge/pcie_root_port.c | 4 ++++ > > include/hw/pci/pcie_port.h | 1 + > > 3 files changed, 9 insertions(+) > > > > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci- > > bridge/gen_pcie_root_port.c > > index 9766edb..26bda73 100644 > > --- a/hw/pci-bridge/gen_pcie_root_port.c > > +++ b/hw/pci-bridge/gen_pcie_root_port.c > > @@ -20,6 +20,9 @@ > > OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT) > > > > #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100 > > +#define GEN_PCIE_ROOT_PORT_ACS_OFFSET \ > > + (GEN_PCIE_ROOT_PORT_AER_OFFSET + PCI_ERR_SIZEOF) > > + > > #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1 > > > > typedef struct GenPCIERootPort { > > @@ -149,6 +152,7 @@ static void gen_rp_dev_class_init(ObjectClass *klass, > > void *data) > > rpc->interrupts_init = gen_rp_interrupts_init; > > rpc->interrupts_uninit = gen_rp_interrupts_uninit; > > rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET; > > + rpc->acs_offset = GEN_PCIE_ROOT_PORT_ACS_OFFSET; > > } > > > > static const TypeInfo gen_rp_dev_info = { > > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c > > index 34ad767..a0b4cf7 100644 > > --- a/hw/pci-bridge/pcie_root_port.c > > +++ b/hw/pci-bridge/pcie_root_port.c > > @@ -47,6 +47,7 @@ static void rp_reset(DeviceState *qdev) > > pcie_cap_deverr_reset(d); > > pcie_cap_slot_reset(d); > > pcie_cap_arifwd_reset(d); > > + pcie_cap_acs_reset(d); > > pcie_aer_root_reset(d); > > pci_bridge_reset(qdev); > > pci_bridge_disable_base_limit(d); > > @@ -106,6 +107,9 @@ static void rp_realize(PCIDevice *d, Error **errp) > > pcie_aer_root_init(d); > > rp_aer_vector_update(d); > > > > + if (rpc->acs_offset) { > > + pcie_acs_init(d, rpc->acs_offset); > > + } > > return; > > > > err: > > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h > > index df242a0..09586f4 100644 > > --- a/include/hw/pci/pcie_port.h > > +++ b/include/hw/pci/pcie_port.h > > @@ -78,6 +78,7 @@ typedef struct PCIERootPortClass { > > int exp_offset; > > int aer_offset; > > int ssvid_offset; > > + int acs_offset; /* If nonzero, optional ACS capability offset */ > > int ssid; > > } PCIERootPortClass; > > > > Reviewed-by: Marcel Apfelbaum Thanks! I added this r-b to v4, but left out the one for patch 1 as it got some more changes based on Alex's feedback and our fruitful discussion afterwards, so you might want to recheck it. Knut > > Thanks, > Marcel