On 25/07/2022 10:30, Tian, Kevin wrote:
<please use plain-text next time>
From: Yishai Hadas <yishaih@xxxxxxxxxx>
Sent: Thursday, July 21, 2022 7:06 PM
+/*
+ * Upon VFIO_DEVICE_FEATURE_SET start device DMA logging.
both 'start'/'stop' are via VFIO_DEVICE_FEATURE_SET
Right, we have a note for that near VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP.
Here it refers to the start option.
let's make it accurate here.
OK
+ * page_size is an input that hints what tracking granularity the device
+ * should try to achieve. If the device cannot do the hinted page size then it
+ * should pick the next closest page size it supports. On output the device
next closest 'smaller' page size?
Not only, it depends on the device capabilities/support and should be a driver choice.
'should pick next closest" is a guideline to the driver. If user requests
8KB while the device supports 4KB and 16KB, which one is closest?
It's probably safer to just say that it's a driver choice when the hinted page
size cannot be set?
Yes, may rephrase in V3 accordingly.
+struct vfio_device_feature_dma_logging_control {
+ __aligned_u64 page_size;
+ __u32 num_ranges;
+ __u32 __reserved;
+ __aligned_u64 ranges;
+};
should we move the definition of LOG_MAX_RANGES to be here
so the user can know the max limits of tracked ranges?
This was raised as an option as part of this mail thread.
However, for now it seems redundant as we may not expect user space to hit this limit and it mainly comes to protect kernel from memory exploding by a malicious user.
No matter how realistic an user might hit an limitation, it doesn't
sound good to not expose it if existing.
As Jason replied at some point here, we need to see a clear use case for
more than a few 10's of ranges before we complicate things.
For now we don't see one. If one does crop up someday it is easy to add
a new query, or some other behavior.
Alex,
Can you please comment here so that we can converge and be ready for V3 ?
+
+struct vfio_device_feature_dma_logging_range {
+ __aligned_u64 iova;
+ __aligned_u64 length;
+};
+
+#define VFIO_DEVICE_FEATURE_DMA_LOGGING_START 3
Can the user update the range list by doing another START?
No, single start to ask the device what to track and a matching single stop should follow at the end.
let's document it then.
OK
Thanks
Kevin
Thanks,
Yishai
