From: Peter Xu <peterx@redhat.com>
To: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: yang.zhong@intel.com, peter.maydell@linaro.org,
kevin.tian@intel.com, tnowicki@marvell.com, mst@redhat.com,
jean-philippe.brucker@arm.com, quintela@redhat.com,
qemu-devel@nongnu.org, armbru@redhat.com,
Auger Eric <eric.auger@redhat.com>,
bharatb.linux@gmail.com, qemu-arm@nongnu.org,
dgilbert@redhat.com, eric.auger.pro@gmail.com
Subject: Re: [PATCH for-5.0 v11 08/20] virtio-iommu: Implement translate
Date: Mon, 6 Jan 2020 12:58:50 -0500 [thread overview]
Message-ID: <20200106175850.GC219677@xz-x1> (raw)
In-Reply-To: <20200106170634.GF2062@myrica>
On Mon, Jan 06, 2020 at 06:06:34PM +0100, Jean-Philippe Brucker wrote:
> On Fri, Dec 20, 2019 at 11:51:00AM -0500, Peter Xu wrote:
> > On Fri, Dec 20, 2019 at 05:26:42PM +0100, Jean-Philippe Brucker wrote:
> > > There is at the virtio transport level: the driver sets status to
> > > FEATURES_OK once it accepted the feature bits, and to DRIVER_OK once its
> > > fully operational. The virtio-iommu spec says:
> > >
> > > If the driver does not accept the VIRTIO_IOMMU_F_BYPASS feature, the
> > > device SHOULD NOT let endpoints access the guest-physical address space.
> > >
> > > So before features negotiation, there is no access. Afterwards it depends
> > > if the VIRTIO_IOMMU_F_BYPASS has been accepted by the driver.
> >
> > Before enabling virtio-iommu device, should we still let the devices
> > to access the whole system address space? I believe that's at least
> > what Intel IOMMUs are doing. From code-wise, its:
> >
> > if (likely(s->dmar_enabled)) {
> > success = vtd_do_iommu_translate(vtd_as, vtd_as->bus, vtd_as->devfn,
> > addr, flag & IOMMU_WO, &iotlb);
> > } else {
> > /* DMAR disabled, passthrough, use 4k-page*/
> > iotlb.iova = addr & VTD_PAGE_MASK_4K;
> > iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
> > iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
> > iotlb.perm = IOMMU_RW;
> > success = true;
> > }
> >
> > From hardware-wise, an IOMMU should be close to transparent if you
> > never enable it, imho.
>
> For hardware that's not necessarily the best choice. As cited in my
> previous reply it has been shown to introduce vulnerabilities since
> malicious devices can DMA during boot, before the OS takes control of the
> IOMMU. The Arm SMMU allows an implementation to adopt a deny policy by
> default.
I see. But then how to read a sector from the block to at least boot
an OS if we use a default-deny policy? Does it still need a mapping
that is established somehow by someone before hand?
>
> > Otherwise I'm confused on how a guest (with virtio-iommu) could boot
> > with a normal BIOS that does not contain a virtio-iommu driver. For
> > example, what if the BIOS needs to read some block sectors (as you
> > mentioned)?
>
> Ideally we should aim at supporting the device in both the BIOS and the
> OS. Failing that, there should at least be a way to instantiate a
> virtio-iommu device that is blocking by default, that cannot be bypassed
> unless the controlling software decides to allow it. Could the bypass
> policy be a command-line option to the virtio-iommu device?
>
> [...]
> > > Yes bypass mode assumes that devices and drivers aren't malicious, and the
> > > IOMMU is only used for things like assigning devices to guest userspace,
> > > or having large contiguous DMA buffers.
> >
> > Yes I agree. However again when the BYPASS flag was introduced, have
> > you thought of introducing that flag per-device? IMHO that could be
> > better because you have a finer granularity on controlling all these,
> > so you'll be able to reject malicious devices but at the meantime
> > grant permission to trusted devices.
>
> At the moment that per-device behavior can be emulated by sending an
> ATTACH request followed by identity MAP. It could be a little more
> efficient to add a "bypass" flag to the ATTACH request and avoid setting
> up the identity mapping manually, since the device then wouldn't need to
> look up the mapping on translation, but I don't know how much it would
> improve performance. The device could also cache the fact that the address
> space is identity-mapped, for the same result. The domain lookup has to
> happen in any case, so you can never get the full iommu-free performance
> with these mechanisms.
IMHO it's really a matter of whether virtio-iommu wants to have a
device layer besides the domain layer for the initial versions (just
like VT-d has a device context, then it points to a domain, so it has
these two layers). But I agree for the bypass feature it should work
(though trying to detect "a device is put into an identital domain is
bypassed" is still a bit tricky to me). And after all virtio-iommu is
always extensible when needs come.
Thanks,
--
Peter Xu
next prev parent reply other threads:[~2020-01-06 17:59 UTC|newest]
Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-22 18:29 [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device Eric Auger
2019-11-22 18:29 ` [PATCH for-5.0 v11 01/20] migration: Support QLIST migration Eric Auger
2019-11-27 11:46 ` Dr. David Alan Gilbert
2020-01-08 13:19 ` Juan Quintela
2020-01-08 13:40 ` Auger Eric
2020-01-08 13:51 ` Juan Quintela
2020-01-08 14:02 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 02/20] virtio-iommu: Add skeleton Eric Auger
2019-12-10 16:31 ` Jean-Philippe Brucker
2019-12-19 10:31 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 03/20] virtio-iommu: Decode the command payload Eric Auger
2019-12-10 16:32 ` Jean-Philippe Brucker
2019-12-10 19:14 ` Peter Xu
2019-11-22 18:29 ` [PATCH for-5.0 v11 04/20] virtio-iommu: Add the iommu regions Eric Auger
2019-12-10 16:34 ` Jean-Philippe Brucker
2019-12-19 18:11 ` Auger Eric
2019-12-10 19:18 ` Peter Xu
2019-11-22 18:29 ` [PATCH for-5.0 v11 05/20] virtio-iommu: Endpoint and domains structs and helpers Eric Auger
2019-12-10 16:37 ` Jean-Philippe Brucker
2019-12-19 18:31 ` Auger Eric
2019-12-20 17:00 ` Jean-Philippe Brucker
2019-12-23 9:11 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 06/20] virtio-iommu: Implement attach/detach command Eric Auger
2019-12-10 16:41 ` Jean-Philippe Brucker
2019-12-23 9:14 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 07/20] virtio-iommu: Implement map/unmap Eric Auger
2019-12-10 16:43 ` Jean-Philippe Brucker
2019-12-23 9:42 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 08/20] virtio-iommu: Implement translate Eric Auger
2019-12-10 16:43 ` Jean-Philippe Brucker
2019-12-10 19:33 ` Peter Xu
2019-12-19 10:30 ` Auger Eric
2019-12-19 13:33 ` Peter Xu
2019-12-19 14:38 ` Auger Eric
2019-12-19 14:49 ` Peter Xu
2019-12-19 15:09 ` Auger Eric
2019-12-20 16:26 ` Jean-Philippe Brucker
2019-12-20 16:51 ` Peter Xu
2020-01-06 17:06 ` Jean-Philippe Brucker
2020-01-06 17:58 ` Peter Xu [this message]
2020-01-07 10:10 ` Jean-Philippe Brucker
2020-01-08 16:55 ` Auger Eric
2020-01-09 8:47 ` Jean-Philippe Brucker
2020-01-09 8:58 ` Auger Eric
2020-01-09 10:40 ` Jean-Philippe Brucker
2020-01-09 11:01 ` Auger Eric
2020-01-09 11:15 ` Jean-Philippe Brucker
2020-01-09 11:32 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 09/20] virtio-iommu: Implement fault reporting Eric Auger
2019-12-10 16:44 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 10/20] virtio-iommu-pci: Add virtio iommu pci support Eric Auger
2019-12-10 16:44 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 11/20] hw/arm/virt: Add the virtio-iommu device tree mappings Eric Auger
2019-12-10 16:45 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 12/20] qapi: Introduce DEFINE_PROP_INTERVAL Eric Auger
2019-11-22 19:03 ` Dr. David Alan Gilbert
2019-11-25 13:12 ` Auger Eric
2019-12-12 12:17 ` Markus Armbruster
2019-12-12 15:13 ` Auger Eric
2019-12-13 10:03 ` Markus Armbruster
2019-11-22 18:29 ` [PATCH for-5.0 v11 13/20] virtio-iommu: Implement probe request Eric Auger
2019-12-10 16:46 ` Jean-Philippe Brucker
2019-12-10 19:36 ` Peter Xu
2019-11-22 18:29 ` [PATCH for-5.0 v11 14/20] virtio-iommu: Handle reserved regions in the translation process Eric Auger
2019-12-10 16:46 ` Jean-Philippe Brucker
2019-12-10 19:39 ` Peter Xu
2019-11-22 18:29 ` [PATCH for-5.0 v11 15/20] virtio-iommu-pci: Add array of Interval properties Eric Auger
2019-12-10 16:47 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 16/20] hw/arm/virt-acpi-build: Introduce fill_iort_idmap helper Eric Auger
2019-12-10 16:47 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 17/20] hw/arm/virt-acpi-build: Add virtio-iommu node in IORT table Eric Auger
2019-12-10 16:48 ` Jean-Philippe Brucker
2019-11-22 18:29 ` [PATCH for-5.0 v11 18/20] virtio-iommu: Support migration Eric Auger
2019-11-27 12:06 ` Dr. David Alan Gilbert
2019-12-10 16:50 ` Jean-Philippe Brucker
2019-12-19 11:03 ` Auger Eric
2019-12-10 20:01 ` Peter Xu
2019-12-24 7:39 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 19/20] pc: Add support for virtio-iommu-pci Eric Auger
2019-12-10 16:50 ` Jean-Philippe Brucker
2019-12-24 7:39 ` Auger Eric
2020-01-09 12:02 ` Michael S. Tsirkin
2020-01-09 13:34 ` Auger Eric
2019-11-22 18:29 ` [PATCH for-5.0 v11 20/20] tests: Add virtio-iommu test Eric Auger
2019-11-22 21:56 ` [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device no-reply
2019-12-11 16:40 ` Michael S. Tsirkin
2019-12-11 16:48 ` Auger Eric
2019-12-11 20:40 ` Michael S. Tsirkin
2019-12-12 15:05 ` Auger Eric
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=20200106175850.GC219677@xz-x1 \
--to=peterx@redhat.com \
--cc=armbru@redhat.com \
--cc=bharatb.linux@gmail.com \
--cc=dgilbert@redhat.com \
--cc=eric.auger.pro@gmail.com \
--cc=eric.auger@redhat.com \
--cc=jean-philippe.brucker@arm.com \
--cc=jean-philippe@linaro.org \
--cc=kevin.tian@intel.com \
--cc=mst@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=tnowicki@marvell.com \
--cc=yang.zhong@intel.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).