| Message ID | 20180627034935.20276-4-venu.busireddy@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > used to pair a virtio device with the passthrough device attached to > that bridge. > > This capability is added to the bridge iff the "uuid" option is specified > for the bridge. I think the name should be more explicit. How about "failover-group-id"? > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> I'd like to also tweak the device id in this case, to make it easier for guests to know it's a grouping bridge. > --- > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ > include/hw/pci/pcie.h | 1 + > 3 files changed, 35 insertions(+) > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > index b2d861d216..bbbc6fa1c6 100644 > --- a/hw/pci-bridge/pci_bridge_dev.c > +++ b/hw/pci-bridge/pci_bridge_dev.c > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) > bridge_dev->msi = ON_OFF_AUTO_OFF; > } > > + err = pci_bridge_vendor_init(dev, 0, errp); > + if (err < 0) { > + error_append_hint(errp, "Can't init group ID, error %d\n", err); > + goto vendor_cap_err; > + } > + > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); > if (err) { > goto slotid_error; > @@ -109,6 +115,7 @@ slotid_error: > if (shpc_present(dev)) { > shpc_cleanup(dev, &bridge_dev->bar); > } > +vendor_cap_err: > shpc_error: > pci_bridge_exitfn(dev); > } > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { > ON_OFF_AUTO_AUTO), > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > PCI_BRIDGE_DEV_F_SHPC_REQ, true), > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 40a39f57cb..cb8b3dad2a 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -34,12 +34,17 @@ > #include "hw/pci/pci_bus.h" > #include "qemu/range.h" > #include "qapi/error.h" > +#include "qemu/uuid.h" > > /* PCI bridge subsystem vendor ID helper functions */ > #define PCI_SSVID_SIZEOF 8 > #define PCI_SSVID_SVID 4 > #define PCI_SSVID_SSID 6 > > +#define PCI_VENDOR_SIZEOF 20 > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 > + > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > uint16_t svid, uint16_t ssid, > Error **errp) > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > return pos; > } > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) > +{ > + int pos; > + > + if (qemu_uuid_is_null(&d->uuid)) { > + return 0; > + } > + > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, > + errp); > + if (pos < 0) { > + return pos; > + } > + > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, > + PCI_VENDOR_SIZEOF); > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, > + sizeof(QemuUUID)); > + return pos; > +} > + > /* Accessor function to get parent bridge device from pci bus. */ > PCIDevice *pci_bridge_get_device(PCIBus *bus) > { > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > index b71e369703..b4189d0ce3 100644 > --- a/include/hw/pci/pcie.h > +++ b/include/hw/pci/pcie.h > @@ -82,6 +82,7 @@ struct PCIExpressDevice { > }; > > #define COMPAT_PROP_PCP "power_controller_present" > +#define COMPAT_PROP_UUID "uuid" > > /* PCI express capability helper functions */ > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type,
On 2018-06-27 07:02:36 +0300, Michael S. Tsirkin wrote: > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > used to pair a virtio device with the passthrough device attached to > > that bridge. > > > > This capability is added to the bridge iff the "uuid" option is specified > > for the bridge. > > I think the name should be more explicit. How about "failover-group-id"? I can change it. But don't you think it is bit long? > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > I'd like to also tweak the device id in this case, > to make it easier for guests to know it's a grouping bridge. Could you please recommend a name for the new ID'd definition? Something in lines of PCI_DEVICE_ID_REDHAT_<blah blah>. Thanks, Venu > > > --- > > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ > > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ > > include/hw/pci/pcie.h | 1 + > > 3 files changed, 35 insertions(+) > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > > index b2d861d216..bbbc6fa1c6 100644 > > --- a/hw/pci-bridge/pci_bridge_dev.c > > +++ b/hw/pci-bridge/pci_bridge_dev.c > > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) > > bridge_dev->msi = ON_OFF_AUTO_OFF; > > } > > > > + err = pci_bridge_vendor_init(dev, 0, errp); > > + if (err < 0) { > > + error_append_hint(errp, "Can't init group ID, error %d\n", err); > > + goto vendor_cap_err; > > + } > > + > > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); > > if (err) { > > goto slotid_error; > > @@ -109,6 +115,7 @@ slotid_error: > > if (shpc_present(dev)) { > > shpc_cleanup(dev, &bridge_dev->bar); > > } > > +vendor_cap_err: > > shpc_error: > > pci_bridge_exitfn(dev); > > } > > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { > > ON_OFF_AUTO_AUTO), > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > > PCI_BRIDGE_DEV_F_SHPC_REQ, true), > > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > > index 40a39f57cb..cb8b3dad2a 100644 > > --- a/hw/pci/pci_bridge.c > > +++ b/hw/pci/pci_bridge.c > > @@ -34,12 +34,17 @@ > > #include "hw/pci/pci_bus.h" > > #include "qemu/range.h" > > #include "qapi/error.h" > > +#include "qemu/uuid.h" > > > > /* PCI bridge subsystem vendor ID helper functions */ > > #define PCI_SSVID_SIZEOF 8 > > #define PCI_SSVID_SVID 4 > > #define PCI_SSVID_SSID 6 > > > > +#define PCI_VENDOR_SIZEOF 20 > > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 > > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 > > + > > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > uint16_t svid, uint16_t ssid, > > Error **errp) > > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > return pos; > > } > > > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) > > +{ > > + int pos; > > + > > + if (qemu_uuid_is_null(&d->uuid)) { > > + return 0; > > + } > > + > > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, > > + errp); > > + if (pos < 0) { > > + return pos; > > + } > > + > > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, > > + PCI_VENDOR_SIZEOF); > > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, > > + sizeof(QemuUUID)); > > + return pos; > > +} > > + > > /* Accessor function to get parent bridge device from pci bus. */ > > PCIDevice *pci_bridge_get_device(PCIBus *bus) > > { > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > > index b71e369703..b4189d0ce3 100644 > > --- a/include/hw/pci/pcie.h > > +++ b/include/hw/pci/pcie.h > > @@ -82,6 +82,7 @@ struct PCIExpressDevice { > > }; > > > > #define COMPAT_PROP_PCP "power_controller_present" > > +#define COMPAT_PROP_UUID "uuid" > > > > /* PCI express capability helper functions */ > > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >
On 2018-06-26 23:08:12 -0500, Venu Busireddy wrote: > On 2018-06-27 07:02:36 +0300, Michael S. Tsirkin wrote: > > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > > used to pair a virtio device with the passthrough device attached to > > > that bridge. > > > > > > This capability is added to the bridge iff the "uuid" option is specified > > > for the bridge. > > > > I think the name should be more explicit. How about "failover-group-id"? > > I can change it. But don't you think it is bit long? > > > > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > > > I'd like to also tweak the device id in this case, > > to make it easier for guests to know it's a grouping bridge. > > Could you please recommend a name for the new ID'd definition? Something > in lines of PCI_DEVICE_ID_REDHAT_<blah blah>. How about these names and values for the device IDs? PCI_DEVICE_ID_REDHAT_PCI_BRIDGE_GRP (0x0010) for pci-bridge, and PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE_GRP (0x0011) for pcie-downstream Thanks, Venu > > > --- > > > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ > > > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ > > > include/hw/pci/pcie.h | 1 + > > > 3 files changed, 35 insertions(+) > > > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > > > index b2d861d216..bbbc6fa1c6 100644 > > > --- a/hw/pci-bridge/pci_bridge_dev.c > > > +++ b/hw/pci-bridge/pci_bridge_dev.c > > > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) > > > bridge_dev->msi = ON_OFF_AUTO_OFF; > > > } > > > > > > + err = pci_bridge_vendor_init(dev, 0, errp); > > > + if (err < 0) { > > > + error_append_hint(errp, "Can't init group ID, error %d\n", err); > > > + goto vendor_cap_err; > > > + } > > > + > > > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); > > > if (err) { > > > goto slotid_error; > > > @@ -109,6 +115,7 @@ slotid_error: > > > if (shpc_present(dev)) { > > > shpc_cleanup(dev, &bridge_dev->bar); > > > } > > > +vendor_cap_err: > > > shpc_error: > > > pci_bridge_exitfn(dev); > > > } > > > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { > > > ON_OFF_AUTO_AUTO), > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > > > PCI_BRIDGE_DEV_F_SHPC_REQ, true), > > > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > > > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > > > index 40a39f57cb..cb8b3dad2a 100644 > > > --- a/hw/pci/pci_bridge.c > > > +++ b/hw/pci/pci_bridge.c > > > @@ -34,12 +34,17 @@ > > > #include "hw/pci/pci_bus.h" > > > #include "qemu/range.h" > > > #include "qapi/error.h" > > > +#include "qemu/uuid.h" > > > > > > /* PCI bridge subsystem vendor ID helper functions */ > > > #define PCI_SSVID_SIZEOF 8 > > > #define PCI_SSVID_SVID 4 > > > #define PCI_SSVID_SSID 6 > > > > > > +#define PCI_VENDOR_SIZEOF 20 > > > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 > > > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 > > > + > > > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > uint16_t svid, uint16_t ssid, > > > Error **errp) > > > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > return pos; > > > } > > > > > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) > > > +{ > > > + int pos; > > > + > > > + if (qemu_uuid_is_null(&d->uuid)) { > > > + return 0; > > > + } > > > + > > > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, > > > + errp); > > > + if (pos < 0) { > > > + return pos; > > > + } > > > + > > > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, > > > + PCI_VENDOR_SIZEOF); > > > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, > > > + sizeof(QemuUUID)); > > > + return pos; > > > +} > > > + > > > /* Accessor function to get parent bridge device from pci bus. */ > > > PCIDevice *pci_bridge_get_device(PCIBus *bus) > > > { > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > > > index b71e369703..b4189d0ce3 100644 > > > --- a/include/hw/pci/pcie.h > > > +++ b/include/hw/pci/pcie.h > > > @@ -82,6 +82,7 @@ struct PCIExpressDevice { > > > }; > > > > > > #define COMPAT_PROP_PCP "power_controller_present" > > > +#define COMPAT_PROP_UUID "uuid" > > > > > > /* PCI express capability helper functions */ > > > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >
On Wed, Jun 27, 2018 at 06:07:59PM -0500, Venu Busireddy wrote: > On 2018-06-26 23:08:12 -0500, Venu Busireddy wrote: > > On 2018-06-27 07:02:36 +0300, Michael S. Tsirkin wrote: > > > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > > > used to pair a virtio device with the passthrough device attached to > > > > that bridge. > > > > > > > > This capability is added to the bridge iff the "uuid" option is specified > > > > for the bridge. > > > > > > I think the name should be more explicit. How about "failover-group-id"? > > > > I can change it. But don't you think it is bit long? > > > > > > > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > > > > > I'd like to also tweak the device id in this case, > > > to make it easier for guests to know it's a grouping bridge. > > > > Could you please recommend a name for the new ID'd definition? Something > > in lines of PCI_DEVICE_ID_REDHAT_<blah blah>. > > How about these names and values for the device IDs? > > PCI_DEVICE_ID_REDHAT_PCI_BRIDGE_GRP (0x0010) for pci-bridge, and You do not need the second PCI, and group is one of the functions of bridge anyway. PCI_DEVICE_ID_REDHAT_BRIDGE_FAILOVER? > PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE_GRP (0x0011) for pcie-downstream > > Thanks, > > Venu PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER? It's becoming annoying though - we'll need one for each type :( Ideas/options: - use revision ID to distinguish from regular bridge - use device serial # cap for the bridge when it's an express device > > > > --- > > > > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ > > > > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ > > > > include/hw/pci/pcie.h | 1 + > > > > 3 files changed, 35 insertions(+) > > > > > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > > > > index b2d861d216..bbbc6fa1c6 100644 > > > > --- a/hw/pci-bridge/pci_bridge_dev.c > > > > +++ b/hw/pci-bridge/pci_bridge_dev.c > > > > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) > > > > bridge_dev->msi = ON_OFF_AUTO_OFF; > > > > } > > > > > > > > + err = pci_bridge_vendor_init(dev, 0, errp); > > > > + if (err < 0) { > > > > + error_append_hint(errp, "Can't init group ID, error %d\n", err); > > > > + goto vendor_cap_err; > > > > + } > > > > + > > > > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); > > > > if (err) { > > > > goto slotid_error; > > > > @@ -109,6 +115,7 @@ slotid_error: > > > > if (shpc_present(dev)) { > > > > shpc_cleanup(dev, &bridge_dev->bar); > > > > } > > > > +vendor_cap_err: > > > > shpc_error: > > > > pci_bridge_exitfn(dev); > > > > } > > > > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { > > > > ON_OFF_AUTO_AUTO), > > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > > > > PCI_BRIDGE_DEV_F_SHPC_REQ, true), > > > > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), > > > > DEFINE_PROP_END_OF_LIST(), > > > > }; > > > > > > > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > > > > index 40a39f57cb..cb8b3dad2a 100644 > > > > --- a/hw/pci/pci_bridge.c > > > > +++ b/hw/pci/pci_bridge.c > > > > @@ -34,12 +34,17 @@ > > > > #include "hw/pci/pci_bus.h" > > > > #include "qemu/range.h" > > > > #include "qapi/error.h" > > > > +#include "qemu/uuid.h" > > > > > > > > /* PCI bridge subsystem vendor ID helper functions */ > > > > #define PCI_SSVID_SIZEOF 8 > > > > #define PCI_SSVID_SVID 4 > > > > #define PCI_SSVID_SSID 6 > > > > > > > > +#define PCI_VENDOR_SIZEOF 20 > > > > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 > > > > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 > > > > + > > > > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > > uint16_t svid, uint16_t ssid, > > > > Error **errp) > > > > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > > return pos; > > > > } > > > > > > > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) > > > > +{ > > > > + int pos; > > > > + > > > > + if (qemu_uuid_is_null(&d->uuid)) { > > > > + return 0; > > > > + } > > > > + > > > > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, > > > > + errp); > > > > + if (pos < 0) { > > > > + return pos; > > > > + } > > > > + > > > > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, > > > > + PCI_VENDOR_SIZEOF); > > > > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, > > > > + sizeof(QemuUUID)); > > > > + return pos; > > > > +} > > > > + > > > > /* Accessor function to get parent bridge device from pci bus. */ > > > > PCIDevice *pci_bridge_get_device(PCIBus *bus) > > > > { > > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > > > > index b71e369703..b4189d0ce3 100644 > > > > --- a/include/hw/pci/pcie.h > > > > +++ b/include/hw/pci/pcie.h > > > > @@ -82,6 +82,7 @@ struct PCIExpressDevice { > > > > }; > > > > > > > > #define COMPAT_PROP_PCP "power_controller_present" > > > > +#define COMPAT_PROP_UUID "uuid" > > > > > > > > /* PCI express capability helper functions */ > > > > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > >
On 2018-06-28 05:14:50 +0300, Michael S. Tsirkin wrote: > On Wed, Jun 27, 2018 at 06:07:59PM -0500, Venu Busireddy wrote: > > On 2018-06-26 23:08:12 -0500, Venu Busireddy wrote: > > > On 2018-06-27 07:02:36 +0300, Michael S. Tsirkin wrote: > > > > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > > > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > > > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > > > > used to pair a virtio device with the passthrough device attached to > > > > > that bridge. > > > > > > > > > > This capability is added to the bridge iff the "uuid" option is specified > > > > > for the bridge. > > > > > > > > I think the name should be more explicit. How about "failover-group-id"? > > > > > > I can change it. But don't you think it is bit long? > > > > > > > > > > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > > > > > > > I'd like to also tweak the device id in this case, > > > > to make it easier for guests to know it's a grouping bridge. > > > > > > Could you please recommend a name for the new ID'd definition? Something > > > in lines of PCI_DEVICE_ID_REDHAT_<blah blah>. > > > > How about these names and values for the device IDs? > > > > PCI_DEVICE_ID_REDHAT_PCI_BRIDGE_GRP (0x0010) for pci-bridge, and > > You do not need the second PCI, and group is one of the > functions of bridge anyway. > > PCI_DEVICE_ID_REDHAT_BRIDGE_FAILOVER? Sounds good. > > PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE_GRP (0x0011) for pcie-downstream > > > > Thanks, > > > > Venu > > PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER? Sounds good. > It's becoming annoying though - we'll need one for each type :( > > Ideas/options: > - use revision ID to distinguish from regular bridge What would we use for the revision? For example, "pci-bridge" (default one) currently uses revision 0. Should we use revision 1 for the group bridge? If we do that, what if in the future we need to up the revision for the default bridge? > - use device serial # cap for the bridge when it's an express device We could do that. But if we are defining a new Device ID for the PCI case, we might as well do so for the PCIe case too, and be consistent? The other advantage in defining a new ID is, 'lspci' could right away tell what type of bridge it is (if we update PCI IDs). No need to map the revision to a bridge type! Unless you have strong reservations against, I think the notion of different Device ID for the grouping bridge sounds cleaner. Do you agree? Venu > > > > > > --- > > > > > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ > > > > > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ > > > > > include/hw/pci/pcie.h | 1 + > > > > > 3 files changed, 35 insertions(+) > > > > > > > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > > > > > index b2d861d216..bbbc6fa1c6 100644 > > > > > --- a/hw/pci-bridge/pci_bridge_dev.c > > > > > +++ b/hw/pci-bridge/pci_bridge_dev.c > > > > > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) > > > > > bridge_dev->msi = ON_OFF_AUTO_OFF; > > > > > } > > > > > > > > > > + err = pci_bridge_vendor_init(dev, 0, errp); > > > > > + if (err < 0) { > > > > > + error_append_hint(errp, "Can't init group ID, error %d\n", err); > > > > > + goto vendor_cap_err; > > > > > + } > > > > > + > > > > > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); > > > > > if (err) { > > > > > goto slotid_error; > > > > > @@ -109,6 +115,7 @@ slotid_error: > > > > > if (shpc_present(dev)) { > > > > > shpc_cleanup(dev, &bridge_dev->bar); > > > > > } > > > > > +vendor_cap_err: > > > > > shpc_error: > > > > > pci_bridge_exitfn(dev); > > > > > } > > > > > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { > > > > > ON_OFF_AUTO_AUTO), > > > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, > > > > > PCI_BRIDGE_DEV_F_SHPC_REQ, true), > > > > > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), > > > > > DEFINE_PROP_END_OF_LIST(), > > > > > }; > > > > > > > > > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > > > > > index 40a39f57cb..cb8b3dad2a 100644 > > > > > --- a/hw/pci/pci_bridge.c > > > > > +++ b/hw/pci/pci_bridge.c > > > > > @@ -34,12 +34,17 @@ > > > > > #include "hw/pci/pci_bus.h" > > > > > #include "qemu/range.h" > > > > > #include "qapi/error.h" > > > > > +#include "qemu/uuid.h" > > > > > > > > > > /* PCI bridge subsystem vendor ID helper functions */ > > > > > #define PCI_SSVID_SIZEOF 8 > > > > > #define PCI_SSVID_SVID 4 > > > > > #define PCI_SSVID_SSID 6 > > > > > > > > > > +#define PCI_VENDOR_SIZEOF 20 > > > > > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 > > > > > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 > > > > > + > > > > > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > > > uint16_t svid, uint16_t ssid, > > > > > Error **errp) > > > > > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, > > > > > return pos; > > > > > } > > > > > > > > > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) > > > > > +{ > > > > > + int pos; > > > > > + > > > > > + if (qemu_uuid_is_null(&d->uuid)) { > > > > > + return 0; > > > > > + } > > > > > + > > > > > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, > > > > > + errp); > > > > > + if (pos < 0) { > > > > > + return pos; > > > > > + } > > > > > + > > > > > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, > > > > > + PCI_VENDOR_SIZEOF); > > > > > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, > > > > > + sizeof(QemuUUID)); > > > > > + return pos; > > > > > +} > > > > > + > > > > > /* Accessor function to get parent bridge device from pci bus. */ > > > > > PCIDevice *pci_bridge_get_device(PCIBus *bus) > > > > > { > > > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > > > > > index b71e369703..b4189d0ce3 100644 > > > > > --- a/include/hw/pci/pcie.h > > > > > +++ b/include/hw/pci/pcie.h > > > > > @@ -82,6 +82,7 @@ struct PCIExpressDevice { > > > > > }; > > > > > > > > > > #define COMPAT_PROP_PCP "power_controller_present" > > > > > +#define COMPAT_PROP_UUID "uuid" > > > > > > > > > > /* PCI express capability helper functions */ > > > > > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >
On Wed, Jun 27, 2018 at 7:14 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Jun 27, 2018 at 06:07:59PM -0500, Venu Busireddy wrote: >> On 2018-06-26 23:08:12 -0500, Venu Busireddy wrote: >> > On 2018-06-27 07:02:36 +0300, Michael S. Tsirkin wrote: >> > > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: >> > > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device >> > > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be >> > > > used to pair a virtio device with the passthrough device attached to >> > > > that bridge. >> > > > >> > > > This capability is added to the bridge iff the "uuid" option is specified >> > > > for the bridge. >> > > >> > > I think the name should be more explicit. How about "failover-group-id"? >> > >> > I can change it. But don't you think it is bit long? >> > >> > > > >> > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> >> > > >> > > I'd like to also tweak the device id in this case, >> > > to make it easier for guests to know it's a grouping bridge. >> > >> > Could you please recommend a name for the new ID'd definition? Something >> > in lines of PCI_DEVICE_ID_REDHAT_<blah blah>. >> >> How about these names and values for the device IDs? >> >> PCI_DEVICE_ID_REDHAT_PCI_BRIDGE_GRP (0x0010) for pci-bridge, and > > You do not need the second PCI, and group is one of the > functions of bridge anyway. > > PCI_DEVICE_ID_REDHAT_BRIDGE_FAILOVER? > > >> PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE_GRP (0x0011) for pcie-downstream >> >> Thanks, >> >> Venu > > PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER? > > > It's becoming annoying though - we'll need one for each type :( > > Ideas/options: > - use revision ID to distinguish from regular bridge > - use device serial # cap for the bridge when it's an express device > IMO, from guest implementation point of view, it would be the best to use a single consistent mechanism all across. The serial number cap is too PCIE specific. -Siwei > >> > > > --- >> > > > hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ >> > > > hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ >> > > > include/hw/pci/pcie.h | 1 + >> > > > 3 files changed, 35 insertions(+) >> > > > >> > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c >> > > > index b2d861d216..bbbc6fa1c6 100644 >> > > > --- a/hw/pci-bridge/pci_bridge_dev.c >> > > > +++ b/hw/pci-bridge/pci_bridge_dev.c >> > > > @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) >> > > > bridge_dev->msi = ON_OFF_AUTO_OFF; >> > > > } >> > > > >> > > > + err = pci_bridge_vendor_init(dev, 0, errp); >> > > > + if (err < 0) { >> > > > + error_append_hint(errp, "Can't init group ID, error %d\n", err); >> > > > + goto vendor_cap_err; >> > > > + } >> > > > + >> > > > err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); >> > > > if (err) { >> > > > goto slotid_error; >> > > > @@ -109,6 +115,7 @@ slotid_error: >> > > > if (shpc_present(dev)) { >> > > > shpc_cleanup(dev, &bridge_dev->bar); >> > > > } >> > > > +vendor_cap_err: >> > > > shpc_error: >> > > > pci_bridge_exitfn(dev); >> > > > } >> > > > @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { >> > > > ON_OFF_AUTO_AUTO), >> > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, >> > > > PCI_BRIDGE_DEV_F_SHPC_REQ, true), >> > > > + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), >> > > > DEFINE_PROP_END_OF_LIST(), >> > > > }; >> > > > >> > > > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c >> > > > index 40a39f57cb..cb8b3dad2a 100644 >> > > > --- a/hw/pci/pci_bridge.c >> > > > +++ b/hw/pci/pci_bridge.c >> > > > @@ -34,12 +34,17 @@ >> > > > #include "hw/pci/pci_bus.h" >> > > > #include "qemu/range.h" >> > > > #include "qapi/error.h" >> > > > +#include "qemu/uuid.h" >> > > > >> > > > /* PCI bridge subsystem vendor ID helper functions */ >> > > > #define PCI_SSVID_SIZEOF 8 >> > > > #define PCI_SSVID_SVID 4 >> > > > #define PCI_SSVID_SSID 6 >> > > > >> > > > +#define PCI_VENDOR_SIZEOF 20 >> > > > +#define PCI_VENDOR_CAP_LEN_OFFSET 2 >> > > > +#define PCI_VENDOR_GROUP_ID_OFFSET 4 >> > > > + >> > > > int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, >> > > > uint16_t svid, uint16_t ssid, >> > > > Error **errp) >> > > > @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, >> > > > return pos; >> > > > } >> > > > >> > > > +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) >> > > > +{ >> > > > + int pos; >> > > > + >> > > > + if (qemu_uuid_is_null(&d->uuid)) { >> > > > + return 0; >> > > > + } >> > > > + >> > > > + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, >> > > > + errp); >> > > > + if (pos < 0) { >> > > > + return pos; >> > > > + } >> > > > + >> > > > + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, >> > > > + PCI_VENDOR_SIZEOF); >> > > > + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, >> > > > + sizeof(QemuUUID)); >> > > > + return pos; >> > > > +} >> > > > + >> > > > /* Accessor function to get parent bridge device from pci bus. */ >> > > > PCIDevice *pci_bridge_get_device(PCIBus *bus) >> > > > { >> > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h >> > > > index b71e369703..b4189d0ce3 100644 >> > > > --- a/include/hw/pci/pcie.h >> > > > +++ b/include/hw/pci/pcie.h >> > > > @@ -82,6 +82,7 @@ struct PCIExpressDevice { >> > > > }; >> > > > >> > > > #define COMPAT_PROP_PCP "power_controller_present" >> > > > +#define COMPAT_PROP_UUID "uuid" >> > > > >> > > > /* PCI express capability helper functions */ >> > > > int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, >> > > >> > > --------------------------------------------------------------------- >> > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org >> > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >> > > >> > >> > --------------------------------------------------------------------- >> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org >> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >
On Wed, Jun 27, 2018 at 07:02:36AM +0300, Michael S. Tsirkin wrote: > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > used to pair a virtio device with the passthrough device attached to > > that bridge. > > > > This capability is added to the bridge iff the "uuid" option is specified > > for the bridge. > > I think the name should be more explicit. How about "failover-group-id"? > > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > I'd like to also tweak the device id in this case, > to make it easier for guests to know it's a grouping bridge. I tend to think this proposal is rather too narrow. The need to tell the guest OS about the intended usage of devices is a pretty common use case, of which setting up bonding NIC pair is just one example. OpenStack already has this problem & took the approach of providing some structured JSON metadata to the guest that lists all devices (well NICs and disks at least) with identifying metadata (eg PCI address, MAC addr, disk serial and anything else relevant), and also assigns freeform string "tags" against each. eg the host admin could specify tags "lanA" and "lanB" for two NICs, so the guest knows which NIC to use for which purpose. This kind of tagging could easily cover setting up bonded pairs. Or for a disk they could tag it "oracledata" or "apachecache" to indicate what kind of data to use on the disk, etc https://specs.openstack.org/openstack/nova-specs/specs/mitaka/approved/virt-device-role-tagging.html I'm not suggesting the impl used in OpenStack is perfect - there's a number of pain points in it, but I think it shows the need for a general solution to the problem of the host admin informing the guest OS about the intended usage of devices being provided. I think freeform string tags do this pretty well. The key problem is how to expose such tags - openstack took approach of an out of band JSON doc exposed from its metadata service and or cloud init config drive, because that was its only viable option. Perhaps there is a way to expose tags at a low level though if we can integrate with QEMU somehow ? Regards, Daniel
On Thu, 28 Jun 2018 09:25:32 +0100 Daniel P. Berrangé <berrange@redhat.com> wrote: > On Wed, Jun 27, 2018 at 07:02:36AM +0300, Michael S. Tsirkin wrote: > > On Tue, Jun 26, 2018 at 10:49:33PM -0500, Venu Busireddy wrote: > > > Add the "Vendor-Specific" capability to the Red Hat PCI bridge device > > > "pci-bridge", to contain the "Group Identifier" (UUID) that will be > > > used to pair a virtio device with the passthrough device attached to > > > that bridge. > > > > > > This capability is added to the bridge iff the "uuid" option is specified > > > for the bridge. As an aside, I don't think that will work for s390, see https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg07785.html. > > > > I think the name should be more explicit. How about "failover-group-id"? > > > > > > > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > > > > I'd like to also tweak the device id in this case, > > to make it easier for guests to know it's a grouping bridge. > > I tend to think this proposal is rather too narrow. The need to tell > the guest OS about the intended usage of devices is a pretty common use > case, of which setting up bonding NIC pair is just one example. FWIW, there was also discussion of making "pair those devices" a more general virtio mechanism (see https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06339.html and around). > > OpenStack already has this problem & took the approach of providing some > structured JSON metadata to the guest that lists all devices (well NICs > and disks at least) with identifying metadata (eg PCI address, MAC addr, > disk serial and anything else relevant), and also assigns freeform string > "tags" against each. eg the host admin could specify tags "lanA" and "lanB" > for two NICs, so the guest knows which NIC to use for which purpose. This > kind of tagging could easily cover setting up bonded pairs. Or for a disk > they could tag it "oracledata" or "apachecache" to indicate what kind of > data to use on the disk, etc > > https://specs.openstack.org/openstack/nova-specs/specs/mitaka/approved/virt-device-role-tagging.html > > I'm not suggesting the impl used in OpenStack is perfect - there's a number > of pain points in it, but I think it shows the need for a general solution > to the problem of the host admin informing the guest OS about the intended > usage of devices being provided. I think freeform string tags do this > pretty well. The key problem is how to expose such tags - openstack took > approach of an out of band JSON doc exposed from its metadata service and > or cloud init config drive, because that was its only viable option. As another example, s390 has a mechanism for providing configuration information to a guest as well (see drivers/s390/char/sclp_sd.c in Linux; for a guest user space exploiter, see https://github.com/ibm-s390-tools/s390-tools/commit/7d355b0fec964ad84ecaf88eb946121d39486070 and follow-ons; the hypervisor implementation is proprietary AFAIK). > > Perhaps there is a way to expose tags at a low level though if we can > integrate with QEMU somehow ? I'd really like to see a data format that is flexible enough to cover existing needs (which can be used with different hypervisor<->guest interfaces.) We'd probably need to standardize it somehow if we want to tie in with the virtio OASIS spec. That said, for the net device standby functionality, it might be easier to just go with the VIRTIO_NET_F_STANDBY_UUID proposal while we discuss a more generic mechanism.
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c index b2d861d216..bbbc6fa1c6 100644 --- a/hw/pci-bridge/pci_bridge_dev.c +++ b/hw/pci-bridge/pci_bridge_dev.c @@ -71,6 +71,12 @@ static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp) bridge_dev->msi = ON_OFF_AUTO_OFF; } + err = pci_bridge_vendor_init(dev, 0, errp); + if (err < 0) { + error_append_hint(errp, "Can't init group ID, error %d\n", err); + goto vendor_cap_err; + } + err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp); if (err) { goto slotid_error; @@ -109,6 +115,7 @@ slotid_error: if (shpc_present(dev)) { shpc_cleanup(dev, &bridge_dev->bar); } +vendor_cap_err: shpc_error: pci_bridge_exitfn(dev); } @@ -162,6 +169,7 @@ static Property pci_bridge_dev_properties[] = { ON_OFF_AUTO_AUTO), DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, PCI_BRIDGE_DEV_F_SHPC_REQ, true), + DEFINE_PROP_UUID(COMPAT_PROP_UUID, PCIDevice, uuid, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 40a39f57cb..cb8b3dad2a 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -34,12 +34,17 @@ #include "hw/pci/pci_bus.h" #include "qemu/range.h" #include "qapi/error.h" +#include "qemu/uuid.h" /* PCI bridge subsystem vendor ID helper functions */ #define PCI_SSVID_SIZEOF 8 #define PCI_SSVID_SVID 4 #define PCI_SSVID_SSID 6 +#define PCI_VENDOR_SIZEOF 20 +#define PCI_VENDOR_CAP_LEN_OFFSET 2 +#define PCI_VENDOR_GROUP_ID_OFFSET 4 + int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, uint16_t svid, uint16_t ssid, Error **errp) @@ -57,6 +62,27 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset, return pos; } +int pci_bridge_vendor_init(PCIDevice *d, uint8_t offset, Error **errp) +{ + int pos; + + if (qemu_uuid_is_null(&d->uuid)) { + return 0; + } + + pos = pci_add_capability(d, PCI_CAP_ID_VNDR, offset, PCI_VENDOR_SIZEOF, + errp); + if (pos < 0) { + return pos; + } + + pci_set_word(d->config + pos + PCI_VENDOR_CAP_LEN_OFFSET, + PCI_VENDOR_SIZEOF); + memcpy(d->config + pos + PCI_VENDOR_GROUP_ID_OFFSET, &d->uuid, + sizeof(QemuUUID)); + return pos; +} + /* Accessor function to get parent bridge device from pci bus. */ PCIDevice *pci_bridge_get_device(PCIBus *bus) { diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h index b71e369703..b4189d0ce3 100644 --- a/include/hw/pci/pcie.h +++ b/include/hw/pci/pcie.h @@ -82,6 +82,7 @@ struct PCIExpressDevice { }; #define COMPAT_PROP_PCP "power_controller_present" +#define COMPAT_PROP_UUID "uuid" /* PCI express capability helper functions */ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type,
Add the "Vendor-Specific" capability to the Red Hat PCI bridge device "pci-bridge", to contain the "Group Identifier" (UUID) that will be used to pair a virtio device with the passthrough device attached to that bridge. This capability is added to the bridge iff the "uuid" option is specified for the bridge. Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> --- hw/pci-bridge/pci_bridge_dev.c | 8 ++++++++ hw/pci/pci_bridge.c | 26 ++++++++++++++++++++++++++ include/hw/pci/pcie.h | 1 + 3 files changed, 35 insertions(+)