Re: [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag

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

On Thu, Feb 17, 2022 at 06:22:25PM +0000, Edward Cree wrote:
> On 17/02/2022 11:49, Michael S. Tsirkin wrote:
> > On Thu, Feb 17, 2022 at 11:55:45AM +0200, Eli Cohen wrote:
> >> This is required by iproute2 to display the capabilities of a vdpa based
> >> virtio device.
> >>
> >> Previously, drivers/net/ethernet/sfc/mcdi_pcol.h made use of a private
> >> definition of this flag. Modify the definition to rely on the new
> >> generic definition.
> >>
> >> Signed-off-by: Eli Cohen <elic@nvidia.com>
> >> ---
> >>  drivers/net/ethernet/sfc/mcdi_pcol.h | 2 +-
> >>  include/uapi/linux/virtio_config.h   | 6 ++++++
> >>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/sfc/mcdi_pcol.h b/drivers/net/ethernet/sfc/mcdi_pcol.h
> >> index d3fcbf930dba..2603e04dae06 100644
> >> --- a/drivers/net/ethernet/sfc/mcdi_pcol.h
> >> +++ b/drivers/net/ethernet/sfc/mcdi_pcol.h
> >> @@ -21477,7 +21477,7 @@
> >>  #define        VIRTIO_BLK_CONFIG_VIRTIO_F_SR_IOV_LBN 37
> >>  #define        VIRTIO_BLK_CONFIG_VIRTIO_F_SR_IOV_WIDTH 1
> >>  #define        VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_OFST 0
> >> -#define        VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN 38
> >> +#define        VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN VIRTIO_F_NOTIFICATION_DATA
> >>  #define        VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_WIDTH 1
> >>  #define       VIRTIO_BLK_CONFIG_FEATURES_LBN 0
> >>  #define       VIRTIO_BLK_CONFIG_FEATURES_WIDTH 64
> > 
> > I don't think we need to bother with macros in mcdi_pcol.h - that header
> > is generated by some kind of script, this probably is why it has
> > all these macros that are otherwise unused.
> > 
> > Cc Edward Cree who added them - Edward, is there a way to
> > get rid of VIRTIO_ macros inmcdi_pcol.h?
> 
> mcdi_pcol.h is indeed generated, and is shared with the MCPU firmware
>  which implements the other side of the MCDI protocol, so I don't know
>  whether we'd be able to carry these kind of changes; every difference
>  between the upstream and internal/fw versions makes for ongoing
>  maintenance pain.  (And obviously Linux' virtio_config.h isn't
>  available to our firmware build.)
> Do these macro definitions lead to any immediate conflicts (if both
>  headers are included), or is it just the worry that conflicts could
>  arise in the future since these #defines aren't EFX_ namespaced?

The latter. One simple way would be ifndef __KERNEL__ around virtio
things in the header.

> If the latter, I would be tempted to leave well alone for now; only
>  sfc driver code is ever likely to #include mcdi_pcol.h, so a proper
>  resolution can safely wait until sfc vDPA support is upstreamed.
> CCing Aleksandar Radovanovic who originally added them internally, in
>  case he has anything to add.  Also CCing Gautam Dawar who is working
>  on vDPA support in the sfc driver.
> 
> -ed
> 
> > 
> >> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> >> index b5eda06f0d57..30eb76dcdcad 100644
> >> --- a/include/uapi/linux/virtio_config.h
> >> +++ b/include/uapi/linux/virtio_config.h
> >> @@ -92,4 +92,10 @@
> >>   * Does the device support Single Root I/O Virtualization?
> >>   */
> >>  #define VIRTIO_F_SR_IOV			37
> >> +
> >> +/* When negotiated, indicates that the driver can pass extra data beyond
> >> + * virtqueue identification when sending notifications
> >> + */
> >> +#define VIRTIO_F_NOTIFICATION_DATA	38
> >> +
> >>  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> >> -- 
> >> 2.32.0
> > 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization