Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

From: Jason Wang
Date: Tue Aug 11 2020 - 22:02:44 EST



On 2020/8/11 下午4:29, Michael S. Tsirkin wrote:
On Tue, Aug 11, 2020 at 10:53:09AM +0800, Jason Wang wrote:
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@xxxxxxxxxx>
---
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
I prefer first/last I think, these are unambiguous.
E.g.

dma_addr_t aperture_start; /* First address that can be mapped */
dma_addr_t aperture_end; /* Last address that can be mapped */

instead of addressing ambiguity with a comment, let's just name the field well.


Ok, will do.

Thanks