Date: Wed, 13 Dec 2017 14:49:36 +0800
From: Peter Xu <peterx@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        intel-gvt-dev@lists.freedesktop.org, aik@ozlabs.ru,
        kwankhede@nvidia.com, zhenyuw@linux.intel.com, zhi.a.wang@intel.com
Subject: Re: [PATCH] vfio: Simplify capability helper
Message-ID: <20171213064936.GC7780@xz-mi>
References: <20171212195850.13691.40496.stgit@gimli.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20171212195850.13691.40496.stgit@gimli.home>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1130
Lines: 28

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@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.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,

-- 
Peter Xu