Re: [PATCH V4 5/5] vdpasim: vDPA device simulator

[Jason Wang]

On 2020/2/21 äå3:57, Jason Wang wrote:
> On 2020/2/20 äå11:12, Jason Gunthorpe wrote:
> > On Thu, Feb 20, 2020 at 02:11:41PM +0800, Jason Wang wrote:
> > > +static void vdpasim_device_release(struct device *dev)
> > > +{
> > > +ÂÂÂ struct vdpasim *vdpasim = dev_to_sim(dev);
> > > +
> > > +ÂÂÂ cancel_work_sync(&vdpasim->work);
> > > +ÂÂÂ kfree(vdpasim->buffer);
> > > +ÂÂÂ vhost_iotlb_free(vdpasim->iommu);
> > > +ÂÂÂ kfree(vdpasim);
> > > +}
> > > +
> > > +static struct vdpasim *vdpasim_create(void)
> > > +{
> > > +ÂÂÂ struct virtio_net_config *config;
> > > +ÂÂÂ struct vhost_iotlb *iommu;
> > > +ÂÂÂ struct vdpasim *vdpasim;
> > > +ÂÂÂ struct device *dev;
> > > +ÂÂÂ void *buffer;
> > > +ÂÂÂ int ret = -ENOMEM;
> > > +
> > > +ÂÂÂ iommu = vhost_iotlb_alloc(2048, 0);
> > > +ÂÂÂ if (!iommu)
> > > +ÂÂÂÂÂÂÂ goto err;
> > > +
> > > +ÂÂÂ buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > > +ÂÂÂ if (!buffer)
> > > +ÂÂÂÂÂÂÂ goto err_buffer;
> > > +
> > > +ÂÂÂ vdpasim = kzalloc(sizeof(*vdpasim), GFP_KERNEL);
> > > +ÂÂÂ if (!vdpasim)
> > > +ÂÂÂÂÂÂÂ goto err_alloc;
> > > +
> > > +ÂÂÂ vdpasim->buffer = buffer;
> > > +ÂÂÂ vdpasim->iommu = iommu;
> > > +
> > > +ÂÂÂ 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);
> > > +
> > > +ÂÂÂ vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
> > > +ÂÂÂ vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
> > > +
> > > +ÂÂÂ dev = &vdpasim->dev;
> > > +ÂÂÂ dev->release = vdpasim_device_release;
> > > +ÂÂÂ dev->coherent_dma_mask = DMA_BIT_MASK(64);
> > > +ÂÂÂ set_dma_ops(dev, &vdpasim_dma_ops);
> > > +ÂÂÂ dev_set_name(dev, "%s", VDPASIM_NAME);
> > > +
> > > +ÂÂÂ ret = device_register(&vdpasim->dev);
> > > +ÂÂÂ if (ret)
> > > +ÂÂÂÂÂÂÂ goto err_init;
> > It is a bit weird to be creating this dummy parent, couldn't this be
> > done by just passing a NULL parent to vdpa_alloc_device, doing
> > set_dma_ops() on the vdpasim->vdpa->dev and setting dma_device to
> > vdpasim->vdpa->dev ?
>
>
> I think it works.


Rethink about this, since most hardware vDPA driver will have a parent 
and will use it to find the parent structure e.g

dev_get_drvdata(vdpa->dev->parent)

So I keep this dummy parent in V5.

Thanks