Re: [PATCH V4 vfio 05/10] vfio: Introduce the DMA logging feature support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26/08/2022 15:52, Jason Gunthorpe wrote:
On Thu, Aug 25, 2022 at 04:46:51PM -0600, Alex Williamson wrote:
On Thu, 25 Aug 2022 23:26:04 +0100
Joao Martins <joao.m.martins@xxxxxxxxxx> wrote:

On 8/25/22 21:49, Alex Williamson wrote:
On Mon, 15 Aug 2022 18:11:04 +0300
Yishai Hadas <yishaih@xxxxxxxxxx> wrote:
+static int
+vfio_ioctl_device_feature_logging_report(struct vfio_device *device,
+					 u32 flags, void __user *arg,
+					 size_t argsz)
+{
+	size_t minsz =
+		offsetofend(struct vfio_device_feature_dma_logging_report,
+			    bitmap);
+	struct vfio_device_feature_dma_logging_report report;
+	struct iova_bitmap_iter iter;
+	int ret;
+
+	if (!device->log_ops)
+		return -ENOTTY;
+
+	ret = vfio_check_feature(flags, argsz,
+				 VFIO_DEVICE_FEATURE_GET,
+				 sizeof(report));
+	if (ret != 1)
+		return ret;
+
+	if (copy_from_user(&report, arg, minsz))
+		return -EFAULT;
+
+	if (report.page_size < PAGE_SIZE || !is_power_of_2(report.page_size))
Why is PAGE_SIZE a factor here?  I'm under the impression that
iova_bitmap is intended to handle arbitrary page sizes.  Thanks,
Arbritary page sizes ... which are powers of 2. We use page shift in iova bitmap.
While it's not hard to lose this restriction (trading a shift over a slower mul)
... I am not sure it is worth supporting said use considering that there aren't
non-powers of 2 page sizes right now?

The PAGE_SIZE restriction might be that it's supposed to be the lowest possible page_size.
Sorry, I was unclear.  Size relative to PAGE_SIZE was my only question,
not that we shouldn't require power of 2 sizes.  We're adding device
level dirty tracking, where the device page size granularity might be
4K on a host with a CPU 64K page size.  Maybe there's a use case for
that.  Given the flexibility claimed by the iova_bitmap support,
requiring reported page size less than system PAGE_SIZE seems
unjustified.  Thanks,
+1

Jason

OK

So in V5 we may come with the below as we don't really expect page size smaller than 4K.

if (report.page_size < SZ_4K || !is_power_of_2(report.page_size))
        return -EINVAL;

Yishai




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux