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

[Zhu, Lingshan]

On 4/9/2020 8:43 PM, Jason Wang wrote:
> On 2020/4/9 äå6:41, Arnd Bergmann wrote:
> > On Thu, Mar 26, 2020 at 3:08 PM Jason Wang <jasowang@xxxxxxxxxx> wrote:
> > > From: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Signed-off-by: Bie Tiwei <tiwei.bie@xxxxxxxxx>
> > > Signed-off-by: Wang Xiao <xiao.w.wang@xxxxxxxxx>
> > > Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> > > +
> > > +#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

sure, we just need it page aligned, this value is only used in 
get_vq_align(). I will try to find a arm64 machine and debug on this issue

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