Re: [PATCH 02/10] block: virtio-blk: check logical block size
Hi Maxim, > Instead of this adding blk_is_valid_logical_block_size allowed me to > trivially convert most of the uses. > > For RFC I converted only some drivers that I am more familiar with > and/or can test but I can remove the driver's own checks in most other > drivers with low chance of introducing a bug, even if I can't test the > driver. > > What do you think? > > I can also both make blk_queue_logical_block_size return an error > value, and have blk_is_valid_logical_block_size and use either of > these checks, depending on the driver with eventual goal of > un-exporting blk_is_valid_logical_block_size. My concern is that blk_is_valid_logical_block_size() deviates from the regular block layer convention where the function to set a given queue limit will either adjust the passed value or print a warning. I guess I won't entirely object to having the actual check in a helper function that drivers with a peculiar initialization pattern can use. And then make blk_queue_logical_block_size() call that helper as well to validate the lbs. But I do think that blk_queue_logical_block_size() should be the preferred interface and that, where possible, drivers should be updated to check the return value of that. -- Martin K. Petersen Oracle Linux Engineering ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 19/19] arm64: lto: Strengthen READ_ONCE() to acquire when CONFIG_LTO=y
On Fri 2020-07-10 17:52:03, Will Deacon wrote:
> When building with LTO, there is an increased risk of the compiler
> converting an address dependency headed by a READ_ONCE() invocation
> into a control dependency and consequently allowing for harmful
> reordering by the CPU.
>
> Ensure that such transformations are harmless by overriding the generic
> READ_ONCE() definition with one that provides acquire semantics when
> building with LTO.
Traditionally, READ_ONCE had only effects on compiler optimalizations, not on
special semantics of the load instruction.
Do you have example how LTO optimalizations break the code?
Should some documentation be added? Because I believe users will need to
understand
what is going on there.
It is not LTO-only problem and it is not arm64-only problem, right?
Best regards,
Pavel
> +#ifdef CONFIG_AS_HAS_LDAPR
> +#define __LOAD_RCPC(sfx, regs...)\
> + ALTERNATIVE(\
> + "ldar" #sfx "\t" #regs,\
> + ".arch_extension rcpc\n"\
> + "ldapr" #sfx "\t" #regs,\
> + ARM64_HAS_LDAPR)
> +#else
> +#define __LOAD_RCPC(sfx, regs...)"ldar" #sfx "\t" #regs
> +#endif /* CONFIG_AS_HAS_LDAPR */
> +
> +#define __READ_ONCE(x)
> \
> +({ \
> + typeof(&(x)) __x = &(x);\
> + int atomic = 1; \
> + union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u; \
> + switch (sizeof(x)) {\
> + case 1: \
> + asm volatile(__LOAD_RCPC(b, %w0, %1)\
> + : "=r" (*(__u8 *)__u.__c) \
> + : "Q" (*__x) : "memory"); \
> + break; \
> + case 2: \
> + asm volatile(__LOAD_RCPC(h, %w0, %1)\
> + : "=r" (*(__u16 *)__u.__c) \
> + : "Q" (*__x) : "memory"); \
> + break; \
> + case 4: \
> + asm volatile(__LOAD_RCPC(, %w0, %1) \
> + : "=r" (*(__u32 *)__u.__c) \
> + : "Q" (*__x) : "memory"); \
> + break; \
> + case 8: \
> + asm volatile(__LOAD_RCPC(, %0, %1) \
> + : "=r" (*(__u64 *)__u.__c) \
> + : "Q" (*__x) : "memory"); \
> + break; \
> + default:\
> + atomic = 0; \
> + } \
> + atomic ? (typeof(*__x))__u.__val : (*(volatile typeof(__x))__x);\
> +})
> +
> +#endif /* !BUILD_VDSO */
> +#endif /* CONFIG_LTO */
> +
> +#include
> +
> +#endif /* __ASM_RWONCE_H */
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index 45d5cfe46429..60df97f2e7de 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -28,7 +28,7 @@ ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1
> --hash-style=sysv\
>$(btildflags-y) -T
>
> ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
> -ccflags-y += -DDISABLE_BRANCH_PROFILING
> +ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
>
> CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS)
> $(GCC_PLUGINS_CFLAGS)
> KBUILD_CFLAGS+= $(DISABLE_LTO)
> diff --git a/arch/arm64/kernel/vdso32/Makefile
> b/arch/arm64/kernel/vdso32/Makefile
> index d88148bef6b0..4fdf3754a058 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -43,7 +43,7 @@ cc32-as-instr = $(call try-run,\
> # As a result we set our own flags here.
>
> # KBUILD_CPPFLAGS and NOSTDINC_FLAGS from top-level Makefile
> -VDSO_CPPFLAGS := -D__KERNEL__ -nostdinc -isystem $(shell $(CC_COMPAT)
> -print-file-name=include)
> +VDSO_CPPFLAGS := -DBUILD_VDSO -D__
Re: [PATCH 02/10] block: virtio-blk: check logical block size
On Tue, Jul 21, 2020 at 01:52:31PM +0300, Maxim Levitsky wrote: > Linux kernel only supports logical block sizes which are power of two, > at least 512 bytes and no more that PAGE_SIZE. > > Check this instead of crashing later on. > > Note that there is no need to check physical block size since it is > only a hint, and virtio-blk already only supports power of two values. > > Bugzilla link: https://bugzilla.redhat.com/show_bug.cgi?id=1664619 > > Signed-off-by: Maxim Levitsky > --- > drivers/block/virtio_blk.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 02/10] block: virtio-blk: check logical block size
On Wed, 2020-07-22 at 12:11 +0300, Maxim Levitsky wrote: > On Tue, 2020-07-21 at 22:55 -0400, Martin K. Petersen wrote: > > Christoph, > > > > > Hmm, I wonder if we should simply add the check and warning to > > > blk_queue_logical_block_size and add an error in that case. Then > > > drivers only have to check the error return, which might add a lot > > > less boiler plate code. > > > > Yep, I agree. > > > > I also agree that this would be cleaner (I actually tried to implement > this the way you suggest), but let me explain my reasoning for doing > it > this way. > > The problem is that most current users of blk_queue_logical_block_size > (43 uses in the tree, out of which only 9 use constant block size) > check > for the block size relatively early, often store it in some internal > struct etc, prior to calling blk_queue_logical_block_size thus making > them only to rely on blk_queue_logical_block_size as the check for > block size validity will need non-trivial changes in their code. > > Instead of this adding blk_is_valid_logical_block_size allowed me > to trivially convert most of the uses. > > For RFC I converted only some drivers that I am more familiar with > and/or can test but I can remove the driver's own checks in most other > drivers with low chance of introducing a bug, even if I can't test the > driver. > > What do you think? > > I can also both make blk_queue_logical_block_size return an error > value, > and have blk_is_valid_logical_block_size and use either of these > checks, > depending on the driver with eventual goal of un-exporting > blk_is_valid_logical_block_size. > > Also note that I did add WARN_ON to blk_queue_logical_block_size. Any update on this? Best regards, Maxim Levitsky > > Best regards, > Maxim Levitsky ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[vhost:vhost 38/45] include/linux/vdpa.h:43:28: sparse: sparse: expected ; at end of declaration
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: 84d40e4b4bc64456abf5ef5663871053b40e84ac
commit: fee8fe6bd8ccacd27e963b71b4f943be3721779e [38/45] vdpa: make sure
set_features in invoked for legacy
:: branch date: 5 hours ago
:: commit date: 6 hours ago
config: x86_64-randconfig-s021-20200727 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-94-geb6779f6-dirty
git checkout fee8fe6bd8ccacd27e963b71b4f943be3721779e
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
drivers/vdpa/vdpa.c: note: in included file:
>> include/linux/vdpa.h:43:28: sparse: sparse: expected ; at end of declaration
include/linux/vdpa.h:43:28: sparse: sparse: Expected } at end of
struct-union-enum-specifier
include/linux/vdpa.h:43:28: sparse: sparse: got .
--
drivers/vdpa/ifcvf/ifcvf_main.c: note: in included file (through
drivers/vdpa/ifcvf/ifcvf_base.h):
>> include/linux/vdpa.h:43:28: sparse: sparse: expected ; at end of declaration
include/linux/vdpa.h:43:28: sparse: sparse: Expected } at end of
struct-union-enum-specifier
include/linux/vdpa.h:43:28: sparse: sparse: got .
--
drivers/vdpa/ifcvf/ifcvf_base.c: note: in included file (through
drivers/vdpa/ifcvf/ifcvf_base.h):
>> include/linux/vdpa.h:43:28: sparse: sparse: expected ; at end of declaration
include/linux/vdpa.h:43:28: sparse: sparse: Expected } at end of
struct-union-enum-specifier
include/linux/vdpa.h:43:28: sparse: sparse: got .
#
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?id=fee8fe6bd8ccacd27e963b71b4f943be3721779e
git remote add vhost
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
git remote update vhost
git checkout fee8fe6bd8ccacd27e963b71b4f943be3721779e
vim +43 include/linux/vdpa.h
c25a26e653a61b Jason Wang 2020-05-29 29
961e9c84077f6c Jason Wang 2020-03-26 30 /**
961e9c84077f6c Jason Wang 2020-03-26 31 * vDPA device -
representation of a vDPA device
961e9c84077f6c Jason Wang 2020-03-26 32 * @dev: underlying device
961e9c84077f6c Jason Wang 2020-03-26 33 * @dma_dev: the actual
device that is performing DMA
961e9c84077f6c Jason Wang 2020-03-26 34 * @config: the configuration
ops for this device.
961e9c84077f6c Jason Wang 2020-03-26 35 * @index: device index
fee8fe6bd8ccac Michael S. Tsirkin 2020-07-27 36 * @features_valid: were
features initialized? for legacy guests
961e9c84077f6c Jason Wang 2020-03-26 37 */
961e9c84077f6c Jason Wang 2020-03-26 38 struct vdpa_device {
961e9c84077f6c Jason Wang 2020-03-26 39struct device dev;
961e9c84077f6c Jason Wang 2020-03-26 40struct device *dma_dev;
961e9c84077f6c Jason Wang 2020-03-26 41const struct
vdpa_config_ops *config;
961e9c84077f6c Jason Wang 2020-03-26 42unsigned int index;
fee8fe6bd8ccac Michael S. Tsirkin 2020-07-27 @43bool features_valid.
961e9c84077f6c Jason Wang 2020-03-26 44 };
961e9c84077f6c Jason Wang 2020-03-26 45
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1 1/6] mm/page_alloc: tweak comments in has_unmovable_pages()
On 28.07.20 15:48, Baoquan He wrote:
> On 06/30/20 at 04:26pm, David Hildenbrand wrote:
>> Let's move the split comment regarding bootmem allocations and memory
>> holes, especially in the context of ZONE_MOVABLE, to the PageReserved()
>> check.
>>
>> Cc: Andrew Morton
>> Cc: Michal Hocko
>> Cc: Michael S. Tsirkin
>> Signed-off-by: David Hildenbrand
>> ---
>> mm/page_alloc.c | 22 ++
>> 1 file changed, 6 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 48eb0f1410d47..bd3ebf08f09b9 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -8207,14 +8207,6 @@ struct page *has_unmovable_pages(struct zone *zone,
>> struct page *page,
>> unsigned long iter = 0;
>> unsigned long pfn = page_to_pfn(page);
>>
>> -/*
>> - * TODO we could make this much more efficient by not checking every
>> - * page in the range if we know all of them are in MOVABLE_ZONE and
>> - * that the movable zone guarantees that pages are migratable but
>> - * the later is not the case right now unfortunatelly. E.g. movablecore
>> - * can still lead to having bootmem allocations in zone_movable.
>> - */
>> -
>> if (is_migrate_cma_page(page)) {
>> /*
>> * CMA allocations (alloc_contig_range) really need to mark
>> @@ -8233,6 +8225,12 @@ struct page *has_unmovable_pages(struct zone *zone,
>> struct page *page,
>>
>> page = pfn_to_page(pfn + iter);
>>
>> +/*
>> + * Both, bootmem allocations and memory holes are marked
>> + * PG_reserved and are unmovable. We can even have unmovable
>> + * allocations inside ZONE_MOVABLE, for example when
>> + * specifying "movable_core".
> should be 'movablecore', we don't
> have kernel parameter 'movable_core'.
Agreed!
>
> Otherwise, this looks good to me. Esp the code comment at below had been
> added very long time ago and obsolete.
>
> Reviewed-by: Baoquan He
>
> By the way, David, do you know what is the situation of having unmovable
> allocations inside ZONE_MOVABLE when specifying 'movablecore'? I quickly
> went through find_zone_movable_pfns_for_nodes(), but didn't get why.
> Could you tell a little more detail about it?
As far as I understand, it can happen that we have memblock allocations
during boot that fall into an area the kernel later configures to span
the movable zone (via movable_core).
>
> Thanks
> Baoquan
--
Thanks,
David / dhildenb
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 vhost next 10/10] vdpa/mlx5: Add VDPA driver for supported mlx5 devices
Hi Eli, Thank you for the patch! Yet something to improve: [auto build test ERROR on next-20200727] url: https://github.com/0day-ci/linux/commits/Eli-Cohen/VDPA-support-for-Mellanox-ConnectX-devices/20200728-140938 base:e9a523ff8f76de0768857f02ea76437d3b39d151 config: mips-allmodconfig (attached as .config) compiler: mips-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! >> ERROR: modpost: "__udivdi3" [drivers/vdpa/mlx5/mlx5_vdpa.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[vhost:vhost 40/45] drivers/virtio/virtio_vdpa.c:301:9: error: void value not ignored as it ought to be
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: 84d40e4b4bc64456abf5ef5663871053b40e84ac
commit: 03750f7fa49f9384911059fe16f5367b7e86f99d [40/45] virtio_vdpa: legacy
features handling
config: nds32-randconfig-r003-20200728 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 03750f7fa49f9384911059fe16f5367b7e86f99d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All error/warnings (new ones prefixed by >>):
In file included from drivers/virtio/virtio_vdpa.c:17:
include/linux/vdpa.h:43:21: error: expected ':', ',', ';', '}' or
'__attribute__' before '.' token
43 | bool features_valid.
| ^
include/linux/vdpa.h: In function 'vdpa_reset':
include/linux/vdpa.h:276:6: error: 'struct vdpa_device' has no member named
'features_valid'
276 | vdev->features_valid = false;
| ^~
include/linux/vdpa.h: In function 'vdpa_set_features':
include/linux/vdpa.h:284:6: error: 'struct vdpa_device' has no member named
'features_valid'
284 | vdev->features_valid = true;
| ^~
include/linux/vdpa.h: In function 'vdpa_get_config':
include/linux/vdpa.h:298:11: error: 'struct vdpa_device' has no member named
'features_valid'
298 | if (!vdev->features_valid)
| ^~
drivers/virtio/virtio_vdpa.c: In function 'virtio_vdpa_get':
drivers/virtio/virtio_vdpa.c:60:32: warning: unused variable 'ops'
[-Wunused-variable]
60 | const struct vdpa_config_ops *ops = vdpa->config;
|^~~
drivers/virtio/virtio_vdpa.c: In function 'virtio_vdpa_finalize_features':
>> drivers/virtio/virtio_vdpa.c:301:9: error: void value not ignored as it
>> ought to be
301 | return vdpa_set_features(vdpa, vdev->features);
| ^~~
drivers/virtio/virtio_vdpa.c:296:32: warning: unused variable 'ops'
[-Wunused-variable]
296 | const struct vdpa_config_ops *ops = vdpa->config;
|^~~
>> drivers/virtio/virtio_vdpa.c:302:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
302 | }
| ^
vim +301 drivers/virtio/virtio_vdpa.c
292
293 static int virtio_vdpa_finalize_features(struct virtio_device *vdev)
294 {
295 struct vdpa_device *vdpa = vd_get_vdpa(vdev);
296 const struct vdpa_config_ops *ops = vdpa->config;
297
298 /* Give virtio_ring a chance to accept features. */
299 vring_transport_features(vdev);
300
> 301 return vdpa_set_features(vdpa, vdev->features);
> 302 }
303
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V4 4/6] vhost_vdpa: implement IRQ offloading in vhost_vdpa
On 2020/7/28 下午5:18, Zhu, Lingshan wrote: * status to 0. @@ -167,6 +220,15 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp) if (status != 0 && (ops->get_status(vdpa) & ~status) != 0) return -EINVAL; + /* vq irq is not expected to be changed once DRIVER_OK is set */ So this basically limit the usage of get_vq_irq() in the context vhost_vdpa_set_status() and other vDPA bus drivers' set_status(). If this is true, there's even no need to introduce any new config ops but just let set_status() to return the irqs used for the device. Or if we want this to be more generic, we need vpda's own irq manager (which should be similar to irq bypass manager). That is: I think there is no need for a driver to free / re-request its irqs after DRIVER_OK though it can do so. If a driver changed its irq of a vq after DRIVER_OK, the vq is still operational but will lose irq offloading that is reasonable. If we want set_status() return irqs, we need to record the irqs somewhere in vdpa_device, Why, we can simply pass an array to the driver I think? void (*set_status)(struct vdpa_device *vdev, u8 status, int *irqs); as we discussed in a previous thread, this may need initialize and cleanup works, so a new ops with proper comments (don't say they could not change irq, but highlight if irq changes, irq offloading will not work till next DRIVER_OK) could be more elegant. However if we really need to change irq after DRIVER_OK, I think maybe we still need vDPA vq irq allocate / free helpers, then the helpers can not be used in probe() as we discussed before, it is a step back to V3 series. Still, it's not about whether driver may change irq after DRIVER_OK but implication of the assumption. If one bus ops must be called in another ops, it's better to just implement them in one ops. - bus driver can register itself as consumer - vDPA device driver can register itself as producer - matching via queue index IMHO, is it too heavy for this feature, Do you mean LOCs? We can: 1) refactor irq bypass manager 2) invent it our own (a much simplified version compared to bypass manager) 3) enforcing them via vDPA bus Each of the above should be not a lot of coding. I think method 3 is partially done in your previous series but in an implicit manner: - bus driver that has alloc_irq/free_irq implemented could be implicitly treated as consumer registering - every vDPA device driver could be treated as producer - vdpa_devm_alloc_irq() could be treated as producer registering - alloc_irq/free_irq is the add_producer/del_procuer We probably just lack some synchronization with driver probe/remove. and how can they match if two individual adapters both have vq idx = 1. The matching is per vDPA device. Thanks Thanks! - deal with registering/unregistering of consumer/producer So there's no need to care when or where the vDPA device driver to allocate the irq, and we don't need to care at which context the vDPA bus driver can use the irq. Either side may get notified when the other side is gone (though we only care about the gone of producer probably). Any thought on this? Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V4 4/6] vhost_vdpa: implement IRQ offloading in vhost_vdpa
On 2020/7/28 下午12:24, Zhu Lingshan wrote:
This patch introduce a set of functions for setup/unsetup
and update irq offloading respectively by register/unregister
and re-register the irq_bypass_producer.
With these functions, this commit can setup/unsetup
irq offloading through setting DRIVER_OK/!DRIVER_OK, and
update irq offloading through SET_VRING_CALL.
Signed-off-by: Zhu Lingshan
Suggested-by: Jason Wang
---
drivers/vhost/Kconfig | 1 +
drivers/vhost/vdpa.c | 79 ++-
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index d3688c6afb87..587fbae06182 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -65,6 +65,7 @@ config VHOST_VDPA
tristate "Vhost driver for vDPA-based backend"
depends on EVENTFD
select VHOST
+ select IRQ_BYPASS_MANAGER
depends on VDPA
help
This kernel module can be loaded in host kernel to accelerate
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index df3cf386b0cd..1dccced321f8 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -115,6 +115,55 @@ static irqreturn_t vhost_vdpa_config_cb(void *private)
return IRQ_HANDLED;
}
+static void vhost_vdpa_setup_vq_irq(struct vhost_vdpa *v, int qid)
+{
+ struct vhost_virtqueue *vq = &v->vqs[qid];
+ const struct vdpa_config_ops *ops = v->vdpa->config;
+ struct vdpa_device *vdpa = v->vdpa;
+ int ret, irq;
+
+ spin_lock(&vq->call_ctx.ctx_lock);
+ irq = ops->get_vq_irq(vdpa, qid);
+ if (!vq->call_ctx.ctx || irq == -EINVAL) {
It's better to check returned irq as "irq < 0" to be more robust.
Forcing a specific errno value is not good.
+ spin_unlock(&vq->call_ctx.ctx_lock);
+ return;
+ }
+
+ vq->call_ctx.producer.token = vq->call_ctx.ctx;
+ vq->call_ctx.producer.irq = irq;
+ ret = irq_bypass_register_producer(&vq->call_ctx.producer);
+ spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
+static void vhost_vdpa_unsetup_vq_irq(struct vhost_vdpa *v, int qid)
+{
+ struct vhost_virtqueue *vq = &v->vqs[qid];
+
+ spin_lock(&vq->call_ctx.ctx_lock);
+ irq_bypass_unregister_producer(&vq->call_ctx.producer);
+ spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
+static void vhost_vdpa_update_vq_irq(struct vhost_virtqueue *vq)
+{
+ spin_lock(&vq->call_ctx.ctx_lock);
+ /*
+* if it has a non-zero irq, means there is a
+* previsouly registered irq_bypass_producer,
+* we should update it when ctx (its token)
+* changes.
+*/
+ if (!vq->call_ctx.producer.irq) {
+ spin_unlock(&vq->call_ctx.ctx_lock);
+ return;
+ }
+
+ irq_bypass_unregister_producer(&vq->call_ctx.producer);
+ vq->call_ctx.producer.token = vq->call_ctx.ctx;
+ irq_bypass_register_producer(&vq->call_ctx.producer);
+ spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
static void vhost_vdpa_reset(struct vhost_vdpa *v)
{
struct vdpa_device *vdpa = v->vdpa;
@@ -155,11 +204,15 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v,
u8 __user *statusp)
{
struct vdpa_device *vdpa = v->vdpa;
const struct vdpa_config_ops *ops = vdpa->config;
- u8 status;
+ u8 status, status_old;
+ int i, nvqs;
if (copy_from_user(&status, statusp, sizeof(status)))
return -EFAULT;
+ status_old = ops->get_status(vdpa);
+ nvqs = v->nvqs;
+
/*
* Userspace shouldn't remove status bits unless reset the
* status to 0.
@@ -167,6 +220,15 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8
__user *statusp)
if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
return -EINVAL;
+ /* vq irq is not expected to be changed once DRIVER_OK is set */
So this basically limit the usage of get_vq_irq() in the context
vhost_vdpa_set_status() and other vDPA bus drivers' set_status(). If
this is true, there's even no need to introduce any new config ops but
just let set_status() to return the irqs used for the device. Or if we
want this to be more generic, we need vpda's own irq manager (which
should be similar to irq bypass manager). That is:
- bus driver can register itself as consumer
- vDPA device driver can register itself as producer
- matching via queue index
- deal with registering/unregistering of consumer/producer
So there's no need to care when or where the vDPA device driver to
allocate the irq, and we don't need to care at which context the vDPA
bus driver can use the irq. Either side may get notified when the other
side is gone (though we only care about the gone of producer probably).
Any thought on this?
Thanks
+ if ((status & VIRTIO_CONFIG_S_DRIVER_OK) && !(status_old &
VIRTIO_CONFIG_S_DRIVER_OK))
+ for (i = 0; i < nvqs; i+
Re: [PATCH V4 3/6] vDPA: add get_vq_irq() in vdpa_config_ops
On 2020/7/28 下午12:24, Zhu Lingshan wrote:
This commit adds a new function get_vq_irq() in struct
vdpa_config_ops, which will return the irq number of a
virtqueue.
Signed-off-by: Zhu Lingshan
Suggested-by: Jason Wang
---
include/linux/vdpa.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 239db794357c..cebc79173aaa 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -87,6 +87,11 @@ struct vdpa_device {
*@vdev: vdpa device
*@idx: virtqueue index
*Returns the notifcation area
+ * @get_vq_irq:Get the irq number of a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * Returns u32: irq number of a virtqueue,
+ * -EINVAL if no irq assigned.
I think we can not get -EINVAL since the function will return a u32.
Thanks
* @get_vq_align: Get the virtqueue align requirement
*for the device
*@vdev: vdpa device
@@ -178,6 +183,7 @@ struct vdpa_config_ops {
u64 (*get_vq_state)(struct vdpa_device *vdev, u16 idx);
struct vdpa_notification_area
(*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
+ u32 (*get_vq_irq)(struct vdpa_device *vdv, u16 idx);
/* Device ops */
u32 (*get_vq_align)(struct vdpa_device *vdev);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
