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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <77962da1-b9ec-2eaf-21d0-9790e7f0366d@intel.com>
Date:   Fri, 10 Apr 2020 11:15:53 +0800
From:   "Zhu, Lingshan" <lingshan.zhu@...el.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Cc:     Jason Wang <jasowang@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm list <kvm@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        Networking <netdev@...r.kernel.org>,
        Jason Gunthorpe <jgg@...lanox.com>, maxime.coquelin@...hat.com,
        cunming.liang@...el.com, zhihong.wang@...el.com,
        rob.miller@...adcom.com, xiao.w.wang@...el.com,
        eperezma@...hat.com, lulu@...hat.com,
        Parav Pandit <parav@...lanox.com>, kevin.tian@...el.com,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Christoph Hellwig <hch@...radead.org>, aadam@...hat.com,
        Jiri Pirko <jiri@...lanox.com>, shahafs@...lanox.com,
        hanand@...inx.com, mhabets@...arflare.com, gdawar@...inx.com,
        saugatm@...inx.com, vmireyno@...vell.com,
        zhangweining@...jie.com.cn, Bie Tiwei <tiwei.bie@...el.com>
Subject: Re: [PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA


On 4/10/2020 4:25 AM, Michael S. Tsirkin wrote:
> 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@...hat.com> wrote:
>>> From: Zhu Lingshan <lingshan.zhu@...el.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@...el.com>
>>> Signed-off-by: Bie Tiwei <tiwei.bie@...el.com>
>>> Signed-off-by: Wang Xiao <xiao.w.wang@...el.com>
>>> Signed-off-by: Jason Wang <jasowang@...hat.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.

Thanks Michael!

@ Arnd, would you please kindly have a try on the attached patch? I 
failed to find an armv8 or above platform for now.

Thanks
BR
Zhu Lingshan

View attachment "0001-vdpa-change-get_vq_align-to-u32-alignment.patch" of type "text/plain" (2076 bytes)