diff mbox series

[2/3] virtio: Define bit numbers for device independent features

Message ID 20220207125537.174619-3-elic@nvidia.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series vdpa tool support for configuring max VQs | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Eli Cohen Feb. 7, 2022, 12:55 p.m. UTC
Define bit fields for device independent feature bits. We need them in a
follow up patch.

Also, define macros for start and end of these feature bits.

Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
 include/uapi/linux/virtio_config.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Jason Wang Feb. 10, 2022, 7:54 a.m. UTC | #1
On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
>
> Define bit fields for device independent feature bits. We need them in a
> follow up patch.
>
> Also, define macros for start and end of these feature bits.
>
> Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
>  include/uapi/linux/virtio_config.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 3bf6c8bf8477..6d92cc31a8d3 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -45,14 +45,14 @@
>  /* We've given up on this device. */
>  #define VIRTIO_CONFIG_S_FAILED         0x80
>
> -/*
> - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> - * being used (e.g. virtio_ring, virtio_pci etc.), the
> - * rest are per-device feature bits.
> - */
> -#define VIRTIO_TRANSPORT_F_START       28
> -#define VIRTIO_TRANSPORT_F_END         38
> +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> +#define VIRTIO_DEV_INDEPENDENT_F_END   38

Haven't gone through patch 3 but I think it's probably better not
touch uapi stuff. Or we can define those macros in other place?

> +
> +#define VIRTIO_F_RING_INDIRECT_DESC 28
> +#define VIRTIO_F_RING_EVENT_IDX 29
> +#define VIRTIO_F_IN_ORDER 35
> +#define VIRTIO_F_NOTIFICATION_DATA 38

This part belongs to the virtio_ring.h any reason not pull that file
instead of squashing those into virtio_config.h?

Thanks

>
>  #ifndef VIRTIO_CONFIG_NO_LEGACY
>  /* Do we get callbacks when the ring is completely used, even if we've
> --
> 2.34.1
>
Eli Cohen Feb. 10, 2022, 8:30 a.m. UTC | #2
On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> >
> > Define bit fields for device independent feature bits. We need them in a
> > follow up patch.
> >
> > Also, define macros for start and end of these feature bits.
> >
> > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > ---
> >  include/uapi/linux/virtio_config.h | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -45,14 +45,14 @@
> >  /* We've given up on this device. */
> >  #define VIRTIO_CONFIG_S_FAILED         0x80
> >
> > -/*
> > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > - * rest are per-device feature bits.
> > - */
> > -#define VIRTIO_TRANSPORT_F_START       28
> > -#define VIRTIO_TRANSPORT_F_END         38
> > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> 
> Haven't gone through patch 3 but I think it's probably better not
> touch uapi stuff. Or we can define those macros in other place?
> 

I can put it in vdpa.c

> > +
> > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > +#define VIRTIO_F_RING_EVENT_IDX 29
> > +#define VIRTIO_F_IN_ORDER 35
> > +#define VIRTIO_F_NOTIFICATION_DATA 38
> 
> This part belongs to the virtio_ring.h any reason not pull that file
> instead of squashing those into virtio_config.h?
> 

Not sure what you mean here. I can't find virtio_ring.h in my tree.

> Thanks
> 
> >
> >  #ifndef VIRTIO_CONFIG_NO_LEGACY
> >  /* Do we get callbacks when the ring is completely used, even if we've
> > --
> > 2.34.1
> >
>
Jason Wang Feb. 10, 2022, 8:35 a.m. UTC | #3
On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
>
> On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > >
> > > Define bit fields for device independent feature bits. We need them in a
> > > follow up patch.
> > >
> > > Also, define macros for start and end of these feature bits.
> > >
> > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > ---
> > >  include/uapi/linux/virtio_config.h | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -45,14 +45,14 @@
> > >  /* We've given up on this device. */
> > >  #define VIRTIO_CONFIG_S_FAILED         0x80
> > >
> > > -/*
> > > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > > - * rest are per-device feature bits.
> > > - */
> > > -#define VIRTIO_TRANSPORT_F_START       28
> > > -#define VIRTIO_TRANSPORT_F_END         38
> > > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> >
> > Haven't gone through patch 3 but I think it's probably better not
> > touch uapi stuff. Or we can define those macros in other place?
> >
>
> I can put it in vdpa.c
>
> > > +
> > > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > > +#define VIRTIO_F_RING_EVENT_IDX 29
> > > +#define VIRTIO_F_IN_ORDER 35
> > > +#define VIRTIO_F_NOTIFICATION_DATA 38
> >
> > This part belongs to the virtio_ring.h any reason not pull that file
> > instead of squashing those into virtio_config.h?
> >
>
> Not sure what you mean here. I can't find virtio_ring.h in my tree.

I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.

Thanks

>
> > Thanks
> >
> > >
> > >  #ifndef VIRTIO_CONFIG_NO_LEGACY
> > >  /* Do we get callbacks when the ring is completely used, even if we've
> > > --
> > > 2.34.1
> > >
> >
>
Eli Cohen Feb. 10, 2022, 9:27 a.m. UTC | #4
On Thu, Feb 10, 2022 at 04:35:28PM +0800, Jason Wang wrote:
> On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
> >
> > On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> > > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > > >
> > > > Define bit fields for device independent feature bits. We need them in a
> > > > follow up patch.
> > > >
> > > > Also, define macros for start and end of these feature bits.
> > > >
> > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > ---
> > > >  include/uapi/linux/virtio_config.h | 16 ++++++++--------
> > > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -45,14 +45,14 @@
> > > >  /* We've given up on this device. */
> > > >  #define VIRTIO_CONFIG_S_FAILED         0x80
> > > >
> > > > -/*
> > > > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > > > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > > > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > > > - * rest are per-device feature bits.
> > > > - */
> > > > -#define VIRTIO_TRANSPORT_F_START       28
> > > > -#define VIRTIO_TRANSPORT_F_END         38
> > > > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > > > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > > > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> > >
> > > Haven't gone through patch 3 but I think it's probably better not
> > > touch uapi stuff. Or we can define those macros in other place?
> > >
> >
> > I can put it in vdpa.c
> >
> > > > +
> > > > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > > > +#define VIRTIO_F_RING_EVENT_IDX 29
> > > > +#define VIRTIO_F_IN_ORDER 35
> > > > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > >
> > > This part belongs to the virtio_ring.h any reason not pull that file
> > > instead of squashing those into virtio_config.h?
> > >
> >
> > Not sure what you mean here. I can't find virtio_ring.h in my tree.
> 
> I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.

I will still miss VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_NOTIFICATION_DATA
which are only defined in drivers/net/ethernet/sfc/mcdi_pcol.h for block
devices only.

What would you suggest to do with them? Maybe define them in vdpa.c?

> 
> Thanks
> 
> >
> > > Thanks
> > >
> > > >
> > > >  #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > >  /* Do we get callbacks when the ring is completely used, even if we've
> > > > --
> > > > 2.34.1
> > > >
> > >
> >
>
Jason Wang Feb. 14, 2022, 6:06 a.m. UTC | #5
在 2022/2/10 下午5:27, Eli Cohen 写道:
> On Thu, Feb 10, 2022 at 04:35:28PM +0800, Jason Wang wrote:
>> On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
>>> On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
>>>> On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
>>>>> Define bit fields for device independent feature bits. We need them in a
>>>>> follow up patch.
>>>>>
>>>>> Also, define macros for start and end of these feature bits.
>>>>>
>>>>> Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
>>>>> Signed-off-by: Eli Cohen <elic@nvidia.com>
>>>>> ---
>>>>>   include/uapi/linux/virtio_config.h | 16 ++++++++--------
>>>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
>>>>> index 3bf6c8bf8477..6d92cc31a8d3 100644
>>>>> --- a/include/uapi/linux/virtio_config.h
>>>>> +++ b/include/uapi/linux/virtio_config.h
>>>>> @@ -45,14 +45,14 @@
>>>>>   /* We've given up on this device. */
>>>>>   #define VIRTIO_CONFIG_S_FAILED         0x80
>>>>>
>>>>> -/*
>>>>> - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
>>>>> - * VIRTIO_TRANSPORT_F_END are reserved for the transport
>>>>> - * being used (e.g. virtio_ring, virtio_pci etc.), the
>>>>> - * rest are per-device feature bits.
>>>>> - */
>>>>> -#define VIRTIO_TRANSPORT_F_START       28
>>>>> -#define VIRTIO_TRANSPORT_F_END         38
>>>>> +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
>>>>> +#define VIRTIO_DEV_INDEPENDENT_F_START 28
>>>>> +#define VIRTIO_DEV_INDEPENDENT_F_END   38
>>>> Haven't gone through patch 3 but I think it's probably better not
>>>> touch uapi stuff. Or we can define those macros in other place?
>>>>
>>> I can put it in vdpa.c
>>>
>>>>> +
>>>>> +#define VIRTIO_F_RING_INDIRECT_DESC 28
>>>>> +#define VIRTIO_F_RING_EVENT_IDX 29
>>>>> +#define VIRTIO_F_IN_ORDER 35
>>>>> +#define VIRTIO_F_NOTIFICATION_DATA 38
>>>> This part belongs to the virtio_ring.h any reason not pull that file
>>>> instead of squashing those into virtio_config.h?
>>>>
>>> Not sure what you mean here. I can't find virtio_ring.h in my tree.
>> I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.
> I will still miss VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_NOTIFICATION_DATA
> which are only defined in drivers/net/ethernet/sfc/mcdi_pcol.h for block
> devices only.
>
> What would you suggest to do with them? Maybe define them in vdpa.c?


I meant maybe we need a full synchronization from the current Linux uapi 
headers for virtio_config.h and and add virtio_ring.h here.

Thanks


>
>> Thanks
>>
>>>> Thanks
>>>>
>>>>>   #ifndef VIRTIO_CONFIG_NO_LEGACY
>>>>>   /* Do we get callbacks when the ring is completely used, even if we've
>>>>> --
>>>>> 2.34.1
>>>>>
Eli Cohen Feb. 16, 2022, 7:15 a.m. UTC | #6
On Mon, Feb 14, 2022 at 02:06:54PM +0800, Jason Wang wrote:
> 
> 在 2022/2/10 下午5:27, Eli Cohen 写道:
> > On Thu, Feb 10, 2022 at 04:35:28PM +0800, Jason Wang wrote:
> > > On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> > > > > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > > Define bit fields for device independent feature bits. We need them in a
> > > > > > follow up patch.
> > > > > > 
> > > > > > Also, define macros for start and end of these feature bits.
> > > > > > 
> > > > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > > > ---
> > > > > >   include/uapi/linux/virtio_config.h | 16 ++++++++--------
> > > > > >   1 file changed, 8 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > > > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > > @@ -45,14 +45,14 @@
> > > > > >   /* We've given up on this device. */
> > > > > >   #define VIRTIO_CONFIG_S_FAILED         0x80
> > > > > > 
> > > > > > -/*
> > > > > > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > > > > > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > > > > > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > > > > > - * rest are per-device feature bits.
> > > > > > - */
> > > > > > -#define VIRTIO_TRANSPORT_F_START       28
> > > > > > -#define VIRTIO_TRANSPORT_F_END         38
> > > > > > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> > > > > Haven't gone through patch 3 but I think it's probably better not
> > > > > touch uapi stuff. Or we can define those macros in other place?
> > > > > 
> > > > I can put it in vdpa.c
> > > > 
> > > > > > +
> > > > > > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > > > > > +#define VIRTIO_F_RING_EVENT_IDX 29
> > > > > > +#define VIRTIO_F_IN_ORDER 35
> > > > > > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > > > > This part belongs to the virtio_ring.h any reason not pull that file
> > > > > instead of squashing those into virtio_config.h?
> > > > > 
> > > > Not sure what you mean here. I can't find virtio_ring.h in my tree.
> > > I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.
> > I will still miss VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_NOTIFICATION_DATA
> > which are only defined in drivers/net/ethernet/sfc/mcdi_pcol.h for block
> > devices only.
> > 
> > What would you suggest to do with them? Maybe define them in vdpa.c?
> 
> 
> I meant maybe we need a full synchronization from the current Linux uapi
> headers for virtio_config.h and and add virtio_ring.h here.
> 

virtio_config.h is updatd already and virtio_ring.h does not add any
flag definition that we're missing.

The flags I was missing are
+#define VIRTIO_F_IN_ORDER 35
+#define VIRTIO_F_NOTIFICATION_DATA 38

and both of these do not appear in the linux headers. They appear as
block specific flags:

drivers/net/ethernet/sfc/mcdi_pcol.h:21471:#define VIRTIO_BLK_CONFIG_VIRTIO_F_IN_ORDER_LBN 35
drivers/net/ethernet/sfc/mcdi_pcol.h:21480:#define VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN 38

So I just defined these two in vdpa.c (in patch v1).

> Thanks
> 
> 
> > 
> > > Thanks
> > > 
> > > > > Thanks
> > > > > 
> > > > > >   #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > > >   /* Do we get callbacks when the ring is completely used, even if we've
> > > > > > --
> > > > > > 2.34.1
> > > > > > 
>
Jason Wang Feb. 17, 2022, 6:06 a.m. UTC | #7
On Wed, Feb 16, 2022 at 3:16 PM Eli Cohen <elic@nvidia.com> wrote:
>
> On Mon, Feb 14, 2022 at 02:06:54PM +0800, Jason Wang wrote:
> >
> > 在 2022/2/10 下午5:27, Eli Cohen 写道:
> > > On Thu, Feb 10, 2022 at 04:35:28PM +0800, Jason Wang wrote:
> > > > On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> > > > > > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > > > Define bit fields for device independent feature bits. We need them in a
> > > > > > > follow up patch.
> > > > > > >
> > > > > > > Also, define macros for start and end of these feature bits.
> > > > > > >
> > > > > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > > > > ---
> > > > > > >   include/uapi/linux/virtio_config.h | 16 ++++++++--------
> > > > > > >   1 file changed, 8 insertions(+), 8 deletions(-)
> > > > > > >
> > > > > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > > > > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > > > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > > > @@ -45,14 +45,14 @@
> > > > > > >   /* We've given up on this device. */
> > > > > > >   #define VIRTIO_CONFIG_S_FAILED         0x80
> > > > > > >
> > > > > > > -/*
> > > > > > > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > > > > > > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > > > > > > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > > > > > > - * rest are per-device feature bits.
> > > > > > > - */
> > > > > > > -#define VIRTIO_TRANSPORT_F_START       28
> > > > > > > -#define VIRTIO_TRANSPORT_F_END         38
> > > > > > > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> > > > > > Haven't gone through patch 3 but I think it's probably better not
> > > > > > touch uapi stuff. Or we can define those macros in other place?
> > > > > >
> > > > > I can put it in vdpa.c
> > > > >
> > > > > > > +
> > > > > > > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > > > > > > +#define VIRTIO_F_RING_EVENT_IDX 29
> > > > > > > +#define VIRTIO_F_IN_ORDER 35
> > > > > > > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > > > > > This part belongs to the virtio_ring.h any reason not pull that file
> > > > > > instead of squashing those into virtio_config.h?
> > > > > >
> > > > > Not sure what you mean here. I can't find virtio_ring.h in my tree.
> > > > I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.
> > > I will still miss VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_NOTIFICATION_DATA
> > > which are only defined in drivers/net/ethernet/sfc/mcdi_pcol.h for block
> > > devices only.
> > >
> > > What would you suggest to do with them? Maybe define them in vdpa.c?
> >
> >
> > I meant maybe we need a full synchronization from the current Linux uapi
> > headers for virtio_config.h and and add virtio_ring.h here.
> >
>
> virtio_config.h is updatd already and virtio_ring.h does not add any
> flag definition that we're missing.
>
> The flags I was missing are
> +#define VIRTIO_F_IN_ORDER 35
> +#define VIRTIO_F_NOTIFICATION_DATA 38

Right, so Gautam posted a path for _F_IN_ORDER [1].

We probably need another patch for NOTIFICATION_DATA.


>
> and both of these do not appear in the linux headers. They appear as
> block specific flags:
>
> drivers/net/ethernet/sfc/mcdi_pcol.h:21471:#define VIRTIO_BLK_CONFIG_VIRTIO_F_IN_ORDER_LBN 35
> drivers/net/ethernet/sfc/mcdi_pcol.h:21480:#define VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN 38
>
> So I just defined these two in vdpa.c (in patch v1).

Fine, but we need to remove them if we get update from linux kernel uapi headers

[1] https://lkml.org/lkml/2022/2/15/43

Thanks

>
> > Thanks
> >
> >
> > >
> > > > Thanks
> > > >
> > > > > > Thanks
> > > > > >
> > > > > > >   #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > > > >   /* Do we get callbacks when the ring is completely used, even if we've
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> >
>
Eli Cohen Feb. 17, 2022, 8:26 a.m. UTC | #8
On Thu, Feb 17, 2022 at 02:06:39PM +0800, Jason Wang wrote:
> On Wed, Feb 16, 2022 at 3:16 PM Eli Cohen <elic@nvidia.com> wrote:
> >
> > On Mon, Feb 14, 2022 at 02:06:54PM +0800, Jason Wang wrote:
> > >
> > > 在 2022/2/10 下午5:27, Eli Cohen 写道:
> > > > On Thu, Feb 10, 2022 at 04:35:28PM +0800, Jason Wang wrote:
> > > > > On Thu, Feb 10, 2022 at 4:31 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > > On Thu, Feb 10, 2022 at 03:54:57PM +0800, Jason Wang wrote:
> > > > > > > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote:
> > > > > > > > Define bit fields for device independent feature bits. We need them in a
> > > > > > > > follow up patch.
> > > > > > > >
> > > > > > > > Also, define macros for start and end of these feature bits.
> > > > > > > >
> > > > > > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
> > > > > > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > > > > > ---
> > > > > > > >   include/uapi/linux/virtio_config.h | 16 ++++++++--------
> > > > > > > >   1 file changed, 8 insertions(+), 8 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > > > > > index 3bf6c8bf8477..6d92cc31a8d3 100644
> > > > > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > > > > @@ -45,14 +45,14 @@
> > > > > > > >   /* We've given up on this device. */
> > > > > > > >   #define VIRTIO_CONFIG_S_FAILED         0x80
> > > > > > > >
> > > > > > > > -/*
> > > > > > > > - * Virtio feature bits VIRTIO_TRANSPORT_F_START through
> > > > > > > > - * VIRTIO_TRANSPORT_F_END are reserved for the transport
> > > > > > > > - * being used (e.g. virtio_ring, virtio_pci etc.), the
> > > > > > > > - * rest are per-device feature bits.
> > > > > > > > - */
> > > > > > > > -#define VIRTIO_TRANSPORT_F_START       28
> > > > > > > > -#define VIRTIO_TRANSPORT_F_END         38
> > > > > > > > +/* Device independent features per virtio spec 1.1 range from 28 to 38 */
> > > > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_START 28
> > > > > > > > +#define VIRTIO_DEV_INDEPENDENT_F_END   38
> > > > > > > Haven't gone through patch 3 but I think it's probably better not
> > > > > > > touch uapi stuff. Or we can define those macros in other place?
> > > > > > >
> > > > > > I can put it in vdpa.c
> > > > > >
> > > > > > > > +
> > > > > > > > +#define VIRTIO_F_RING_INDIRECT_DESC 28
> > > > > > > > +#define VIRTIO_F_RING_EVENT_IDX 29
> > > > > > > > +#define VIRTIO_F_IN_ORDER 35
> > > > > > > > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > > > > > > This part belongs to the virtio_ring.h any reason not pull that file
> > > > > > > instead of squashing those into virtio_config.h?
> > > > > > >
> > > > > > Not sure what you mean here. I can't find virtio_ring.h in my tree.
> > > > > I meant just copy the virtio_ring.h in the linux tree. It seems cleaner.
> > > > I will still miss VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_NOTIFICATION_DATA
> > > > which are only defined in drivers/net/ethernet/sfc/mcdi_pcol.h for block
> > > > devices only.
> > > >
> > > > What would you suggest to do with them? Maybe define them in vdpa.c?
> > >
> > >
> > > I meant maybe we need a full synchronization from the current Linux uapi
> > > headers for virtio_config.h and and add virtio_ring.h here.
> > >
> >
> > virtio_config.h is updatd already and virtio_ring.h does not add any
> > flag definition that we're missing.
> >
> > The flags I was missing are
> > +#define VIRTIO_F_IN_ORDER 35
> > +#define VIRTIO_F_NOTIFICATION_DATA 38
> 
> Right, so Gautam posted a path for _F_IN_ORDER [1].
> 
> We probably need another patch for NOTIFICATION_DATA.
> 

OK, I will send a patch NOTIFICATION_DATA. Once they are accepted, I
will follow up with a patch to remove from iproute2.
> 
> >
> > and both of these do not appear in the linux headers. They appear as
> > block specific flags:
> >
> > drivers/net/ethernet/sfc/mcdi_pcol.h:21471:#define VIRTIO_BLK_CONFIG_VIRTIO_F_IN_ORDER_LBN 35
> > drivers/net/ethernet/sfc/mcdi_pcol.h:21480:#define VIRTIO_BLK_CONFIG_VIRTIO_F_NOTIFICATION_DATA_LBN 38
> >
> > So I just defined these two in vdpa.c (in patch v1).
> 
> Fine, but we need to remove them if we get update from linux kernel uapi headers
> 
> [1] https://lkml.org/lkml/2022/2/15/43
> 
> Thanks
> 
> >
> > > Thanks
> > >
> > >
> > > >
> > > > > Thanks
> > > > >
> > > > > > > Thanks
> > > > > > >
> > > > > > > >   #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > > > > >   /* Do we get callbacks when the ring is completely used, even if we've
> > > > > > > > --
> > > > > > > > 2.34.1
> > > > > > > >
> > >
> >
>
diff mbox series

Patch

diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 3bf6c8bf8477..6d92cc31a8d3 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -45,14 +45,14 @@ 
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED		0x80
 
-/*
- * Virtio feature bits VIRTIO_TRANSPORT_F_START through
- * VIRTIO_TRANSPORT_F_END are reserved for the transport
- * being used (e.g. virtio_ring, virtio_pci etc.), the
- * rest are per-device feature bits.
- */
-#define VIRTIO_TRANSPORT_F_START	28
-#define VIRTIO_TRANSPORT_F_END		38
+/* Device independent features per virtio spec 1.1 range from 28 to 38 */
+#define VIRTIO_DEV_INDEPENDENT_F_START 28
+#define VIRTIO_DEV_INDEPENDENT_F_END   38
+
+#define VIRTIO_F_RING_INDIRECT_DESC 28
+#define VIRTIO_F_RING_EVENT_IDX 29
+#define VIRTIO_F_IN_ORDER 35
+#define VIRTIO_F_NOTIFICATION_DATA 38
 
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 /* Do we get callbacks when the ring is completely used, even if we've