From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754053AbdG3Kjq (ORCPT ); Sun, 30 Jul 2017 06:39:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48886 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753998AbdG3Kjn (ORCPT ); Sun, 30 Jul 2017 06:39:43 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A788FC05A1C5 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=eric.auger@redhat.com Subject: Re: [PATCH 2/2] vfio/type1: Give hardware MSI regions precedence To: Robin Murphy , alex.williamson@redhat.com References: Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, shameerali.kolothum.thodi@huawei.com, marc.zyngier@arm.com From: Auger Eric Message-ID: <60052d02-276e-ed12-f372-14ce166f50ed@redhat.com> Date: Sun, 30 Jul 2017 12:39:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Sun, 30 Jul 2017 10:39:42 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Robin, On 27/07/2017 16:54, Robin Murphy wrote: > If the IOMMU driver advertises 'real' reserved regions for MSIs, but > still includes the software-managed region as well, we are currently > blind to the former and will configure the IOMMU domain to map MSIs into > the latter, which is unlikely to work as expected. > > Since it would take a ridiculous hardware topology for both regions to > be valid (which would be rather difficult to support in general), we > should be safe to assume that the presence of any hardware regions makes > the software region irrelevant. However, the IOMMU driver might still > advertise the software region by default, particularly if the hardware > regions are filled in elsewhere by generic code, so it might not be fair > for VFIO to be super-strict about not mixing them. To that end, make > vfio_iommu_has_sw_msi() robust against the presence of both region types > at once, so that we end up doing what is almost certainly right, rather > than what is almost certainly wrong. > > Signed-off-by: Robin Murphy Reviewed-by: Eric Auger Thanks Eric > --- > drivers/vfio/vfio_iommu_type1.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 2328be628f21..92155cce926d 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -1169,13 +1169,21 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) > INIT_LIST_HEAD(&group_resv_regions); > iommu_get_group_resv_regions(group, &group_resv_regions); > list_for_each_entry(region, &group_resv_regions, list) { > + /* > + * The presence of any 'real' MSI regions should take > + * precedence over the software-managed one if the > + * IOMMU driver happens to advertise both types. > + */ > + if (region->type == IOMMU_RESV_MSI) { > + ret = false; > + break; > + } > + > if (region->type == IOMMU_RESV_SW_MSI) { > *base = region->start; > ret = true; > - goto out; > } > } > -out: > list_for_each_entry_safe(region, next, &group_resv_regions, list) > kfree(region); > return ret; >