On 11/10/2023 11:02, Michael S. Tsirkin wrote:
On Wed, Oct 11, 2023 at 10:44:49AM +0300, Yishai Hadas wrote:
On 10/10/2023 23:42, Michael S. Tsirkin wrote:
On Tue, Oct 10, 2023 at 07:09:08PM +0300, Yishai Hadas wrote:
Assuming that we'll put each command inside virtio as the generic layer, we
won't be able to call/use this API internally to get the PF as of cyclic
dependencies between the modules, link will fail.
I just mean:
virtio_admin_legacy_io_write(sruct pci_device *, ....)
internally it starts from vf gets the pf (or vf itself or whatever
the transport is) sends command gets status returns.
what is cyclic here?
virtio-pci depends on virtio [1].
If we put the commands in the generic layer as we expect it to be (i.e.
virtio), then trying to call internally call for virtio_pci_vf_get_pf_dev()
to get the PF from the VF will end-up by a linker cyclic error as of below
[2].
As of that, someone can suggest to put the commands in virtio-pci, however
this will fully bypass the generic layer of virtio and future clients won't
be able to use it.
virtio_pci would get pci device.
virtio pci convers that to virtio device of owner + group member id and calls virtio.
Do you suggest another set of exported symbols (i.e per command ) in
virtio which will get the owner device + group member + the extra
specific command parameters ?
This will end-up duplicating the number of export symbols per command.
no cycles and minimal transport specific code, right?
See my above note, if we may just call virtio without any further work
on the command's input, than YES.
If so, virtio will prepare the command by setting the relevant SG lists
and other data and finally will call:
vdev->config->exec_admin_cmd(vdev, cmd);
Was that your plan ?
In addition, passing in the VF PCI pointer instead of the VF group member ID
+ the VIRTIO PF device, will require in the future to duplicate each command
once we'll use SIOV devices.
I don't think anyone knows how will SIOV look. But shuffling
APIs around is not a big deal. We'll see.
As you are the maintainer it's up-to-you, just need to consider another
further duplication here.
Yishai
Instead, we suggest the below API for the above example.
virtio_admin_legacy_io_write(virtio_device *virtio_dev, u64
group_member_id, ....)
[1]
[yishaih@reg-l-vrt-209 linux]$ modinfo virtio-pci
filename: /lib/modules/6.6.0-rc2+/kernel/drivers/virtio/virtio_pci.ko
version: 1
license: GPL
description: virtio-pci
author: Anthony Liguori <aliguori@xxxxxxxxxx>
srcversion: 7355EAC9408D38891938391
alias: pci:v00001AF4d*sv*sd*bc*sc*i*
depends: virtio_pci_modern_dev,virtio,virtio_ring,virtio_pci_legacy_dev
retpoline: Y
intree: Y
name: virtio_pci
vermagic: 6.6.0-rc2+ SMP preempt mod_unload modversions
parm: force_legacy:Force legacy mode for transitional virtio 1
devices (bool)
[2]
depmod: ERROR: Cycle detected: virtio -> virtio_pci -> virtio
depmod: ERROR: Found 2 modules in dependency cycles!
make[2]: *** [scripts/Makefile.modinst:128: depmod] Error 1
make[1]: *** [/images/yishaih/src/kernel/linux/Makefile:1821:
modules_install] Error 2
Yishai
virtio absolutely must not depend on virtio pci, it is used on
systems without pci at all.
