Re: [PATCH] vfio: Simplify capability helper

[Peter Xu]

On Wed, Dec 13, 2017 at 04:32:10PM +0100, Auger Eric wrote:
> Hi,
> 
> On 13/12/17 16:04, Auger Eric wrote:
> > 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@xxxxxxxxxx>
> >>
> >> Reviewed-by: Peter Xu <peterx@xxxxxxxxxx>
> >>
> >> 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
> > 
> looks fixed by
> 
> [RFC PATCH kernel] vfio-pci: Fix sparse capability when no parts of MSIX
> BAR can be mapped
> 
> Sorry for the noise.

Hi, Eric,

Thanks for the link anyways. :)

-- 
Peter Xu