thr3ads.net - Linux Virtualization - [PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA [Apr 2020]

If this information is useful, please help other people find it:
Share via:

Arnd Bergmann

2020-Apr-09 10:41 UTC

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

On Thu, Mar 26, 2020 at 3:08 PM Jason Wang <jasowang at redhat.com>
wrote:>
> From: Zhu Lingshan <lingshan.zhu at intel.com>
>
> This commit introduced two layers to drive IFC VF:
>
> (1) ifcvf_base layer, which handles IFC VF NIC hardware operations and
>     configurations.
>
> (2) ifcvf_main layer, which complies to VDPA bus framework,
>     implemented device operations for VDPA bus, handles device probe,
>     bus attaching, vring operations, etc.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com>
> Signed-off-by: Bie Tiwei <tiwei.bie at intel.com>
> Signed-off-by: Wang Xiao <xiao.w.wang at intel.com>
> Signed-off-by: Jason Wang <jasowang at redhat.com>
> +
> +#define IFCVF_QUEUE_ALIGNMENT  PAGE_SIZE
> +#define IFCVF_QUEUE_MAX                32768
> +static u16 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
> +{
> +       return IFCVF_QUEUE_ALIGNMENT;
> +}
This fails to build on arm64 with 64kb page size (found in linux-next):

/drivers/vdpa/ifcvf/ifcvf_main.c: In function 'ifcvf_vdpa_get_vq_align':
arch/arm64/include/asm/page-def.h:17:20: error: conversion from 'long
unsigned int' to 'u16' {aka 'short unsigned int'} changes
value from
'65536' to '0' [-Werror=overflow]
   17 | #define PAGE_SIZE  (_AC(1, UL) << PAGE_SHIFT)
      |                    ^
drivers/vdpa/ifcvf/ifcvf_base.h:37:31: note: in expansion of macro
'PAGE_SIZE'
   37 | #define IFCVF_QUEUE_ALIGNMENT PAGE_SIZE
      |                               ^~~~~~~~~
drivers/vdpa/ifcvf/ifcvf_main.c:231:9: note: in expansion of macro
'IFCVF_QUEUE_ALIGNMENT'
  231 |  return IFCVF_QUEUE_ALIGNMENT;
      |         ^~~~~~~~~~~~~~~~~~~~~

It's probably good enough to just not allow the driver to be built in that
configuration as it's fairly rare but unfortunately there is no simple
Kconfig
symbol for it.

In a similar driver, we did

config VMXNET3
        tristate "VMware VMXNET3 ethernet driver"
        depends on PCI && INET
        depends on !(PAGE_SIZE_64KB || ARM64_64K_PAGES || \
                     IA64_PAGE_SIZE_64KB || MICROBLAZE_64K_PAGES || \
                     PARISC_PAGE_SIZE_64KB || PPC_64K_PAGES)

I think we should probably make PAGE_SIZE_64KB a global symbol
in arch/Kconfig and have it selected by the other symbols so drivers
like yours can add a dependency for it.

         Arnd

Jason Wang

2020-Apr-09 12:43 UTC

head link

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

On 2020/4/9 ??6:41, Arnd Bergmann wrote:> On Thu, Mar 26, 2020 at 3:08 PM Jason Wang <jasowang at redhat.com>
wrote:
>> From: Zhu Lingshan <lingshan.zhu at intel.com>
>>
>> This commit introduced two layers to drive IFC VF:
>>
>> (1) ifcvf_base layer, which handles IFC VF NIC hardware operations and
>>      configurations.
>>
>> (2) ifcvf_main layer, which complies to VDPA bus framework,
>>      implemented device operations for VDPA bus, handles device probe,
>>      bus attaching, vring operations, etc.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com>
>> Signed-off-by: Bie Tiwei <tiwei.bie at intel.com>
>> Signed-off-by: Wang Xiao <xiao.w.wang at intel.com>
>> Signed-off-by: Jason Wang <jasowang at redhat.com>
>> +
>> +#define IFCVF_QUEUE_ALIGNMENT  PAGE_SIZE
>> +#define IFCVF_QUEUE_MAX                32768
>> +static u16 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
>> +{
>> +       return IFCVF_QUEUE_ALIGNMENT;
>> +}
> This fails to build on arm64 with 64kb page size (found in linux-next):
>
> /drivers/vdpa/ifcvf/ifcvf_main.c: In function
'ifcvf_vdpa_get_vq_align':
> arch/arm64/include/asm/page-def.h:17:20: error: conversion from 'long
> unsigned int' to 'u16' {aka 'short unsigned int'}
changes value from
> '65536' to '0' [-Werror=overflow]
>     17 | #define PAGE_SIZE  (_AC(1, UL) << PAGE_SHIFT)
>        |                    ^
> drivers/vdpa/ifcvf/ifcvf_base.h:37:31: note: in expansion of macro
'PAGE_SIZE'
>     37 | #define IFCVF_QUEUE_ALIGNMENT PAGE_SIZE
>        |                               ^~~~~~~~~
> drivers/vdpa/ifcvf/ifcvf_main.c:231:9: note: in expansion of macro
> 'IFCVF_QUEUE_ALIGNMENT'
>    231 |  return IFCVF_QUEUE_ALIGNMENT;
>        |         ^~~~~~~~~~~~~~~~~~~~~
>
> It's probably good enough to just not allow the driver to be built in
that
> configuration as it's fairly rare but unfortunately there is no simple
Kconfig
> symbol for it.

Or I think the 64KB alignment is probably more than enough.

Ling Shan, can we use smaller value here?

Thanks

>
> In a similar driver, we did
>
> config VMXNET3
>          tristate "VMware VMXNET3 ethernet driver"
>          depends on PCI && INET
>          depends on !(PAGE_SIZE_64KB || ARM64_64K_PAGES || \
>                       IA64_PAGE_SIZE_64KB || MICROBLAZE_64K_PAGES || \
>                       PARISC_PAGE_SIZE_64KB || PPC_64K_PAGES)
>
> I think we should probably make PAGE_SIZE_64KB a global symbol
> in arch/Kconfig and have it selected by the other symbols so drivers
> like yours can add a dependency for it.
>
>           Arnd
>

Michael S. Tsirkin

2020-Apr-09 20:25 UTC

head link

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

On Thu, Apr 09, 2020 at 12:41:13PM +0200, Arnd Bergmann
wrote:> On Thu, Mar 26, 2020 at 3:08 PM Jason Wang <jasowang at redhat.com>
wrote:
> >
> > From: Zhu Lingshan <lingshan.zhu at intel.com>
> >
> > This commit introduced two layers to drive IFC VF:
> >
> > (1) ifcvf_base layer, which handles IFC VF NIC hardware operations and
> >     configurations.
> >
> > (2) ifcvf_main layer, which complies to VDPA bus framework,
> >     implemented device operations for VDPA bus, handles device probe,
> >     bus attaching, vring operations, etc.
> >
> > Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com>
> > Signed-off-by: Bie Tiwei <tiwei.bie at intel.com>
> > Signed-off-by: Wang Xiao <xiao.w.wang at intel.com>
> > Signed-off-by: Jason Wang <jasowang at redhat.com>
> 
> > +
> > +#define IFCVF_QUEUE_ALIGNMENT  PAGE_SIZE
> > +#define IFCVF_QUEUE_MAX                32768
> > +static u16 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
> > +{
> > +       return IFCVF_QUEUE_ALIGNMENT;
> > +}
> 
> This fails to build on arm64 with 64kb page size (found in linux-next):
> 
> /drivers/vdpa/ifcvf/ifcvf_main.c: In function
'ifcvf_vdpa_get_vq_align':
> arch/arm64/include/asm/page-def.h:17:20: error: conversion from 'long
> unsigned int' to 'u16' {aka 'short unsigned int'}
changes value from
> '65536' to '0' [-Werror=overflow]
>    17 | #define PAGE_SIZE  (_AC(1, UL) << PAGE_SHIFT)
>       |                    ^
> drivers/vdpa/ifcvf/ifcvf_base.h:37:31: note: in expansion of macro
'PAGE_SIZE'
>    37 | #define IFCVF_QUEUE_ALIGNMENT PAGE_SIZE
>       |                               ^~~~~~~~~
> drivers/vdpa/ifcvf/ifcvf_main.c:231:9: note: in expansion of macro
> 'IFCVF_QUEUE_ALIGNMENT'
>   231 |  return IFCVF_QUEUE_ALIGNMENT;
>       |         ^~~~~~~~~~~~~~~~~~~~~
> 
> It's probably good enough to just not allow the driver to be built in
that
> configuration as it's fairly rare but unfortunately there is no simple
Kconfig
> symbol for it.
> 
> In a similar driver, we did
> 
> config VMXNET3
>         tristate "VMware VMXNET3 ethernet driver"
>         depends on PCI && INET
>         depends on !(PAGE_SIZE_64KB || ARM64_64K_PAGES || \
>                      IA64_PAGE_SIZE_64KB || MICROBLAZE_64K_PAGES || \
>                      PARISC_PAGE_SIZE_64KB || PPC_64K_PAGES)
> 
> I think we should probably make PAGE_SIZE_64KB a global symbol
> in arch/Kconfig and have it selected by the other symbols so drivers
> like yours can add a dependency for it.
> 
>          Arnd
It's probably easier to make the alignment u32 - I don't really know why
it's
u16, all callers seem to assign the result to a u32 value.

Possibly Parallel Threads

Search for more possibly parallel threads

Linux Virtualization - Apr 2020 - [PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

[PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA

Possibly Parallel Threads