From: Jason Wang <hidden> Date: 2020-01-16 12:42:58
Hi all:
Based on the comments and discussion for mdev based hardware virtio
offloading support[1]. A different approach to support vDPA device is
proposed in this series.
Instead of leveraging VFIO/mdev which may not work for some
vendors. This series tries to introduce a dedicated vDPA bus and
leverage vhost for userspace drivers. This help for the devices that
are not fit for VFIO and may reduce the conflict when try to propose a
bus template for virtual devices in [1].
The vDPA support is split into following parts:
1) vDPA core (bus, device and driver abstraction)
2) virtio vDPA transport for kernel virtio driver to control vDPA
device
3) vhost vDPA bus driver for userspace vhost driver to control vDPA
device
4) vendor vDPA drivers
5) management API
Both 1) and 2) are included in this series. Tiwei will work on part
3). For 4), Ling Shan will work and post IFCVF driver. For 5) we leave
it to vendor to implement, but it's better to come into an agreement
for management to create/configure/destroy vDPA device.
The sample driver is kept but renamed to vdap_sim. An on-chip IOMMU
implementation is added to sample device to make it work for both
kernel virtio driver and userspace vhost driver. It implements a sysfs
based management API, but it can switch to any other (e.g devlink) if
necessary.
Please refer each patch for more information.
Comments are welcomed.
[1] https://lkml.org/lkml/2019/11/18/261
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 | 434 +++++++++++++++++-
drivers/virtio/Kconfig | 15 +
drivers/virtio/Makefile | 2 +
drivers/virtio/vdpa/Kconfig | 26 ++
drivers/virtio/vdpa/Makefile | 3 +
drivers/virtio/vdpa/vdpa.c | 141 ++++++
drivers/virtio/vdpa/vdpa_sim.c | 796 +++++++++++++++++++++++++++++++++
drivers/virtio/virtio_vdpa.c | 400 +++++++++++++++++
include/linux/vdpa.h | 191 ++++++++
include/linux/vhost_iotlb.h | 45 ++
include/linux/vringh.h | 36 ++
19 files changed, 2327 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-01-16 12:43:10
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-01-16 12:43:29
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 | 434 +++++++++++++++++++++++++++++++++--
include/linux/vringh.h | 36 +++
3 files changed, 448 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,375 @@ 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)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)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)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)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)+{+/* Sane power of 2 please! */+if(!num||num>0xffff||(num&(num-1))){+vringh_bad("Bad ring size %u",num);+return-EINVAL;+}++vrh->little_endian=(features&(1ULL<<VIRTIO_F_VERSION_1));+vrh->event_indices=(features&(1<<VIRTIO_RING_F_EVENT_IDX));+vrh->weak_barriers=weak_barriers;+vrh->completed=0;+vrh->last_avail_idx=0;+vrh->last_used_idx=0;+vrh->vring.num=num;+vrh->vring.desc=desc;+vrh->vring.avail=avail;+vrh->vring.used=used;+return0;+}+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-01-16 12:43:43
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
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 | 141 ++++++++++++++++++++++++++
include/linux/vdpa.h | 191 +++++++++++++++++++++++++++++++++++
7 files changed, 347 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-01-16 12:43:58
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 | 400 +++++++++++++++++++++++++++++++++++
3 files changed, 414 insertions(+)
create mode 100644 drivers/virtio/virtio_vdpa.c
@@ -0,0 +1,400 @@+// 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"++#define to_virtio_vdpa_device(dev) \+container_of(dev,structvirtio_vdpa_device,vdev)++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;+};++staticstructvdpa_device*vd_get_vdpa(structvirtio_device*vdev)+{+structvirtio_vdpa_device*vd_dev=to_virtio_vdpa_device(vdev);+structvdpa_device*vdpa=vd_dev->vdpa;++returnvdpa;+}++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);+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_to_dev(vdpa));+}++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);+structvdpa_device*vdpa=vd_dev->vdpa;++devm_kfree(&vdpa->dev,vd_dev);+}++staticintvirtio_vdpa_probe(structdevice*dev)+{+structvdpa_device*vdpa=dev_to_vdpa(dev);+conststructvdpa_config_ops*ops=vdpa->config;+structvirtio_vdpa_device*vd_dev;+intrc;++vd_dev=devm_kzalloc(dev,sizeof(*vd_dev),GFP_KERNEL);+if(!vd_dev)+return-ENOMEM;++vd_dev->vdev.dev.parent=&vdpa->dev;+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)+return-ENODEV;++vd_dev->vdev.id.vendor=ops->get_vendor_id(vdpa);+rc=register_virtio_device(&vd_dev->vdev);+if(rc)+put_device(dev);+else+dev_set_drvdata(dev,vd_dev);++returnrc;+}++staticvoidvirtio_vdpa_remove(structdevice*dev)+{+structvirtio_vdpa_device*vd_dev=dev_get_drvdata(dev);++unregister_virtio_device(&vd_dev->vdev);+}++staticstructvdpa_drivervirtio_vdpa_driver={+.drv={+.name="virtio_vdpa",+},+.probe=virtio_vdpa_probe,+.remove=virtio_vdpa_remove,+};++staticint__initvirtio_vdpa_init(void)+{+returnregister_vdpa_driver(&virtio_vdpa_driver);+}++staticvoid__exitvirtio_vdpa_exit(void)+{+unregister_vdpa_driver(&virtio_vdpa_driver);+}++module_init(virtio_vdpa_init)+module_exit(virtio_vdpa_exit)++MODULE_VERSION(MOD_VERSION);+MODULE_LICENSE(MOD_LICENSE);+MODULE_AUTHOR(MOD_AUTHOR);+MODULE_DESCRIPTION(MOD_DESC);
From: Jason Wang <hidden> Date: 2020-01-16 12:44:07
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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
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.
Signed-off-by: Jason Wang <redacted>
---
drivers/virtio/vdpa/Kconfig | 17 +
drivers/virtio/vdpa/Makefile | 1 +
drivers/virtio/vdpa/vdpa_sim.c | 796 +++++++++++++++++++++++++++++++++
3 files changed, 814 insertions(+)
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
@@ -0,0 +1,796 @@+// 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{+structclass*vd_class;+structidrvd_idr;+structdevicedev;+structkobject*devices_kobj;+};++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_CLASS_NAME "vdpa_simulator"+#define VDPASIM_NAME "netdev"++u64vdpasim_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;+structlist_headnext;+guid_tuuid;+charname[64];+};++staticstructmutexvsim_list_lock;+staticstructlist_headvsim_devices_list;++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*)vq->desc_addr,+(structvring_avail*)vq->driver_addr,+(structvring_used*)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){+kfree(addr);+*dma_addr=DMA_MAPPING_ERROR;+}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((void*)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);++sysfs_remove_link(vdpasim_dev->devices_kobj,vdpasim->name);++mutex_lock(&vsim_list_lock);+list_del(&vdpasim->next);+mutex_unlock(&vsim_list_lock);++kfree(vdpasim->buffer);+kfree(vdpasim);+}++staticconststructvdpa_config_opsvdpasim_net_config_ops;++staticintvdpasim_create(constguid_t*uuid)+{+structvdpasim*vdpasim,*tmp;+structvirtio_net_config*config;+structvdpa_device*vdpa;+structdevice*dev;+intret=-ENOMEM;++mutex_lock(&vsim_list_lock);+list_for_each_entry(tmp,&vsim_devices_list,next){+if(guid_equal(&tmp->uuid,uuid)){+mutex_unlock(&vsim_list_lock);+return-EEXIST;+}+}++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);++guid_copy(&vdpasim->uuid,uuid);++list_add(&vdpasim->next,&vsim_devices_list);+vdpa=&vdpasim->vdpa;++mutex_unlock(&vsim_list_lock);++vdpa=&vdpasim->vdpa;+vdpa->config=&vdpasim_net_config_ops;+vdpa_set_parent(vdpa,&vdpasim_dev->dev);+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=register_vdpa_device(vdpa);+if(ret)+gotoerr_register;++sprintf(vdpasim->name,"%pU",uuid);++ret=sysfs_create_link(vdpasim_dev->devices_kobj,&vdpa->dev.kobj,+vdpasim->name);+if(ret)+gotoerr_link;++return0;++err_link:+err_register:+vhost_iotlb_free(vdpasim->iommu);+mutex_lock(&vsim_list_lock);+list_del(&vdpasim->next);+mutex_unlock(&vsim_list_lock);+err_iotlb:+kfree(vdpasim->buffer);+err_buffer_alloc:+kfree(vdpasim);+err_vdpa_alloc:+returnret;+}++staticintvdpasim_remove(constguid_t*uuid)+{+structvdpasim*vds,*tmp;+structvdpa_device*vdpa=NULL;+intret=-EINVAL;++mutex_lock(&vsim_list_lock);+list_for_each_entry_safe(vds,tmp,&vsim_devices_list,next){+if(guid_equal(&vds->uuid,uuid)){+vdpa=&vds->vdpa;+ret=0;+break;+}+}+mutex_unlock(&vsim_list_lock);++if(vdpa)+unregister_vdpa_device(vdpa);++returnret;+}++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;+}++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,+};++staticvoidvdpasim_device_release(structdevice*dev)+{+structvdpasim_dev*vdpasim_dev=+container_of(dev,structvdpasim_dev,dev);++vdpasim_dev->dev.bus=NULL;+idr_destroy(&vdpasim_dev->vd_idr);+class_destroy(vdpasim_dev->vd_class);+vdpasim_dev->vd_class=NULL;+kfree(vdpasim_dev);+}++staticssize_tcreate_store(structkobject*kobj,structkobj_attribute*attr,+constchar*buf,size_tcount)+{+char*str;+guid_tuuid;+intret;++if((count<UUID_STRING_LEN)||(count>UUID_STRING_LEN+1))+return-EINVAL;++str=kstrndup(buf,count,GFP_KERNEL);+if(!str)+return-ENOMEM;++ret=guid_parse(str,&uuid);+kfree(str);+if(ret)+returnret;++ret=vdpasim_create(&uuid);+if(ret)+returnret;++returncount;+}++staticssize_tremove_store(structkobject*kobj,structkobj_attribute*attr,+constchar*buf,size_tcount)+{+char*str;+guid_tuuid;+intret;++if((count<UUID_STRING_LEN)||(count>UUID_STRING_LEN+1))+return-EINVAL;++str=kstrndup(buf,count,GFP_KERNEL);+if(!str)+return-ENOMEM;++ret=guid_parse(str,&uuid);+kfree(str);+if(ret)+returnret;++ret=vdpasim_remove(&uuid);+if(ret)+returnret;++returncount;+}++staticstructkobj_attributecreate_attribute=__ATTR_WO(create);+staticstructkobj_attributeremove_attribute=__ATTR_WO(remove);++staticstructattribute*attrs[]={+&create_attribute.attr,+&remove_attribute.attr,+NULL,+};++staticstructattribute_groupattr_group={+.attrs=attrs,+};++staticint__initvdpasim_dev_init(void)+{+structdevice*dev;+intret=0;++vdpasim_dev=kzalloc(sizeof(*vdpasim_dev),GFP_KERNEL);+if(!vdpasim_dev)+return-ENOMEM;++idr_init(&vdpasim_dev->vd_idr);++vdpasim_dev->vd_class=class_create(THIS_MODULE,VDPASIM_CLASS_NAME);++if(IS_ERR(vdpasim_dev->vd_class)){+pr_err("Error: failed to register vdpasim_dev class\n");+ret=PTR_ERR(vdpasim_dev->vd_class);+gotoerr_class;+}++dev=&vdpasim_dev->dev;+dev->class=vdpasim_dev->vd_class;+dev->release=vdpasim_device_release;+dev_set_name(dev,"%s",VDPASIM_NAME);++ret=device_register(&vdpasim_dev->dev);+if(ret)+gotoerr_register;++ret=sysfs_create_group(&vdpasim_dev->dev.kobj,&attr_group);+if(ret)+gotoerr_create;++vdpasim_dev->devices_kobj=kobject_create_and_add("devices",+&dev->kobj);+if(!vdpasim_dev->devices_kobj){+ret=-ENOMEM;+gotoerr_devices;+}++mutex_init(&vsim_list_lock);+INIT_LIST_HEAD(&vsim_devices_list);++return0;++err_devices:+sysfs_remove_group(&vdpasim_dev->dev.kobj,&attr_group);+err_create:+device_unregister(&vdpasim_dev->dev);+err_register:+class_destroy(vdpasim_dev->vd_class);+err_class:+kfree(vdpasim_dev);+vdpasim_dev=NULL;+returnret;+}++staticvoid__exitvdpasim_dev_exit(void)+{+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-01-16 15:22:18
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
Usually we want to see symbols consistently prefixed with vdpa_*, is
there a reason why register/unregister are swapped?
+ int err;
+
+ if (!vdpa_get_parent(vdpa))
+ return -EINVAL;
+
+ if (!vdpa->config)
+ return -EINVAL;
+
+ err = ida_simple_get(&vdpa_index_ida, 0, 0, GFP_KERNEL);
+ if (err < 0)
+ return -EFAULT;
+
+ vdpa->dev.bus = &vdpa_bus;
+ device_initialize(&vdpa->dev);
IMHO device_initialize should not be called inside something called
register, toooften we find out that the caller drivers need the device
to be initialized earlier, ie to use the kref, or something.
I find the best flow is to have some init function that does the
device_initialize and sets the device_name that the driver can call
early.
Shouldn't there be a device/driver matching process of some kind?
Jason
It is unusual for the release function to not be owned by the
subsystem, through the class. I'm not sure there are enough module ref
counts to ensure that this function is not unloaded?
Usually to make this all work sanely the subsytem provides some
allocation function
vdpa_dev = vdpa_alloc_dev(parent, ops, sizeof(struct virtio_vdpa_device))
struct virtio_vdpa_device *priv = vdpa_priv(vdpa_dev)
Then the subsystem naturally owns all the memory.
Otherwise it gets tricky to ensure that the module doesn't unload
before all the krefs are put.
This is not right, the struct device lifetime is controled by a kref,
not via devm. If you want to use a devm unwind then the unwind is
put_device, not devm_kfree.
In this simple situation I don't see a reason to use devm.
And a ugly unwind like this is why you want to have device_initialize()
exposed to the driver, so there is a clear pairing that calling
device_initialize() must be followed by put_device. This should also
use the goto unwind style
Still a little unclear on binding, is this supposed to bind to all
vdpa devices?
Where is the various THIS_MODULE's I expect to see in a scheme like
this?
All function pointers must be protected by a held module reference
count, ie the above probe/remove and all the pointers in ops.
From: Jason Gunthorpe <hidden> Date: 2020-01-16 15:47:07
On Thu, Jan 16, 2020 at 08:42:31PM +0800, Jason Wang wrote:
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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
This is very gross, creating a class just to get a create/remove and
then not using the class for anything else? Yuk.
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
This is just begging for a netlink based approach.
Certainly netlink driven removal should be an agreeable standard for
all devices, I think.
The goto err_link does the wrong unwind, once register is completed
the error unwind is unregister & put_device, not kfree. This is why I
recommend to always initalize the device early, and always using
put_device during error unwinds.
This whole guid thing seems unncessary when the device is immediately
assigned a vdpa index from the ida. If you were not using syfs you'd
just return that index from the creation netlink.
Jason
From: Jason Wang <hidden> Date: 2020-01-17 03:03:38
On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
I think you meant ADI vs SF. It looks to me that ADI is limited to the
scope of scalable IOV but SF not.
Usually we want to see symbols consistently prefixed with vdpa_*, is
there a reason why register/unregister are swapped?
I follow the name from virtio. I will switch to vdpa_*.
quoted
+ int err;
+
+ if (!vdpa_get_parent(vdpa))
+ return -EINVAL;
+
+ if (!vdpa->config)
+ return -EINVAL;
+
+ err = ida_simple_get(&vdpa_index_ida, 0, 0, GFP_KERNEL);
+ if (err < 0)
+ return -EFAULT;
+
+ vdpa->dev.bus = &vdpa_bus;
+ device_initialize(&vdpa->dev);
IMHO device_initialize should not be called inside something called
register, toooften we find out that the caller drivers need the device
to be initialized earlier, ie to use the kref, or something.
I find the best flow is to have some init function that does the
device_initialize and sets the device_name that the driver can call
early.
Ok, will do.
Shouldn't there be a device/driver matching process of some kind?
The question is what do we want do match here.
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about
this. Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider
the two drivers so far (virtio/vhost) are all bus drivers.
Thanks
+ device driver over vDPA bus. For this to be useful, you need
+ an appropriate vDPA device implementation that operates on a
+ physical device to allow the datapath of virtio to be
use tab + 2 spaces above for indentation, not lots of spaces.
+ offloaded to hardware.
+
+ If unsure, say M.
+
config VIRTIO_PMEM
tristate "Support for virtio pmem driver"
depends on VIRTIO
From: Jason Wang <hidden> Date: 2020-01-17 09:33:22
On 2020/1/16 下午11:47, Jason Gunthorpe wrote:
On Thu, Jan 16, 2020 at 08:42:31PM +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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
This is very gross, creating a class just to get a create/remove and
then not using the class for anything else? Yuk.
It includes more information, e.g the devices and the link from vdpa_sim
device and vdpa device.
quoted
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
This is just begging for a netlink based approach.
Certainly netlink driven removal should be an agreeable standard for
all devices, I think.
Well, I think Parav had some proposals during the discussion of mdev
approach. But I'm not sure if he had any RFC codes for me to integrate
it into vdpasim.
Or do you want me to propose the netlink API? If yes, would you prefer
to a new virtio dedicated one or be a subset of devlink?
But it might be better to reach an agreement for all the vendors here.
Rob, Steve, Tiwei, Lingshan, Harpreet, Martin, Jakub, please share your
thoughts about the management API here.
The goto err_link does the wrong unwind, once register is completed
the error unwind is unregister & put_device, not kfree. This is why I
recommend to always initalize the device early, and always using
put_device during error unwinds.
Will fix.
This whole guid thing seems unncessary when the device is immediately
assigned a vdpa index from the ida.
The problem here is that user need to know which vdpa_sim is the one
that is just created.
If you were not using syfs you'd
just return that index from the creation netlink.
It is unusual for the release function to not be owned by the
subsystem, through the class.
This is how virtio_pci and virtio_mmio work now. Virtio devices may have
different transports which require different release functions. I think
this is the reason why virtio
I'm not sure there are enough module ref
counts to ensure that this function is not unloaded?
Let me double check this.
Usually to make this all work sanely the subsytem provides some
allocation function
vdpa_dev = vdpa_alloc_dev(parent, ops, sizeof(struct virtio_vdpa_device))
struct virtio_vdpa_device *priv = vdpa_priv(vdpa_dev)
Then the subsystem naturally owns all the memory.
Otherwise it gets tricky to ensure that the module doesn't unload
before all the krefs are put.
This is not right, the struct device lifetime is controled by a kref,
not via devm. If you want to use a devm unwind then the unwind is
put_device, not devm_kfree.
I'm not sure I get the point here. The lifetime is bound to underlying
vDPA device and devres allow to be freed before the vpda device is
released. But I agree using devres of underlying vdpa device looks wired.
In this simple situation I don't see a reason to use devm.
Still a little unclear on binding, is this supposed to bind to all
vdpa devices?
Yes, it expected to drive all vDPA devices.
Where is the various THIS_MODULE's I expect to see in a scheme like
this?
All function pointers must be protected by a held module reference
count, ie the above probe/remove and all the pointers in ops.
Will double check, since I don't see this in other virtio transport
drivers (PCI or MMIO).
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-01-17 12:13:52
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted hunk
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
quoted
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 | 141 ++++++++++++++++++++++++++
include/linux/vdpa.h | 191 +++++++++++++++++++++++++++++++++++
7 files changed, 347 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
This seems wrong: kicks are data path so drivers should not
do it in a vendor specific way. How about an API
returning the device/resource that can then be
mapped as appropriate?
+ * @set_vq_cb: Set the interrupt callback function for
+ * a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @cb: virtio-vdev interrupt callback structure
Calls are data path too, I think we need some way to map MSI?
+ * @set_vq_ready: Set ready status for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @ready: ready (true) not ready(false)
+ * @get_vq_ready: Get ready status for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * Returns boolean: ready (true) or not (false)
+ * @set_vq_state: Set the state for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @state: virtqueue state (last_avail_idx)
+ * Returns integer: success (0) or error (< 0)
+ * @get_vq_state: Get the state for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * Returns virtqueue state (last_avail_idx)
+ * @get_vq_align: Get the virtqueue align requirement
+ * for the device
+ * @vdev: vdpa device
+ * Returns virtqueue algin requirement
Where does this come from? Spec dictates that for a data path,
vendor specific values for this will break userspace ...
+ * @get_features: Get virtio features supported by the device
+ * @vdev: vdpa device
+ * Returns the virtio features support by the
+ * device
+ * @set_features: Set virtio features supported by the driver
+ * @vdev: vdpa device
+ * @features: feature support by the driver
+ * Returns integer: success (0) or error (< 0)
+ * @set_config_cb: Set the config interrupt callback
+ * @vdev: vdpa device
+ * @cb: virtio-vdev interrupt callback structure
+ * @get_vq_num_max: Get the max size of virtqueue
+ * @vdev: vdpa device
+ * Returns u16: max size of virtqueue
I'm not sure this has to be uniform across VQs.
+ * @get_device_id: Get virtio device id
+ * @vdev: vdpa device
+ * Returns u32: virtio device id
is this the virtio ID? PCI ID?
+ * @get_vendor_id: Get id for the vendor that provides this device
+ * @vdev: vdpa device
+ * Returns u32: virtio vendor id
what's the idea behind this? userspace normally doesn't interact with
this ... debugging?
+ * @get_status: Get the device status
+ * @vdev: vdpa device
+ * Returns u8: virtio device status
+ * @set_status: Set the device status
+ * @vdev: vdpa device
+ * @status: virtio device status
+ * @get_config: Read from device specific configuration space
+ * @vdev: vdpa device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to read to
+ * @len: the length to read from
+ * configuration space
+ * @set_config: Write to device specific configuration space
+ * @vdev: vdpa device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to write from
+ * @len: the length to write to
+ * configuration space
+ * @get_generation: Get device config generation (optional)
+ * @vdev: vdpa device
+ * Returns u32: device generation
+ * @set_map: Set device memory mapping, optional
+ * and only needed for device that using
+ * device specific DMA translation
+ * (on-chip IOMMU)
+ * @vdev: vdpa device
+ * @iotlb: vhost memory mapping to be
+ * used by the vDPA
+ * Returns integer: success (0) or error (< 0)
OK so any change just swaps in a completely new mapping?
Wouldn't this make minor changes such as memory hotplug
quite expensive?
From: Jason Wang <hidden> Date: 2020-01-17 13:53:00
On 2020/1/17 下午8:13, Michael S. Tsirkin wrote:
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
quoted
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 | 141 ++++++++++++++++++++++++++
include/linux/vdpa.h | 191 +++++++++++++++++++++++++++++++++++
7 files changed, 347 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
This seems wrong: kicks are data path so drivers should not
do it in a vendor specific way.
I'm not sure I get this since the doorbell is pretty vendor specific.
The idea here is to start form simple and common cases that can work for
both kernel virtio drivers and vhost:
- For kernel, kick_vq() is called from vq->notify() directly
- For vhost, vhost is in charge of hook eventfd to kick_vq()
How about an API
returning the device/resource that can then be
mapped as appropriate?
Yes, this could be a further optimization on top but not a must (only
work for e.g the doorbell does not share MMIO space with other
functions). For vhost we need something like this and need to hook it to
mmap() of vhost file descriptor.
quoted
+ * @set_vq_cb: Set the interrupt callback function for
+ * a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @cb: virtio-vdev interrupt callback structure
Calls are data path too, I think we need some way to map MSI?
Similarly, this could be a optimization on top, and we can start from
simple and common cases:
- For kernel, the vq callback could be mapped to MSI interrupt handler
directly
- For vhost, eventfd wakeup could be hook in the cb here
quoted
+ * @set_vq_ready: Set ready status for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @ready: ready (true) not ready(false)
+ * @get_vq_ready: Get ready status for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * Returns boolean: ready (true) or not (false)
+ * @set_vq_state: Set the state for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * @state: virtqueue state (last_avail_idx)
+ * Returns integer: success (0) or error (< 0)
+ * @get_vq_state: Get the state for a virtqueue
+ * @vdev: vdpa device
+ * @idx: virtqueue index
+ * Returns virtqueue state (last_avail_idx)
+ * @get_vq_align: Get the virtqueue align requirement
+ * for the device
+ * @vdev: vdpa device
+ * Returns virtqueue algin requirement
Where does this come from? Spec dictates that for a data path,
vendor specific values for this will break userspace ...
It comes from the align parameter of vring_create_virtqueue(). We can
expose the alignment to userspace if necessary. If it's not necessary, I
can drop this method here.
quoted
+ * @get_features: Get virtio features supported by the device
+ * @vdev: vdpa device
+ * Returns the virtio features support by the
+ * device
+ * @set_features: Set virtio features supported by the driver
+ * @vdev: vdpa device
+ * @features: feature support by the driver
+ * Returns integer: success (0) or error (< 0)
+ * @set_config_cb: Set the config interrupt callback
+ * @vdev: vdpa device
+ * @cb: virtio-vdev interrupt callback structure
+ * @get_vq_num_max: Get the max size of virtqueue
+ * @vdev: vdpa device
+ * Returns u16: max size of virtqueue
I'm not sure this has to be uniform across VQs.
Let me add an index parameter to this.
quoted
+ * @get_device_id: Get virtio device id
+ * @vdev: vdpa device
+ * Returns u32: virtio device id
is this the virtio ID? PCI ID?
Virtio ID
quoted
+ * @get_vendor_id: Get id for the vendor that provides this device
+ * @vdev: vdpa device
+ * Returns u32: virtio vendor id
what's the idea behind this? userspace normally doesn't interact with
this ... debugging?
This allows some vendor specific driver on top of vDPA bus. If this is
not interested, I can drop this.
quoted
+ * @get_status: Get the device status
+ * @vdev: vdpa device
+ * Returns u8: virtio device status
+ * @set_status: Set the device status
+ * @vdev: vdpa device
+ * @status: virtio device status
+ * @get_config: Read from device specific configuration space
+ * @vdev: vdpa device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to read to
+ * @len: the length to read from
+ * configuration space
+ * @set_config: Write to device specific configuration space
+ * @vdev: vdpa device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to write from
+ * @len: the length to write to
+ * configuration space
+ * @get_generation: Get device config generation (optional)
+ * @vdev: vdpa device
+ * Returns u32: device generation
+ * @set_map: Set device memory mapping, optional
+ * and only needed for device that using
+ * device specific DMA translation
+ * (on-chip IOMMU)
+ * @vdev: vdpa device
+ * @iotlb: vhost memory mapping to be
+ * used by the vDPA
+ * Returns integer: success (0) or error (< 0)
OK so any change just swaps in a completely new mapping?
Wouldn't this make minor changes such as memory hotplug
quite expensive?
My understanding is that the incremental updating of the on chip IOMMU
may degrade the performance. So vendor vDPA drivers may want to know
all the mappings at once. Technically, we can keep the incremental API
here and let the vendor vDPA drivers to record the full mapping
internally which may slightly increase the complexity of vendor driver.
We need more inputs from vendors here.
Thanks
From: Jason Gunthorpe <hidden> Date: 2020-01-17 13:54:49
On Fri, Jan 17, 2020 at 11:03:12AM +0800, Jason Wang wrote:
On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
I think you meant ADI vs SF. It looks to me that ADI is limited to the scope
of scalable IOV but SF not.
I think if one looks carefully you'd find that SF and ADI are using
very similar techiniques. For instance we'd also like to use the code
reorg of the MSIX vector setup with SFs that Intel is calling IMS.
Really SIOV is simply a bundle of pre-existing stuff under a tidy
name, whatever code skeleton we come up with for SFs should be re-used
for ADI.
quoted
Shouldn't there be a device/driver matching process of some kind?
The question is what do we want do match here.
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about this.
Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider the
two drivers so far (virtio/vhost) are all bus drivers.
As we seem to be contemplating some dynamic creation of vdpa devices I
think upon creation time it should be specified what mode they should
run it and then all driver binding and autoloading should happen
automatically. Telling the user to bind/unbind is a very poor
experience.
Jason
This is not right, the struct device lifetime is controled by a kref,
not via devm. If you want to use a devm unwind then the unwind is
put_device, not devm_kfree.
I'm not sure I get the point here. The lifetime is bound to underlying vDPA
device and devres allow to be freed before the vpda device is released. But
I agree using devres of underlying vdpa device looks wired.
Once device_initialize is called the only way to free a struct device
is via put_device, while here you have a devm trigger that will
unconditionally do kfree on a struct device without respecting the
reference count.
reference counted memory must never be allocated with devm.
And a ugly unwind like this is why you want to have device_initialize()
exposed to the driver,
In this context, which "driver" did you mean here? (Note, virtio-vdpa is the
driver for vDPA bus here).
'driver' is the thing using the 'core' library calls to implement a
device, so here the 'vd_dev' is the driver and
'register_virtio_device' is the core
quoted
Where is the various THIS_MODULE's I expect to see in a scheme like
this?
All function pointers must be protected by a held module reference
count, ie the above probe/remove and all the pointers in ops.
Will double check, since I don't see this in other virtio transport drivers
(PCI or MMIO).
pci_register_driver is a macro that provides a THIS_MODULE, and the
pci core code sets driver.owner, then the rest of the stuff related to
driver ops is supposed to work against that to protect the driver ops.
For the device module refcounting you either need to ensure that
'unregister' is a strong fence and guanentees that no device ops are
called past unregister (noting that this is impossible for release),
or you need to hold the module lock until release.
It is common to see non-core subsystems get this stuff wrong.
Jason
From: Jason Gunthorpe <hidden> Date: 2020-01-17 14:10:30
On Fri, Jan 17, 2020 at 05:32:39PM +0800, Jason Wang wrote:
On 2020/1/16 下午11:47, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:31PM +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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
This is very gross, creating a class just to get a create/remove and
then not using the class for anything else? Yuk.
It includes more information, e.g the devices and the link from vdpa_sim
device and vdpa device.
I feel like regardless of how the device is created there should be a
consistent virtio centric management for post-creation tasks, such as
introspection and destruction
A virto struct device should already have back pointers to it's parent
device, which should be enough to discover the vdpa_sim, none of the
extra sysfs munging should be needed.
quoted
quoted
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
This is just begging for a netlink based approach.
Certainly netlink driven removal should be an agreeable standard for
all devices, I think.
Well, I think Parav had some proposals during the discussion of mdev
approach. But I'm not sure if he had any RFC codes for me to integrate it
into vdpasim.
Or do you want me to propose the netlink API? If yes, would you prefer to a
new virtio dedicated one or be a subset of devlink?
From: kbuild test robot <hidden> Date: 2020-01-17 21:55:47
Hi Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linux/master linus/master v5.5-rc6 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: nios2-randconfig-a001-20200117 (attached as .config)
compiler: nios2-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All errors (new ones prefixed by >>):
drivers/vhost/vringh.o: In function `iotlb_translate':
quoted
drivers/vhost/vringh.c:1079: undefined reference to `vhost_iotlb_itree_first'
drivers/vhost/vringh.c:1079:(.text+0x45c): relocation truncated to fit: R_NIOS2_CALL26 against `vhost_iotlb_itree_first'
vim +1079 drivers/vhost/vringh.c
1061
1062 static int iotlb_translate(const struct vringh *vrh,
1063 u64 addr, u64 len, struct bio_vec iov[],
1064 int iov_size, u32 perm)
1065 {
1066 struct vhost_iotlb_map *map;
1067 struct vhost_iotlb *iotlb = vrh->iotlb;
1068 int ret = 0;
1069 u64 s = 0;
1070
1071 while (len > s) {
1072 u64 size, pa, pfn;
1073
1074 if (unlikely(ret >= iov_size)) {
1075 ret = -ENOBUFS;
1076 break;
1077 }
1078
1079 map = vhost_iotlb_itree_first(iotlb, addr,
1080 addr + len - 1);
1081 if (!map || map->start > addr) {
1082 ret = -EINVAL;
1083 break;
1084 } else if (!(map->perm & perm)) {
1085 ret = -EPERM;
1086 break;
1087 }
1088
1089 size = map->size - addr + map->start;
1090 pa = map->addr + addr - map->start;
1091 pfn = pa >> PAGE_SHIFT;
1092 iov[ret].bv_page = pfn_to_page(pfn);
1093 iov[ret].bv_len = min(len - s, size);
1094 iov[ret].bv_offset = pa & (PAGE_SIZE - 1);
1095 s += size;
1096 addr += size;
1097 ++ret;
1098 }
1099
1100 return ret;
1101 }
1102
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
From: kbuild test robot <hidden> Date: 2020-01-17 22:34:40
Hi Jason,
I love your patch! Perhaps something to improve:
[auto build test WARNING on vhost/linux-next]
[also build test WARNING on linux/master linus/master v5.5-rc6 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: sh-randconfig-a001-20200117 (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All warnings (new ones prefixed by >>):
drivers/vhost/vringh.c: In function 'copy_from_iotlb':
quoted
drivers/vhost/vringh.c:1110:29: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
ret = iotlb_translate(vrh, (u64)src, len, iov, 16, VHOST_MAP_RO);
^
drivers/vhost/vringh.c: In function 'copy_to_iotlb':
drivers/vhost/vringh.c:1128:29: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
ret = iotlb_translate(vrh, (u64)dst, len, iov, 16, VHOST_MAP_WO);
^
drivers/vhost/vringh.c: In function 'getu16_iotlb':
drivers/vhost/vringh.c:1145:29: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
ret = iotlb_translate(vrh, (u64)p, sizeof(*p),
^
drivers/vhost/vringh.c: In function 'putu16_iotlb':
drivers/vhost/vringh.c:1166:29: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
ret = iotlb_translate(vrh, (u64)p, sizeof(*p),
^
vim +1110 drivers/vhost/vringh.c
1102
1103 static inline int copy_from_iotlb(const struct vringh *vrh, void *dst,
1104 void *src, size_t len)
1105 {
1106 struct iov_iter iter;
1107 struct bio_vec iov[16];
1108 int ret;
1109
1110 ret = iotlb_translate(vrh, (u64)src, len, iov, 16, VHOST_MAP_RO);
1111 if (ret < 0)
1112 return ret;
1113
1114 iov_iter_bvec(&iter, READ, iov, ret, len);
1115
1116 ret = copy_from_iter(dst, len, &iter);
1117
1118 return ret;
1119 }
1120
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
From: kbuild test robot <hidden> Date: 2020-01-18 00:02:41
Hi Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linux/master linus/master v5.5-rc6 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-g003-20200117 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All errors (new ones prefixed by >>):
ld: drivers/vhost/vhost.o: in function `vhost_dev_reset_owner_prepare':
quoted
(.text+0x18b): undefined reference to `vhost_iotlb_alloc'
ld: drivers/vhost/vhost.o: in function `vhost_init_device_iotlb':
(.text+0x204): undefined reference to `vhost_iotlb_alloc'
quoted
ld: (.text+0x26c): undefined reference to `vhost_iotlb_free'
ld: drivers/vhost/vhost.o: in function `iotlb_access_ok':
quoted
vhost.c:(.text+0x60e): undefined reference to `vhost_iotlb_itree_first'
ld: drivers/vhost/vhost.o: in function `translate_desc':
vhost.c:(.text+0x85f): undefined reference to `vhost_iotlb_itree_first'
ld: drivers/vhost/vhost.o: in function `vhost_dev_cleanup':
quoted
(.text+0xd4a): undefined reference to `vhost_iotlb_free'
ld: (.text+0xd59): undefined reference to `vhost_iotlb_free'
ld: drivers/vhost/vhost.o: in function `vhost_chr_write_iter':
quoted
(.text+0x1c3b): undefined reference to `vhost_iotlb_del_range'
ld: (.text+0x1d78): undefined reference to `vhost_iotlb_add_range'
ld: drivers/vhost/vhost.o: in function `vhost_dev_ioctl':
(.text+0x3d6d): undefined reference to `vhost_iotlb_alloc'
ld: (.text+0x3dd5): undefined reference to `vhost_iotlb_add_range'
ld: (.text+0x3de4): undefined reference to `vhost_iotlb_free'
ld: (.text+0x3e7e): undefined reference to `vhost_iotlb_free'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
From: kbuild test robot <hidden> Date: 2020-01-18 00:41:16
Hi Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linux/master linus/master v5.5-rc6 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-d002-20200117 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All errors (new ones prefixed by >>):
ld: drivers/vhost/vhost.o: in function `iotlb_alloc':
quoted
drivers/vhost/vhost.c:582: undefined reference to `vhost_iotlb_alloc'
ld: drivers/vhost/vhost.c:582: undefined reference to `vhost_iotlb_alloc'
ld: drivers/vhost/vhost.o: in function `vhost_init_device_iotlb':
quoted
drivers/vhost/vhost.c:1667: undefined reference to `vhost_iotlb_free'
ld: drivers/vhost/vhost.o: in function `iotlb_access_ok':
quoted
drivers/vhost/vhost.c:1271: undefined reference to `vhost_iotlb_itree_first'
ld: drivers/vhost/vhost.o: in function `translate_desc':
drivers/vhost/vhost.c:1981: undefined reference to `vhost_iotlb_itree_first'
ld: drivers/vhost/vhost.o: in function `vhost_dev_cleanup':
drivers/vhost/vhost.c:658: undefined reference to `vhost_iotlb_free'
quoted
ld: drivers/vhost/vhost.c:660: undefined reference to `vhost_iotlb_free'
ld: drivers/vhost/vhost.o: in function `vhost_process_iotlb_msg':
quoted
drivers/vhost/vhost.c:1070: undefined reference to `vhost_iotlb_del_range'
ld: drivers/vhost/vhost.c:1056: undefined reference to `vhost_iotlb_add_range'
ld: drivers/vhost/vhost.o: in function `iotlb_alloc':
quoted
drivers/vhost/vhost.c:582: undefined reference to `vhost_iotlb_alloc'
ld: drivers/vhost/vhost.o: in function `vhost_set_memory':
quoted
drivers/vhost/vhost.c:1380: undefined reference to `vhost_iotlb_add_range'
ld: drivers/vhost/vhost.c:1407: undefined reference to `vhost_iotlb_free'
ld: drivers/vhost/vhost.c:1403: undefined reference to `vhost_iotlb_free'
vim +582 drivers/vhost/vhost.c
579
580 static struct vhost_iotlb *iotlb_alloc(void)
581 {
> 582 return vhost_iotlb_alloc(max_iotlb_entries,
583 VHOST_IOTLB_FLAG_RETIRE);
584 }
585
586 struct vhost_iotlb *vhost_dev_reset_owner_prepare(void)
587 {
588 return iotlb_alloc();
589 }
590 EXPORT_SYMBOL_GPL(vhost_dev_reset_owner_prepare);
591
592 /* Caller should have device mutex */
593 void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
594 {
595 int i;
596
597 vhost_dev_cleanup(dev);
598
599 dev->umem = umem;
600 /* We don't need VQ locks below since vhost_dev_cleanup makes sure
601 * VQs aren't running.
602 */
603 for (i = 0; i < dev->nvqs; ++i)
604 dev->vqs[i]->umem = umem;
605 }
606 EXPORT_SYMBOL_GPL(vhost_dev_reset_owner);
607
608 void vhost_dev_stop(struct vhost_dev *dev)
609 {
610 int i;
611
612 for (i = 0; i < dev->nvqs; ++i) {
613 if (dev->vqs[i]->kick && dev->vqs[i]->handle_kick) {
614 vhost_poll_stop(&dev->vqs[i]->poll);
615 vhost_poll_flush(&dev->vqs[i]->poll);
616 }
617 }
618 }
619 EXPORT_SYMBOL_GPL(vhost_dev_stop);
620
621 static void vhost_clear_msg(struct vhost_dev *dev)
622 {
623 struct vhost_msg_node *node, *n;
624
625 spin_lock(&dev->iotlb_lock);
626
627 list_for_each_entry_safe(node, n, &dev->read_list, node) {
628 list_del(&node->node);
629 kfree(node);
630 }
631
632 list_for_each_entry_safe(node, n, &dev->pending_list, node) {
633 list_del(&node->node);
634 kfree(node);
635 }
636
637 spin_unlock(&dev->iotlb_lock);
638 }
639
640 void vhost_dev_cleanup(struct vhost_dev *dev)
641 {
642 int i;
643
644 for (i = 0; i < dev->nvqs; ++i) {
645 if (dev->vqs[i]->error_ctx)
646 eventfd_ctx_put(dev->vqs[i]->error_ctx);
647 if (dev->vqs[i]->kick)
648 fput(dev->vqs[i]->kick);
649 if (dev->vqs[i]->call_ctx)
650 eventfd_ctx_put(dev->vqs[i]->call_ctx);
651 vhost_vq_reset(dev, dev->vqs[i]);
652 }
653 vhost_dev_free_iovecs(dev);
654 if (dev->log_ctx)
655 eventfd_ctx_put(dev->log_ctx);
656 dev->log_ctx = NULL;
657 /* No one will access memory at this point */
658 vhost_iotlb_free(dev->umem);
659 dev->umem = NULL;
> 660 vhost_iotlb_free(dev->iotlb);
661 dev->iotlb = NULL;
662 vhost_clear_msg(dev);
663 wake_up_interruptible_poll(&dev->wait, EPOLLIN | EPOLLRDNORM);
664 WARN_ON(!llist_empty(&dev->work_list));
665 if (dev->worker) {
666 kthread_stop(dev->worker);
667 dev->worker = NULL;
668 dev->kcov_handle = 0;
669 }
670 if (dev->mm)
671 mmput(dev->mm);
672 dev->mm = NULL;
673 }
674 EXPORT_SYMBOL_GPL(vhost_dev_cleanup);
675
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
From: kbuild test robot <hidden> Date: 2020-01-18 18:19:27
Hi Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linux/master linus/master v5.5-rc6 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All error/warnings (new ones prefixed by >>):
drivers/virtio/vdpa/vdpa_sim.c: In function 'vdpasim_queue_ready':
quoted
drivers/virtio/vdpa/vdpa_sim.c:101:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
false, (struct vring_desc *)vq->desc_addr,
^
drivers/virtio/vdpa/vdpa_sim.c:102:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
(struct vring_avail *)vq->driver_addr,
^
drivers/virtio/vdpa/vdpa_sim.c:103:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
(struct vring_used *)vq->device_addr);
^
--
From: Jason Wang <hidden> Date: 2020-01-20 07:50:37
On 2020/1/17 下午9:54, Jason Gunthorpe wrote:
On Fri, Jan 17, 2020 at 11:03:12AM +0800, Jason Wang wrote:
quoted
On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
I think you meant ADI vs SF. It looks to me that ADI is limited to the scope
of scalable IOV but SF not.
I think if one looks carefully you'd find that SF and ADI are using
very similar techiniques. For instance we'd also like to use the code
reorg of the MSIX vector setup with SFs that Intel is calling IMS.
Really SIOV is simply a bundle of pre-existing stuff under a tidy
name, whatever code skeleton we come up with for SFs should be re-used
for ADI.
Ok, but do you prefer to mention ADI only for the next version?
quoted
quoted
Shouldn't there be a device/driver matching process of some kind?
The question is what do we want do match here.
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about this.
Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider the
two drivers so far (virtio/vhost) are all bus drivers.
As we seem to be contemplating some dynamic creation of vdpa devices I
think upon creation time it should be specified what mode they should
run it and then all driver binding and autoloading should happen
automatically. Telling the user to bind/unbind is a very poor
experience.
Jason
Ok, I will add the type (virtio vs vhost) and driver matching method back.
Thanks
This is not right, the struct device lifetime is controled by a kref,
not via devm. If you want to use a devm unwind then the unwind is
put_device, not devm_kfree.
I'm not sure I get the point here. The lifetime is bound to underlying vDPA
device and devres allow to be freed before the vpda device is released. But
I agree using devres of underlying vdpa device looks wired.
Once device_initialize is called the only way to free a struct device
is via put_device, while here you have a devm trigger that will
unconditionally do kfree on a struct device without respecting the
reference count.
reference counted memory must never be allocated with devm.
And a ugly unwind like this is why you want to have device_initialize()
exposed to the driver,
In this context, which "driver" did you mean here? (Note, virtio-vdpa is the
driver for vDPA bus here).
'driver' is the thing using the 'core' library calls to implement a
device, so here the 'vd_dev' is the driver and
'register_virtio_device' is the core
Ok.
quoted
quoted
Where is the various THIS_MODULE's I expect to see in a scheme like
this?
All function pointers must be protected by a held module reference
count, ie the above probe/remove and all the pointers in ops.
Will double check, since I don't see this in other virtio transport drivers
(PCI or MMIO).
pci_register_driver is a macro that provides a THIS_MODULE, and the
pci core code sets driver.owner, then the rest of the stuff related to
driver ops is supposed to work against that to protect the driver ops.
For the device module refcounting you either need to ensure that
'unregister' is a strong fence and guanentees that no device ops are
called past unregister (noting that this is impossible for release),
or you need to hold the module lock until release.
It is common to see non-core subsystems get this stuff wrong.
Jason
From: Jason Wang <hidden> Date: 2020-01-20 08:01:37
On 2020/1/17 下午10:10, Jason Gunthorpe wrote:
On Fri, Jan 17, 2020 at 05:32:39PM +0800, Jason Wang wrote:
quoted
On 2020/1/16 下午11:47, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:31PM +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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
This is very gross, creating a class just to get a create/remove and
then not using the class for anything else? Yuk.
It includes more information, e.g the devices and the link from vdpa_sim
device and vdpa device.
I feel like regardless of how the device is created there should be a
consistent virtio centric management for post-creation tasks, such as
introspection and destruction
Right, actually, this is something that could be done by sysfs as well.
Having an intermediate steps as "activate" and introducing attributes
for post-creation tasks.
A virto struct device should already have back pointers to it's parent
device, which should be enough to discover the vdpa_sim, none of the
extra sysfs munging should be needed.
quoted
quoted
quoted
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
This is just begging for a netlink based approach.
Certainly netlink driven removal should be an agreeable standard for
all devices, I think.
Well, I think Parav had some proposals during the discussion of mdev
approach. But I'm not sure if he had any RFC codes for me to integrate it
into vdpasim.
Or do you want me to propose the netlink API? If yes, would you prefer to a
new virtio dedicated one or be a subset of devlink?
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-01-20 12:17:41
On Fri, Jan 17, 2020 at 01:54:42PM +0000, Jason Gunthorpe wrote:
quoted
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about this.
Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider the
two drivers so far (virtio/vhost) are all bus drivers.
As we seem to be contemplating some dynamic creation of vdpa devices I
think upon creation time it should be specified what mode they should
run it and then all driver binding and autoloading should happen
automatically. Telling the user to bind/unbind is a very poor
experience.
Maybe but OTOH it's an existing interface. I think we can reasonably
start with bind/unbind and then add ability to specify
the mode later. bind/unbind come from core so they will be
maintained anyway.
--
MST
From: Jason Gunthorpe <hidden> Date: 2020-01-20 17:51:01
On Mon, Jan 20, 2020 at 07:17:26AM -0500, Michael S. Tsirkin wrote:
On Fri, Jan 17, 2020 at 01:54:42PM +0000, Jason Gunthorpe wrote:
quoted
quoted
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about this.
Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider the
two drivers so far (virtio/vhost) are all bus drivers.
As we seem to be contemplating some dynamic creation of vdpa devices I
think upon creation time it should be specified what mode they should
run it and then all driver binding and autoloading should happen
automatically. Telling the user to bind/unbind is a very poor
experience.
Maybe but OTOH it's an existing interface. I think we can reasonably
start with bind/unbind and then add ability to specify
the mode later. bind/unbind come from core so they will be
maintained anyway.
Existing where? For vfio? vfio is the only thing I am aware doing
that, and this is not vfio..
Jason
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-01-20 21:56:22
On Mon, Jan 20, 2020 at 05:50:55PM +0000, Jason Gunthorpe wrote:
On Mon, Jan 20, 2020 at 07:17:26AM -0500, Michael S. Tsirkin wrote:
quoted
On Fri, Jan 17, 2020 at 01:54:42PM +0000, Jason Gunthorpe wrote:
quoted
quoted
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about this.
Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider the
two drivers so far (virtio/vhost) are all bus drivers.
As we seem to be contemplating some dynamic creation of vdpa devices I
think upon creation time it should be specified what mode they should
run it and then all driver binding and autoloading should happen
automatically. Telling the user to bind/unbind is a very poor
experience.
Maybe but OTOH it's an existing interface. I think we can reasonably
start with bind/unbind and then add ability to specify
the mode later. bind/unbind come from core so they will be
maintained anyway.
Existing where?
Driver core.
For vfio? vfio is the only thing I am aware doing
that, and this is not vfio..
Jason
vfio is not doing anything. anyone can use a combination
of unbind and driver_override to attach a driver to a device.
It's not a great interface but it's there without any code,
and it will stay there without maintainance overhead
if we later add a nicer one.
--
MST
From: Jason Wang <redacted>
Sent: Friday, January 17, 2020 11:03 AM
On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
the concept of VDEV includes both software bits and ADIs. If you
only take about hardware types, using ADI is more accurate.
quoted
quoted
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
I think you meant ADI vs SF. It looks to me that ADI is limited to the
scope of scalable IOV but SF not.
ADI is just a term for minimally assignable resource in Scalable IOV.
'assignable' implies several things, e.g. the resource can be independently
mapped to/accessed by user space or guest, DMAs between two
ADIs are isolated, operating one ADI doesn't affecting another ADI,
etc. I'm not clear about other vendor specific interfaces, but supposing
they need match the similar requirements. Then do we really want to
differentiate ADI vs. SF? What about merging them with ADI as just
one example of finer-grained slicing?
Usually we want to see symbols consistently prefixed with vdpa_*, is
there a reason why register/unregister are swapped?
I follow the name from virtio. I will switch to vdpa_*.
quoted
quoted
+ int err;
+
+ if (!vdpa_get_parent(vdpa))
+ return -EINVAL;
+
+ if (!vdpa->config)
+ return -EINVAL;
+
+ err = ida_simple_get(&vdpa_index_ida, 0, 0, GFP_KERNEL);
+ if (err < 0)
+ return -EFAULT;
+
+ vdpa->dev.bus = &vdpa_bus;
+ device_initialize(&vdpa->dev);
IMHO device_initialize should not be called inside something called
register, toooften we find out that the caller drivers need the device
to be initialized earlier, ie to use the kref, or something.
I find the best flow is to have some init function that does the
device_initialize and sets the device_name that the driver can call
early.
Ok, will do.
quoted
Shouldn't there be a device/driver matching process of some kind?
The question is what do we want do match here.
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about
this. Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider
the two drivers so far (virtio/vhost) are all bus drivers.
Thanks
From: Jason Wang
Sent: Thursday, January 16, 2020 8:42 PM
Hi all:
Based on the comments and discussion for mdev based hardware virtio
offloading support[1]. A different approach to support vDPA device is
proposed in this series.
Can you point to the actual link which triggered the direction change?
A quick glimpse in that thread doesn't reveal such information...
Instead of leveraging VFIO/mdev which may not work for some
vendors. This series tries to introduce a dedicated vDPA bus and
leverage vhost for userspace drivers. This help for the devices that
are not fit for VFIO and may reduce the conflict when try to propose a
bus template for virtual devices in [1].
The vDPA support is split into following parts:
1) vDPA core (bus, device and driver abstraction)
2) virtio vDPA transport for kernel virtio driver to control vDPA
device
3) vhost vDPA bus driver for userspace vhost driver to control vDPA
device
4) vendor vDPA drivers
5) management API
Both 1) and 2) are included in this series. Tiwei will work on part
3). For 4), Ling Shan will work and post IFCVF driver. For 5) we leave
it to vendor to implement, but it's better to come into an agreement
for management to create/configure/destroy vDPA device.
The sample driver is kept but renamed to vdap_sim. An on-chip IOMMU
implementation is added to sample device to make it work for both
kernel virtio driver and userspace vhost driver. It implements a sysfs
based management API, but it can switch to any other (e.g devlink) if
necessary.
Please refer each patch for more information.
Comments are welcomed.
[1] https://lkml.org/lkml/2019/11/18/261
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 | 434 +++++++++++++++++-
drivers/virtio/Kconfig | 15 +
drivers/virtio/Makefile | 2 +
drivers/virtio/vdpa/Kconfig | 26 ++
drivers/virtio/vdpa/Makefile | 3 +
drivers/virtio/vdpa/vdpa.c | 141 ++++++
drivers/virtio/vdpa/vdpa_sim.c | 796
+++++++++++++++++++++++++++++++++
drivers/virtio/virtio_vdpa.c | 400 +++++++++++++++++
include/linux/vdpa.h | 191 ++++++++
include/linux/vhost_iotlb.h | 45 ++
include/linux/vringh.h | 36 ++
19 files changed, 2327 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-01-21 09:39:42
On 2020/1/21 下午4:44, Tian, Kevin wrote:
quoted
From: Jason Wang
Sent: Thursday, January 16, 2020 8:42 PM
Hi all:
Based on the comments and discussion for mdev based hardware virtio
offloading support[1]. A different approach to support vDPA device is
proposed in this series.
Can you point to the actual link which triggered the direction change?
A quick glimpse in that thread doesn't reveal such information...
Instead of leveraging VFIO/mdev which may not work for some
vendors. This series tries to introduce a dedicated vDPA bus and
leverage vhost for userspace drivers. This help for the devices that
are not fit for VFIO and may reduce the conflict when try to propose a
bus template for virtual devices in [1].
The vDPA support is split into following parts:
1) vDPA core (bus, device and driver abstraction)
2) virtio vDPA transport for kernel virtio driver to control vDPA
device
3) vhost vDPA bus driver for userspace vhost driver to control vDPA
device
4) vendor vDPA drivers
5) management API
Both 1) and 2) are included in this series. Tiwei will work on part
3). For 4), Ling Shan will work and post IFCVF driver. For 5) we leave
it to vendor to implement, but it's better to come into an agreement
for management to create/configure/destroy vDPA device.
The sample driver is kept but renamed to vdap_sim. An on-chip IOMMU
implementation is added to sample device to make it work for both
kernel virtio driver and userspace vhost driver. It implements a sysfs
based management API, but it can switch to any other (e.g devlink) if
necessary.
Please refer each patch for more information.
Comments are welcomed.
[1] https://lkml.org/lkml/2019/11/18/261
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 | 434 +++++++++++++++++-
drivers/virtio/Kconfig | 15 +
drivers/virtio/Makefile | 2 +
drivers/virtio/vdpa/Kconfig | 26 ++
drivers/virtio/vdpa/Makefile | 3 +
drivers/virtio/vdpa/vdpa.c | 141 ++++++
drivers/virtio/vdpa/vdpa_sim.c | 796
+++++++++++++++++++++++++++++++++
drivers/virtio/virtio_vdpa.c | 400 +++++++++++++++++
include/linux/vdpa.h | 191 ++++++++
include/linux/vhost_iotlb.h | 45 ++
include/linux/vringh.h | 36 ++
19 files changed, 2327 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-01-21 09:41:51
On 2020/1/21 下午4:40, Tian, Kevin wrote:
quoted
From: Jason Wang <redacted>
Sent: Friday, January 17, 2020 11:03 AM
On 2020/1/16 下午11:22, Jason Gunthorpe wrote:
quoted
On Thu, Jan 16, 2020 at 08:42:29PM +0800, Jason Wang wrote:
quoted
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
- VDEV (Virtual Device) - With technologies such as Intel Scalable
IOV, a virtual device composed by host OS utilizing one or more
ADIs.
the concept of VDEV includes both software bits and ADIs. If you
only take about hardware types, using ADI is more accurate.
Ok.
quoted
quoted
quoted
- SF (Sub function) - Vendor specific interface to slice the Physical
Function to multiple sub functions that can be assigned to different
partitions as virtual devices.
I really hope we don't end up with two different ways to spell this
same thing.
I think you meant ADI vs SF. It looks to me that ADI is limited to the
scope of scalable IOV but SF not.
ADI is just a term for minimally assignable resource in Scalable IOV.
'assignable' implies several things, e.g. the resource can be independently
mapped to/accessed by user space or guest, DMAs between two
ADIs are isolated, operating one ADI doesn't affecting another ADI,
etc. I'm not clear about other vendor specific interfaces, but supposing
they need match the similar requirements. Then do we really want to
differentiate ADI vs. SF? What about merging them with ADI as just
one example of finer-grained slicing?
I think so. That what Jason G want as well.
Thanks
Usually we want to see symbols consistently prefixed with vdpa_*, is
there a reason why register/unregister are swapped?
I follow the name from virtio. I will switch to vdpa_*.
quoted
quoted
+ int err;
+
+ if (!vdpa_get_parent(vdpa))
+ return -EINVAL;
+
+ if (!vdpa->config)
+ return -EINVAL;
+
+ err = ida_simple_get(&vdpa_index_ida, 0, 0, GFP_KERNEL);
+ if (err < 0)
+ return -EFAULT;
+
+ vdpa->dev.bus = &vdpa_bus;
+ device_initialize(&vdpa->dev);
IMHO device_initialize should not be called inside something called
register, toooften we find out that the caller drivers need the device
to be initialized earlier, ie to use the kref, or something.
I find the best flow is to have some init function that does the
device_initialize and sets the device_name that the driver can call
early.
Ok, will do.
quoted
Shouldn't there be a device/driver matching process of some kind?
The question is what do we want do match here.
1) "virtio" vs "vhost", I implemented matching method for this in mdev
series, but it looks unnecessary for vDPA device driver to know about
this. Anyway we can use sysfs driver bind/unbind to switch drivers
2) virtio device id and vendor id. I'm not sure we need this consider
the two drivers so far (virtio/vhost) are all bus drivers.
Thanks
From: Jason Gunthorpe <hidden> Date: 2020-01-21 14:12:14
On Mon, Jan 20, 2020 at 04:56:06PM -0500, Michael S. Tsirkin wrote:
quoted
For vfio? vfio is the only thing I am aware doing
that, and this is not vfio..
vfio is not doing anything. anyone can use a combination
of unbind and driver_override to attach a driver to a device.
It's not a great interface but it's there without any code,
and it will stay there without maintainance overhead
if we later add a nicer one.
Well, it is not a great interface, and it is only really used in
normal cases by vfio.
I don't think it is a good idea to design new subsystems with that
idea in mind, particularly since detatching the vdpa driver would not
trigger destruction of the underlying dynamic resource (ie the SF).
We need a way to trigger that destruction..
Jason
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-01-21 14:15:33
On Tue, Jan 21, 2020 at 02:12:05PM +0000, Jason Gunthorpe wrote:
On Mon, Jan 20, 2020 at 04:56:06PM -0500, Michael S. Tsirkin wrote:
quoted
quoted
For vfio? vfio is the only thing I am aware doing
that, and this is not vfio..
vfio is not doing anything. anyone can use a combination
of unbind and driver_override to attach a driver to a device.
It's not a great interface but it's there without any code,
and it will stay there without maintainance overhead
if we later add a nicer one.
Well, it is not a great interface, and it is only really used in
normal cases by vfio.
I don't think it is a good idea to design new subsystems with that
idea in mind, particularly since detatching the vdpa driver would not
trigger destruction of the underlying dynamic resource (ie the SF).
We need a way to trigger that destruction..
Jason
You wanted a netlink command for this, right?
--
MST
From: Jason Gunthorpe <hidden> Date: 2020-01-21 14:16:45
On Tue, Jan 21, 2020 at 09:15:14AM -0500, Michael S. Tsirkin wrote:
On Tue, Jan 21, 2020 at 02:12:05PM +0000, Jason Gunthorpe wrote:
quoted
On Mon, Jan 20, 2020 at 04:56:06PM -0500, Michael S. Tsirkin wrote:
quoted
quoted
For vfio? vfio is the only thing I am aware doing
that, and this is not vfio..
vfio is not doing anything. anyone can use a combination
of unbind and driver_override to attach a driver to a device.
It's not a great interface but it's there without any code,
and it will stay there without maintainance overhead
if we later add a nicer one.
Well, it is not a great interface, and it is only really used in
normal cases by vfio.
I don't think it is a good idea to design new subsystems with that
idea in mind, particularly since detatching the vdpa driver would not
trigger destruction of the underlying dynamic resource (ie the SF).
We need a way to trigger that destruction..
Jason
You wanted a netlink command for this, right?
It is my suggestion.
Based on experiance here we started out with sysfs and it was OK, but
slow. When we added container support the entire sysfs thing
completely exploded and we had to replace it with netlink.
Jason
From: Dan Carpenter <hidden> Date: 2020-01-28 03:33:46
Hi Jason,
url: https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
Reported-by: Dan Carpenter <redacted>
smatch warnings:
drivers/virtio/vdpa/vdpa_sim.c:288 vdpasim_alloc_coherent() warn: returning freed memory 'addr'
# https://github.com/0day-ci/linux/commit/55047769b3e974d68b2aab5ce0022459b172a23f
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 55047769b3e974d68b2aab5ce0022459b172a23f
vim +/addr +288 drivers/virtio/vdpa/vdpa_sim.c
55047769b3e974 Jason Wang 2020-01-16 263 static void *vdpasim_alloc_coherent(struct device *dev, size_t size,
55047769b3e974 Jason Wang 2020-01-16 264 dma_addr_t *dma_addr, gfp_t flag,
55047769b3e974 Jason Wang 2020-01-16 265 unsigned long attrs)
55047769b3e974 Jason Wang 2020-01-16 266 {
55047769b3e974 Jason Wang 2020-01-16 267 struct vdpa_device *vdpa = dev_to_vdpa(dev);
55047769b3e974 Jason Wang 2020-01-16 268 struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
55047769b3e974 Jason Wang 2020-01-16 269 struct vhost_iotlb *iommu = vdpasim->iommu;
55047769b3e974 Jason Wang 2020-01-16 270 void *addr = kmalloc(size, flag);
55047769b3e974 Jason Wang 2020-01-16 271 int ret;
55047769b3e974 Jason Wang 2020-01-16 272
55047769b3e974 Jason Wang 2020-01-16 273 if (!addr)
55047769b3e974 Jason Wang 2020-01-16 274 *dma_addr = DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 275 else {
55047769b3e974 Jason Wang 2020-01-16 276 u64 pa = virt_to_phys(addr);
55047769b3e974 Jason Wang 2020-01-16 277
55047769b3e974 Jason Wang 2020-01-16 278 ret = vhost_iotlb_add_range(iommu, (u64)pa,
55047769b3e974 Jason Wang 2020-01-16 279 (u64)pa + size - 1,
55047769b3e974 Jason Wang 2020-01-16 280 pa, VHOST_MAP_RW);
55047769b3e974 Jason Wang 2020-01-16 281 if (ret) {
55047769b3e974 Jason Wang 2020-01-16 282 kfree(addr);
^^^^^^^^^^^
55047769b3e974 Jason Wang 2020-01-16 283 *dma_addr = DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 284 } else
55047769b3e974 Jason Wang 2020-01-16 285 *dma_addr = (dma_addr_t)pa;
55047769b3e974 Jason Wang 2020-01-16 286 }
55047769b3e974 Jason Wang 2020-01-16 287
55047769b3e974 Jason Wang 2020-01-16 @288 return addr;
^^^^^^^^^^^^
55047769b3e974 Jason Wang 2020-01-16 289 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
Reported-by: Dan Carpenter <redacted>
smatch warnings:
drivers/virtio/vdpa/vdpa_sim.c:288 vdpasim_alloc_coherent() warn: returning freed memory 'addr'
# https://github.com/0day-ci/linux/commit/55047769b3e974d68b2aab5ce0022459b172a23f
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 55047769b3e974d68b2aab5ce0022459b172a23f
vim +/addr +288 drivers/virtio/vdpa/vdpa_sim.c
55047769b3e974 Jason Wang 2020-01-16 263 static void *vdpasim_alloc_coherent(struct device *dev, size_t size,
55047769b3e974 Jason Wang 2020-01-16 264 dma_addr_t *dma_addr, gfp_t flag,
55047769b3e974 Jason Wang 2020-01-16 265 unsigned long attrs)
55047769b3e974 Jason Wang 2020-01-16 266 {
55047769b3e974 Jason Wang 2020-01-16 267 struct vdpa_device *vdpa = dev_to_vdpa(dev);
55047769b3e974 Jason Wang 2020-01-16 268 struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
55047769b3e974 Jason Wang 2020-01-16 269 struct vhost_iotlb *iommu = vdpasim->iommu;
55047769b3e974 Jason Wang 2020-01-16 270 void *addr = kmalloc(size, flag);
55047769b3e974 Jason Wang 2020-01-16 271 int ret;
55047769b3e974 Jason Wang 2020-01-16 272
55047769b3e974 Jason Wang 2020-01-16 273 if (!addr)
55047769b3e974 Jason Wang 2020-01-16 274 *dma_addr = DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 275 else {
55047769b3e974 Jason Wang 2020-01-16 276 u64 pa = virt_to_phys(addr);
55047769b3e974 Jason Wang 2020-01-16 277
55047769b3e974 Jason Wang 2020-01-16 278 ret = vhost_iotlb_add_range(iommu, (u64)pa,
55047769b3e974 Jason Wang 2020-01-16 279 (u64)pa + size - 1,
55047769b3e974 Jason Wang 2020-01-16 280 pa, VHOST_MAP_RW);
55047769b3e974 Jason Wang 2020-01-16 281 if (ret) {
55047769b3e974 Jason Wang 2020-01-16 282 kfree(addr);
^^^^^^^^^^^
55047769b3e974 Jason Wang 2020-01-16 283 *dma_addr = DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 284 } else
55047769b3e974 Jason Wang 2020-01-16 285 *dma_addr = (dma_addr_t)pa;
55047769b3e974 Jason Wang 2020-01-16 286 }
55047769b3e974 Jason Wang 2020-01-16 287
55047769b3e974 Jason Wang 2020-01-16 @288 return addr;
^^^^^^^^^^^^
55047769b3e974 Jason Wang 2020-01-16 289 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
From: Jason Wang <hidden> Date: 2020-02-04 04:19:55
On 2020/1/17 下午10:10, Jason Gunthorpe wrote:
quoted
quoted
quoted
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
This is just begging for a netlink based approach.
Certainly netlink driven removal should be an agreeable standard for
all devices, I think.
Well, I think Parav had some proposals during the discussion of mdev
approach. But I'm not sure if he had any RFC codes for me to integrate it
into vdpasim.
Or do you want me to propose the netlink API? If yes, would you prefer to a
new virtio dedicated one or be a subset of devlink?
Well, lets see what feed back Parav has
Jason
Hi Parav:
Do you have any update on this? If it still require sometime, I will
post V2 that sticks to sysfs based API.
Thanks
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.
A sysfs based management interface is implemented, devices are
created and removed through:
/sys/devices/virtual/vdpa_simulator/netdev/{create|remove}
Netlink based lifecycle management could be implemented for vDPA
simulator as well.
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.
Signed-off-by: Jason Wang <redacted>
---
drivers/virtio/vdpa/Kconfig | 17 +
drivers/virtio/vdpa/Makefile | 1 +
drivers/virtio/vdpa/vdpa_sim.c | 796 +++++++++++++++++++++++++++++++++
3 files changed, 814 insertions(+)
create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
@@ -0,0 +1,796 @@+// 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{+structclass*vd_class;+structidrvd_idr;+structdevicedev;+structkobject*devices_kobj;+};++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_CLASS_NAME "vdpa_simulator"+#define VDPASIM_NAME "netdev"++u64vdpasim_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;+structlist_headnext;+guid_tuuid;+charname[64];+};++staticstructmutexvsim_list_lock;+staticstructlist_headvsim_devices_list;++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*)vq->desc_addr,+(structvring_avail*)vq->driver_addr,+(structvring_used*)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){+kfree(addr);+*dma_addr=DMA_MAPPING_ERROR;+}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((void*)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,+};+
Hey Jason,
IMHO, it would be nice if dma_ops of the parent device could be re-used.
vdpa_device is expecting to represent a physical device except this
simulator, however, there are not enough information in vdpa_device.dev
to indicating which kind physical device it attached to. Namely
get_arch_dma_ops(struct bus type) can not work on vdpa_device.dev. Then
it seems device drivers need to implement a wrap of dma_ops of parent
devices. Can this work be done in the vdpa framework since it looks like
a common task? Can "vd_dev->vdev.dev.parent = vdpa->dev->parent;" in
virtio_vdpa_probe() do the work?
Thanks,
BR
Zhu Lingshan
Hey Jason,
IMHO, it would be nice if dma_ops of the parent device could be
re-used. vdpa_device is expecting to represent a physical device
except this simulator, however, there are not enough information in
vdpa_device.dev to indicating which kind physical device it attached
to. Namely get_arch_dma_ops(struct bus type) can not work on
vdpa_device.dev. Then it seems device drivers need to implement a wrap
of dma_ops of parent devices. Can this work be done in the vdpa
framework since it looks like a common task? Can
"vd_dev->vdev.dev.parent = vdpa->dev->parent;" in virtio_vdpa_probe()
do the work?
Thanks,
BR
Zhu Lingshan
Hey Jason,
IMHO, it would be nice if dma_ops of the parent device could be re-used.
vdpa_device is expecting to represent a physical device except this
simulator, however, there are not enough information in vdpa_device.dev
to indicating which kind physical device it attached to. Namely
get_arch_dma_ops(struct bus type) can not work on vdpa_device.dev. Then
it seems device drivers need to implement a wrap of dma_ops of parent
devices. Can this work be done in the vdpa framework since it looks like
a common task? Can "vd_dev->vdev.dev.parent = vdpa->dev->parent;" in
virtio_vdpa_probe() do the work?
Thanks,
BR
Zhu Lingshan
Good catch.
I think we can.
IMHO you need to specify some 'dma_device', not try and play tricks
with dma_ops, or assuming the parent is always the device used for
dma.
Jason
Hey Jason,
IMHO, it would be nice if dma_ops of the parent device could be re-used.
vdpa_device is expecting to represent a physical device except this
simulator, however, there are not enough information in vdpa_device.dev
to indicating which kind physical device it attached to. Namely
get_arch_dma_ops(struct bus type) can not work on vdpa_device.dev. Then
it seems device drivers need to implement a wrap of dma_ops of parent
devices. Can this work be done in the vdpa framework since it looks like
a common task? Can "vd_dev->vdev.dev.parent = vdpa->dev->parent;" in
virtio_vdpa_probe() do the work?
Thanks,
BR
Zhu Lingshan
Good catch.
I think we can.
IMHO you need to specify some 'dma_device', not try and play tricks
with dma_ops, or assuming the parent is always the device used for
dma.
Jason
Right, this is what in my mind and discussed in the vhost-vdpa thread.
Will go this way.
Thanks