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: <YwjCDSPVa0uNy6vX@nvidia.com>
Date:   Fri, 26 Aug 2022 09:52:29 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Joao Martins <joao.m.martins@...cle.com>,
        Yishai Hadas <yishaih@...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 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@...cle.com> wrote:
> 
> > 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.
> 
> 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