search for: vdpa_iova_rang

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Changes from V1: - do not mandate

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Thanks Jason Wang (4): vdpa:

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Thanks Jason Wang (4): vdpa:

...pa.h > +++ b/include/linux/vdpa.h > @@ -41,6 +41,16 @@ struct vdpa_device { > unsigned int index; > }; > > +/** > + * vDPA IOVA range - the IOVA range support by the device > + * @start: start of the IOVA range > + * @end: end of the IOVA range > + */ > +struct vdpa_iova_range { > + u64 start; > + u64 end; > +}; > + This is ambiguous. Is end in the range or just behind it? How about first/last? > /** > * vDPA_config_ops - operations for configuring a vDPA device. > * Note: vDPA device drivers are required to implement all of the > @@ -1...

...pa.h > +++ b/include/linux/vdpa.h > @@ -41,6 +41,16 @@ struct vdpa_device { > unsigned int index; > }; > > +/** > + * vDPA IOVA range - the IOVA range support by the device > + * @start: start of the IOVA range > + * @end: end of the IOVA range > + */ > +struct vdpa_iova_range { > + u64 start; > + u64 end; > +}; > + This is ambiguous. Is end in the range or just behind it? How about first/last? > /** > * vDPA_config_ops - operations for configuring a vDPA device. > * Note: vDPA device drivers are required to implement all of the > @@ -1...

...39db794357c..b7633ed2500c 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -41,6 +41,16 @@ struct vdpa_device { unsigned int index; }; +/** + * vDPA IOVA range - the IOVA range support by the device + * @start: start of the IOVA range + * @end: end of the IOVA range + */ +struct vdpa_iova_range { + u64 start; + u64 end; +}; + /** * vDPA_config_ops - operations for configuring a vDPA device. * Note: vDPA device drivers are required to implement all of the @@ -134,6 +144,9 @@ struct vdpa_device { * @get_generation: Get device config generation (optional) * @vdev: vdpa device...

...; > unsigned int index; > > > }; > > > > > > +/** > > > + * vDPA IOVA range - the IOVA range support by the device > > > + * @start: start of the IOVA range > > > + * @end: end of the IOVA range > > > + */ > > > +struct vdpa_iova_range { > > > + u64 start; > > > + u64 end; > > > +}; > > > + > > > > > > This is ambiguous. Is end in the range or just behind it? > > How about first/last? > > It is customary in the kernel to use start-end where end corresponds to &...

...vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c index 62d640327145..89854e17c3c2 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c @@ -574,6 +574,16 @@ static u32 vdpasim_get_generation(struct vdpa_device *vdpa) return vdpasim->generation; } +struct vdpa_iova_range vdpasim_get_iova_range(struct vdpa_device *vdpa) +{ + struct vdpa_iova_range range = { + .first = 0ULL, + .last = ULLONG_MAX, + }; + + return range; +} + static int vdpasim_set_map(struct vdpa_device *vdpa, struct vhost_iotlb *iotlb) { @@ -657,6 +667,7 @@ static const struct vdpa_config...

...; >>>>>> +/** >>>>>> + * vDPA IOVA range - the IOVA range support by the device >>>>>> + * @start: start of the IOVA range >>>>>> + * @end: end of the IOVA range >>>>>> + */ >>>>>> +struct vdpa_iova_range { >>>>>> + u64 start; >>>>>> + u64 end; >>>>>> +}; >>>>>> + >>>>> >>>>> This is ambiguous. Is end in the range or just behind it? >>>>> How about first/last? >>>> It is...

...; >>>>>> +/** >>>>>> + * vDPA IOVA range - the IOVA range support by the device >>>>>> + * @start: start of the IOVA range >>>>>> + * @end: end of the IOVA range >>>>>> + */ >>>>>> +struct vdpa_iova_range { >>>>>> + u64 start; >>>>>> + u64 end; >>>>>> +}; >>>>>> + >>>>> >>>>> This is ambiguous. Is end in the range or just behind it? >>>>> How about first/last? >>>> It is...

...) > > return 0; > } > + > +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp) > +{ > + struct iommu_domain_geometry geo; > + struct vdpa_device *vdpa = v->vdpa; > + const struct vdpa_config_ops *ops = vdpa->config; > + struct vhost_vdpa_iova_range range; > + struct vdpa_iova_range vdpa_range; > + > + if (!ops->set_map && !ops->dma_map) { Why not just check if (ops->get_iova_range) directly? > + iommu_domain_get_attr(v->domain, > + DOMAIN_ATTR_GEOMETRY, &geo); > + range.start = geo.ap...

...) > > return 0; > } > + > +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp) > +{ > + struct iommu_domain_geometry geo; > + struct vdpa_device *vdpa = v->vdpa; > + const struct vdpa_config_ops *ops = vdpa->config; > + struct vhost_vdpa_iova_range range; > + struct vdpa_iova_range vdpa_range; > + > + if (!ops->set_map && !ops->dma_map) { Why not just check if (ops->get_iova_range) directly? > + iommu_domain_get_attr(v->domain, > + DOMAIN_ATTR_GEOMETRY, &geo); > + range.start = geo.ap...

...config_call(struct vhost_vdpa *v, u32 __user *argp) return 0; } + +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp) +{ + struct iommu_domain_geometry geo; + struct vdpa_device *vdpa = v->vdpa; + const struct vdpa_config_ops *ops = vdpa->config; + struct vhost_vdpa_iova_range range; + struct vdpa_iova_range vdpa_range; + + if (!ops->set_map && !ops->dma_map) { + iommu_domain_get_attr(v->domain, + DOMAIN_ATTR_GEOMETRY, &geo); + range.start = geo.aperture_start; + range.end = geo.aperture_end; + } else { + vdpa_range = ops->get_iova_r...

...t; @@ -41,6 +41,16 @@ struct vdpa_device { >> unsigned int index; >> }; >> >> +/** >> + * vDPA IOVA range - the IOVA range support by the device >> + * @start: start of the IOVA range >> + * @end: end of the IOVA range >> + */ >> +struct vdpa_iova_range { >> + u64 start; >> + u64 end; >> +}; >> + > This is ambiguous. Is end in the range or just behind it? In the range. > How about first/last? Sure. Thanks > > >

...t; @@ -41,6 +41,16 @@ struct vdpa_device { >> unsigned int index; >> }; >> >> +/** >> + * vDPA IOVA range - the IOVA range support by the device >> + * @start: start of the IOVA range >> + * @end: end of the IOVA range >> + */ >> +struct vdpa_iova_range { >> + u64 start; >> + u64 end; >> +}; >> + > What do you do with this information? Suppose some device tells you it > supports some limited range, say, from 0x40000000 to 0x80000000. What > does qemu do with this information? For qemu, when qemu will fail the vD...

...t; > > > > > +/** > > > > > + * vDPA IOVA range - the IOVA range support by the device > > > > > + * @start: start of the IOVA range > > > > > + * @end: end of the IOVA range > > > > > + */ > > > > > +struct vdpa_iova_range { > > > > > + u64 start; > > > > > + u64 end; > > > > > +}; > > > > > + > > > > > > > > > > > > This is ambiguous. Is end in the range or just behind it? > > > > How about first/last? >...

...> > > > + * vDPA IOVA range - the IOVA range support by the device > > > > > > > + * @start: start of the IOVA range > > > > > > > + * @end: end of the IOVA range > > > > > > > + */ > > > > > > > +struct vdpa_iova_range { > > > > > > > + u64 start; > > > > > > > + u64 end; > > > > > > > +}; > > > > > > > + > > > > > > > > > > > > This is ambiguous. Is end in the range or just behind it? > &g...

On 2020/8/23 ??2:40, Eli Cohen wrote: >> +static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v) >> +{ >> + struct vdpa_iova_range *range = &v->range; >> + struct iommu_domain_geometry geo; >> + struct vdpa_device *vdpa = v->vdpa; >> + const struct vdpa_config_ops *ops = vdpa->config; >> + >> + if (ops->get_iova_range) { >> + *range = ops->get_iova_range(vdpa); >>...

...; } >> + >> +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp) >> +{ >> + struct iommu_domain_geometry geo; >> + struct vdpa_device *vdpa = v->vdpa; >> + const struct vdpa_config_ops *ops = vdpa->config; >> + struct vhost_vdpa_iova_range range; >> + struct vdpa_iova_range vdpa_range; >> + >> + if (!ops->set_map && !ops->dma_map) { > Why not just check if (ops->get_iova_range) directly? Because set_map || dma_ops is a hint that the device has its own DMA translation logic. Device without ge...

Live migration of vdpa would typically require re-instate vdpa device with an idential set of configs on the destination node, same way as how source node created the device in the first place. In order to allow live migration orchestration software to export the initial set of vdpa attributes with which the device was created, it will be useful if the vdpa tool can report the config on demand