From: Robin Murphy <robin.murphy@arm.com>
To: Alex Williamson <alex.williamson@redhat.com>,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Cc: Auger Eric <eric.auger@redhat.com>,
"pmorel@linux.vnet.ibm.com" <pmorel@linux.vnet.ibm.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Linuxarm <linuxarm@huawei.com>,
John Garry <john.garry@huawei.com>,
"xuwei (O)" <xuwei5@huawei.com>
Subject: Re: [PATCH v4 4/6] vfio/type1: check dma map request is within a valid iova range
Date: Fri, 2 Mar 2018 17:16:35 +0000 [thread overview]
Message-ID: <22b61209-61b6-207d-9d70-1db6ae630fc1@arm.com> (raw)
In-Reply-To: <20180302090417.05ade6df@t450s.home>
On 02/03/18 16:04, Alex Williamson wrote:
[...]
>>> I still think you're overstretching the IOMMU reserved region interface
>>> by trying to report possible ACS conflicts. Are we going to update the
>>> reserved list on device hotplug? Are we going to update the list when
>>> MMIO is enabled or disabled for each device? What if the BARs are
>>> reprogrammed or bridge apertures changed? IMO, the IOMMU reserved list
>>> should account for specific IOVA exclusions that apply to transactions
>>> that actually reach the IOMMU, not aliasing that might occur in the
>>> downstream topology. Additionally, the IOMMU group composition must be
>>> such that these sorts of aliasing issues can only occur within an IOMMU
>>> group. If a transaction can be affected by the MMIO programming of
>>> another group, then the groups are drawn incorrectly for the isolation
>>> capabilities of the hardware. Thanks,
>>
>> Agree that this is not a perfect thing to do covering all scenarios. As Robin
>> pointed out, the current code is playing safe by reserving the full windows.
>>
>> My suggestion will be to limit this reservation to non-ACS cases only. This will
>> make sure that ACS ARM hardware is not broken by this series and nos-ACS
>> ones has a chance to run once Qemu is updated to take care of the reserved
>> regions (at least in some user scenarios).
>>
>> If you all are fine with this, I can include the ACS check I mentioned earlier in
>> iommu_dma_get_resv_regions() and sent out the revised series.
>>
>> Please let me know your thoughts.
>
> IMO, the IOMMU should be concerned with ACS as far as isolation is
> concerned and reporting reserved ranges that are imposed at the IOMMU
> and leave any aliasing or routing issues in the downstream topology to
> other layers, or perhaps to the user. Unfortunately, enforcing the
> iova list in vfio is gated by some movement here since we can't break
> existing users. Thanks,
FWIW, given the discussion we've had here I wouldn't object to pushing
the PCI window reservation back into the DMA-specific path, such that it
doesn't get exposed via the general IOMMU API interface. We *do* want to
do it there where we are in total control of our own address space and
it just avoids a whole class of problems (even with ACS I'm not sure the
root complex can be guaranteed to send a "bad" IOVA out to the SMMU
instead of just terminating it for looking like a peer-to-peer attempt).
I do agree that it's not scalable for the IOMMU layer to attempt to
detect and describe upstream PCI limitations to userspace by itself -
they are "reserved regions" rather than "may or may not work regions"
after all. If there is a genuine integration issue between an IOMMU and
an upstream PCI RC which restricts usable addresses on a given system,
that probably needs to be explicitly communicated from firmware to the
IOMMU driver anyway, at which point that driver can report the relevant
region(s) directly from its own callback.
I suppose there's an in-between option of keeping generic window
reservations but giving them a new "only reserve this if you're being
super-cautious or don't know better" region type which we hide from
userspace and ignore in VFIO, but maybe that leaves the lines a but too
blurred still.
Robin.
next prev parent reply other threads:[~2018-03-02 17:16 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-21 12:22 [PATCH v4 0/6] vfio/type1: Add support for valid iova list management Shameer Kolothum
2018-02-21 12:22 ` [PATCH v4 1/6] vfio/type1: Introduce iova list and add iommu aperture validity check Shameer Kolothum
2018-02-21 12:22 ` [PATCH v4 2/6] vfio/type1: Check reserve region conflict and update iova list Shameer Kolothum
2018-02-21 12:22 ` [PATCH v4 3/6] vfio/type1: Update iova list on detach Shameer Kolothum
2018-02-21 12:22 ` [PATCH v4 4/6] vfio/type1: check dma map request is within a valid iova range Shameer Kolothum
2018-02-26 22:05 ` Auger Eric
2018-02-26 23:13 ` Alex Williamson
2018-02-27 8:26 ` Auger Eric
2018-02-27 9:57 ` Shameerali Kolothum Thodi
2018-02-27 17:13 ` Alex Williamson
2018-02-28 9:02 ` Auger Eric
2018-02-28 9:25 ` Shameerali Kolothum Thodi
2018-02-28 11:53 ` Auger Eric
2018-02-28 13:39 ` Shameerali Kolothum Thodi
2018-02-28 15:32 ` Auger Eric
2018-02-28 15:24 ` Alex Williamson
2018-03-02 13:19 ` Shameerali Kolothum Thodi
2018-03-02 16:04 ` Alex Williamson
2018-03-02 17:16 ` Robin Murphy [this message]
2018-03-05 11:44 ` Shameerali Kolothum Thodi
2018-03-14 18:12 ` Robin Murphy
2018-03-08 9:35 ` Shameerali Kolothum Thodi
2018-02-27 16:57 ` Alex Williamson
2018-02-27 12:40 ` Robin Murphy
2018-02-21 12:22 ` [PATCH v4 5/6] vfio/type1: Add IOVA range capability support Shameer Kolothum
2018-02-22 22:54 ` Alex Williamson
2018-02-23 10:56 ` Shameerali Kolothum Thodi
2018-02-21 12:22 ` [PATCH v4 6/6] vfio/type1: remove duplicate retrieval of reserved regions Shameer Kolothum
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=22b61209-61b6-207d-9d70-1db6ae630fc1@arm.com \
--to=robin.murphy@arm.com \
--cc=alex.williamson@redhat.com \
--cc=eric.auger@redhat.com \
--cc=john.garry@huawei.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=pmorel@linux.vnet.ibm.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=xuwei5@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).