* [Qemu-devel] [PATCH v6 0/2] pcie: Add simple ACS "support" to the generic PCIe root port @ 2019-02-21 18:13 Knut Omang 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function Knut Omang 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability Knut Omang 0 siblings, 2 replies; 5+ messages in thread From: Knut Omang @ 2019-02-21 18:13 UTC (permalink / raw) To: qemu-devel Cc: Michael S . Tsirkin, Marcel Apfelbaum, Alex Williamson, Stefan Hajnoczi, Elijah Shakkour, Tal Attaly, Knut Omang These two patches together implements a PCIe capability config space header for Access Control Services (ACS) for the new Qemu specific generic root port. ACS support in the associated root port is needed for passing individual functions of a device populating the port through to an L2 guest from an unmodified kernel. Without this, the IOMMU group the device belongs to will also include the root port itself, and all functions the device provides. ACS is thus necessary to support SR/IOV where the primary purpose is to be able to share out individual VFs to different guests, which will not be permitted by VFIO or the Windows Hyper-V equivalent unless ACS is supported by the root port. These patches can also be found as part of an updated version of my SR/IOV emulation patch set at https://github.com/knuto/qemu/tree/sriov_patches_v12 The patches' basic operation with VFIO and iommu groups have been tested with the above patch set and a rebased version of an in progress igb ethernet device, which needs some more care before I can let it go out. Changes from v5: ---------------- - Fix comment on cap.bits Changes from v4: ---------------- - Set the DT bit for downstream ports - Fix the assertion guard against uses violating the spec - Use pci_is_express_downstream_port() instead of type casts for discrimination. Changes from v3: ---------------- - rebased to the latest qemu master - Revised commit message and comments for patch #1 to make it clearer that VFIO works for single function devices even without ACS. - Improved checking for valid endpoints for ACS. - Fixed comment style issue - Co-locate the pci_acs_init and _reset functions and rename pci_cap_acs_reset to pci_acs_reset to adhere to the naming conventions that _cap_ functions in pcie is for changing state in the main pcie capability and not the individual extended capabilities. - Added Marcel's r-b to patch 2, which did not change Changes from v2: ---------------- - rebased to the latest qemu master Incorporated further feedback from Alex: - Make sure slot/downstream capability bits are only set for slots. - Make acs reset callback do nothing if no acs capability exists - Set correct acs version - div simplification Changes from v1: ---------------- Incorporated feedback from Alex Williamson: - Make commit messages reflect a more correct understanding of how this affects VFIO operation. - Implemented the CTRL register properly (reset callback + making non-implemented capabilities RO, default value 0) - removed the egress ctrl vector parameter to the init function - Fixed some whitespace issues Knut Omang (2): pcie: Add a simple PCIe ACS (Access Control Services) helper function gen_pcie_root_port: Add ACS (Access Control Services) capability hw/pci-bridge/gen_pcie_root_port.c | 4 +++- hw/pci-bridge/pcie_root_port.c | 4 +++- hw/pci/pcie.c | 38 +++++++++++++++++++++++++++++++- include/hw/pci/pcie.h | 6 +++++- include/hw/pci/pcie_port.h | 1 +- include/hw/pci/pcie_regs.h | 4 +++- 6 files changed, 57 insertions(+) base-commit: 0b5e750bea635b167eb03d86c3d9a09bbd43bc06 -- git-series 0.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function 2019-02-21 18:13 [Qemu-devel] [PATCH v6 0/2] pcie: Add simple ACS "support" to the generic PCIe root port Knut Omang @ 2019-02-21 18:13 ` Knut Omang 2019-02-21 21:24 ` Alex Williamson 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability Knut Omang 1 sibling, 1 reply; 5+ messages in thread From: Knut Omang @ 2019-02-21 18:13 UTC (permalink / raw) To: qemu-devel Cc: Michael S . Tsirkin, Marcel Apfelbaum, Alex Williamson, Stefan Hajnoczi, Elijah Shakkour, Tal Attaly, Knut Omang Implementing an ACS capability on downstream ports and multifunction endpoints indicates isolation and IOMMU visibility to a finer granularity. This creates smaller IOMMU groups in the guest and thus more flexibility in assigning endpoints to guest userspace or an L2 guest. Signed-off-by: Knut Omang <knut.omang@oracle.com> --- hw/pci/pcie.c | 38 ++++++++++++++++++++++++++++++++++++++ include/hw/pci/pcie.h | 6 ++++++ include/hw/pci/pcie_regs.h | 4 ++++ 3 files changed, 48 insertions(+) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 230478f..09ebf11 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -906,3 +906,41 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset) pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f); } + +/* ACS (Access Control Services) */ +void pcie_acs_init(PCIDevice *dev, uint16_t offset) +{ + bool is_downstream = pci_is_express_downstream_port(dev); + uint16_t cap_bits = 0; + + /* For endpoints, only multifunction devs may have an ACS capability: */ + assert(is_downstream || + (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) || + PCI_FUNC(dev->devfn)); + + pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset, + PCI_ACS_SIZEOF); + dev->exp.acs_cap = offset; + + if (is_downstream) { + /* + * Downstream ports must implement SV, TB, RR, CR, UF, and DT (with + * caveats on the latter four that we ignore for simplicity). + * Endpoints may also implement a subset of ACS capabilities, + * but these are optional if the endpoint does not support + * peer-to-peer between functions and thus omitted here. + */ + cap_bits = PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | + PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT; + } + + pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits); + pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits); +} + +void pcie_acs_reset(PCIDevice *dev) +{ + if (dev->exp.acs_cap) { + pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0); + } +} diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h index 5b82a0d..e30334d 100644 --- a/include/hw/pci/pcie.h +++ b/include/hw/pci/pcie.h @@ -79,6 +79,9 @@ struct PCIExpressDevice { /* Offset of ATS capability in config space */ uint16_t ats_cap; + + /* ACS */ + uint16_t acs_cap; }; #define COMPAT_PROP_PCP "power_controller_present" @@ -128,6 +131,9 @@ void pcie_add_capability(PCIDevice *dev, uint16_t offset, uint16_t size); void pcie_sync_bridge_lnk(PCIDevice *dev); +void pcie_acs_init(PCIDevice *dev, uint16_t offset); +void pcie_acs_reset(PCIDevice *dev); + void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn); void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num); void pcie_ats_init(PCIDevice *dev, uint16_t offset); diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h index ad4e780..1db86b0 100644 --- a/include/hw/pci/pcie_regs.h +++ b/include/hw/pci/pcie_regs.h @@ -175,4 +175,8 @@ typedef enum PCIExpLinkWidth { PCI_ERR_COR_INTERNAL | \ PCI_ERR_COR_HL_OVERFLOW) +/* ACS */ +#define PCI_ACS_VER 0x1 +#define PCI_ACS_SIZEOF 8 + #endif /* QEMU_PCIE_REGS_H */ -- git-series 0.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function Knut Omang @ 2019-02-21 21:24 ` Alex Williamson 0 siblings, 0 replies; 5+ messages in thread From: Alex Williamson @ 2019-02-21 21:24 UTC (permalink / raw) To: Knut Omang Cc: qemu-devel, Michael S . Tsirkin, Marcel Apfelbaum, Stefan Hajnoczi, Elijah Shakkour, Tal Attaly On Thu, 21 Feb 2019 19:13:22 +0100 Knut Omang <knut.omang@oracle.com> wrote: > Implementing an ACS capability on downstream ports and multifunction > endpoints indicates isolation and IOMMU visibility to a finer > granularity. This creates smaller IOMMU groups in the guest and thus > more flexibility in assigning endpoints to guest userspace or an L2 > guest. > > Signed-off-by: Knut Omang <knut.omang@oracle.com> > --- > hw/pci/pcie.c | 38 ++++++++++++++++++++++++++++++++++++++ > include/hw/pci/pcie.h | 6 ++++++ > include/hw/pci/pcie_regs.h | 4 ++++ > 3 files changed, 48 insertions(+) Reviewed-by: Alex Williamson <alex.williamson@redhat.com> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 230478f..09ebf11 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -906,3 +906,41 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset) > > pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f); > } > + > +/* ACS (Access Control Services) */ > +void pcie_acs_init(PCIDevice *dev, uint16_t offset) > +{ > + bool is_downstream = pci_is_express_downstream_port(dev); > + uint16_t cap_bits = 0; > + > + /* For endpoints, only multifunction devs may have an ACS capability: */ > + assert(is_downstream || > + (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) || > + PCI_FUNC(dev->devfn)); > + > + pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset, > + PCI_ACS_SIZEOF); > + dev->exp.acs_cap = offset; > + > + if (is_downstream) { > + /* > + * Downstream ports must implement SV, TB, RR, CR, UF, and DT (with > + * caveats on the latter four that we ignore for simplicity). > + * Endpoints may also implement a subset of ACS capabilities, > + * but these are optional if the endpoint does not support > + * peer-to-peer between functions and thus omitted here. > + */ > + cap_bits = PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | > + PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT; > + } > + > + pci_set_word(dev->config + offset + PCI_ACS_CAP, cap_bits); > + pci_set_word(dev->wmask + offset + PCI_ACS_CTRL, cap_bits); > +} > + > +void pcie_acs_reset(PCIDevice *dev) > +{ > + if (dev->exp.acs_cap) { > + pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0); > + } > +} > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > index 5b82a0d..e30334d 100644 > --- a/include/hw/pci/pcie.h > +++ b/include/hw/pci/pcie.h > @@ -79,6 +79,9 @@ struct PCIExpressDevice { > > /* Offset of ATS capability in config space */ > uint16_t ats_cap; > + > + /* ACS */ > + uint16_t acs_cap; > }; > > #define COMPAT_PROP_PCP "power_controller_present" > @@ -128,6 +131,9 @@ void pcie_add_capability(PCIDevice *dev, > uint16_t offset, uint16_t size); > void pcie_sync_bridge_lnk(PCIDevice *dev); > > +void pcie_acs_init(PCIDevice *dev, uint16_t offset); > +void pcie_acs_reset(PCIDevice *dev); > + > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn); > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num); > void pcie_ats_init(PCIDevice *dev, uint16_t offset); > diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h > index ad4e780..1db86b0 100644 > --- a/include/hw/pci/pcie_regs.h > +++ b/include/hw/pci/pcie_regs.h > @@ -175,4 +175,8 @@ typedef enum PCIExpLinkWidth { > PCI_ERR_COR_INTERNAL | \ > PCI_ERR_COR_HL_OVERFLOW) > > +/* ACS */ > +#define PCI_ACS_VER 0x1 > +#define PCI_ACS_SIZEOF 8 > + > #endif /* QEMU_PCIE_REGS_H */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability 2019-02-21 18:13 [Qemu-devel] [PATCH v6 0/2] pcie: Add simple ACS "support" to the generic PCIe root port Knut Omang 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function Knut Omang @ 2019-02-21 18:13 ` Knut Omang 2019-02-21 21:24 ` Alex Williamson 1 sibling, 1 reply; 5+ messages in thread From: Knut Omang @ 2019-02-21 18:13 UTC (permalink / raw) To: qemu-devel Cc: Michael S . Tsirkin, Marcel Apfelbaum, Alex Williamson, Stefan Hajnoczi, Elijah Shakkour, Tal Attaly, Knut Omang 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 <knut.omang@oracle.com> Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> --- 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..e94d918 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_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; -- git-series 0.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability Knut Omang @ 2019-02-21 21:24 ` Alex Williamson 0 siblings, 0 replies; 5+ messages in thread From: Alex Williamson @ 2019-02-21 21:24 UTC (permalink / raw) To: Knut Omang Cc: qemu-devel, Michael S . Tsirkin, Marcel Apfelbaum, Stefan Hajnoczi, Elijah Shakkour, Tal Attaly On Thu, 21 Feb 2019 19:13:23 +0100 Knut Omang <knut.omang@oracle.com> 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 <knut.omang@oracle.com> > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > --- > 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(+) Reviewed-by: Alex Williamson <alex.williamson@redhat.com> > 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..e94d918 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_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; > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-21 21:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-21 18:13 [Qemu-devel] [PATCH v6 0/2] pcie: Add simple ACS "support" to the generic PCIe root port Knut Omang 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function Knut Omang 2019-02-21 21:24 ` Alex Williamson 2019-02-21 18:13 ` [Qemu-devel] [PATCH v6 2/2] gen_pcie_root_port: Add ACS (Access Control Services) capability Knut Omang 2019-02-21 21:24 ` Alex Williamson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.