Messages in this thread Patch in this message | | | Subject | Re: [PATCH V9 9/9] virtio: Intel IFC VF driver for VDPA | From | "Zhu, Lingshan" <> | Date | Fri, 10 Apr 2020 11:15:53 +0800 |
| |
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@redhat.com> wrote: >>> From: Zhu Lingshan <lingshan.zhu@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@intel.com> >>> Signed-off-by: Bie Tiwei <tiwei.bie@intel.com> >>> Signed-off-by: Wang Xiao <xiao.w.wang@intel.com> >>> Signed-off-by: Jason Wang <jasowang@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.
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 From bb2019f0620609fbbdc6854bccc5fefdb53f3d9d Mon Sep 17 00:00:00 2001 From: Zhu Lingshan <lingshan.zhu@intel.com> Date: Fri, 10 Apr 2020 11:04:38 +0800 Subject: [PATCH] vdpa: change get_vq_align() to u32 alignment
This commit change the return value of get_vq_align() to u32. this can help resolve a build issue of arm64 platform, and adapting to the callers.
Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +- include/linux/vdpa.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 8d54dc5..970895c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -228,7 +228,7 @@ static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) return IFCVF_SUBSYS_VENDOR_ID; } -static u16 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) +static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) { return IFCVF_QUEUE_ALIGNMENT; } diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c index 6e8a0cf..a4b3f0d 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c @@ -435,7 +435,7 @@ static u64 vdpasim_get_vq_state(struct vdpa_device *vdpa, u16 idx) return vrh->last_avail_idx; } -static u16 vdpasim_get_vq_align(struct vdpa_device *vdpa) +static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa) { return VDPASIM_QUEUE_ALIGN; } diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 733acfb..5453af8 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -164,7 +164,7 @@ struct vdpa_config_ops { u64 (*get_vq_state)(struct vdpa_device *vdev, u16 idx); /* Device ops */ - u16 (*get_vq_align)(struct vdpa_device *vdev); + u32 (*get_vq_align)(struct vdpa_device *vdev); u64 (*get_features)(struct vdpa_device *vdev); int (*set_features)(struct vdpa_device *vdev, u64 features); void (*set_config_cb)(struct vdpa_device *vdev, -- 1.8.3.1
| |