5:51 a.m.
We can treat mdevs the same as all other PCI hostdevs
and figure out whether they are PCI Express or legacy PCI
by checking the size of their config space.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 2106b34..3277d18 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -645,9 +645,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
return pcieFlags;
}
- if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
- return pcieFlags;
-
if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
hostAddr->bus,
hostAddr->slot,
--
2.7.4
8:24 p.m.
On 05/26/2017 06:51 AM, Andrea Bolognani wrote:
We can treat mdevs the same as all other PCI hostdevs
and figure out whether they are PCI Express or legacy PCI
by checking the size of their config space.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 2106b34..3277d18 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -645,9 +645,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
return pcieFlags;
}
- if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
- return pcieFlags;
-
if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
hostAddr->bus,
hostAddr->slot,
This can't work. From the host's point of view, there is no PCI device
whose config space can be read, and from the code's point of view,
hostAddr->domain|bus|slot are invalid (they are in
hostdev->source.subsys.u.pci.addr, which is only valid when
(hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI).
Is there maybe something standard in the mdev's sysfs entry that could
be used to determine PCI vs PCIe? I think back when we were discussing
the implementation of this, Alex had said there wasn't.
1:20 a.m.
On Thu, 2017-06-01 at 21:24 -0400, Laine Stump wrote:
> @@ -645,9 +645,6 @@
qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> return pcieFlags;
> }
>
> - if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
> - return pcieFlags;
> -
> if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
> hostAddr->bus,
> hostAddr->slot,
This can't work. From the host's point of view, there is no PCI device
whose config space can be read, and from the code's point of view,
hostAddr->domain|bus|slot are invalid (they are in
hostdev->source.subsys.u.pci.addr, which is only valid when
(hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI).
Surely you meant to use '==' here?
Is there maybe something standard in the mdev's sysfs entry that
could
be used to determine PCI vs PCIe? I think back when we were discussing
the implementation of this, Alex had said there wasn't.
If that's the case, then there should be a comment explaining
why mdevs are treated this way. It's not at all obvious to a
reader who's not familiar with them :)
--
Andrea Bolognani / Red Hat / Virtualization
9:55 a.m.
On Fri, Jun 02, 2017 at 08:20:12AM +0200, Andrea Bolognani wrote:
On Thu, 2017-06-01 at 21:24 -0400, Laine Stump wrote:
> > @@ -645,9 +645,6 @@
qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> > return pcieFlags;
> > }
> >
> > - if (hostdev->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
> > - return pcieFlags;
> > -
> > if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
> > hostAddr->bus,
> > hostAddr->slot,
>
> This can't work. From the host's point of view, there is no PCI device
> whose config space can be read, and from the code's point of view,
> hostAddr->domain|bus|slot are invalid (they are in
> hostdev->source.subsys.u.pci.addr, which is only valid when
> (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI).
In fact it does work, because we return pcieFlags on error which might only be
a problem if the backing device itself would be a legacy PCI.
Surely you meant to use '==' here?
> Is there maybe something standard in the mdev's sysfs entry that could
> be used to determine PCI vs PCIe? I think back when we were discussing
> the implementation of this, Alex had said there wasn't.
No, there's not without looking at the parent PCI device. IIRC Alex said we
would have to go to the parent device's sysfs, read the config space and
determine it from there.
If that's the case, then there should be a comment explaining
why mdevs are treated this way. It's not at all obvious to a
reader who's not familiar with them :)
You're absolutely right, I have to admit, I don't remember the exact reason why
I added that check, whether it was just to follow a private discussion about
this or I actually checked the log for errors and figured this myself. Feel
free to send another patch adding the comment.
Anyhow, thank you Laine for providing feedback, since when Andrea described the
issue to me, it made sense and looked to me like a feasible thing to do the
whole time and I couldn't come up with an argument why it wasn't OK, so,
appreciated.
Erik
--
Andrea Bolognani / Red Hat / Virtualization
2760
days inactive
2767
days old
3 comments
3 participants
participants (3)
-
Andrea Bolognani -
Erik Skultety -
Laine Stump