linux-kernel - Re: [PATCH] vfio: Simplify capability helper

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: <0bb4e51a-6fd2-e946-8927-b8d9fe491b07@redhat.com>
Date:   Wed, 13 Dec 2017 16:04:48 +0100
From:   Auger Eric <eric.auger@...hat.com>
To:     Peter Xu <peterx@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        intel-gvt-dev@...ts.freedesktop.org, aik@...abs.ru,
        kwankhede@...dia.com, zhenyuw@...ux.intel.com, zhi.a.wang@...el.com
Subject: Re: [PATCH] vfio: Simplify capability helper

Hi Peter,

On 13/12/17 07:49, Peter Xu wrote:
> On Tue, Dec 12, 2017 at 12:59:39PM -0700, Alex Williamson wrote:
>> The vfio_info_add_capability() helper requires the caller to pass a
>> capability ID, which it then uses to fill in header fields, assuming
>> hard coded versions.  This makes for an awkward and rigid interface.
>> The only thing we want this helper to do is allocate sufficient
>> space in the caps buffer and chain this capability into the list.
>> Reduce it to that simple task.
>>
>> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> 
> Reviewed-by: Peter Xu <peterx@...hat.com>
> 
> Though during review I had a question related to the function
> msix_sparse_mmap_cap(): Is it possible that one PCI device BAR is very
> small (e.g., 4K) that only contains the MSI-X table (and another small
> PBA area)?  If so, should we just mark that region unmappable instead
> of setting vfio_region_info_cap_sparse_mmap.nr_areas to 1 in
> msix_sparse_mmap_cap()?
> 
> 	/* If MSI-X table is aligned to the start or end, only one area */
> 	if (((vdev->msix_offset & PAGE_MASK) == 0) ||
> 	    (PAGE_ALIGN(vdev->msix_offset + vdev->msix_size) >= end))
> 		nr_areas = 1;
> 
> Thanks,
> 
if I understand the code correctly, if the MSI-X table exactly matches
the BAR, a sparse mmap region is reported with offset/size = 0. Is that
correct?

Thanks

Eric