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: introduce config op to get valid iova range
vdpa_sim: implement get_iova_range bus operation
vdpa: get_iova_range() is mandatory for device specific DMA
translation
vhost: vdpa: report iova range
drivers/vdpa/vdpa.c | 4 ++++
drivers/vdpa/vdpa_sim/vdpa_sim.c | 11 +++++++++++
drivers/vhost/vdpa.c | 27 +++++++++++++++++++++++++++
include/linux/vdpa.h | 14 ++++++++++++++
include/uapi/linux/vhost.h | 4 ++++
include/uapi/linux/vhost_types.h | 5 +++++
6 files changed, 65 insertions(+)
--
2.20.1
Jason Wang
2020-Jun-17 03:29 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
This patch introduce a config op to get valid iova range from the vDPA
device.
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
include/linux/vdpa.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 239db794357c..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
* Returns u32: device generation
+ * @get_iova_range: Get supported iova range (on-chip IOMMU)
+ * @vdev: vdpa device
+ * Returns the iova range supported by the device
* @set_map: Set device memory mapping (optional)
* Needed for device that using device
* specific DMA translation (on-chip IOMMU)
@@ -195,6 +208,7 @@ struct vdpa_config_ops {
void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
const void *buf, unsigned int len);
u32 (*get_generation)(struct vdpa_device *vdev);
+ struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
/* DMA ops */
int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb);
--
2.20.1
Jason Wang
2020-Jun-17 03:29 UTC
[PATCH 2/4] vdpa_sim: implement get_iova_range bus operation
This patch implements get_iova_range method for vdpa_sim. Since
vdpa_sim is a software device, simply advertise a [0ULL, ~0ULL] range.
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c7334cc65bb2..b3a6dc5b9984 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -524,6 +524,16 @@ static u32 vdpasim_get_generation(struct vdpa_device *vdpa)
return vdpasim->generation;
}
+static struct vdpa_iova_range vdpasim_get_iova_range(struct vdpa_device *vdpa)
+{
+ struct vdpa_iova_range range;
+
+ range.start = 0ULL;
+ range.end = ~0ULL;
+
+ return range;
+}
+
static int vdpasim_set_map(struct vdpa_device *vdpa,
struct vhost_iotlb *iotlb)
{
@@ -597,6 +607,7 @@ static const struct vdpa_config_ops vdpasim_net_config_ops =
{
.get_config = vdpasim_get_config,
.set_config = vdpasim_set_config,
.get_generation = vdpasim_get_generation,
+ .get_iova_range = vdpasim_get_iova_range,
.set_map = vdpasim_set_map,
.dma_map = vdpasim_dma_map,
.dma_unmap = vdpasim_dma_unmap,
--
2.20.1
Jason Wang
2020-Jun-17 03:29 UTC
[PATCH 3/4] vdpa: get_iova_range() is mandatory for device specific DMA translation
In order to let userspace work correctly, get_iova_range() is a must for the device that has its own DMA translation logic. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vdpa/vdpa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index de211ef3738c..ab7af978ef70 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -82,6 +82,10 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent, if (!!config->dma_map != !!config->dma_unmap) goto err; + if ((config->dma_map || config->set_map) && + !config->get_iova_range) + goto err; + err = -ENOMEM; vdev = kzalloc(size, GFP_KERNEL); if (!vdev) -- 2.20.1
This patch introduces a new ioctl for vhost-vdpa device that can
report the iova range by the device. For device that depends on
platform IOMMU, we fetch the iova range via DOMAIN_ATTR_GEOMETRY. For
devices that has its own DMA translation unit, we fetch it directly
from vDPA bus operation.
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
drivers/vhost/vdpa.c | 27 +++++++++++++++++++++++++++
include/uapi/linux/vhost.h | 4 ++++
include/uapi/linux/vhost_types.h | 5 +++++
3 files changed, 36 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 77a0c9fb6cc3..ad23e66cbf57 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -332,6 +332,30 @@ static long vhost_vdpa_set_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_range(vdpa);
+ range.start = vdpa_range.start;
+ range.end = vdpa_range.end;
+ }
+
+ return copy_to_user(argp, &range, sizeof(range));
+
+}
+
static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
void __user *argp)
{
@@ -442,6 +466,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
case VHOST_VDPA_SET_CONFIG_CALL:
r = vhost_vdpa_set_config_call(v, argp);
break;
+ case VHOST_VDPA_GET_IOVA_RANGE:
+ r = vhost_vdpa_get_iova_range(v, argp);
+ break;
default:
r = vhost_dev_ioctl(&v->vdev, cmd, argp);
if (r == -ENOIOCTLCMD)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 0c2349612e77..850956980e27 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -144,4 +144,8 @@
/* Set event fd for config interrupt*/
#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, int)
+
+/* Get the valid iova range */
+#define VHOST_VDPA_GET_IOVA_RANGE _IOW(VHOST_VIRTIO, 0x78, \
+ struct vhost_vdpa_iova_range)
#endif
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index 669457ce5c48..4025b5a36177 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -127,6 +127,11 @@ struct vhost_vdpa_config {
__u8 buf[0];
};
+struct vhost_vdpa_iova_range {
+ __u64 start;
+ __u64 end;
+};
+
/* Feature bits */
/* Log all write descriptors. Can be changed while device is active. */
#define VHOST_F_LOG_ALL 26
--
2.20.1
Michael S. Tsirkin
2020-Aug-05 12:51 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:> This patch introduce a config op to get valid iova range from the vDPA > device. > > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > include/linux/vdpa.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 239db794357c..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; > +}; > +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 > @@ -134,6 +144,9 @@ struct vdpa_device { > * @get_generation: Get device config generation (optional) > * @vdev: vdpa device > * Returns u32: device generation > + * @get_iova_range: Get supported iova range (on-chip IOMMU) > + * @vdev: vdpa device > + * Returns the iova range supported by the device > * @set_map: Set device memory mapping (optional) > * Needed for device that using device > * specific DMA translation (on-chip IOMMU) > @@ -195,6 +208,7 @@ struct vdpa_config_ops { > void (*set_config)(struct vdpa_device *vdev, unsigned int offset, > const void *buf, unsigned int len); > u32 (*get_generation)(struct vdpa_device *vdev); > + struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev); > > /* DMA ops */ > int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb); > -- > 2.20.1
Michael S. Tsirkin
2020-Aug-05 12:55 UTC
[PATCH 3/4] vdpa: get_iova_range() is mandatory for device specific DMA translation
On Wed, Jun 17, 2020 at 11:29:46AM +0800, Jason Wang wrote:> In order to let userspace work correctly, get_iova_range() is a must > for the device that has its own DMA translation logic.I guess you mean for a device. However in absence of ths op, I don't see what is wrong with just assuming device can access any address.> > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > drivers/vdpa/vdpa.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index de211ef3738c..ab7af978ef70 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -82,6 +82,10 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent, > if (!!config->dma_map != !!config->dma_unmap) > goto err; > > + if ((config->dma_map || config->set_map) && > + !config->get_iova_range) > + goto err; > + > err = -ENOMEM; > vdev = kzalloc(size, GFP_KERNEL); > if (!vdev)What about devices using an IOMMU for translation? IOMMUs generally have a limited IOVA range too, right?> -- > 2.20.1
On Wed, Jun 17, 2020 at 11:29:47AM +0800, Jason Wang wrote:> This patch introduces a new ioctl for vhost-vdpa device that can > report the iova range by the device. For device that depends on > platform IOMMU, we fetch the iova range via DOMAIN_ATTR_GEOMETRY. For > devices that has its own DMA translation unit, we fetch it directly > from vDPA bus operation. > > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > drivers/vhost/vdpa.c | 27 +++++++++++++++++++++++++++ > include/uapi/linux/vhost.h | 4 ++++ > include/uapi/linux/vhost_types.h | 5 +++++ > 3 files changed, 36 insertions(+) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 77a0c9fb6cc3..ad23e66cbf57 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -332,6 +332,30 @@ static long vhost_vdpa_set_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) {Why not just check if (ops->get_iova_range) directly?> + 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_range(vdpa); > + range.start = vdpa_range.start; > + range.end = vdpa_range.end; > + } > + > + return copy_to_user(argp, &range, sizeof(range)); > + > +} > + > static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, > void __user *argp) > { > @@ -442,6 +466,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > case VHOST_VDPA_SET_CONFIG_CALL: > r = vhost_vdpa_set_config_call(v, argp); > break; > + case VHOST_VDPA_GET_IOVA_RANGE: > + r = vhost_vdpa_get_iova_range(v, argp); > + break; > default: > r = vhost_dev_ioctl(&v->vdev, cmd, argp); > if (r == -ENOIOCTLCMD) > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h > index 0c2349612e77..850956980e27 100644 > --- a/include/uapi/linux/vhost.h > +++ b/include/uapi/linux/vhost.h > @@ -144,4 +144,8 @@ > > /* Set event fd for config interrupt*/ > #define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, int) > + > +/* Get the valid iova range */ > +#define VHOST_VDPA_GET_IOVA_RANGE _IOW(VHOST_VIRTIO, 0x78, \ > + struct vhost_vdpa_iova_range) > #endif > diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h > index 669457ce5c48..4025b5a36177 100644 > --- a/include/uapi/linux/vhost_types.h > +++ b/include/uapi/linux/vhost_types.h > @@ -127,6 +127,11 @@ struct vhost_vdpa_config { > __u8 buf[0]; > }; > > +struct vhost_vdpa_iova_range { > + __u64 start; > + __u64 end; > +}; > +Pls document fields. And I think first/last is a better API ...> /* Feature bits */ > /* Log all write descriptors. Can be changed while device is active. */ > #define VHOST_F_LOG_ALL 26 > -- > 2.20.1
Michael S. Tsirkin
2020-Aug-06 12:29 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote:> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: > > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote: > > > This patch introduce a config op to get valid iova range from the vDPA > > > device. > > > > > > Signed-off-by: Jason Wang <jasowang at redhat.com> > > > --- > > > include/linux/vdpa.h | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > > > index 239db794357c..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; > > > +}; > > > + > > > > > > 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 > the byte following the last in the range. See struct vm_area_struct > vm_start and vm_end fieldsExactly my point: include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address in this case Jason wants it to be the last byte, not one behind.> > > > > > > > > /** > > > * 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 > > > * Returns u32: device generation > > > + * @get_iova_range: Get supported iova range (on-chip IOMMU) > > > + * @vdev: vdpa device > > > + * Returns the iova range supported by the device > > > * @set_map: Set device memory mapping (optional) > > > * Needed for device that using device > > > * specific DMA translation (on-chip IOMMU) > > > @@ -195,6 +208,7 @@ struct vdpa_config_ops { > > > void (*set_config)(struct vdpa_device *vdev, unsigned int offset, > > > const void *buf, unsigned int len); > > > u32 (*get_generation)(struct vdpa_device *vdev); > > > + struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev); > > > > > > /* DMA ops */ > > > int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb); > > > -- > > > 2.20.1 > >
Jason Wang
2020-Aug-07 03:04 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On 2020/8/6 ??8:10, Eli Cohen wrote:> On Wed, Jun 17, 2020 at 06:29:44AM +0300, Jason Wang wrote: >> This patch introduce a config op to get valid iova range from the vDPA >> device. >> >> Signed-off-by: Jason Wang<jasowang at redhat.com> >> --- >> include/linux/vdpa.h | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h >> index 239db794357c..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; >> +}; >> + > 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 vDPA device creation when: 1) vIOMMU is not enabled and GPA is out of this range 2) vIOMMU is enabled but it can't report such range to guest For other userspace application, it will know it can only use this range as its IOVA. Thanks
Jason Wang
2020-Aug-07 03:23 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On 2020/8/6 ??8:29, Michael S. Tsirkin wrote:> On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote: >> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: >>> On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote: >>>> This patch introduce a config op to get valid iova range from the vDPA >>>> device. >>>> >>>> Signed-off-by: Jason Wang<jasowang at redhat.com> >>>> --- >>>> include/linux/vdpa.h | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h >>>> index 239db794357c..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; >>>> +}; >>>> + >>> 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 >> the byte following the last in the range. See struct vm_area_struct >> vm_start and vm_end fields > Exactly my point: > > include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address > > in this case Jason wants it to be the last byte, not one behind.Ok, I somehow recall the reason :) See: struct iommu_domain_geometry { ??? dma_addr_t aperture_start; /* First address that can be mapped??? */ ??? dma_addr_t aperture_end;?? /* Last address that can be mapped???? */ ??? bool force_aperture;?????? /* DMA only allowed in mappable range? */ }; So what I proposed here is to be consistent with it. Thanks> >
Michael S. Tsirkin
2020-Aug-10 12:05 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote:> On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote: > > On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote: > > > On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: > > > > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote: > > > > > This patch introduce a config op to get valid iova range from the vDPA > > > > > device. > > > > > > > > > > Signed-off-by: Jason Wang <jasowang at redhat.com> > > > > > --- > > > > > include/linux/vdpa.h | 14 ++++++++++++++ > > > > > 1 file changed, 14 insertions(+) > > > > > > > > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > > > > > index 239db794357c..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; > > > > > +}; > > > > > + > > > > > > > > > > > > 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 > > > the byte following the last in the range. See struct vm_area_struct > > > vm_start and vm_end fields > > > > Exactly my point: > > > > include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address > > > > in this case Jason wants it to be the last byte, not one behind. > > > > > Maybe start, size? Not ambiguous, and you don't need to do annoying > calculations like size = last - start + 1Size has a bunch of issues: can overlap, can not cover the entire 64 bit range. The requisite checks are arguably easier to get wrong than getting the size if you need it.
Jason Wang
2020-Aug-11 02:53 UTC
[PATCH 1/4] vdpa: introduce config op to get valid iova range
On 2020/8/10 ??8:05, Michael S. Tsirkin wrote:> On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote: >> On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote: >>> On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote: >>>> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: >>>>> On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote: >>>>>> This patch introduce a config op to get valid iova range from the vDPA >>>>>> device. >>>>>> >>>>>> Signed-off-by: Jason Wang <jasowang at redhat.com> >>>>>> --- >>>>>> include/linux/vdpa.h | 14 ++++++++++++++ >>>>>> 1 file changed, 14 insertions(+) >>>>>> >>>>>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h >>>>>> index 239db794357c..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; >>>>>> +}; >>>>>> + >>>>> >>>>> 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 >>>> the byte following the last in the range. See struct vm_area_struct >>>> vm_start and vm_end fields >>> Exactly my point: >>> >>> include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address >>> >>> in this case Jason wants it to be the last byte, not one behind. >>> >>> >> Maybe start, size? Not ambiguous, and you don't need to do annoying >> calculations like size = last - start + 1 > Size has a bunch of issues: can overlap, can not cover the entire 64 bit > range. The requisite checks are arguably easier to get wrong than > getting the size if you need it.Yes, so do you still prefer first/last or just begin/end which is consistent with iommu_domain_geometry? Thanks>