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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8342117f-87ab-d38e-6fcd-aaa947dbeaaf@oracle.com>
Date:   Thu, 25 Aug 2022 23:26:04 +0100
From:   Joao Martins <joao.m.martins@...cle.com>
To:     Alex Williamson <alex.williamson@...hat.com>,
        Yishai Hadas <yishaih@...dia.com>
Cc:     jgg@...dia.com, saeedm@...dia.com, kvm@...r.kernel.org,
        netdev@...r.kernel.org, kuba@...nel.org, kevin.tian@...el.com,
        leonro@...dia.com, maorg@...dia.com, cohuck@...hat.com
Subject: Re: [PATCH V4 vfio 05/10] vfio: Introduce the DMA logging feature
 support

On 8/25/22 21:49, Alex Williamson wrote:
> On Mon, 15 Aug 2022 18:11:04 +0300
> Yishai Hadas <yishaih@...dia.com> 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.

> 
> Alex
> 
>> +		return -EINVAL;
>> +
>> +	ret = iova_bitmap_iter_init(&iter, report.iova, report.length,
>> +				    report.page_size,
>> +				    u64_to_user_ptr(report.bitmap));
>> +	if (ret)
>> +		return ret;
>> +
>> +	for (; !iova_bitmap_iter_done(&iter) && !ret;
>> +	     ret = iova_bitmap_iter_advance(&iter)) {
>> +		ret = device->log_ops->log_read_and_clear(device,
>> +			iova_bitmap_iova(&iter),
>> +			iova_bitmap_length(&iter), &iter.dirty);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	iova_bitmap_iter_free(&iter);
>> +	return ret;
>> +}
>