Jason Wang
2021-Jul-01 07:55 UTC
[PATCH v8 09/10] vduse: Introduce VDUSE - vDPA Device in Userspace
? 2021/7/1 ??2:50, Yongji Xie ??:> On Wed, Jun 30, 2021 at 5:51 PM Stefan Hajnoczi <stefanha at redhat.com> wrote: >> On Tue, Jun 29, 2021 at 10:59:51AM +0800, Yongji Xie wrote: >>> On Mon, Jun 28, 2021 at 9:02 PM Stefan Hajnoczi <stefanha at redhat.com> wrote: >>>> On Tue, Jun 15, 2021 at 10:13:30PM +0800, Xie Yongji wrote: >>>>> +/* ioctls */ >>>>> + >>>>> +struct vduse_dev_config { >>>>> + char name[VDUSE_NAME_MAX]; /* vduse device name */ >>>>> + __u32 vendor_id; /* virtio vendor id */ >>>>> + __u32 device_id; /* virtio device id */ >>>>> + __u64 features; /* device features */ >>>>> + __u64 bounce_size; /* bounce buffer size for iommu */ >>>>> + __u16 vq_size_max; /* the max size of virtqueue */ >>>> The VIRTIO specification allows per-virtqueue sizes. A device can have >>>> two virtqueues, where the first one allows up to 1024 descriptors and >>>> the second one allows only 128 descriptors, for example. >>>> >>> Good point! But it looks like virtio-vdpa/virtio-pci doesn't support >>> that now. All virtqueues have the same maximum size. >> I see struct vpda_config_ops only supports a per-device max vq size: >> u16 (*get_vq_num_max)(struct vdpa_device *vdev); >> >> virtio-pci supports per-virtqueue sizes because the struct >> virtio_pci_common_cfg->queue_size register is per-queue (controlled by >> queue_select). >> > Oh, yes. I miss queue_select. > >> I guess this is a question for Jason: will vdpa will keep this limitation? >> If yes, then VDUSE can stick to it too without running into problems in >> the future.I think it's better to extend the get_vq_num_max() per virtqueue. Currently, vDPA assumes the parent to have a global max size. This seems to work on most of the parents but not vp-vDPA (which could be backed by QEMU, in that case cvq's size is smaller). Fortunately, we haven't enabled had cvq support in the userspace now. I can post the fixes.>> >>>>> + __u16 padding; /* padding */ >>>>> + __u32 vq_num; /* the number of virtqueues */ >>>>> + __u32 vq_align; /* the allocation alignment of virtqueue's metadata */ >>>> I'm not sure what this is? >>>> >>> This will be used by vring_create_virtqueue() too. >> If there is no official definition for the meaning of this value then >> "/* same as vring_create_virtqueue()'s vring_align parameter */" would >> be clearer. That way the reader knows what to research in order to >> understand how this field works. >> > OK. > >> I don't remember but maybe it was used to support vrings when the >> host/guest have non-4KB page sizes. I wonder if anyone has an official >> definition for this value? > Not sure. Maybe we might need some alignment which is less than > PAGE_SIZE sometimes.So I see CCW always use 4096, but I'm not sure whether or not it's smaller than PAGE_SIZE. Thanks> > Thanks, > Yongji >