From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751794AbdLOGS2 (ORCPT ); Fri, 15 Dec 2017 01:18:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47324 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751266AbdLOGS1 (ORCPT ); Fri, 15 Dec 2017 01:18:27 -0500 Date: Fri, 15 Dec 2017 14:18:17 +0800 From: Peter Xu To: Alex Williamson Cc: Auger Eric , 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: <20171215061817.GG7780@xz-mi> References: <20171212195850.13691.40496.stgit@gimli.home> <20171213064936.GC7780@xz-mi> <0bb4e51a-6fd2-e946-8927-b8d9fe491b07@redhat.com> <20171213083539.768ed23f@t450s.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20171213083539.768ed23f@t450s.home> User-Agent: Mutt/1.9.1 (2017-09-22) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 15 Dec 2017 06:18:27 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 13, 2017 at 08:35:39AM -0700, Alex Williamson wrote: > On Wed, 13 Dec 2017 16:04:48 +0100 > 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 > > > > > > Reviewed-by: Peter Xu > > > > > > 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? > > Yes, and that was a compatibility choice when the sparse mmap was > added, retaining the per region mmap flag, but essentially excluding > the whole area with the sparse mmap. It seemed like it'd be easier for > userspace to understand the distinction. I see. > Now we're trying to remove > the whole mess and allow mmaps covering the MSI-X vector table because > it's a performance killer for systems where the page size is >4K. Yeah, I just noticed that. Thanks for explaining! -- Peter Xu