* Re: [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag
[not found] <20220217095545.195426-1-elic@nvidia.com>
@ 2022-02-17 10:55 ` Stefano Garzarella
2022-02-17 11:49 ` Michael S. Tsirkin
1 sibling, 0 replies; 3+ messages in thread
From: Stefano Garzarella @ 2022-02-17 10:55 UTC (permalink / raw)
To: Eli Cohen; +Cc: lvivier, mst, virtualization, stephen, eperezma, si-wei.liu
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
It seems to me that VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN is
not used, otherwise I think now we need to add #include
<linux/virtio_config.h> where it is used.
Anyway it seems a right change to me.
>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
>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag
[not found] <20220217095545.195426-1-elic@nvidia.com>
2022-02-17 10:55 ` [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag Stefano Garzarella
@ 2022-02-17 11:49 ` Michael S. Tsirkin
[not found] ` <b52aebae-44c8-fb4a-f7f3-6da7cd4075fc@xilinx.com>
1 sibling, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2022-02-17 11:49 UTC (permalink / raw)
To: Eli Cohen
Cc: lvivier, virtualization, stephen, eperezma, Edward Cree,
si-wei.liu
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?
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag
[not found] ` <b52aebae-44c8-fb4a-f7f3-6da7cd4075fc@xilinx.com>
@ 2022-02-17 21:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2022-02-17 21:45 UTC (permalink / raw)
To: Edward Cree
Cc: lvivier, gdawar, virtualization, stephen, eperezma, si-wei.liu,
Eli Cohen, radovano
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-17 21:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220217095545.195426-1-elic@nvidia.com>
2022-02-17 10:55 ` [PATCH] virtio: Add definition for VIRTIO_F_NOTIFICATION_DATA feature flag Stefano Garzarella
2022-02-17 11:49 ` Michael S. Tsirkin
[not found] ` <b52aebae-44c8-fb4a-f7f3-6da7cd4075fc@xilinx.com>
2022-02-17 21:45 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).