On 26/10/2023 20:55, Alex Williamson wrote:
On Thu, 26 Oct 2023 15:08:12 +0300
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:
On 25/10/2023 22:13, Alex Williamson wrote:
On Wed, 25 Oct 2023 17:35:51 +0300
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:
On 24/10/2023 22:57, Alex Williamson wrote:
On Tue, 17 Oct 2023 16:42:17 +0300
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:
+ if (copy_to_user(buf + copy_offset, &val32, copy_count))
+ return -EFAULT;
+ }
+
+ if (range_intersect_range(pos, count, PCI_SUBSYSTEM_ID, sizeof(val16),
+ ©_offset, ©_count, NULL)) {
+ /*
+ * Transitional devices use the PCI subsystem device id as
+ * virtio device id, same as legacy driver always did.
Where did we require the subsystem vendor ID to be 0x1af4? This
subsystem device ID really only makes since given that subsystem
vendor ID, right? Otherwise I don't see that non-transitional devices,
such as the VF, have a hard requirement per the spec for the subsystem
vendor ID.
Do we want to make this only probe the correct subsystem vendor ID or do
we want to emulate the subsystem vendor ID as well? I don't see this is
correct without one of those options.
Looking in the 1.x spec we can see the below.
Legacy Interfaces: A Note on PCI Device Discovery
"Transitional devices MUST have the PCI Subsystem
Device ID matching the Virtio Device ID, as indicated in section 5 ...
This is to match legacy drivers."
However, there is no need to enforce Subsystem Vendor ID.
This is what we followed here.
Makes sense ?
So do I understand correctly that virtio dictates the subsystem device
ID for all subsystem vendor IDs that implement a legacy virtio
interface? Ok, but this device didn't actually implement a legacy
virtio interface. The device itself is not tranistional, we're imposing
an emulated transitional interface onto it. So did the subsystem vendor
agree to have their subsystem device ID managed by the virtio committee
or might we create conflicts? I imagine we know we don't have a
conflict if we also virtualize the subsystem vendor ID.
The non transitional net device in the virtio spec defined as the below
tuple.
T_A: VID=0x1AF4, DID=0x1040, Subsys_VID=FOO, Subsys_DID=0x40.
And transitional net device in the virtio spec for a vendor FOO is
defined as:
T_B: VID=0x1AF4,DID=0x1000,Subsys_VID=FOO, subsys_DID=0x1
This driver is converting T_A to T_B, which both are defined by the
virtio spec.
Hence, it does not conflict for the subsystem vendor, it is fine.
Surprising to me that the virtio spec dictates subsystem device ID in
all cases. The further discussion in this thread seems to indicate we
need to virtualize subsystem vendor ID for broader driver compatibility
anyway.
BTW, it would be a lot easier for all of the config space emulation here
if we could make use of the existing field virtualization in
vfio-pci-core. In fact you'll see in vfio_config_init() that
PCI_DEVICE_ID is already virtualized for VFs, so it would be enough to
simply do the following to report the desired device ID:
*(__le16 *)&vconfig[PCI_DEVICE_ID] = cpu_to_le16(0x1000);
I would prefer keeping things simple and have one place/flow that
handles all the fields as we have now as part of the driver.
That's the same argument I'd make for re-using the core code, we don't
need multiple implementations handling merging physical and virtual
bits within config space.
In any case, I'll further look at that option for managing the DEVICE_ID
towards V2.
It appears everything in this function could be handled similarly by
vfio-pci-core if the right fields in the perm_bits.virt and .write
bits could be manipulated and vconfig modified appropriately. I'd look
for a way that a variant driver could provide an alternate set of
permissions structures for various capabilities. Thanks,
OK
However, let's not block V2 and the series acceptance as of that.
It can always be some future refactoring as part of other series that
will bring the infra-structure that is needed for that.
We're already on the verge of the v6.7 merge window, so this looks like
v6.8 material anyway. We have time. Thanks,
OK
I sent V2 having all the other notes handled to share and get feedback
from both you and Michael.
Let's continue from there to see what is needed towards v6.8.
Thanks,
Yishai
Alex
