Re: [PATCH v2] virtio-iommu: Fix the partial copy of probe request

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Thu, Jun 23, 2022 at 01:40:58AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >Sent: Wednesday, June 22, 2022 9:58 PM
> >To: Eric Auger <eric.auger@redhat.com>
> >Cc: Duan, Zhenzhong <zhenzhong.duan@intel.com>; qemu-
> >devel@nongnu.org; mst@redhat.com
> >Subject: Re: [PATCH v2] virtio-iommu: Fix the partial copy of probe request
> >
> >On Wed, Jun 22, 2022 at 02:22:18PM +0200, Eric Auger wrote:
> >> >> the spec is pretty confusing here though (virtio-v1.2-csd01.pdf) as
> >> >> it presents the struct as follows:
> >> >>
> >> >> struct virtio_iommu_req_probe {
> >> >> struct virtio_iommu_req_head head;
> >> >> /* Device-readable */
> >> >> le32 endpoint;
> >> >> u8 reserved[64];
> >> >>
> >> >> /* Device-writable */
> >> >> u8 properties[probe_size];
> >> >> struct virtio_iommu_req_tail tail;
> >> >> };
> >> > Hm, which part is confusing?  Yes it's not valid C since probe_size
> >> > is defined dynamically ('probe_size' in the device config), but I
> >> > thought it would be nicer to show the whole request layout this way.
> >> > Besides, at least virtio-blk and virtio-scsi have similar
> >> > variable-sized arrays in their definitions
> >> the fact "struct virtio_iommu_req_tail tail;" was part of the
> >>
> >> virtio_iommu_req_probe struct
> >
> >Right, it would have been better to use a different name than
> >virtio_iommu_req_probe in virtio_iommu.h, to make the pitfall clear.
> >
> Maybe virtio_iommu_req_probe_no_tail?

Yes, we can't change the probe struct anymore since it's API, but we could
use the no_tail prefix on future structs

Thanks,
Jean