Re: [PATCH] virtio-pci: PCI extended capabilities for virtio

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 25, 2024 at 02:39:37PM -0400, sahanlb wrote:
> PCI legacy configuration space does not have sufficient space for a device
> that supports all kinds of virtio structures via PCI capabilities. This is
> especially true if one were to use virtio drivers with physical devices.
> Link: https://par.nsf.gov/servlets/purl/10463939
> A physical device may already have many capabilities in the legacy space.
> 
> This patch adds support to place virtio capabilities in the PCI extended
> configuration space and makes the driver search both legacy and extended
> PCI configuration spaces.
> 
> Add new argument to vp_modern_map_capability to indicate whether mapping
> a legacy or extended capability.
> Add new function virtio_pci_find_ext_capability to walk extended
> capabilities and find virtio capabilities.
> 
> Modify vp_modern_probe to search both legacy and extended configuration
> spaces.
> If virtio_pci_find_capability fails to find common, isr, notify, or device
> virtio structures, call virtio_pci_find_ext_capability.
> 
> Notify virtio structure can get mapped either in vp_modern_probe or in
> vp_modern_map_vq_notify. Add new attribute 'notify_ecap' to
> struct virtio_pci_modern_device to indicate whether the notify capability
> is in the extended congiguration structure.
> 
> Add virtio extended capability structures to
> "include/uapi/linux/virtio_pci.h".
> Format for the extended structures derived from
> Link: https://lore.kernel.org/all/20220112055755.41011-2-jasowang@redhat.com/
> 
> This patch has been validated using an FPGA development board to implement 
> a virtio interface.
> 
> Signed-off-by: sahanlb <sahanb@bu.edu>


Thanks for the patch! As any UAPI change, this one has to also
be accompanied by a spec patch documenting the capabilities.


...

> +struct virtio_pci_cfg_ecap {
> +	struct virtio_pci_ecap cap;
> +	__u8 pci_cfg_data[4]; /* Data for BAR access. */
> +};

Hmm, a weird thing to do. The reason we have it is because
a legacy bios has trouble accessing BAR directly (e.g. if BAR
is 64 bit).  Is there still an issue even for bios with
support for pcie extended config space?


> +
>  /* Macro versions of offsets for the Old Timers! */
>  #define VIRTIO_PCI_CAP_VNDR		0
>  #define VIRTIO_PCI_CAP_NEXT		1


This comes at an interesting time, there is an interest
in order to get virtio working over EP protocol,
in exposing all this information as part of a BAR
as opposed as a capability. I wonder whether that's
also palatable.


> -- 
> 2.42.0