From: Jason Wang <hidden> Date: 2020-02-10 03:57:36
Hi all:
This is an updated version of kernel support for vDPA device. Various
changes were made based on the feedback since last verion. One major
change is to drop the sysfs API and leave the management interface for
future development, and introudce the incremental DMA bus
operations. Please see changelog for more information.
The work on vhost, IFCVF (intel VF driver for vDPA) and qemu is
ongoing.
Please provide feedback.
Thanks
Change from V1:
- drop sysfs API, leave the management interface to future development
(Michael)
- introduce incremental DMA ops (dma_map/dma_unmap) (Michael)
- introduce dma_device and use it instead of parent device for doing
IOMMU or DMA from bus driver (Michael, Jason, Ling Shan, Tiwei)
- accept parent device and dma device when register vdpa device
- coding style and compile fixes (Randy)
- using vdpa_xxx instead of xxx_vdpa (Jason)
- ove vDPA accessors to header and make it static inline (Jason)
- split vdp_register_device() into two helpers vdpa_init_device() and
vdpa_register_device() which allows intermediate step to be done (Jason)
- warn on invalidate queue state when fail to creating virtqueue (Jason)
- make to_virtio_vdpa_device() static (Jason)
- use kmalloc/kfree instead of devres for virtio vdpa device (Jason)
- avoid using cast in vdpa bus function (Jason)
- introduce module_vdpa_driver and fix module refcnt (Jason)
- fix returning freed address in vdapsim coherent DMA addr allocation (Dan)
- various other fixes and tweaks
V1: https://lkml.org/lkml/2020/1/16/353
Jason Wang (5):
vhost: factor out IOTLB
vringh: IOTLB support
vDPA: introduce vDPA bus
virtio: introduce a vDPA based transport
vdpasim: vDPA device simulator
MAINTAINERS | 2 +
drivers/vhost/Kconfig | 7 +
drivers/vhost/Kconfig.vringh | 1 +
drivers/vhost/Makefile | 2 +
drivers/vhost/net.c | 2 +-
drivers/vhost/vhost.c | 221 ++++-------
drivers/vhost/vhost.h | 36 +-
drivers/vhost/vhost_iotlb.c | 171 +++++++++
drivers/vhost/vringh.c | 421 ++++++++++++++++++--
drivers/virtio/Kconfig | 15 +
drivers/virtio/Makefile | 2 +
drivers/virtio/vdpa/Kconfig | 26 ++
drivers/virtio/vdpa/Makefile | 3 +
drivers/virtio/vdpa/vdpa.c | 160 ++++++++
drivers/virtio/vdpa/vdpa_sim.c | 678 +++++++++++++++++++++++++++++++++
drivers/virtio/virtio_vdpa.c | 392 +++++++++++++++++++
include/linux/vdpa.h | 233 +++++++++++
include/linux/vhost_iotlb.h | 45 +++
include/linux/vringh.h | 36 ++
19 files changed, 2249 insertions(+), 204 deletions(-)
create mode 100644 drivers/vhost/vhost_iotlb.c
create mode 100644 drivers/virtio/vdpa/Kconfig
create mode 100644 drivers/virtio/vdpa/Makefile
create mode 100644 drivers/virtio/vdpa/vdpa.c
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
create mode 100644 drivers/virtio/virtio_vdpa.c
create mode 100644 include/linux/vdpa.h
create mode 100644 include/linux/vhost_iotlb.h
--
2.19.1
From: Jason Wang <hidden> Date: 2020-02-10 03:58:39
This patch factors out IOTLB into a dedicated module in order to be
reused by other modules like vringh. User may choose to enable the
automatic retiring by specifying VHOST_IOTLB_FLAG_RETIRE flag to fit
for the case of vhost device IOTLB implementation.
Signed-off-by: Jason Wang <redacted>
---
MAINTAINERS | 1 +
drivers/vhost/Kconfig | 7 ++
drivers/vhost/Makefile | 2 +
drivers/vhost/net.c | 2 +-
drivers/vhost/vhost.c | 221 +++++++++++-------------------------
drivers/vhost/vhost.h | 36 ++----
drivers/vhost/vhost_iotlb.c | 171 ++++++++++++++++++++++++++++
include/linux/vhost_iotlb.h | 45 ++++++++
8 files changed, 304 insertions(+), 181 deletions(-)
create mode 100644 drivers/vhost/vhost_iotlb.c
create mode 100644 include/linux/vhost_iotlb.h
@@ -581,21 +577,25 @@ long vhost_dev_set_owner(struct vhost_dev *dev)}EXPORT_SYMBOL_GPL(vhost_dev_set_owner);-structvhost_umem*vhost_dev_reset_owner_prepare(void)+staticstructvhost_iotlb*iotlb_alloc(void)+{+returnvhost_iotlb_alloc(max_iotlb_entries,+VHOST_IOTLB_FLAG_RETIRE);+}++structvhost_iotlb*vhost_dev_reset_owner_prepare(void){-returnkvzalloc(sizeof(structvhost_umem),GFP_KERNEL);+returniotlb_alloc();}EXPORT_SYMBOL_GPL(vhost_dev_reset_owner_prepare);/* Caller should have device mutex */-voidvhost_dev_reset_owner(structvhost_dev*dev,structvhost_umem*umem)+voidvhost_dev_reset_owner(structvhost_dev*dev,structvhost_iotlb*umem){inti;vhost_dev_cleanup(dev);-/* Restore memory to default empty mapping. */-INIT_LIST_HEAD(&umem->umem_list);dev->umem=umem;/* We don't need VQ locks below since vhost_dev_cleanup makes sure*VQsaren'trunning.
@@ -677,9 +655,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)eventfd_ctx_put(dev->log_ctx);dev->log_ctx=NULL;/* No one will access memory at this point */-vhost_umem_clean(dev->umem);+vhost_iotlb_free(dev->umem);dev->umem=NULL;-vhost_umem_clean(dev->iotlb);+vhost_iotlb_free(dev->iotlb);dev->iotlb=NULL;vhost_clear_msg(dev);wake_up_interruptible_poll(&dev->wait,EPOLLIN|EPOLLRDNORM);
@@ -715,27 +693,26 @@ static bool vhost_overflow(u64 uaddr, u64 size)}/* Caller should have vq mutex and device mutex. */-staticboolvq_memory_access_ok(void__user*log_base,structvhost_umem*umem,+staticboolvq_memory_access_ok(void__user*log_base,structvhost_iotlb*umem,intlog_all){-structvhost_umem_node*node;+structvhost_iotlb_map*map;if(!umem)returnfalse;-list_for_each_entry(node,&umem->umem_list,link){-unsignedlonga=node->userspace_addr;+list_for_each_entry(map,&umem->list,link){+unsignedlonga=map->addr;-if(vhost_overflow(node->userspace_addr,node->size))+if(vhost_overflow(map->addr,map->size))returnfalse;-if(!access_ok((void__user*)a,-node->size))+if(!access_ok((void__user*)a,map->size))returnfalse;elseif(log_all&&!log_access_ok(log_base,-node->start,-node->size))+map->start,+map->size))returnfalse;}returntrue;
@@ -745,17 +722,17 @@ static inline void __user *vhost_vq_meta_fetch(struct vhost_virtqueue *vq,u64addr,unsignedintsize,inttype){-conststructvhost_umem_node*node=vq->meta_iotlb[type];+conststructvhost_iotlb_map*map=vq->meta_iotlb[type];-if(!node)+if(!map)returnNULL;-return(void*)(uintptr_t)(node->userspace_addr+addr-node->start);+return(void*)(uintptr_t)(map->addr+addr-map->start);}/* Can we switch to this memory table? *//* Caller should have device mutex but not vq mutex */-staticboolmemory_access_ok(structvhost_dev*d,structvhost_umem*umem,+staticboolmemory_access_ok(structvhost_dev*d,structvhost_iotlb*umem,intlog_all){inti;
@@ -1020,47 +997,6 @@ static inline int vhost_get_desc(struct vhost_virtqueue *vq,returnvhost_copy_from_user(vq,desc,vq->desc+idx,sizeof(*desc));}-staticintvhost_new_umem_range(structvhost_umem*umem,-u64start,u64size,u64end,-u64userspace_addr,intperm)-{-structvhost_umem_node*tmp,*node;--if(!size)-return-EFAULT;--node=kmalloc(sizeof(*node),GFP_ATOMIC);-if(!node)-return-ENOMEM;--if(umem->numem==max_iotlb_entries){-tmp=list_first_entry(&umem->umem_list,typeof(*tmp),link);-vhost_umem_free(umem,tmp);-}--node->start=start;-node->size=size;-node->last=end;-node->userspace_addr=userspace_addr;-node->perm=perm;-INIT_LIST_HEAD(&node->link);-list_add_tail(&node->link,&umem->umem_list);-vhost_umem_interval_tree_insert(node,&umem->umem_tree);-umem->numem++;--return0;-}--staticvoidvhost_del_umem_range(structvhost_umem*umem,-u64start,u64end)-{-structvhost_umem_node*node;--while((node=vhost_umem_interval_tree_iter_first(&umem->umem_tree,-start,end)))-vhost_umem_free(umem,node);-}-staticvoidvhost_iotlb_notify_vq(structvhost_dev*d,structvhost_iotlb_msg*msg){
@@ -1117,9 +1053,9 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,break;}vhost_vq_meta_reset(dev);-if(vhost_new_umem_range(dev->iotlb,msg->iova,msg->size,-msg->iova+msg->size-1,-msg->uaddr,msg->perm)){+if(vhost_iotlb_add_range(dev->iotlb,msg->iova,+msg->iova+msg->size-1,+msg->uaddr,msg->perm)){ret=-ENOMEM;break;}
@@ -1131,8 +1067,8 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,break;}vhost_vq_meta_reset(dev);-vhost_del_umem_range(dev->iotlb,msg->iova,-msg->iova+msg->size-1);+vhost_iotlb_del_range(dev->iotlb,msg->iova,+msg->iova+msg->size-1);break;default:ret=-EINVAL;
@@ -1311,44 +1247,42 @@ static bool vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,}staticvoidvhost_vq_meta_update(structvhost_virtqueue*vq,-conststructvhost_umem_node*node,+conststructvhost_iotlb_map*map,inttype){intaccess=(type==VHOST_ADDR_USED)?VHOST_ACCESS_WO:VHOST_ACCESS_RO;-if(likely(node->perm&access))-vq->meta_iotlb[type]=node;+if(likely(map->perm&access))+vq->meta_iotlb[type]=map;}staticbooliotlb_access_ok(structvhost_virtqueue*vq,intaccess,u64addr,u64len,inttype){-conststructvhost_umem_node*node;-structvhost_umem*umem=vq->iotlb;+conststructvhost_iotlb_map*map;+structvhost_iotlb*umem=vq->iotlb;u64s=0,size,orig_addr=addr,last=addr+len-1;if(vhost_vq_meta_fetch(vq,addr,len,type))returntrue;while(len>s){-node=vhost_umem_interval_tree_iter_first(&umem->umem_tree,-addr,-last);-if(node==NULL||node->start>addr){+map=vhost_iotlb_itree_first(umem,addr,last);+if(map==NULL||map->start>addr){vhost_iotlb_miss(vq,addr,access);returnfalse;-}elseif(!(node->perm&access)){+}elseif(!(map->perm&access)){/* Report the possible access violation by*requestanothertranslationfromuserspace.*/returnfalse;}-size=node->size-addr+node->start;+size=map->size-addr+map->start;if(orig_addr==addr&&size>=len)-vhost_vq_meta_update(vq,node,type);+vhost_vq_meta_update(vq,map,type);s+=size;addr+=size;
@@ -1364,12 +1298,12 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)if(!vq->iotlb)return1;-returniotlb_access_ok(vq,VHOST_ACCESS_RO,(u64)(uintptr_t)vq->desc,+returniotlb_access_ok(vq,VHOST_MAP_RO,(u64)(uintptr_t)vq->desc,vhost_get_desc_size(vq,num),VHOST_ADDR_DESC)&&-iotlb_access_ok(vq,VHOST_ACCESS_RO,(u64)(uintptr_t)vq->avail,+iotlb_access_ok(vq,VHOST_MAP_RO,(u64)(uintptr_t)vq->avail,vhost_get_avail_size(vq,num),VHOST_ADDR_AVAIL)&&-iotlb_access_ok(vq,VHOST_ACCESS_WO,(u64)(uintptr_t)vq->used,+iotlb_access_ok(vq,VHOST_MAP_WO,(u64)(uintptr_t)vq->used,vhost_get_used_size(vq,num),VHOST_ADDR_USED);}EXPORT_SYMBOL_GPL(vq_meta_prefetch);
@@ -1745,7 +1664,7 @@ int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled)mutex_unlock(&vq->mutex);}-vhost_umem_clean(oiotlb);+vhost_iotlb_free(oiotlb);return0;}
@@ -1875,8 +1794,8 @@ static int log_write(void __user *log_base,staticintlog_write_hva(structvhost_virtqueue*vq,u64hva,u64len){-structvhost_umem*umem=vq->umem;-structvhost_umem_node*u;+structvhost_iotlb*umem=vq->umem;+structvhost_iotlb_map*u;u64start,end,l,min;intr;boolhit=false;
@@ -1886,16 +1805,15 @@ static int log_write_hva(struct vhost_virtqueue *vq, u64 hva, u64 len)/* More than one GPAs can be mapped into a single HVA. So*iterateallpossibleumemsheretobesafe.*/-list_for_each_entry(u,&umem->umem_list,link){-if(u->userspace_addr>hva-1+len||-u->userspace_addr-1+u->size<hva)+list_for_each_entry(u,&umem->list,link){+if(u->addr>hva-1+len||+u->addr-1+u->size<hva)continue;-start=max(u->userspace_addr,hva);-end=min(u->userspace_addr-1+u->size,-hva-1+len);+start=max(u->addr,hva);+end=min(u->addr-1+u->size,hva-1+len);l=end-start+1;r=log_write(vq->log_base,-u->start+start-u->userspace_addr,+u->start+start-u->addr,l);if(r<0)returnr;
From: Jason Wang <hidden> Date: 2020-02-10 03:59:50
This patch implements the third memory accessor for vringh besides
current kernel and userspace accessors. This idea is to allow vringh
to do the address translation through an IOTLB which is implemented
via vhost_map interval tree. Users should setup and IOVA to PA mapping
in this IOTLB.
This allows us to:
- Using vringh to access virtqueues with vIOMMU
- Using vringh to implement software vDPA devices
Signed-off-by: Jason Wang <redacted>
---
drivers/vhost/Kconfig.vringh | 1 +
drivers/vhost/vringh.c | 421 +++++++++++++++++++++++++++++++++--
include/linux/vringh.h | 36 +++
3 files changed, 435 insertions(+), 23 deletions(-)
@@ -96,6 +101,7 @@ static inline ssize_t vringh_iov_xfer(struct vringh_kiov *iov,/* Fix up old iov element then increment. */iov->iov[iov->i].iov_len=iov->consumed;iov->iov[iov->i].iov_base-=iov->consumed;+iov->consumed=0;iov->i++;
@@ -1042,4 +1059,362 @@ int vringh_need_notify_kern(struct vringh *vrh)}EXPORT_SYMBOL(vringh_need_notify_kern);+staticintiotlb_translate(conststructvringh*vrh,+u64addr,u64len,structbio_veciov[],+intiov_size,u32perm)+{+structvhost_iotlb_map*map;+structvhost_iotlb*iotlb=vrh->iotlb;+intret=0;+u64s=0;++while(len>s){+u64size,pa,pfn;++if(unlikely(ret>=iov_size)){+ret=-ENOBUFS;+break;+}++map=vhost_iotlb_itree_first(iotlb,addr,+addr+len-1);+if(!map||map->start>addr){+ret=-EINVAL;+break;+}elseif(!(map->perm&perm)){+ret=-EPERM;+break;+}++size=map->size-addr+map->start;+pa=map->addr+addr-map->start;+pfn=pa>>PAGE_SHIFT;+iov[ret].bv_page=pfn_to_page(pfn);+iov[ret].bv_len=min(len-s,size);+iov[ret].bv_offset=pa&(PAGE_SIZE-1);+s+=size;+addr+=size;+++ret;+}++returnret;+}++staticinlineintcopy_from_iotlb(conststructvringh*vrh,void*dst,+void*src,size_tlen)+{+structiov_iteriter;+structbio_veciov[16];+intret;++ret=iotlb_translate(vrh,(u64)(uintptr_t)src,+len,iov,16,VHOST_MAP_RO);+if(ret<0)+returnret;++iov_iter_bvec(&iter,READ,iov,ret,len);++ret=copy_from_iter(dst,len,&iter);++returnret;+}++staticinlineintcopy_to_iotlb(conststructvringh*vrh,void*dst,+void*src,size_tlen)+{+structiov_iteriter;+structbio_veciov[16];+intret;++ret=iotlb_translate(vrh,(u64)(uintptr_t)dst,+len,iov,16,VHOST_MAP_WO);+if(ret<0)+returnret;++iov_iter_bvec(&iter,WRITE,iov,ret,len);++returncopy_to_iter(src,len,&iter);+}++staticinlineintgetu16_iotlb(conststructvringh*vrh,+u16*val,const__virtio16*p)+{+structbio_veciov;+void*kaddr,*from;+intret;++/* Atomic read is needed for getu16 */+ret=iotlb_translate(vrh,(u64)(uintptr_t)p,sizeof(*p),+&iov,1,VHOST_MAP_RO);+if(ret<0)+returnret;++kaddr=kmap_atomic(iov.bv_page);+from=kaddr+iov.bv_offset;+*val=vringh16_to_cpu(vrh,READ_ONCE(*(__virtio16*)from));+kunmap_atomic(kaddr);++return0;+}++staticinlineintputu16_iotlb(conststructvringh*vrh,+__virtio16*p,u16val)+{+structbio_veciov;+void*kaddr,*to;+intret;++/* Atomic write is needed for putu16 */+ret=iotlb_translate(vrh,(u64)(uintptr_t)p,sizeof(*p),+&iov,1,VHOST_MAP_WO);+if(ret<0)+returnret;++kaddr=kmap_atomic(iov.bv_page);+to=kaddr+iov.bv_offset;+WRITE_ONCE(*(__virtio16*)to,cpu_to_vringh16(vrh,val));+kunmap_atomic(kaddr);++return0;+}++staticinlineintcopydesc_iotlb(conststructvringh*vrh,+void*dst,constvoid*src,size_tlen)+{+intret;++ret=copy_from_iotlb(vrh,dst,(void*)src,len);+if(ret!=len)+return-EFAULT;++return0;+}++staticinlineintxfer_from_iotlb(conststructvringh*vrh,void*src,+void*dst,size_tlen)+{+intret;++ret=copy_from_iotlb(vrh,dst,src,len);+if(ret!=len)+return-EFAULT;++return0;+}++staticinlineintxfer_to_iotlb(conststructvringh*vrh,+void*dst,void*src,size_tlen)+{+intret;++ret=copy_to_iotlb(vrh,dst,src,len);+if(ret!=len)+return-EFAULT;++return0;+}++staticinlineintputused_iotlb(conststructvringh*vrh,+structvring_used_elem*dst,+conststructvring_used_elem*src,+unsignedintnum)+{+intsize=num*sizeof(*dst);+intret;++ret=copy_to_iotlb(vrh,dst,(void*)src,num*sizeof(*dst));+if(ret!=size)+return-EFAULT;++return0;+}++/**+*vringh_init_iotlb-initializeavringhforaringwithIOTLB.+*@vrh:thevringhtoinitialize.+*@features:thefeaturebitsforthisring.+*@num:thenumberofelements.+*@weak_barriers:trueifweonlyneedmemorybarriers,notI/O.+*@desc:theuserpacedescriptorpointer.+*@avail:theuserpaceavailpointer.+*@used:theuserpaceusedpointer.+*+*Returnsanerrorifnumisinvalid.+*/+intvringh_init_iotlb(structvringh*vrh,u64features,+unsignedintnum,boolweak_barriers,+structvring_desc*desc,+structvring_avail*avail,+structvring_used*used)+{+returnvringh_init_kern(vrh,features,num,weak_barriers,+desc,avail,used);+}+EXPORT_SYMBOL(vringh_init_iotlb);++/**+*vringh_set_iotlb-initializeavringhforaringwithIOTLB.+*@vrh:thevring+*@iotlb:iotlbassociatedwiththisvring+*/+voidvringh_set_iotlb(structvringh*vrh,structvhost_iotlb*iotlb)+{+vrh->iotlb=iotlb;+}+EXPORT_SYMBOL(vringh_set_iotlb);++/**+*vringh_getdesc_iotlb-getnextavailabledescriptorfromringwith+*IOTLB.+*@vrh:thekernelspacevring.+*@riov:wheretoputthereadabledescriptors(orNULL)+*@wiov:wheretoputthewritabledescriptors(orNULL)+*@head:headindexwereceived,forpassingtovringh_complete_iotlb().+*@gfp:flagsforallocatinglargerriov/wiov.+*+*Returns0iftherewasnodescriptor,1iftherewas,or-errno.+*+*Notethatonerrorreturn,youcantellthedifferencebetweenan+*invalidringandasingleinvaliddescriptor:intheformercase,+**headwillbevrh->vring.num.Youmaybeabletoignoreaninvalid+*descriptor,butthere'snotmuchyoucandowithaninvalidring.+*+*Notethatyoumayneedtocleanupriovandwiov,evenonerror!+*/+intvringh_getdesc_iotlb(structvringh*vrh,+structvringh_kiov*riov,+structvringh_kiov*wiov,+u16*head,+gfp_tgfp)+{+interr;++err=__vringh_get_head(vrh,getu16_iotlb,&vrh->last_avail_idx);+if(err<0)+returnerr;++/* Empty... */+if(err==vrh->vring.num)+return0;++*head=err;+err=__vringh_iov(vrh,*head,riov,wiov,no_range_check,NULL,+gfp,copydesc_iotlb);+if(err)+returnerr;++return1;+}+EXPORT_SYMBOL(vringh_getdesc_iotlb);++/**+*vringh_iov_pull_iotlb-copybytesfromvring_iov.+*@vrh:thevring.+*@riov:theriovaspassedtovringh_getdesc_iotlb()(updatedasweconsume)+*@dst:theplacetocopy.+*@len:themaximumlengthtocopy.+*+*Returnsthebytescopied<=lenoranegativeerrno.+*/+ssize_tvringh_iov_pull_iotlb(structvringh*vrh,+structvringh_kiov*riov,+void*dst,size_tlen)+{+returnvringh_iov_xfer(vrh,riov,dst,len,xfer_from_iotlb);+}+EXPORT_SYMBOL(vringh_iov_pull_iotlb);++/**+*vringh_iov_push_iotlb-copybytesintovring_iov.+*@vrh:thevring.+*@wiov:thewiovaspassedtovringh_getdesc_iotlb()(updatedasweconsume)+*@dst:theplacetocopy.+*@len:themaximumlengthtocopy.+*+*Returnsthebytescopied<=lenoranegativeerrno.+*/+ssize_tvringh_iov_push_iotlb(structvringh*vrh,+structvringh_kiov*wiov,+constvoid*src,size_tlen)+{+returnvringh_iov_xfer(vrh,wiov,(void*)src,len,xfer_to_iotlb);+}+EXPORT_SYMBOL(vringh_iov_push_iotlb);++/**+*vringh_abandon_iotlb-we'vedecidednottohandlethedescriptor(s).+*@vrh:thevring.+*@num:thenumberofdescriptorstoputback(ie.num+*vringh_get_iotlb()toundo).+*+*Thenextvringh_get_iotlb()willreturntheolddescriptor(s)again.+*/+voidvringh_abandon_iotlb(structvringh*vrh,unsignedintnum)+{+/* We only update vring_avail_event(vr) when we want to be notified,+*sowehaven'tchangedthatyet.+*/+vrh->last_avail_idx-=num;+}+EXPORT_SYMBOL(vringh_abandon_iotlb);++/**+*vringh_complete_iotlb-we'vefinishedwithdescriptor,publishit.+*@vrh:thevring.+*@head:theheadasfilledinbyvringh_getdesc_iotlb.+*@len:thelengthofdatawehavewritten.+*+*Youshouldcheckvringh_need_notify_iotlb()afteroneormorecalls+*tothisfunction.+*/+intvringh_complete_iotlb(structvringh*vrh,u16head,u32len)+{+structvring_used_elemused;++used.id=cpu_to_vringh32(vrh,head);+used.len=cpu_to_vringh32(vrh,len);++return__vringh_complete(vrh,&used,1,putu16_iotlb,putused_iotlb);+}+EXPORT_SYMBOL(vringh_complete_iotlb);++/**+*vringh_notify_enable_iotlb-wewanttoknowifsomethingchanges.+*@vrh:thevring.+*+*Thisalwaysenablesnotifications,butreturnsfalseifthereare+*nowmorebuffersavailableinthevring.+*/+boolvringh_notify_enable_iotlb(structvringh*vrh)+{+return__vringh_notify_enable(vrh,getu16_iotlb,putu16_iotlb);+}+EXPORT_SYMBOL(vringh_notify_enable_iotlb);++/**+*vringh_notify_disable_iotlb-don'ttellusifsomethingchanges.+*@vrh:thevring.+*+*Thisisournormalrunningstate:wedisableandthenonlyenablewhen+*we'regoingtosleep.+*/+voidvringh_notify_disable_iotlb(structvringh*vrh)+{+__vringh_notify_disable(vrh,putu16_iotlb);+}+EXPORT_SYMBOL(vringh_notify_disable_iotlb);++/**+*vringh_need_notify_iotlb-mustwetelltheothersideaboutusedbuffers?+*@vrh:thevringwe'vecalledvringh_complete_iotlb()on.+*+*Returns-errnoor0ifwedon'tneedtotelltheotherside,1ifwedo.+*/+intvringh_need_notify_iotlb(structvringh*vrh)+{+return__vringh_need_notify(vrh,getu16_iotlb);+}+EXPORT_SYMBOL(vringh_need_notify_iotlb);++MODULE_LICENSE("GPL");
@@ -14,6 +14,8 @@#include<linux/virtio_byteorder.h>#include<linux/uio.h>#include<linux/slab.h>+#include<linux/dma-direction.h>+#include<linux/vhost_iotlb.h>#include<asm/barrier.h>/* virtio_ring with information needed for host access. */
@@ -39,6 +41,9 @@ struct vringh {/* The vring (note: it may contain user pointers!) */structvringvring;+/* IOTLB for this vring */+structvhost_iotlb*iotlb;+/* The function to call to notify the guest about added buffers */void(*notify)(structvringh*);};
From: Jason Wang <hidden> Date: 2020-02-10 04:00:48
vDPA device is a device that uses a datapath which complies with the
virtio specifications with vendor specific control path. vDPA devices
can be both physically located on the hardware or emulated by
software. vDPA hardware devices are usually implemented through PCIE
with the following types:
- PF (Physical Function) - A single Physical Function
- VF (Virtual Function) - Device that supports single root I/O
virtualization (SR-IOV). Its Virtual Function (VF) represents a
virtualized instance of the device that can be assigned to different
partitions
- ADI (Assignable Device Interface) and its equivalents - With
technologies such as Intel Scalable IOV, a virtual device (VDEV)
composed by host OS utilizing one or more ADIs. Or its equivalent
like SF (Sub function) from Mellanox.
From a driver's perspective, depends on how and where the DMA
translation is done, vDPA devices are split into two types:
- Platform specific DMA translation - From the driver's perspective,
the device can be used on a platform where device access to data in
memory is limited and/or translated. An example is a PCIE vDPA whose
DMA request was tagged via a bus (e.g PCIE) specific way. DMA
translation and protection are done at PCIE bus IOMMU level.
- Device specific DMA translation - The device implements DMA
isolation and protection through its own logic. An example is a vDPA
device which uses on-chip IOMMU.
To hide the differences and complexity of the above types for a vDPA
device/IOMMU options and in order to present a generic virtio device
to the upper layer, a device agnostic framework is required.
This patch introduces a software vDPA bus which abstracts the
common attributes of vDPA device, vDPA bus driver and the
communication method (vdpa_config_ops) between the vDPA device
abstraction and the vDPA bus driver:
With the abstraction of vDPA bus and vDPA bus operations, the
difference and complexity of the under layer hardware is hidden from
upper layer. The vDPA bus drivers on top can use a unified
vdpa_config_ops to control different types of vDPA device.
Signed-off-by: Jason Wang <redacted>
---
MAINTAINERS | 1 +
drivers/virtio/Kconfig | 2 +
drivers/virtio/Makefile | 1 +
drivers/virtio/vdpa/Kconfig | 9 ++
drivers/virtio/vdpa/Makefile | 2 +
drivers/virtio/vdpa/vdpa.c | 160 ++++++++++++++++++++++++
include/linux/vdpa.h | 233 +++++++++++++++++++++++++++++++++++
7 files changed, 408 insertions(+)
create mode 100644 drivers/virtio/vdpa/Kconfig
create mode 100644 drivers/virtio/vdpa/Makefile
create mode 100644 drivers/virtio/vdpa/vdpa.c
create mode 100644 include/linux/vdpa.h
From: Jason Wang <hidden> Date: 2020-02-10 04:01:54
This patch introduces a vDPA transport for virtio. This is used to
use kernel virtio driver to drive the mediated device that is capable
of populating virtqueue directly.
A new virtio-vdpa driver will be registered to the vDPA bus, when a
new virtio-vdpa device is probed, it will register the device with
vdpa based config ops. This means it is a software transport between
vDPA driver and vDPA device. The transport was implemented through
bus_ops of vDPA parent.
Signed-off-by: Jason Wang <redacted>
---
drivers/virtio/Kconfig | 13 ++
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio_vdpa.c | 392 +++++++++++++++++++++++++++++++++++
3 files changed, 406 insertions(+)
create mode 100644 drivers/virtio/virtio_vdpa.c
@@ -0,0 +1,392 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*VIRTIObaseddriverforvDPAdevice+*+*Copyright(c)2020,RedHat.Allrightsreserved.+*Author:JasonWang<jasowang@redhat.com>+*+*/++#include<linux/init.h>+#include<linux/module.h>+#include<linux/device.h>+#include<linux/kernel.h>+#include<linux/slab.h>+#include<linux/uuid.h>+#include<linux/virtio.h>+#include<linux/vdpa.h>+#include<linux/virtio_config.h>+#include<linux/virtio_ring.h>++#define MOD_VERSION "0.1"+#define MOD_AUTHOR "Jason Wang <jasowang@redhat.com>"+#define MOD_DESC "vDPA bus driver for virtio devices"+#define MOD_LICENSE "GPL v2"++structvirtio_vdpa_device{+structvirtio_devicevdev;+structvdpa_device*vdpa;+u64features;++/* The lock to protect virtqueue list */+spinlock_tlock;+/* List of virtio_vdpa_vq_info */+structlist_headvirtqueues;+};++structvirtio_vdpa_vq_info{+/* the actual virtqueue */+structvirtqueue*vq;++/* the list node for the virtqueues list */+structlist_headnode;+};++staticinlinestructvirtio_vdpa_device*+to_virtio_vdpa_device(structvirtio_device*dev)+{+returncontainer_of(dev,structvirtio_vdpa_device,vdev);+}++staticstructvdpa_device*vd_get_vdpa(structvirtio_device*vdev)+{+returnto_virtio_vdpa_device(vdev)->vdpa;+}++staticvoidvirtio_vdpa_get(structvirtio_device*vdev,unsignedoffset,+void*buf,unsignedlen)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++ops->get_config(vdpa,offset,buf,len);+}++staticvoidvirtio_vdpa_set(structvirtio_device*vdev,unsignedoffset,+constvoid*buf,unsignedlen)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++ops->set_config(vdpa,offset,buf,len);+}++staticu32virtio_vdpa_generation(structvirtio_device*vdev)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++if(ops->get_generation)+returnops->get_generation(vdpa);++return0;+}++staticu8virtio_vdpa_get_status(structvirtio_device*vdev)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++returnops->get_status(vdpa);+}++staticvoidvirtio_vdpa_set_status(structvirtio_device*vdev,u8status)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++returnops->set_status(vdpa,status);+}++staticvoidvirtio_vdpa_reset(structvirtio_device*vdev)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++returnops->set_status(vdpa,0);+}++staticboolvirtio_vdpa_notify(structvirtqueue*vq)+{+structvdpa_device*vdpa=vd_get_vdpa(vq->vdev);+conststructvdpa_config_ops*ops=vdpa->config;++ops->kick_vq(vdpa,vq->index);++returntrue;+}++staticirqreturn_tvirtio_vdpa_config_cb(void*private)+{+structvirtio_vdpa_device*vd_dev=private;++virtio_config_changed(&vd_dev->vdev);++returnIRQ_HANDLED;+}++staticirqreturn_tvirtio_vdpa_virtqueue_cb(void*private)+{+structvirtio_vdpa_vq_info*info=private;++returnvring_interrupt(0,info->vq);+}++staticstructvirtqueue*+virtio_vdpa_setup_vq(structvirtio_device*vdev,unsignedintindex,+void(*callback)(structvirtqueue*vq),+constchar*name,boolctx)+{+structvirtio_vdpa_device*vd_dev=to_virtio_vdpa_device(vdev);+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;+structvirtio_vdpa_vq_info*info;+structvdpa_callbackcb;+structvirtqueue*vq;+u64desc_addr,driver_addr,device_addr;+unsignedlongflags;+u32align,num;+interr;++if(!name)+returnNULL;++/* Queue shouldn't already be set up. */+if(ops->get_vq_ready(vdpa,index))+returnERR_PTR(-ENOENT);++/* Allocate and fill out our active queue description */+info=kmalloc(sizeof(*info),GFP_KERNEL);+if(!info)+returnERR_PTR(-ENOMEM);++num=ops->get_vq_num_max(vdpa);+if(num==0){+err=-ENOENT;+gotoerror_new_virtqueue;+}++/* Create the vring */+align=ops->get_vq_align(vdpa);+vq=vring_create_virtqueue(index,num,align,vdev,+true,true,ctx,+virtio_vdpa_notify,callback,name);+if(!vq){+err=-ENOMEM;+gotoerror_new_virtqueue;+}++/* Setup virtqueue callback */+cb.callback=virtio_vdpa_virtqueue_cb;+cb.private=info;+ops->set_vq_cb(vdpa,index,&cb);+ops->set_vq_num(vdpa,index,virtqueue_get_vring_size(vq));++desc_addr=virtqueue_get_desc_addr(vq);+driver_addr=virtqueue_get_avail_addr(vq);+device_addr=virtqueue_get_used_addr(vq);++if(ops->set_vq_address(vdpa,index,+desc_addr,driver_addr,+device_addr)){+err=-EINVAL;+gotoerr_vq;+}++ops->set_vq_ready(vdpa,index,1);++vq->priv=info;+info->vq=vq;++spin_lock_irqsave(&vd_dev->lock,flags);+list_add(&info->node,&vd_dev->virtqueues);+spin_unlock_irqrestore(&vd_dev->lock,flags);++returnvq;++err_vq:+vring_del_virtqueue(vq);+error_new_virtqueue:+ops->set_vq_ready(vdpa,index,0);+/* VDPA driver should make sure vq is stopeed here */+WARN_ON(ops->get_vq_ready(vdpa,index));+kfree(info);+returnERR_PTR(err);+}++staticvoidvirtio_vdpa_del_vq(structvirtqueue*vq)+{+structvirtio_vdpa_device*vd_dev=to_virtio_vdpa_device(vq->vdev);+structvdpa_device*vdpa=vd_dev->vdpa;+conststructvdpa_config_ops*ops=vdpa->config;+structvirtio_vdpa_vq_info*info=vq->priv;+unsignedintindex=vq->index;+unsignedlongflags;++spin_lock_irqsave(&vd_dev->lock,flags);+list_del(&info->node);+spin_unlock_irqrestore(&vd_dev->lock,flags);++/* Select and deactivate the queue */+ops->set_vq_ready(vdpa,index,0);+WARN_ON(ops->get_vq_ready(vdpa,index));++vring_del_virtqueue(vq);++kfree(info);+}++staticvoidvirtio_vdpa_del_vqs(structvirtio_device*vdev)+{+structvirtqueue*vq,*n;++list_for_each_entry_safe(vq,n,&vdev->vqs,list)+virtio_vdpa_del_vq(vq);+}++staticintvirtio_vdpa_find_vqs(structvirtio_device*vdev,unsignednvqs,+structvirtqueue*vqs[],+vq_callback_t*callbacks[],+constchar*constnames[],+constbool*ctx,+structirq_affinity*desc)+{+structvirtio_vdpa_device*vd_dev=to_virtio_vdpa_device(vdev);+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;+structvdpa_callbackcb;+inti,err,queue_idx=0;++for(i=0;i<nvqs;++i){+if(!names[i]){+vqs[i]=NULL;+continue;+}++vqs[i]=virtio_vdpa_setup_vq(vdev,queue_idx++,+callbacks[i],names[i],ctx?+ctx[i]:false);+if(IS_ERR(vqs[i])){+err=PTR_ERR(vqs[i]);+gotoerr_setup_vq;+}+}++cb.callback=virtio_vdpa_config_cb;+cb.private=vd_dev;+ops->set_config_cb(vdpa,&cb);++return0;++err_setup_vq:+virtio_vdpa_del_vqs(vdev);+returnerr;+}++staticu64virtio_vdpa_get_features(structvirtio_device*vdev)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++returnops->get_features(vdpa);+}++staticintvirtio_vdpa_finalize_features(structvirtio_device*vdev)+{+structvdpa_device*vdpa=vd_get_vdpa(vdev);+conststructvdpa_config_ops*ops=vdpa->config;++/* Give virtio_ring a chance to accept features. */+vring_transport_features(vdev);++returnops->set_features(vdpa,vdev->features);+}++staticconstchar*virtio_vdpa_bus_name(structvirtio_device*vdev)+{+structvirtio_vdpa_device*vd_dev=to_virtio_vdpa_device(vdev);+structvdpa_device*vdpa=vd_dev->vdpa;++returndev_name(&vdpa->dev);+}++staticconststructvirtio_config_opsvirtio_vdpa_config_ops={+.get=virtio_vdpa_get,+.set=virtio_vdpa_set,+.generation=virtio_vdpa_generation,+.get_status=virtio_vdpa_get_status,+.set_status=virtio_vdpa_set_status,+.reset=virtio_vdpa_reset,+.find_vqs=virtio_vdpa_find_vqs,+.del_vqs=virtio_vdpa_del_vqs,+.get_features=virtio_vdpa_get_features,+.finalize_features=virtio_vdpa_finalize_features,+.bus_name=virtio_vdpa_bus_name,+};++staticvoidvirtio_vdpa_release_dev(structdevice*_d)+{+structvirtio_device*vdev=+container_of(_d,structvirtio_device,dev);+structvirtio_vdpa_device*vd_dev=+container_of(vdev,structvirtio_vdpa_device,vdev);++kfree(vd_dev);+}++staticintvirtio_vdpa_probe(structvdpa_device*vdpa)+{+conststructvdpa_config_ops*ops=vdpa->config;+structvirtio_vdpa_device*vd_dev;+intret=-EINVAL;++vd_dev=kzalloc(sizeof(*vd_dev),GFP_KERNEL);+if(!vd_dev)+return-ENOMEM;++vd_dev->vdev.dev.parent=vdpa_get_dma_dev(vdpa);+vd_dev->vdev.dev.release=virtio_vdpa_release_dev;+vd_dev->vdev.config=&virtio_vdpa_config_ops;+vd_dev->vdpa=vdpa;+INIT_LIST_HEAD(&vd_dev->virtqueues);+spin_lock_init(&vd_dev->lock);++vd_dev->vdev.id.device=ops->get_device_id(vdpa);+if(vd_dev->vdev.id.device==0)+gotoerr;++vd_dev->vdev.id.vendor=ops->get_vendor_id(vdpa);+ret=register_virtio_device(&vd_dev->vdev);+if(ret)+gotoerr;++vdpa_set_drvdata(vdpa,vd_dev);++return0;++err:+kfree(vd_dev);+returnret;+}++staticvoidvirtio_vdpa_remove(structvdpa_device*vdpa)+{+structvirtio_vdpa_device*vd_dev=vdpa_get_drvdata(vdpa);++unregister_virtio_device(&vd_dev->vdev);+}++staticstructvdpa_drivervirtio_vdpa_driver={+.driver={+.name="virtio_vdpa",+},+.probe=virtio_vdpa_probe,+.remove=virtio_vdpa_remove,+};++module_vdpa_driver(virtio_vdpa_driver);++MODULE_VERSION(MOD_VERSION);+MODULE_LICENSE(MOD_LICENSE);+MODULE_AUTHOR(MOD_AUTHOR);+MODULE_DESCRIPTION(MOD_DESC);
From: Jason Wang <hidden> Date: 2020-02-10 04:02:45
This patch implements a software vDPA networking device. The datapath
is implemented through vringh and workqueue. The device has an on-chip
IOMMU which translates IOVA to PA. For kernel virtio drivers, vDPA
simulator driver provides dma_ops. For vhost driers, set_map() methods
of vdpa_config_ops is implemented to accept mappings from vhost.
Currently, vDPA device simulator will loopback TX traffic to RX. So
the main use case for the device is vDPA feature testing, prototyping
and development.
Note, there's no management API implemented, a vDPA device will be
registered once the module is probed. We need to handle this in the
future development.
Signed-off-by: Jason Wang <redacted>
---
drivers/virtio/vdpa/Kconfig | 17 +
drivers/virtio/vdpa/Makefile | 1 +
drivers/virtio/vdpa/vdpa_sim.c | 678 +++++++++++++++++++++++++++++++++
3 files changed, 696 insertions(+)
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
@@ -0,0 +1,678 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*VDPAnetworkingdevicesimulator.+*+*Copyright(c)2020,RedHatInc.Allrightsreserved.+*Author:JasonWang<jasowang@redhat.com>+*+*/++#include<linux/init.h>+#include<linux/module.h>+#include<linux/device.h>+#include<linux/kernel.h>+#include<linux/fs.h>+#include<linux/poll.h>+#include<linux/slab.h>+#include<linux/sched.h>+#include<linux/wait.h>+#include<linux/uuid.h>+#include<linux/iommu.h>+#include<linux/sysfs.h>+#include<linux/file.h>+#include<linux/etherdevice.h>+#include<linux/vringh.h>+#include<linux/vdpa.h>+#include<linux/vhost_iotlb.h>+#include<uapi/linux/virtio_config.h>+#include<uapi/linux/virtio_net.h>++#define DRV_VERSION "0.1"+#define DRV_AUTHOR "Jason Wang <jasowang@redhat.com>"+#define DRV_DESC "vDPA Device Simulator"+#define DRV_LICENSE "GPL v2"++structvdpasim_dev{+structdevicedev;+};++structvdpasim_dev*vdpasim_dev;++structvdpasim_virtqueue{+structvringhvring;+structvringh_kioviov;+unsignedshorthead;+boolready;+u64desc_addr;+u64device_addr;+u64driver_addr;+u32num;+void*private;+irqreturn_t(*cb)(void*data);+};++#define VDPASIM_QUEUE_ALIGN PAGE_SIZE+#define VDPASIM_QUEUE_MAX 256+#define VDPASIM_DEVICE_ID 0x1+#define VDPASIM_VENDOR_ID 0+#define VDPASIM_VQ_NUM 0x2+#define VDPASIM_NAME "vdpasim-netdev"++staticu64vdpasim_features=(1ULL<<VIRTIO_F_ANY_LAYOUT)|+(1ULL<<VIRTIO_F_VERSION_1)|+(1ULL<<VIRTIO_F_IOMMU_PLATFORM);++/* State of each vdpasim device */+structvdpasim{+structvdpasim_virtqueuevqs[2];+structwork_structwork;+/* spinlock to synchronize virtqueue state */+spinlock_tlock;+structvdpa_devicevdpa;+structvirtio_net_configconfig;+structvhost_iotlb*iommu;+void*buffer;+u32status;+u32generation;+u64features;+};++structvdpasim*vdpa_sim;++staticstructvdpasim*vdpa_to_sim(structvdpa_device*vdpa)+{+returncontainer_of(vdpa,structvdpasim,vdpa);+}++staticvoidvdpasim_queue_ready(structvdpasim*vdpasim,unsignedintidx)+{+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+intret;++ret=vringh_init_iotlb(&vq->vring,vdpasim_features,+VDPASIM_QUEUE_MAX,false,+(structvring_desc*)(uintptr_t)vq->desc_addr,+(structvring_avail*)+(uintptr_t)vq->driver_addr,+(structvring_used*)+(uintptr_t)vq->device_addr);+}++staticvoidvdpasim_vq_reset(structvdpasim_virtqueue*vq)+{+vq->ready=0;+vq->desc_addr=0;+vq->driver_addr=0;+vq->device_addr=0;+vq->cb=NULL;+vq->private=NULL;+vringh_init_iotlb(&vq->vring,vdpasim_features,VDPASIM_QUEUE_MAX,+false,0,0,0);+}++staticvoidvdpasim_reset(structvdpasim*vdpasim)+{+inti;++for(i=0;i<VDPASIM_VQ_NUM;i++)+vdpasim_vq_reset(&vdpasim->vqs[i]);++vhost_iotlb_reset(vdpasim->iommu);++vdpasim->features=0;+vdpasim->status=0;+++vdpasim->generation;+}++staticvoidvdpasim_work(structwork_struct*work)+{+structvdpasim*vdpasim=container_of(work,struct+vdpasim,work);+structvdpasim_virtqueue*txq=&vdpasim->vqs[1];+structvdpasim_virtqueue*rxq=&vdpasim->vqs[0];+size_tread,write,total_write;+interr;+intpkts=0;++spin_lock(&vdpasim->lock);++if(!(vdpasim->status&VIRTIO_CONFIG_S_DRIVER_OK))+gotoout;++if(!txq->ready||!rxq->ready)+gotoout;++while(true){+total_write=0;+err=vringh_getdesc_iotlb(&txq->vring,&txq->iov,NULL,+&txq->head,GFP_ATOMIC);+if(err<=0)+break;++err=vringh_getdesc_iotlb(&rxq->vring,NULL,&rxq->iov,+&rxq->head,GFP_ATOMIC);+if(err<=0){+vringh_complete_iotlb(&txq->vring,txq->head,0);+break;+}++while(true){+read=vringh_iov_pull_iotlb(&txq->vring,&txq->iov,+vdpasim->buffer,+PAGE_SIZE);+if(read<=0)+break;++write=vringh_iov_push_iotlb(&rxq->vring,&rxq->iov,+vdpasim->buffer,read);+if(write<=0)+break;++total_write+=write;+}++/* Make sure data is wrote before advancing index */+smp_wmb();++vringh_complete_iotlb(&txq->vring,txq->head,0);+vringh_complete_iotlb(&rxq->vring,rxq->head,total_write);++/* Make sure used is visible before rasing the interrupt. */+smp_wmb();++local_bh_disable();+if(txq->cb)+txq->cb(txq->private);+if(rxq->cb)+rxq->cb(rxq->private);+local_bh_enable();++if(++pkts>4){+schedule_work(&vdpasim->work);+gotoout;+}+}++out:+spin_unlock(&vdpasim->lock);+}++staticintdir_to_perm(enumdma_data_directiondir)+{+intperm=-EFAULT;++switch(dir){+caseDMA_FROM_DEVICE:+perm=VHOST_MAP_WO;+break;+caseDMA_TO_DEVICE:+perm=VHOST_MAP_RO;+break;+caseDMA_BIDIRECTIONAL:+perm=VHOST_MAP_RW;+break;+default:+break;+}++returnperm;+}++staticdma_addr_tvdpasim_map_page(structdevice*dev,structpage*page,+unsignedlongoffset,size_tsize,+enumdma_data_directiondir,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;+u64pa=(page_to_pfn(page)<<PAGE_SHIFT)+offset;+intret,perm=dir_to_perm(dir);++if(perm<0)+returnDMA_MAPPING_ERROR;++/* For simplicity, use identical mapping to avoid e.g iova+*allocator.+*/+ret=vhost_iotlb_add_range(iommu,pa,pa+size-1,+pa,dir_to_perm(dir));+if(ret)+returnDMA_MAPPING_ERROR;++return(dma_addr_t)(pa);+}++staticvoidvdpasim_unmap_page(structdevice*dev,dma_addr_tdma_addr,+size_tsize,enumdma_data_directiondir,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;++vhost_iotlb_del_range(iommu,(u64)dma_addr,+(u64)dma_addr+size-1);+}++staticvoid*vdpasim_alloc_coherent(structdevice*dev,size_tsize,+dma_addr_t*dma_addr,gfp_tflag,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;+void*addr=kmalloc(size,flag);+intret;++if(!addr)+*dma_addr=DMA_MAPPING_ERROR;+else{+u64pa=virt_to_phys(addr);++ret=vhost_iotlb_add_range(iommu,(u64)pa,+(u64)pa+size-1,+pa,VHOST_MAP_RW);+if(ret){+*dma_addr=DMA_MAPPING_ERROR;+kfree(addr);+addr=NULL;+}else+*dma_addr=(dma_addr_t)pa;+}++returnaddr;+}++staticvoidvdpasim_free_coherent(structdevice*dev,size_tsize,+void*vaddr,dma_addr_tdma_addr,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;++vhost_iotlb_del_range(iommu,(u64)dma_addr,+(u64)dma_addr+size-1);+kfree(phys_to_virt((uintptr_t)dma_addr));+}++staticconststructdma_map_opsvdpasim_dma_ops={+.map_page=vdpasim_map_page,+.unmap_page=vdpasim_unmap_page,+.alloc=vdpasim_alloc_coherent,+.free=vdpasim_free_coherent,+};++staticvoidvdpasim_release_dev(structdevice*_d)+{+structvdpa_device*vdpa=dev_to_vdpa(_d);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++kfree(vdpasim->buffer);+kfree(vdpasim);+}++staticconststructvdpa_config_opsvdpasim_net_config_ops;++staticstructvdpasim*vdpasim_create(void)+{+structvdpasim*vdpasim;+structvirtio_net_config*config;+structvdpa_device*vdpa;+structdevice*dev;+intret=-ENOMEM;++vdpasim=kzalloc(sizeof(*vdpasim),GFP_KERNEL);+if(!vdpasim)+gotoerr_vdpa_alloc;++vdpasim->buffer=kmalloc(PAGE_SIZE,GFP_KERNEL);+if(!vdpasim->buffer)+gotoerr_buffer_alloc;++vdpasim->iommu=vhost_iotlb_alloc(2048,0);+if(!vdpasim->iommu)+gotoerr_iotlb;++config=&vdpasim->config;+config->mtu=1500;+config->status=VIRTIO_NET_S_LINK_UP;+eth_random_addr(config->mac);++INIT_WORK(&vdpasim->work,vdpasim_work);+spin_lock_init(&vdpasim->lock);++vdpa=&vdpasim->vdpa;+vdpa->dev.release=vdpasim_release_dev;++vringh_set_iotlb(&vdpasim->vqs[0].vring,vdpasim->iommu);+vringh_set_iotlb(&vdpasim->vqs[1].vring,vdpasim->iommu);++dev=&vdpa->dev;+dev->coherent_dma_mask=DMA_BIT_MASK(64);+set_dma_ops(dev,&vdpasim_dma_ops);++ret=vdpa_init_device(vdpa,&vdpasim_dev->dev,dev,+&vdpasim_net_config_ops);+if(ret)+gotoerr_init;++ret=vdpa_register_device(vdpa);+if(ret)+gotoerr_register;++returnvdpasim;++err_register:+put_device(&vdpa->dev);+err_init:+vhost_iotlb_free(vdpasim->iommu);+err_iotlb:+kfree(vdpasim->buffer);+err_buffer_alloc:+kfree(vdpasim);+err_vdpa_alloc:+returnERR_PTR(ret);+}++staticintvdpasim_set_vq_address(structvdpa_device*vdpa,u16idx,+u64desc_area,u64driver_area,+u64device_area)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->desc_addr=desc_area;+vq->driver_addr=driver_area;+vq->device_addr=device_area;++return0;+}++staticvoidvdpasim_set_vq_num(structvdpa_device*vdpa,u16idx,u32num)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->num=num;+}++staticvoidvdpasim_kick_vq(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++if(vq->ready)+schedule_work(&vdpasim->work);+}++staticvoidvdpasim_set_vq_cb(structvdpa_device*vdpa,u16idx,+structvdpa_callback*cb)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->cb=cb->callback;+vq->private=cb->private;+}++staticvoidvdpasim_set_vq_ready(structvdpa_device*vdpa,u16idx,boolready)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++spin_lock(&vdpasim->lock);+vq->ready=ready;+if(vq->ready)+vdpasim_queue_ready(vdpasim,idx);+spin_unlock(&vdpasim->lock);+}++staticboolvdpasim_get_vq_ready(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++returnvq->ready;+}++staticintvdpasim_set_vq_state(structvdpa_device*vdpa,u16idx,u64state)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+structvringh*vrh=&vq->vring;++spin_lock(&vdpasim->lock);+vrh->last_avail_idx=state;+spin_unlock(&vdpasim->lock);++return0;+}++staticu64vdpasim_get_vq_state(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+structvringh*vrh=&vq->vring;++returnvrh->last_avail_idx;+}++staticu16vdpasim_get_vq_align(structvdpa_device*vdpa)+{+returnVDPASIM_QUEUE_ALIGN;+}++staticu64vdpasim_get_features(structvdpa_device*vdpa)+{+returnvdpasim_features;+}++staticintvdpasim_set_features(structvdpa_device*vdpa,u64features)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++/* DMA mapping must be done by driver */+if(!(features&(1ULL<<VIRTIO_F_IOMMU_PLATFORM)))+return-EINVAL;++vdpasim->features=features&vdpasim_features;++return0;+}++staticvoidvdpasim_set_config_cb(structvdpa_device*vdpa,+structvdpa_callback*cb)+{+/* We don't support config interrupt */+}++staticu16vdpasim_get_vq_num_max(structvdpa_device*vdpa)+{+returnVDPASIM_QUEUE_MAX;+}++staticu32vdpasim_get_device_id(structvdpa_device*vdpa)+{+returnVDPASIM_DEVICE_ID;+}++staticu32vdpasim_get_vendor_id(structvdpa_device*vdpa)+{+returnVDPASIM_VENDOR_ID;+}++staticu8vdpasim_get_status(structvdpa_device*vdpa)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+u8status;++spin_lock(&vdpasim->lock);+status=vdpasim->status;+spin_unlock(&vdpasim->lock);++returnvdpasim->status;+}++staticvoidvdpasim_set_status(structvdpa_device*vdpa,u8status)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++spin_lock(&vdpasim->lock);+vdpasim->status=status;+if(status==0)+vdpasim_reset(vdpasim);+spin_unlock(&vdpasim->lock);+}++staticvoidvdpasim_get_config(structvdpa_device*vdpa,unsignedintoffset,+void*buf,unsignedintlen)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++if(offset+len<sizeof(structvirtio_net_config))+memcpy(buf,&vdpasim->config+offset,len);+}++staticvoidvdpasim_set_config(structvdpa_device*vdpa,unsignedintoffset,+constvoid*buf,unsignedintlen)+{+/* No writable config supportted by vdpasim */+}++staticu32vdpasim_get_generation(structvdpa_device*vdpa)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++returnvdpasim->generation;+}++staticintvdpasim_set_map(structvdpa_device*vdpa,+structvhost_iotlb*iotlb)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb_map*map;+u64start=0ULL,last=0ULL-1;+intret;++vhost_iotlb_reset(vdpasim->iommu);++for(map=vhost_iotlb_itree_first(iotlb,start,last);map;+map=vhost_iotlb_itree_next(map,start,last)){+ret=vhost_iotlb_add_range(vdpasim->iommu,map->start,+map->last,map->addr,map->perm);+if(ret)+gotoerr;+}+return0;++err:+vhost_iotlb_reset(vdpasim->iommu);+returnret;+}++staticintvdpasim_dma_map(structvdpa_device*vdpa,u64iova,u64size,+u64pa,u32perm)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++returnvhost_iotlb_add_range(vdpasim->iommu,iova,+iova+size-1,pa,perm);+}++staticintvdpasim_dma_unmap(structvdpa_device*vdpa,u64iova,u64size)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++vhost_iotlb_del_range(vdpasim->iommu,iova,iova+size-1);++return0;+}++staticconststructvdpa_config_opsvdpasim_net_config_ops={+.set_vq_address=vdpasim_set_vq_address,+.set_vq_num=vdpasim_set_vq_num,+.kick_vq=vdpasim_kick_vq,+.set_vq_cb=vdpasim_set_vq_cb,+.set_vq_ready=vdpasim_set_vq_ready,+.get_vq_ready=vdpasim_get_vq_ready,+.set_vq_state=vdpasim_set_vq_state,+.get_vq_state=vdpasim_get_vq_state,+.get_vq_align=vdpasim_get_vq_align,+.get_features=vdpasim_get_features,+.set_features=vdpasim_set_features,+.set_config_cb=vdpasim_set_config_cb,+.get_vq_num_max=vdpasim_get_vq_num_max,+.get_device_id=vdpasim_get_device_id,+.get_vendor_id=vdpasim_get_vendor_id,+.get_status=vdpasim_get_status,+.set_status=vdpasim_set_status,+.get_config=vdpasim_get_config,+.set_config=vdpasim_set_config,+.get_generation=vdpasim_get_generation,+.set_map=vdpasim_set_map,+.dma_map=vdpasim_dma_map,+.dma_unmap=vdpasim_dma_unmap,+};++staticvoidvdpasim_device_release(structdevice*dev)+{+structvdpasim_dev*vdpasim_dev=+container_of(dev,structvdpasim_dev,dev);++vdpasim_dev->dev.bus=NULL;+kfree(vdpasim_dev);+}++staticint__initvdpasim_dev_init(void)+{+structdevice*dev;+intret=0;++vdpasim_dev=kzalloc(sizeof(*vdpasim_dev),GFP_KERNEL);+if(!vdpasim_dev)+return-ENOMEM;++dev=&vdpasim_dev->dev;+dev->release=vdpasim_device_release;+dev_set_name(dev,"%s",VDPASIM_NAME);++ret=device_register(&vdpasim_dev->dev);+if(ret)+gotoerr_register;++if(!vdpasim_create())+gotoerr_register;++return0;++err_register:+kfree(vdpasim_dev);+vdpasim_dev=NULL;+returnret;+}++staticintvdpasim_device_remove_cb(structdevice*dev,void*data)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);++vdpa_unregister_device(vdpa);++return0;+}++staticvoid__exitvdpasim_dev_exit(void)+{+device_for_each_child(&vdpasim_dev->dev,NULL,+vdpasim_device_remove_cb);+device_unregister(&vdpasim_dev->dev);+}++module_init(vdpasim_dev_init)+module_exit(vdpasim_dev_exit)++MODULE_VERSION(DRV_VERSION);+MODULE_LICENSE(DRV_LICENSE);+MODULE_AUTHOR(DRV_AUTHOR);+MODULE_DESCRIPTION(DRV_DESC);
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-10 11:23:48
On Mon, Feb 10, 2020 at 11:56:08AM +0800, Jason Wang wrote:
quoted hunk
This patch implements a software vDPA networking device. The datapath
is implemented through vringh and workqueue. The device has an on-chip
IOMMU which translates IOVA to PA. For kernel virtio drivers, vDPA
simulator driver provides dma_ops. For vhost driers, set_map() methods
of vdpa_config_ops is implemented to accept mappings from vhost.
Currently, vDPA device simulator will loopback TX traffic to RX. So
the main use case for the device is vDPA feature testing, prototyping
and development.
Note, there's no management API implemented, a vDPA device will be
registered once the module is probed. We need to handle this in the
future development.
Signed-off-by: Jason Wang <redacted>
---
drivers/virtio/vdpa/Kconfig | 17 +
drivers/virtio/vdpa/Makefile | 1 +
drivers/virtio/vdpa/vdpa_sim.c | 678 +++++++++++++++++++++++++++++++++
3 files changed, 696 insertions(+)
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
@@ -0,0 +1,678 @@+// SPDX-License-Identifier: GPL-2.0-only+/*+*VDPAnetworkingdevicesimulator.+*+*Copyright(c)2020,RedHatInc.Allrightsreserved.+*Author:JasonWang<jasowang@redhat.com>+*+*/++#include<linux/init.h>+#include<linux/module.h>+#include<linux/device.h>+#include<linux/kernel.h>+#include<linux/fs.h>+#include<linux/poll.h>+#include<linux/slab.h>+#include<linux/sched.h>+#include<linux/wait.h>+#include<linux/uuid.h>+#include<linux/iommu.h>+#include<linux/sysfs.h>+#include<linux/file.h>+#include<linux/etherdevice.h>+#include<linux/vringh.h>+#include<linux/vdpa.h>+#include<linux/vhost_iotlb.h>+#include<uapi/linux/virtio_config.h>+#include<uapi/linux/virtio_net.h>++#define DRV_VERSION "0.1"+#define DRV_AUTHOR "Jason Wang <jasowang@redhat.com>"+#define DRV_DESC "vDPA Device Simulator"+#define DRV_LICENSE "GPL v2"++structvdpasim_dev{+structdevicedev;+};++structvdpasim_dev*vdpasim_dev;++structvdpasim_virtqueue{+structvringhvring;+structvringh_kioviov;+unsignedshorthead;+boolready;+u64desc_addr;+u64device_addr;+u64driver_addr;+u32num;+void*private;+irqreturn_t(*cb)(void*data);+};++#define VDPASIM_QUEUE_ALIGN PAGE_SIZE+#define VDPASIM_QUEUE_MAX 256+#define VDPASIM_DEVICE_ID 0x1+#define VDPASIM_VENDOR_ID 0+#define VDPASIM_VQ_NUM 0x2+#define VDPASIM_NAME "vdpasim-netdev"++staticu64vdpasim_features=(1ULL<<VIRTIO_F_ANY_LAYOUT)|+(1ULL<<VIRTIO_F_VERSION_1)|+(1ULL<<VIRTIO_F_IOMMU_PLATFORM);++/* State of each vdpasim device */+structvdpasim{+structvdpasim_virtqueuevqs[2];+structwork_structwork;+/* spinlock to synchronize virtqueue state */+spinlock_tlock;+structvdpa_devicevdpa;+structvirtio_net_configconfig;+structvhost_iotlb*iommu;+void*buffer;+u32status;+u32generation;+u64features;+};++structvdpasim*vdpa_sim;++staticstructvdpasim*vdpa_to_sim(structvdpa_device*vdpa)+{+returncontainer_of(vdpa,structvdpasim,vdpa);+}++staticvoidvdpasim_queue_ready(structvdpasim*vdpasim,unsignedintidx)+{+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+intret;++ret=vringh_init_iotlb(&vq->vring,vdpasim_features,+VDPASIM_QUEUE_MAX,false,+(structvring_desc*)(uintptr_t)vq->desc_addr,+(structvring_avail*)+(uintptr_t)vq->driver_addr,+(structvring_used*)+(uintptr_t)vq->device_addr);+}++staticvoidvdpasim_vq_reset(structvdpasim_virtqueue*vq)+{+vq->ready=0;+vq->desc_addr=0;+vq->driver_addr=0;+vq->device_addr=0;+vq->cb=NULL;+vq->private=NULL;+vringh_init_iotlb(&vq->vring,vdpasim_features,VDPASIM_QUEUE_MAX,+false,0,0,0);+}++staticvoidvdpasim_reset(structvdpasim*vdpasim)+{+inti;++for(i=0;i<VDPASIM_VQ_NUM;i++)+vdpasim_vq_reset(&vdpasim->vqs[i]);++vhost_iotlb_reset(vdpasim->iommu);++vdpasim->features=0;+vdpasim->status=0;+++vdpasim->generation;+}++staticvoidvdpasim_work(structwork_struct*work)+{+structvdpasim*vdpasim=container_of(work,struct+vdpasim,work);+structvdpasim_virtqueue*txq=&vdpasim->vqs[1];+structvdpasim_virtqueue*rxq=&vdpasim->vqs[0];+size_tread,write,total_write;+interr;+intpkts=0;++spin_lock(&vdpasim->lock);++if(!(vdpasim->status&VIRTIO_CONFIG_S_DRIVER_OK))+gotoout;++if(!txq->ready||!rxq->ready)+gotoout;++while(true){+total_write=0;+err=vringh_getdesc_iotlb(&txq->vring,&txq->iov,NULL,+&txq->head,GFP_ATOMIC);+if(err<=0)+break;++err=vringh_getdesc_iotlb(&rxq->vring,NULL,&rxq->iov,+&rxq->head,GFP_ATOMIC);+if(err<=0){+vringh_complete_iotlb(&txq->vring,txq->head,0);+break;+}++while(true){+read=vringh_iov_pull_iotlb(&txq->vring,&txq->iov,+vdpasim->buffer,+PAGE_SIZE);+if(read<=0)+break;++write=vringh_iov_push_iotlb(&rxq->vring,&rxq->iov,+vdpasim->buffer,read);+if(write<=0)+break;++total_write+=write;+}++/* Make sure data is wrote before advancing index */+smp_wmb();++vringh_complete_iotlb(&txq->vring,txq->head,0);+vringh_complete_iotlb(&rxq->vring,rxq->head,total_write);++/* Make sure used is visible before rasing the interrupt. */+smp_wmb();++local_bh_disable();+if(txq->cb)+txq->cb(txq->private);+if(rxq->cb)+rxq->cb(rxq->private);+local_bh_enable();++if(++pkts>4){+schedule_work(&vdpasim->work);+gotoout;+}+}++out:+spin_unlock(&vdpasim->lock);+}++staticintdir_to_perm(enumdma_data_directiondir)+{+intperm=-EFAULT;++switch(dir){+caseDMA_FROM_DEVICE:+perm=VHOST_MAP_WO;+break;+caseDMA_TO_DEVICE:+perm=VHOST_MAP_RO;+break;+caseDMA_BIDIRECTIONAL:+perm=VHOST_MAP_RW;+break;+default:+break;+}++returnperm;+}++staticdma_addr_tvdpasim_map_page(structdevice*dev,structpage*page,+unsignedlongoffset,size_tsize,+enumdma_data_directiondir,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;+u64pa=(page_to_pfn(page)<<PAGE_SHIFT)+offset;+intret,perm=dir_to_perm(dir);++if(perm<0)+returnDMA_MAPPING_ERROR;++/* For simplicity, use identical mapping to avoid e.g iova+*allocator.+*/+ret=vhost_iotlb_add_range(iommu,pa,pa+size-1,+pa,dir_to_perm(dir));+if(ret)+returnDMA_MAPPING_ERROR;++return(dma_addr_t)(pa);+}++staticvoidvdpasim_unmap_page(structdevice*dev,dma_addr_tdma_addr,+size_tsize,enumdma_data_directiondir,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;++vhost_iotlb_del_range(iommu,(u64)dma_addr,+(u64)dma_addr+size-1);+}++staticvoid*vdpasim_alloc_coherent(structdevice*dev,size_tsize,+dma_addr_t*dma_addr,gfp_tflag,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;+void*addr=kmalloc(size,flag);+intret;++if(!addr)+*dma_addr=DMA_MAPPING_ERROR;+else{+u64pa=virt_to_phys(addr);++ret=vhost_iotlb_add_range(iommu,(u64)pa,+(u64)pa+size-1,+pa,VHOST_MAP_RW);+if(ret){+*dma_addr=DMA_MAPPING_ERROR;+kfree(addr);+addr=NULL;+}else+*dma_addr=(dma_addr_t)pa;+}++returnaddr;+}++staticvoidvdpasim_free_coherent(structdevice*dev,size_tsize,+void*vaddr,dma_addr_tdma_addr,+unsignedlongattrs)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb*iommu=vdpasim->iommu;++vhost_iotlb_del_range(iommu,(u64)dma_addr,+(u64)dma_addr+size-1);+kfree(phys_to_virt((uintptr_t)dma_addr));+}++staticconststructdma_map_opsvdpasim_dma_ops={+.map_page=vdpasim_map_page,+.unmap_page=vdpasim_unmap_page,+.alloc=vdpasim_alloc_coherent,+.free=vdpasim_free_coherent,+};++staticvoidvdpasim_release_dev(structdevice*_d)+{+structvdpa_device*vdpa=dev_to_vdpa(_d);+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++kfree(vdpasim->buffer);+kfree(vdpasim);+}++staticconststructvdpa_config_opsvdpasim_net_config_ops;++staticstructvdpasim*vdpasim_create(void)+{+structvdpasim*vdpasim;+structvirtio_net_config*config;+structvdpa_device*vdpa;+structdevice*dev;+intret=-ENOMEM;++vdpasim=kzalloc(sizeof(*vdpasim),GFP_KERNEL);+if(!vdpasim)+gotoerr_vdpa_alloc;++vdpasim->buffer=kmalloc(PAGE_SIZE,GFP_KERNEL);+if(!vdpasim->buffer)+gotoerr_buffer_alloc;++vdpasim->iommu=vhost_iotlb_alloc(2048,0);+if(!vdpasim->iommu)+gotoerr_iotlb;++config=&vdpasim->config;+config->mtu=1500;+config->status=VIRTIO_NET_S_LINK_UP;+eth_random_addr(config->mac);++INIT_WORK(&vdpasim->work,vdpasim_work);+spin_lock_init(&vdpasim->lock);++vdpa=&vdpasim->vdpa;+vdpa->dev.release=vdpasim_release_dev;++vringh_set_iotlb(&vdpasim->vqs[0].vring,vdpasim->iommu);+vringh_set_iotlb(&vdpasim->vqs[1].vring,vdpasim->iommu);++dev=&vdpa->dev;+dev->coherent_dma_mask=DMA_BIT_MASK(64);+set_dma_ops(dev,&vdpasim_dma_ops);++ret=vdpa_init_device(vdpa,&vdpasim_dev->dev,dev,+&vdpasim_net_config_ops);+if(ret)+gotoerr_init;++ret=vdpa_register_device(vdpa);+if(ret)+gotoerr_register;++returnvdpasim;++err_register:+put_device(&vdpa->dev);+err_init:+vhost_iotlb_free(vdpasim->iommu);+err_iotlb:+kfree(vdpasim->buffer);+err_buffer_alloc:+kfree(vdpasim);+err_vdpa_alloc:+returnERR_PTR(ret);+}++staticintvdpasim_set_vq_address(structvdpa_device*vdpa,u16idx,+u64desc_area,u64driver_area,+u64device_area)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->desc_addr=desc_area;+vq->driver_addr=driver_area;+vq->device_addr=device_area;++return0;+}++staticvoidvdpasim_set_vq_num(structvdpa_device*vdpa,u16idx,u32num)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->num=num;+}++staticvoidvdpasim_kick_vq(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++if(vq->ready)+schedule_work(&vdpasim->work);+}++staticvoidvdpasim_set_vq_cb(structvdpa_device*vdpa,u16idx,+structvdpa_callback*cb)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++vq->cb=cb->callback;+vq->private=cb->private;+}++staticvoidvdpasim_set_vq_ready(structvdpa_device*vdpa,u16idx,boolready)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++spin_lock(&vdpasim->lock);+vq->ready=ready;+if(vq->ready)+vdpasim_queue_ready(vdpasim,idx);+spin_unlock(&vdpasim->lock);+}++staticboolvdpasim_get_vq_ready(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];++returnvq->ready;+}++staticintvdpasim_set_vq_state(structvdpa_device*vdpa,u16idx,u64state)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+structvringh*vrh=&vq->vring;++spin_lock(&vdpasim->lock);+vrh->last_avail_idx=state;+spin_unlock(&vdpasim->lock);++return0;+}++staticu64vdpasim_get_vq_state(structvdpa_device*vdpa,u16idx)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvdpasim_virtqueue*vq=&vdpasim->vqs[idx];+structvringh*vrh=&vq->vring;++returnvrh->last_avail_idx;+}++staticu16vdpasim_get_vq_align(structvdpa_device*vdpa)+{+returnVDPASIM_QUEUE_ALIGN;+}++staticu64vdpasim_get_features(structvdpa_device*vdpa)+{+returnvdpasim_features;+}++staticintvdpasim_set_features(structvdpa_device*vdpa,u64features)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++/* DMA mapping must be done by driver */+if(!(features&(1ULL<<VIRTIO_F_IOMMU_PLATFORM)))+return-EINVAL;++vdpasim->features=features&vdpasim_features;++return0;+}++staticvoidvdpasim_set_config_cb(structvdpa_device*vdpa,+structvdpa_callback*cb)+{+/* We don't support config interrupt */+}++staticu16vdpasim_get_vq_num_max(structvdpa_device*vdpa)+{+returnVDPASIM_QUEUE_MAX;+}++staticu32vdpasim_get_device_id(structvdpa_device*vdpa)+{+returnVDPASIM_DEVICE_ID;+}++staticu32vdpasim_get_vendor_id(structvdpa_device*vdpa)+{+returnVDPASIM_VENDOR_ID;+}++staticu8vdpasim_get_status(structvdpa_device*vdpa)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+u8status;++spin_lock(&vdpasim->lock);+status=vdpasim->status;+spin_unlock(&vdpasim->lock);++returnvdpasim->status;+}++staticvoidvdpasim_set_status(structvdpa_device*vdpa,u8status)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++spin_lock(&vdpasim->lock);+vdpasim->status=status;+if(status==0)+vdpasim_reset(vdpasim);+spin_unlock(&vdpasim->lock);+}++staticvoidvdpasim_get_config(structvdpa_device*vdpa,unsignedintoffset,+void*buf,unsignedintlen)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++if(offset+len<sizeof(structvirtio_net_config))+memcpy(buf,&vdpasim->config+offset,len);+}++staticvoidvdpasim_set_config(structvdpa_device*vdpa,unsignedintoffset,+constvoid*buf,unsignedintlen)+{+/* No writable config supportted by vdpasim */+}++staticu32vdpasim_get_generation(structvdpa_device*vdpa)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++returnvdpasim->generation;+}++staticintvdpasim_set_map(structvdpa_device*vdpa,+structvhost_iotlb*iotlb)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);+structvhost_iotlb_map*map;+u64start=0ULL,last=0ULL-1;+intret;++vhost_iotlb_reset(vdpasim->iommu);++for(map=vhost_iotlb_itree_first(iotlb,start,last);map;+map=vhost_iotlb_itree_next(map,start,last)){+ret=vhost_iotlb_add_range(vdpasim->iommu,map->start,+map->last,map->addr,map->perm);+if(ret)+gotoerr;+}+return0;++err:+vhost_iotlb_reset(vdpasim->iommu);+returnret;+}++staticintvdpasim_dma_map(structvdpa_device*vdpa,u64iova,u64size,+u64pa,u32perm)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++returnvhost_iotlb_add_range(vdpasim->iommu,iova,+iova+size-1,pa,perm);+}++staticintvdpasim_dma_unmap(structvdpa_device*vdpa,u64iova,u64size)+{+structvdpasim*vdpasim=vdpa_to_sim(vdpa);++vhost_iotlb_del_range(vdpasim->iommu,iova,iova+size-1);++return0;+}++staticconststructvdpa_config_opsvdpasim_net_config_ops={+.set_vq_address=vdpasim_set_vq_address,+.set_vq_num=vdpasim_set_vq_num,+.kick_vq=vdpasim_kick_vq,+.set_vq_cb=vdpasim_set_vq_cb,+.set_vq_ready=vdpasim_set_vq_ready,+.get_vq_ready=vdpasim_get_vq_ready,+.set_vq_state=vdpasim_set_vq_state,+.get_vq_state=vdpasim_get_vq_state,+.get_vq_align=vdpasim_get_vq_align,+.get_features=vdpasim_get_features,+.set_features=vdpasim_set_features,+.set_config_cb=vdpasim_set_config_cb,+.get_vq_num_max=vdpasim_get_vq_num_max,+.get_device_id=vdpasim_get_device_id,+.get_vendor_id=vdpasim_get_vendor_id,+.get_status=vdpasim_get_status,+.set_status=vdpasim_set_status,+.get_config=vdpasim_get_config,+.set_config=vdpasim_set_config,+.get_generation=vdpasim_get_generation,+.set_map=vdpasim_set_map,+.dma_map=vdpasim_dma_map,+.dma_unmap=vdpasim_dma_unmap,+};++staticvoidvdpasim_device_release(structdevice*dev)+{+structvdpasim_dev*vdpasim_dev=+container_of(dev,structvdpasim_dev,dev);++vdpasim_dev->dev.bus=NULL;+kfree(vdpasim_dev);+}++staticint__initvdpasim_dev_init(void)+{+structdevice*dev;+intret=0;++vdpasim_dev=kzalloc(sizeof(*vdpasim_dev),GFP_KERNEL);+if(!vdpasim_dev)+return-ENOMEM;++dev=&vdpasim_dev->dev;+dev->release=vdpasim_device_release;+dev_set_name(dev,"%s",VDPASIM_NAME);++ret=device_register(&vdpasim_dev->dev);+if(ret)+gotoerr_register;++if(!vdpasim_create())+gotoerr_register;++return0;++err_register:+kfree(vdpasim_dev);+vdpasim_dev=NULL;+returnret;+}++staticintvdpasim_device_remove_cb(structdevice*dev,void*data)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);++vdpa_unregister_device(vdpa);++return0;+}++staticvoid__exitvdpasim_dev_exit(void)+{+device_for_each_child(&vdpasim_dev->dev,NULL,+vdpasim_device_remove_cb);+device_unregister(&vdpasim_dev->dev);+}++module_init(vdpasim_dev_init)+module_exit(vdpasim_dev_exit)++MODULE_VERSION(DRV_VERSION);+MODULE_LICENSE(DRV_LICENSE);+MODULE_AUTHOR(DRV_AUTHOR);+MODULE_DESCRIPTION(DRV_DESC);
From: Jason Gunthorpe <hidden> Date: 2020-02-10 13:34:59
On Mon, Feb 10, 2020 at 11:56:07AM +0800, Jason Wang wrote:
This patch introduces a vDPA transport for virtio. This is used to
use kernel virtio driver to drive the mediated device that is capable
of populating virtqueue directly.
Is this comment still right? Is there a mediated device still?
Jason
From: Jason Wang <hidden> Date: 2020-02-11 03:05:24
On 2020/2/10 下午9:34, Jason Gunthorpe wrote:
On Mon, Feb 10, 2020 at 11:56:07AM +0800, Jason Wang wrote:
quoted
This patch introduces a vDPA transport for virtio. This is used to
use kernel virtio driver to drive the mediated device that is capable
of populating virtqueue directly.
Is this comment still right? Is there a mediated device still?
Jason
From: Jason Wang <hidden> Date: 2020-02-11 03:12:47
On 2020/2/10 下午7:23, Michael S. Tsirkin wrote:
On Mon, Feb 10, 2020 at 11:56:08AM +0800, Jason Wang wrote:
quoted
This patch implements a software vDPA networking device. The datapath
is implemented through vringh and workqueue. The device has an on-chip
IOMMU which translates IOVA to PA. For kernel virtio drivers, vDPA
simulator driver provides dma_ops. For vhost driers, set_map() methods
of vdpa_config_ops is implemented to accept mappings from vhost.
Currently, vDPA device simulator will loopback TX traffic to RX. So
the main use case for the device is vDPA feature testing, prototyping
and development.
Note, there's no management API implemented, a vDPA device will be
registered once the module is probed. We need to handle this in the
future development.
Signed-off-by: Jason Wang<redacted>
---
drivers/virtio/vdpa/Kconfig | 17 +
drivers/virtio/vdpa/Makefile | 1 +
drivers/virtio/vdpa/vdpa_sim.c | 678 +++++++++++++++++++++++++++++++++
3 files changed, 696 insertions(+)
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
From: Jason Gunthorpe <hidden> Date: 2020-02-11 13:47:57
On Mon, Feb 10, 2020 at 11:56:06AM +0800, Jason Wang wrote:
+/**
+ * vdpa_register_device - register a vDPA device
+ * Callers must have a succeed call of vdpa_init_device() before.
+ * @vdev: the vdpa device to be registered to vDPA bus
+ *
+ * Returns an error when fail to add to vDPA bus
+ */
+int vdpa_register_device(struct vdpa_device *vdev)
+{
+ int err = device_add(&vdev->dev);
+
+ if (err) {
+ put_device(&vdev->dev);
+ ida_simple_remove(&vdpa_index_ida, vdev->index);
+ }
This is a very dangerous construction, I've seen it lead to driver
bugs. Better to require the driver to always do the put_device on
error unwind
The ida_simple_remove should probably be part of the class release
function to make everything work right
+/**
+ * vdpa_unregister_driver - unregister a vDPA device driver
+ * @drv: the vdpa device driver to be unregistered
+ */
+void vdpa_unregister_driver(struct vdpa_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(vdpa_unregister_driver);
+
+static int vdpa_init(void)
+{
+ if (bus_register(&vdpa_bus) != 0)
+ panic("virtio bus registration failed");
+ return 0;
+}
Linus will tell you not to kill the kernel - return the error code and
propagate it up to the module init function.
+/**
+ * vDPA device - representation of a vDPA device
+ * @dev: underlying device
+ * @dma_dev: the actual device that is performing DMA
+ * @config: the configuration ops for this device.
+ * @index: device index
+ */
+struct vdpa_device {
+ struct device dev;
+ struct device *dma_dev;
+ const struct vdpa_config_ops *config;
+ int index;
The driver should not provide the release function.
Again the safest model is 'vdpa_alloc_device' which combines the
kzalloc and the vdpa_init_device() and returns something that is
error unwound with put_device()
The subsystem owns the release and does the kfree and other cleanup
like releasing the IDA.
+ vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
+ vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
+
+ dev = &vdpa->dev;
+ dev->coherent_dma_mask = DMA_BIT_MASK(64);
+ set_dma_ops(dev, &vdpasim_dma_ops);
+
+ ret = vdpa_init_device(vdpa, &vdpasim_dev->dev, dev,
+ &vdpasim_net_config_ops);
+ if (ret)
+ goto err_init;
+
+ ret = vdpa_register_device(vdpa);
+ if (ret)
+ goto err_register;
From: Jason Wang <hidden> Date: 2020-02-12 07:55:56
On 2020/2/11 下午9:47, Jason Gunthorpe wrote:
On Mon, Feb 10, 2020 at 11:56:06AM +0800, Jason Wang wrote:
quoted
+/**
+ * vdpa_register_device - register a vDPA device
+ * Callers must have a succeed call of vdpa_init_device() before.
+ * @vdev: the vdpa device to be registered to vDPA bus
+ *
+ * Returns an error when fail to add to vDPA bus
+ */
+int vdpa_register_device(struct vdpa_device *vdev)
+{
+ int err = device_add(&vdev->dev);
+
+ if (err) {
+ put_device(&vdev->dev);
+ ida_simple_remove(&vdpa_index_ida, vdev->index);
+ }
This is a very dangerous construction, I've seen it lead to driver
bugs. Better to require the driver to always do the put_device on
error unwind
Ok.
The ida_simple_remove should probably be part of the class release
function to make everything work right
It looks to me bus instead of class is the correct abstraction here
since the devices share a set of programming interface but not the
semantics.
Or do you actually mean type here?
quoted
+/**
+ * vdpa_unregister_driver - unregister a vDPA device driver
+ * @drv: the vdpa device driver to be unregistered
+ */
+void vdpa_unregister_driver(struct vdpa_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(vdpa_unregister_driver);
+
+static int vdpa_init(void)
+{
+ if (bus_register(&vdpa_bus) != 0)
+ panic("virtio bus registration failed");
+ return 0;
+}
Linus will tell you not to kill the kernel - return the error code and
propagate it up to the module init function.
Yes, will fix.
quoted
+/**
+ * vDPA device - representation of a vDPA device
+ * @dev: underlying device
+ * @dma_dev: the actual device that is performing DMA
+ * @config: the configuration ops for this device.
+ * @index: device index
+ */
+struct vdpa_device {
+ struct device dev;
+ struct device *dma_dev;
+ const struct vdpa_config_ops *config;
+ int index;
The driver should not provide the release function.
Again the safest model is 'vdpa_alloc_device' which combines the
kzalloc and the vdpa_init_device() and returns something that is
error unwound with put_device()
The subsystem owns the release and does the kfree and other cleanup
like releasing the IDA.
So I think if we agree bus instead of class is used. vDPA bus can
provide a release function in vdpa_alloc_device()?
quoted
+ vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
+ vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
+
+ dev = &vdpa->dev;
+ dev->coherent_dma_mask = DMA_BIT_MASK(64);
+ set_dma_ops(dev, &vdpasim_dma_ops);
+
+ ret = vdpa_init_device(vdpa, &vdpasim_dev->dev, dev,
+ &vdpasim_net_config_ops);
+ if (ret)
+ goto err_init;
+
+ ret = vdpa_register_device(vdpa);
+ if (ret)
+ goto err_register;
From: Jason Gunthorpe <hidden> Date: 2020-02-12 12:51:33
On Wed, Feb 12, 2020 at 03:55:31PM +0800, Jason Wang wrote:
quoted
The ida_simple_remove should probably be part of the class release
function to make everything work right
It looks to me bus instead of class is the correct abstraction here since
the devices share a set of programming interface but not the semantics.
device_release() doesn't call the bus release? You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
From: Jason Wang <hidden> Date: 2020-02-13 03:34:34
On 2020/2/12 下午8:51, Jason Gunthorpe wrote:
On Wed, Feb 12, 2020 at 03:55:31PM +0800, Jason Wang wrote:
quoted
quoted
The ida_simple_remove should probably be part of the class release
function to make everything work right
It looks to me bus instead of class is the correct abstraction here since
the devices share a set of programming interface but not the semantics.
device_release() doesn't call the bus release?
What it did is:
if (dev->release)
dev->release(dev);
else if (dev->type && dev->type->release)
dev->type->release(dev);
else if (dev->class && dev->class->dev_release)
dev->class->dev_release(dev);
else
WARN(1, KERN_ERR "Device '%s' does not have a release()
function, it is broken and must be fixed. See Documentation/kobject.txt.\n",
dev_name(dev));
So it looks not.
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
Thanks
From: Jason Gunthorpe <hidden> Date: 2020-02-13 13:41:42
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
For the PCI VF case this driver would bind to a PCI device like
everything else
For our future SF/ADI cases the driver would bind to some
SF/ADI/whatever device on a bus.
I don't see a reason for VDPA to be creating busses..
Jason
From: Jason Wang <hidden> Date: 2020-02-13 14:59:09
On 2020/2/13 下午9:41, Jason Gunthorpe wrote:
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
This is because we want vDPA to be generic for being used by different
drivers which is not limited to vhost-vdpa. E.g in this series, it
allows vDPA to be used by kernel virtio drivers. And in the future, we
will probably introduce more drivers in the future.
For the PCI VF case this driver would bind to a PCI device like
everything else
For our future SF/ADI cases the driver would bind to some
SF/ADI/whatever device on a bus.
All these driver will still be bound to their own bus (PCI or other).
And what the driver needs is to present a vDPA device to virtual vDPA
bus on top.
Thanks
I don't see a reason for VDPA to be creating busses..
Jason
From: Jason Gunthorpe <hidden> Date: 2020-02-13 15:05:53
On Thu, Feb 13, 2020 at 10:58:44PM +0800, Jason Wang wrote:
On 2020/2/13 下午9:41, Jason Gunthorpe wrote:
quoted
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface. All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
quoted
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
This is because we want vDPA to be generic for being used by different
drivers which is not limited to vhost-vdpa. E.g in this series, it allows
vDPA to be used by kernel virtio drivers. And in the future, we will
probably introduce more drivers in the future.
I don't see how that connects with using a bus.
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
quoted
For the PCI VF case this driver would bind to a PCI device like
everything else
For our future SF/ADI cases the driver would bind to some
SF/ADI/whatever device on a bus.
All these driver will still be bound to their own bus (PCI or other). And
what the driver needs is to present a vDPA device to virtual vDPA bus on
top.
Again, I can't see any reason to inject a 'vdpa virtual bus' on
top. That seems like mis-using the driver core.
Jason
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-13 15:41:24
On Thu, Feb 13, 2020 at 11:05:42AM -0400, Jason Gunthorpe wrote:
On Thu, Feb 13, 2020 at 10:58:44PM +0800, Jason Wang wrote:
quoted
On 2020/2/13 下午9:41, Jason Gunthorpe wrote:
quoted
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface. All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
quoted
quoted
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
This is because we want vDPA to be generic for being used by different
drivers which is not limited to vhost-vdpa. E.g in this series, it allows
vDPA to be used by kernel virtio drivers. And in the future, we will
probably introduce more drivers in the future.
I don't see how that connects with using a bus.
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
That's just a ton of useless code duplication, and a good chance
to have minor variations in implementations confusing
userspace.
Instead, each device implement the same interface, and then
vhost sits on top.
quoted
quoted
For the PCI VF case this driver would bind to a PCI device like
everything else
For our future SF/ADI cases the driver would bind to some
SF/ADI/whatever device on a bus.
All these driver will still be bound to their own bus (PCI or other). And
what the driver needs is to present a vDPA device to virtual vDPA bus on
top.
Again, I can't see any reason to inject a 'vdpa virtual bus' on
top. That seems like mis-using the driver core.
Jason
That bus is exactly what Greg KH proposed. There are other ways
to solve this I guess but this bikeshedding is getting tiring.
Come on it's an internal kernel interface, if we feel
it was a wrong direction to take we can change our minds later.
Main thing is getting UAPI right.
--
MST
From: Jason Gunthorpe <hidden> Date: 2020-02-13 15:52:08
On Thu, Feb 13, 2020 at 10:41:06AM -0500, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 11:05:42AM -0400, Jason Gunthorpe wrote:
quoted
On Thu, Feb 13, 2020 at 10:58:44PM +0800, Jason Wang wrote:
quoted
On 2020/2/13 下午9:41, Jason Gunthorpe wrote:
quoted
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface. All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
quoted
quoted
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
This is because we want vDPA to be generic for being used by different
drivers which is not limited to vhost-vdpa. E.g in this series, it allows
vDPA to be used by kernel virtio drivers. And in the future, we will
probably introduce more drivers in the future.
I don't see how that connects with using a bus.
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
That's just a ton of useless code duplication, and a good chance
to have minor variations in implementations confusing
userspace.
What? Why? This is how almost every user of the driver core works.
I don't see how you get any duplication unless the subsystem core is
badly done wrong.
The 'class' is supposed to provide all the library functions to remove
this duplication. Instead of plugging the HW driver in via some bus
scheme every subsystem has its own 'ops' that the HW driver provides
to the subsystem's class via subsystem_register()
This is the *standard* pattern to use the driver core.
This is almost there, it just has this extra bus part to convey the HW
ops instead of directly.
Instead, each device implement the same interface, and then
vhost sits on top.
Sure, but plugging in via ops/etc not via a bus and another struct
device.
That bus is exactly what Greg KH proposed. There are other ways
to solve this I guess but this bikeshedding is getting tiring.
This discussion was for a different goal, IMHO.
Come on it's an internal kernel interface, if we feel
it was a wrong direction to take we can change our minds later.
Main thing is getting UAPI right.
This discussion started because the refcounting has been busted up in
every version posted so far. It is not bikeshedding when these bugs
are actually being caused by trying to abuse the driver core and
shoehorn in a bus that isn't needed when a class is the correct thing
to use.
Jason
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-13 15:59:53
On Thu, Feb 13, 2020 at 11:51:54AM -0400, Jason Gunthorpe wrote:
The 'class' is supposed to provide all the library functions to remove
this duplication. Instead of plugging the HW driver in via some bus
scheme every subsystem has its own 'ops' that the HW driver provides
to the subsystem's class via subsystem_register()
Hmm I'm not familiar with subsystem_register. A grep didn't find it
in the kernel either ...
--
MST
From: Jason Gunthorpe <hidden> Date: 2020-02-13 16:13:32
On Thu, Feb 13, 2020 at 10:59:34AM -0500, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 11:51:54AM -0400, Jason Gunthorpe wrote:
quoted
The 'class' is supposed to provide all the library functions to remove
this duplication. Instead of plugging the HW driver in via some bus
scheme every subsystem has its own 'ops' that the HW driver provides
to the subsystem's class via subsystem_register()
Hmm I'm not familiar with subsystem_register. A grep didn't find it
in the kernel either ...
I mean it is the registration function provided by the subsystem that
owns the class, for instance tpm_chip_register(),
ib_register_device(), register_netdev(), rtc_register_device() etc
So if you have some vhost (vhost net?) class then you'd have some
vhost_vdpa_init/alloc(); vhost_vdpa_register(), sequence
presumably. (vs trying to do it with a bus matcher)
I recommend to look at rtc and tpm for fairly simple easy to follow
patterns for creating a subsystem in the kernel. A subsystem owns a class,
allows HW drivers to plug in to it, and provides a consistent user
API via a cdev/sysfs/etc.
The driver model class should revolve around the char dev and sysfs
uABI - if you enumerate the devices on the class then they should all
follow the char dev and sysfs interfaces contract of that class.
Those examples show how to do all the refcounting semi-sanely,
introduce sysfs, cdevs, etc.
I thought the latest proposal was to use the existing vhost class and
largely the existing vhost API, so it probably just needs to make sure
the common class-wide stuff is split from the 'driver' stuff of the
existing vhost to netdev.
Jason
From: Jason Gunthorpe <hidden> Date: 2020-02-13 16:24:19
On Thu, Feb 13, 2020 at 10:56:00AM -0500, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 11:51:54AM -0400, Jason Gunthorpe wrote:
quoted
quoted
That bus is exactly what Greg KH proposed. There are other ways
to solve this I guess but this bikeshedding is getting tiring.
This discussion was for a different goal, IMHO.
Hmm couldn't find it anymore. What was the goal there in your opinion?
I think it was largely talking about how to model things like
ADI/SF/etc, plus stuff got very confused when the discussion tried to
explain what mdev's role was vs the driver core.
The standard driver model is a 'bus' driver provides the HW access
(think PCI level things), and a 'hw driver' attaches to the bus
device, and instantiates a 'subsystem device' (think netdev, rdma,
etc) using some per-subsystem XXX_register(). The 'hw driver' pulls in
functions from the 'subsystem' using a combination of callbacks and
library-style calls so there is no code duplication.
As a subsystem, vhost&vdpa should expect its 'HW driver' to bind to
devices on busses, for instance I would expect:
- A future SF/ADI/'virtual bus' as a child of multi-functional PCI device
Exactly how this works is still under active discussion and is
one place where Greg said 'use a bus'.
- An existing PCI, platform, or other bus and device. No need for an
extra bus here, PCI is the bus.
- No bus, ie for a simulator or binding to a netdev. (existing vhost?)
They point is that the HW driver's job is to adapt from the bus level
interfaces (eg readl/writel) to the subsystem level (eg something like
the vdpa_ops).
For instance that Intel driver should be a pci_driver to bind to a
struct pci_device for its VF and then call some 'vhost&vdpa'
_register() function to pass its ops to the subsystem which in turn
creates the struct device of the subsystem calls, common char devices,
sysfs, etc and calls the driver's ops in response to uAPI calls.
This is already almost how things were setup in v2 of the patches,
near as I can see, just that a bus was inserted somehow instead of
having only the vhost class. So it iwas confusing and the lifetime
model becomes too complicated to implement correctly...
Jason
From: Jason Wang <hidden> Date: 2020-02-14 03:23:55
On 2020/2/13 下午11:05, Jason Gunthorpe wrote:
On Thu, Feb 13, 2020 at 10:58:44PM +0800, Jason Wang wrote:
quoted
On 2020/2/13 下午9:41, Jason Gunthorpe wrote:
quoted
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:
quoted
quoted
You have dev, type or
class to choose from. Type is rarely used and doesn't seem to be used
by vdpa, so class seems the right choice
Jason
Yes, but my understanding is class and bus are mutually exclusive. So we
can't add a class to a device which is already attached on a bus.
While I suppose there are variations, typically 'class' devices are
user facing things and 'bus' devices are internal facing (ie like a
PCI device)
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface.
I'm confused here, are you suggesting to use class to create char device
in vhost-vdpa? That's fine but the comment should go for vhost-vdpa patch.
All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
That's not true, char interface is only used for vhost. Kernel virtio
driver does not need char dev but a device on the virtio bus.
quoted
quoted
So why is this using a bus? VDPA is a user facing object, so the
driver should create a class vhost_vdpa device directly, and that
driver should live in the drivers/vhost/ directory.
This is because we want vDPA to be generic for being used by different
drivers which is not limited to vhost-vdpa. E.g in this series, it allows
vDPA to be used by kernel virtio drivers. And in the future, we will
probably introduce more drivers in the future.
I don't see how that connects with using a bus.
This is demonstrated in the virito-vdpa driver. So if you want to use
kernel virito driver for vDPA device, a bus is most straight forward.
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
Are you saying, e.g it's the charge of IFCVF driver to create vhost char
dev and other stuffs?
quoted
quoted
For the PCI VF case this driver would bind to a PCI device like
everything else
For our future SF/ADI cases the driver would bind to some
SF/ADI/whatever device on a bus.
All these driver will still be bound to their own bus (PCI or other). And
what the driver needs is to present a vDPA device to virtual vDPA bus on
top.
Again, I can't see any reason to inject a 'vdpa virtual bus' on
top. That seems like mis-using the driver core.
I don't think so. Vhost is not the only programming interface for vDPA.
We don't want a device that can only work for userspace drivers and only
have a single set of userspace APIs.
Thanks
From: Jason Wang <hidden> Date: 2020-02-14 04:06:02
On 2020/2/14 上午12:24, Jason Gunthorpe wrote:
On Thu, Feb 13, 2020 at 10:56:00AM -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Feb 13, 2020 at 11:51:54AM -0400, Jason Gunthorpe wrote:
quoted
quoted
That bus is exactly what Greg KH proposed. There are other ways
to solve this I guess but this bikeshedding is getting tiring.
This discussion was for a different goal, IMHO.
Hmm couldn't find it anymore. What was the goal there in your opinion?
I think it was largely talking about how to model things like
ADI/SF/etc, plus stuff got very confused when the discussion tried to
explain what mdev's role was vs the driver core.
The standard driver model is a 'bus' driver provides the HW access
(think PCI level things), and a 'hw driver' attaches to the bus
device,
This is not true, kernel had already had plenty virtual bus where
virtual devices and drivers could be attached, besides mdev and virtio,
you can see vop, rpmsg, visorbus etc.
and instantiates a 'subsystem device' (think netdev, rdma,
etc) using some per-subsystem XXX_register().
Well, if you go through virtio spec, we support ~20 types of different
devices. Classes like netdev and rdma are correct since they have a
clear set of semantics their own. But grouping network and scsi into a
single class looks wrong, that's the work of a virtual bus.
The class should be done on top of vDPA device instead of vDPA device
itself:
- For kernel driver, netdev, blk dev could be done on top
- For userspace driver, the class could be done by the drivers inside VM
or userspace (dpdk)
The 'hw driver' pulls in
functions from the 'subsystem' using a combination of callbacks and
library-style calls so there is no code duplication.
The point is we want vDPA devices to be used by different subsystems,
not only vhost, but also netdev, blk, crypto (every subsystem that can
use virtio devices). That's why we introduce vDPA bus and introduce
different drivers on top.
As a subsystem, vhost&vdpa should expect its 'HW driver' to bind to
devices on busses, for instance I would expect:
- A future SF/ADI/'virtual bus' as a child of multi-functional PCI device
Exactly how this works is still under active discussion and is
one place where Greg said 'use a bus'.
That's ok but it's something that is not directly related to vDPA which
can be implemented by any kinds of devices/buses:
struct XXX_device {
struct vdpa_device vdpa;
struct adi_device/pci_device *lowerdev;
}
...
- An existing PCI, platform, or other bus and device. No need for an
extra bus here, PCI is the bus.
There're several examples that a bus is needed on top.
A good example is Mellanox TmFIFO driver which is a platform device
driver but register itself as a virtio device in order to be used by
virito-console driver on the virtio bus.
But it's a pity that the device can not be used by userspace driver due
to the limitation of virito bus which is designed for kernel driver.
That's why vDPA bus is introduced which abstract the common requirements
of both kernel and userspace drivers which allow the a single HW driver
to be used by kernel drivers (and the subsystems on top) and userspace
drivers.
- No bus, ie for a simulator or binding to a netdev. (existing vhost?)
Note, simulator can have its own class (sysfs etc.).
They point is that the HW driver's job is to adapt from the bus level
interfaces (eg readl/writel) to the subsystem level (eg something like
the vdpa_ops).
For instance that Intel driver should be a pci_driver to bind to a
struct pci_device for its VF and then call some 'vhost&vdpa'
_register() function to pass its ops to the subsystem which in turn
creates the struct device of the subsystem calls, common char devices,
sysfs, etc and calls the driver's ops in response to uAPI calls.
This is already almost how things were setup in v2 of the patches,
near as I can see, just that a bus was inserted somehow instead of
having only the vhost class.
Well the series (plus mdev part) uses a bus since day 0. It's not
something new.
Thanks
So it iwas confusing and the lifetime
model becomes too complicated to implement correctly...
Jason
From: Jason Wang <hidden> Date: 2020-02-14 04:40:17
On 2020/2/14 上午12:13, Jason Gunthorpe wrote:
On Thu, Feb 13, 2020 at 10:59:34AM -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Feb 13, 2020 at 11:51:54AM -0400, Jason Gunthorpe wrote:
quoted
The 'class' is supposed to provide all the library functions to remove
this duplication. Instead of plugging the HW driver in via some bus
scheme every subsystem has its own 'ops' that the HW driver provides
to the subsystem's class via subsystem_register()
Hmm I'm not familiar with subsystem_register. A grep didn't find it
in the kernel either ...
I mean it is the registration function provided by the subsystem that
owns the class, for instance tpm_chip_register(),
ib_register_device(), register_netdev(), rtc_register_device() etc
So if you have some vhost (vhost net?) class then you'd have some
vhost_vdpa_init/alloc(); vhost_vdpa_register(), sequence
presumably. (vs trying to do it with a bus matcher)
I recommend to look at rtc and tpm for fairly simple easy to follow
patterns for creating a subsystem in the kernel. A subsystem owns a class,
allows HW drivers to plug in to it, and provides a consistent user
API via a cdev/sysfs/etc.
The driver model class should revolve around the char dev and sysfs
uABI - if you enumerate the devices on the class then they should all
follow the char dev and sysfs interfaces contract of that class.
Those examples show how to do all the refcounting semi-sanely,
introduce sysfs, cdevs, etc.
I thought the latest proposal was to use the existing vhost class and
largely the existing vhost API, so it probably just needs to make sure
the common class-wide stuff is split from the 'driver' stuff of the
existing vhost to netdev.
Still, netdev is only one of the type we want to support. And we can not
guarantee or forecast that vhost is the only API that is used.
Let's take virtio as an example, it is implemented through a bus which
allows different subsystems on top. And it can provide a variety of
different uAPIs. For best performance, VFIO could be used for userspace
drivers, but it requires the bus has support from VFIO.
For vDPA devices, it's just the same logic. A bus allows different
drivers and subsystems on top. One of the subsystem could be vhost that
provides a unified API for userspace driver.
Thanks
From: Jason Gunthorpe <hidden> Date: 2020-02-14 13:52:44
On Fri, Feb 14, 2020 at 11:23:27AM +0800, Jason Wang wrote:
quoted
quoted
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface.
I'm confused here, are you suggesting to use class to create char device in
vhost-vdpa? That's fine but the comment should go for vhost-vdpa patch.
Certainly yes, something creating many char devs should have a
class. That makes the sysfs work as expected
I suppose this is vhost user? I admit I don't really see how this
vhost stuff works, all I see are global misc devices? Very unusual for
a new subsystem to be using global misc devices..
I would have expected that a single VDPA device comes out as a single
char dev linked to only that VDPA device.
quoted
All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
That's not true, char interface is only used for vhost. Kernel virtio driver
does not need char dev but a device on the virtio bus.
Okay, this is fine, but why do you need two busses to accomplish this?
Shouldn't the 'struct virito_device' be the plug in point for HW
drivers I was talking about - and from there a vhost-user can connect
to the struct virtio_device to give it a char dev or a kernel driver
can connect to link it to another subsystem?
It is easy to see something is going wrong with this design because
the drivers/virtio/virtio_vdpa.c mainly contains a bunch of trampoline
functions reflecting identical calls from one ops struct to a
different ops struct. This suggests the 'vdpa' is some subclass of
'virtio' and it is possibly better to model it by extending 'struct
virito_device' to include the vdpa specific stuff.
Where does the vhost-user char dev get invovled in with the v2 series?
Is that included?
quoted
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
Are you saying, e.g it's the charge of IFCVF driver to create vhost char dev
and other stuffs?
From: Jason Gunthorpe <hidden> Date: 2020-02-14 14:04:58
On Fri, Feb 14, 2020 at 12:05:32PM +0800, Jason Wang wrote:
quoted
The standard driver model is a 'bus' driver provides the HW access
(think PCI level things), and a 'hw driver' attaches to the bus
device,
This is not true, kernel had already had plenty virtual bus where virtual
devices and drivers could be attached, besides mdev and virtio, you can see
vop, rpmsg, visorbus etc.
Sure, but those are not connecting HW into the kernel..
quoted
and instantiates a 'subsystem device' (think netdev, rdma,
etc) using some per-subsystem XXX_register().
Well, if you go through virtio spec, we support ~20 types of different
devices. Classes like netdev and rdma are correct since they have a clear
set of semantics their own. But grouping network and scsi into a single
class looks wrong, that's the work of a virtual bus.
rdma also has about 20 different types of things it supports on top of
the generic ib_device.
The central point in RDMA is the 'struct ib_device' which is a device
class. You can discover all RDMA devices by looking in /sys/class/infiniband/
It has an internal bus like thing (which probably should have been an
actual bus, but this was done 15 years ago) which allows other
subsystems to have drivers to match and bind their own drivers to the
struct ib_device.
So you'd have a chain like:
struct pci_device -> struct ib_device -> [ib client bus thing] -> struct net_device
And the various char devs are created by clients connecting to the
ib_device and creating char devs on their own classes.
Since ib_devices are multi-queue we can have all 20 devices running
concurrently and there are various schemes to manage when the various
things are created.
quoted
The 'hw driver' pulls in
functions from the 'subsystem' using a combination of callbacks and
library-style calls so there is no code duplication.
The point is we want vDPA devices to be used by different subsystems, not
only vhost, but also netdev, blk, crypto (every subsystem that can use
virtio devices). That's why we introduce vDPA bus and introduce different
drivers on top.
See the other mail, it seems struct virtio_device serves this purpose
already, confused why a struct vdpa_device and another bus is being
introduced
There're several examples that a bus is needed on top.
A good example is Mellanox TmFIFO driver which is a platform device driver
but register itself as a virtio device in order to be used by virito-console
driver on the virtio bus.
How is that another bus? The platform bus is the HW bus, the TmFIFO is
the HW driver, and virtio_device is the subsystem.
This seems reasonable/normal so far..
But it's a pity that the device can not be used by userspace driver due to
the limitation of virito bus which is designed for kernel driver. That's why
vDPA bus is introduced which abstract the common requirements of both kernel
and userspace drivers which allow the a single HW driver to be used by
kernel drivers (and the subsystems on top) and userspace drivers.
Ah! Maybe this is the source of all this strangeness - the userspace
driver is something parallel to the struct virtio_device instead of
being a consumer of it?? That certianly would mess up the driver model
quite a lot.
Then you want to add another bus to switch between vhost and struct
virtio_device? But only for vdpa?
But as you point out something like TmFIFO is left hanging. Seems like
the wrong abstraction point..
Jason
From: Jason Wang <hidden> Date: 2020-02-17 06:08:02
On 2020/2/14 下午10:04, Jason Gunthorpe wrote:
On Fri, Feb 14, 2020 at 12:05:32PM +0800, Jason Wang wrote:
quoted
quoted
The standard driver model is a 'bus' driver provides the HW access
(think PCI level things), and a 'hw driver' attaches to the bus
device,
This is not true, kernel had already had plenty virtual bus where virtual
devices and drivers could be attached, besides mdev and virtio, you can see
vop, rpmsg, visorbus etc.
Sure, but those are not connecting HW into the kernel..
Well the virtual devices are normally implemented via a real HW driver.
E.g for virio bus, its transport driver could be driver of real hardware
(e.g PCI).
quoted
quoted
and instantiates a 'subsystem device' (think netdev, rdma,
etc) using some per-subsystem XXX_register().
Well, if you go through virtio spec, we support ~20 types of different
devices. Classes like netdev and rdma are correct since they have a clear
set of semantics their own. But grouping network and scsi into a single
class looks wrong, that's the work of a virtual bus.
rdma also has about 20 different types of things it supports on top of
the generic ib_device.
The central point in RDMA is the 'struct ib_device' which is a device
class. You can discover all RDMA devices by looking in /sys/class/infiniband/
It has an internal bus like thing (which probably should have been an
actual bus, but this was done 15 years ago) which allows other
subsystems to have drivers to match and bind their own drivers to the
struct ib_device.
Right.
So you'd have a chain like:
struct pci_device -> struct ib_device -> [ib client bus thing] -> struct net_device
And the various char devs are created by clients connecting to the
ib_device and creating char devs on their own classes.
Since ib_devices are multi-queue we can have all 20 devices running
concurrently and there are various schemes to manage when the various
things are created.
quoted
quoted
The 'hw driver' pulls in
functions from the 'subsystem' using a combination of callbacks and
library-style calls so there is no code duplication.
The point is we want vDPA devices to be used by different subsystems, not
only vhost, but also netdev, blk, crypto (every subsystem that can use
virtio devices). That's why we introduce vDPA bus and introduce different
drivers on top.
See the other mail, it seems struct virtio_device serves this purpose
already, confused why a struct vdpa_device and another bus is being
introduced
quoted
There're several examples that a bus is needed on top.
A good example is Mellanox TmFIFO driver which is a platform device driver
but register itself as a virtio device in order to be used by virito-console
driver on the virtio bus.
How is that another bus? The platform bus is the HW bus, the TmFIFO is
the HW driver, and virtio_device is the subsystem.
This seems reasonable/normal so far..
Yes, that's reasonable. This example is to answer the question why bus
is used instead of class here.
quoted
But it's a pity that the device can not be used by userspace driver due to
the limitation of virito bus which is designed for kernel driver. That's why
vDPA bus is introduced which abstract the common requirements of both kernel
and userspace drivers which allow the a single HW driver to be used by
kernel drivers (and the subsystems on top) and userspace drivers.
Ah! Maybe this is the source of all this strangeness - the userspace
driver is something parallel to the struct virtio_device instead of
being a consumer of it??
userspace driver is not parallel to virtio_device. The vhost_device is
parallel to virtio_device actually.
That certianly would mess up the driver model
quite a lot.
Then you want to add another bus to switch between vhost and struct
virtio_device? But only for vdpa?
Still, vhost works on top of vDPA bus directly (see the reply above).
But as you point out something like TmFIFO is left hanging. Seems like
the wrong abstraction point..
You know, even refactoring virtio-bus is not for free, TmFIFO driver
needs changes anyhow.
Thanks
From: Jason Wang <hidden> Date: 2020-02-17 06:08:52
On 2020/2/14 下午9:52, Jason Gunthorpe wrote:
On Fri, Feb 14, 2020 at 11:23:27AM +0800, Jason Wang wrote:
quoted
quoted
quoted
Though all vDPA devices have the same programming interface, but the
semantic is different. So it looks to me that use bus complies what
class.rst said:
"
Each device class defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
"
Here we are talking about the /dev/XX node that provides the
programming interface.
I'm confused here, are you suggesting to use class to create char device in
vhost-vdpa? That's fine but the comment should go for vhost-vdpa patch.
Certainly yes, something creating many char devs should have a
class. That makes the sysfs work as expected
I suppose this is vhost user?
Actually not.
Vhost-user is the vhost protocol that is used for userspace vhost
backend (usually though a UNIX domain socket).
What's being done in the vhost-vpda is a new type of the vhost in kernel.
I admit I don't really see how this
vhost stuff works, all I see are global misc devices? Very unusual for
a new subsystem to be using global misc devices..
Vhost is not a subsystem right now, e.g for it's net implementation, it
was loosely coupled with a socket.
I thought you were copied in the patch [1], maybe we can move vhost
related discussion there to avoid confusion.
[1] https://lwn.net/Articles/811210/
I would have expected that a single VDPA device comes out as a single
char dev linked to only that VDPA device.
quoted
quoted
All the vdpa devices have the same basic
chardev interface and discover any semantic variations 'in band'
That's not true, char interface is only used for vhost. Kernel virtio driver
does not need char dev but a device on the virtio bus.
Okay, this is fine, but why do you need two busses to accomplish this?
The reasons are:
- vDPA ops is designed to be functional as a software assisted transport
(control path) for virtio, so it's fit for a new transport driver but
not directly into virtio bus. VOP use similar design.
- virtio bus is designed for kernel drivers but not userspace, and it
can not be easily extended to support userspace driver but requires some
major refactoring. E.g the virtio bus operations requires the virtqueue
to be allocated by the transport driver.
So it's cheaper and simpler to introduce a new bus instead of
refactoring a well known bus and API where brunches of drivers and
devices had been implemented for years.
Shouldn't the 'struct virito_device' be the plug in point for HW
drivers I was talking about - and from there a vhost-user can connect
to the struct virtio_device to give it a char dev or a kernel driver
can connect to link it to another subsystem?
From vhost point of view, it would only need to connect vDPA bus, no
need to go for virtio bus. Vhost device talks to vDPA device through
vDPA bus. Virito device talks to vDPA device through a new vDPA
transport driver.
It is easy to see something is going wrong with this design because
the drivers/virtio/virtio_vdpa.c mainly contains a bunch of trampoline
functions reflecting identical calls from one ops struct to a
different ops struct.
That's pretty normal, since part of the virtio ops could be 1:1 mapped
to some device function. If you see MMIO and PCI transport, you can see
something similar. The only difference is that in the case of VDPA the
function is assisted or emulated by hardware vDPA driver.
This suggests the 'vdpa' is some subclass of
'virtio' and it is possibly better to model it by extending 'struct
virito_device' to include the vdpa specific stuff.
Going for such kind of modeling, virtio-pci and virtio-mmio could be
also treated as a subclass of virtio as well, they were all implemented
via a dedicated transport driver.
Where does the vhost-user char dev get invovled in with the v2 series?
Is that included?
We're working on the a new version, but for the bus/driver part it
should be the same as version 1.
Thanks
quoted
quoted
Every class of virtio traffic is going to need a special HW driver to
enable VDPA, that special driver can create the correct vhost side
class device.
Are you saying, e.g it's the charge of IFCVF driver to create vhost char dev
and other stuffs?
From: Jason Gunthorpe <hidden> Date: 2020-02-18 13:56:18
On Mon, Feb 17, 2020 at 02:08:03PM +0800, Jason Wang wrote:
I thought you were copied in the patch [1], maybe we can move vhost related
discussion there to avoid confusion.
[1] https://lwn.net/Articles/811210/
Wow, that is .. confusing.
So this is supposed to duplicate the uAPI of vhost-user? But it is
open coded and duplicated because .. vdpa?
So it's cheaper and simpler to introduce a new bus instead of refactoring a
well known bus and API where brunches of drivers and devices had been
implemented for years.
If you reason for this approach is to ease the implementation then you
should talk about it in the cover letters/etc
Maybe it is reasonable to do this because the rework is too great, I
don't know, but to me this whole thing looks rather messy.
Remember this stuff is all uAPI as it shows up in sysfs, so you can
easilly get stuck with it forever.
Jason
On Tue, Feb 18, 2020 at 01:56:12PM +0000, Jason Gunthorpe wrote:
On Mon, Feb 17, 2020 at 02:08:03PM +0800, Jason Wang wrote:
quoted
I thought you were copied in the patch [1], maybe we can move vhost related
discussion there to avoid confusion.
[1] https://lwn.net/Articles/811210/
Wow, that is .. confusing.
So this is supposed to duplicate the uAPI of vhost-user? But it is
open coded and duplicated because .. vdpa?
Do you mean the vhost-user in DPDK? There is no vhost-user
in Linux kernel.
Thanks,
Tiwei
quoted
So it's cheaper and simpler to introduce a new bus instead of refactoring a
well known bus and API where brunches of drivers and devices had been
implemented for years.
If you reason for this approach is to ease the implementation then you
should talk about it in the cover letters/etc
Maybe it is reasonable to do this because the rework is too great, I
don't know, but to me this whole thing looks rather messy.
Remember this stuff is all uAPI as it shows up in sysfs, so you can
easilly get stuck with it forever.
Jason
From: Jason Wang <hidden> Date: 2020-02-19 05:36:00
On 2020/2/18 下午9:56, Jason Gunthorpe wrote:
On Mon, Feb 17, 2020 at 02:08:03PM +0800, Jason Wang wrote:
quoted
I thought you were copied in the patch [1], maybe we can move vhost related
discussion there to avoid confusion.
[1] https://lwn.net/Articles/811210/
Wow, that is .. confusing.
So this is supposed to duplicate the uAPI of vhost-user?
It tries to reuse the uAPI of vhost with some extension.
But it is
open coded and duplicated because .. vdpa?
I'm not sure I get here, vhost module is reused for vhost-vdpa and all
current vhost device (e.g net) uses their own char device.
quoted
So it's cheaper and simpler to introduce a new bus instead of refactoring a
well known bus and API where brunches of drivers and devices had been
implemented for years.
If you reason for this approach is to ease the implementation then you
should talk about it in the cover letters/etc
I will add more rationale in both cover letter and this patch.
Thanks
Maybe it is reasonable to do this because the rework is too great, I
don't know, but to me this whole thing looks rather messy.
Remember this stuff is all uAPI as it shows up in sysfs, so you can
easilly get stuck with it forever.
Jason