From: Antonios Motakis <a.motakis@virtualopensystems.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: kvm-arm <kvmarm@lists.cs.columbia.edu>,
Linux IOMMU <iommu@lists.linux-foundation.org>,
VirtualOpenSystems Technical Team <tech@virtualopensystems.com>,
KVM devel mailing list <kvm@vger.kernel.org>,
Christoffer Dall <christoffer.dall@linaro.org>,
Will Deacon <will.deacon@arm.com>,
Kim Phillips <kim.phillips@freescale.com>,
Eric Auger <eric.auger@linaro.org>,
Marc Zyngier <marc.zyngier@arm.com>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv7 06/26] vfio/iommu_type1: implement the VFIO_DMA_MAP_FLAG_NOEXEC flag
Date: Mon, 29 Sep 2014 12:38:10 +0200 [thread overview]
Message-ID: <CAG8rG2ykooVtOo83hMrDvaeWvpvOpDi5X6XsHBf-BeD5WpG9Nw@mail.gmail.com> (raw)
In-Reply-To: <1411763257.7360.49.camel@ul30vt.home>
On Fri, Sep 26, 2014 at 10:27 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> On Fri, 2014-09-26 at 17:39 +0200, Antonios Motakis wrote:
>> On Wed, Sep 24, 2014 at 12:40 AM, Alex Williamson
>> <alex.williamson@redhat.com> wrote:
>> > On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
>> >> Some IOMMU drivers, such as the ARM SMMU driver, make available the
>> >> IOMMU_NOEXEC flag, to set the page tables for a device as XN (execute never).
>> >> This affects devices such as the ARM PL330 DMA Controller, which respects
>> >> this flag and will refuse to fetch DMA instructions from memory where the
>> >> XN flag has been set.
>> >>
>> >> The flag can be used only if all IOMMU domains behind the container support
>> >> the IOMMU_NOEXEC flag. Also, if any mappings are created with the flag, any
>> >> new domains with devices will have to support it as well.
>> >>
>> >> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
>> >> ---
>> >> drivers/vfio/vfio_iommu_type1.c | 38 +++++++++++++++++++++++++++++++++++++-
>> >> 1 file changed, 37 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>> >> index 0734fbe..09e5064 100644
>> >> --- a/drivers/vfio/vfio_iommu_type1.c
>> >> +++ b/drivers/vfio/vfio_iommu_type1.c
>> >> @@ -81,6 +81,26 @@ struct vfio_group {
>> >> };
>> >>
>> >> /*
>> >> + * This function returns true only if _all_ domains support the capability.
>> >> + */
>> >> +static int vfio_all_domains_have_iommu_noexec(struct vfio_iommu *iommu)
>> >
>> > Rename to vfio_domains_have_iommu_noexec() for consistency with the
>> > cache version.
>> >
>>
>> The logic here is a slightly different logic between the two. For
>> IOMMU_CACHE we generally check if any domain includes it,
>
> Not true, all the domains must support IOMMU_CACHE for the function to
> return 1. In fact, the code is so identical that if we were to cache
> IOMMU_CAP_NOEXEC into domain->prot, we should probably only have one
> function:
>
> static int vfio_domains_have_iommu_flag(struct vfio_iommu *iommu, int flag);
>
You are absolutely correct, I managed to confuse myself when switching
from vfio_domains_have_iommu_exec to vfio_domains_have_iommu_noexec.
I will implement the shared function.
>> for NOEXEC
>> in contract we need all domains to support it, otherwise we can't
>> expose the capability. Hence the _all_ addition in the name of the
>> function.
>>
>> >> +{
>> >> + struct vfio_domain *d;
>> >> + int ret = 1;
>> >> +
>> >> + mutex_lock(&iommu->lock);
>> >> + list_for_each_entry(d, &iommu->domain_list, next) {
>> >> + if (!iommu_domain_has_cap(d->domain, IOMMU_CAP_NOEXEC)) {
>> >
>> > Should we cache this in domain->prot like we do for IOMMU_CACHE?
>> >
>> >> + ret = 0;
>> >> + break;
>> >> + }
>> >> + }
>> >> + mutex_unlock(&iommu->lock);
>> >> +
>> >> + return ret;
>> >> +}
>> >> +
>> >> +/*
>> >> * This code handles mapping and unmapping of user data buffers
>> >> * into DMA'ble space using the IOMMU
>> >> */
>> >> @@ -546,6 +566,11 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>> >> prot |= IOMMU_WRITE;
>> >> if (map->flags & VFIO_DMA_MAP_FLAG_READ)
>> >> prot |= IOMMU_READ;
>> >> + if (map->flags & VFIO_DMA_MAP_FLAG_NOEXEC) {
>> >> + if (!vfio_all_domains_have_iommu_noexec(iommu))
>> >> + return -EINVAL;
>> >> + prot |= IOMMU_NOEXEC;
>> >> + }
>> >>
>> >> if (!prot || !size || (size | iova | vaddr) & mask)
>> >> return -EINVAL;
>> >> @@ -636,6 +661,12 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
>> >> dma = rb_entry(n, struct vfio_dma, node);
>> >> iova = dma->iova;
>> >>
>> >> + /* if any of the mappings to be replayed has the NOEXEC flag
>> >> + * set, then the new iommu domain must support it */
>> >
>> > nit, please fix the comment style to match the rest of the file.
>> >
>>
>> Ack
>>
>> >> + if ((dma->prot | IOMMU_NOEXEC) &&
>> >> + !iommu_domain_has_cap(domain->domain, IOMMU_CAP_NOEXEC))
>> >> + return -EINVAL;
>> >> +
>> >> while (iova < dma->iova + dma->size) {
>> >> phys_addr_t phys = iommu_iova_to_phys(d->domain, iova);
>> >> size_t size;
>> >> @@ -890,6 +921,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>> >> if (!iommu)
>> >> return 0;
>> >> return vfio_domains_have_iommu_cache(iommu);
>> >> + case VFIO_IOMMU_PROT_NOEXEC:
>> >> + if (!iommu)
>> >> + return 0;
>> >> + return vfio_all_domains_have_iommu_noexec(iommu);
>> >> default:
>> >> return 0;
>> >> }
>> >> @@ -913,7 +948,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>> >> } else if (cmd == VFIO_IOMMU_MAP_DMA) {
>> >> struct vfio_iommu_type1_dma_map map;
>> >> uint32_t mask = VFIO_DMA_MAP_FLAG_READ |
>> >> - VFIO_DMA_MAP_FLAG_WRITE;
>> >> + VFIO_DMA_MAP_FLAG_WRITE |
>> >> + VFIO_DMA_MAP_FLAG_NOEXEC;
>> >>
>> >> minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
>> >>
>> >
>> >
>> >
>>
>>
>>
>
>
>
--
Antonios Motakis
Virtual Open Systems
next prev parent reply other threads:[~2014-09-29 10:38 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1411483586-29304-1-git-send-email-a.motakis@virtualopensystems.com>
2014-09-23 14:46 ` [PATCHv7 01/26] iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC Antonios Motakis
2014-09-23 14:58 ` Will Deacon
2014-09-23 22:14 ` Alex Williamson
2014-09-23 14:46 ` [PATCHv7 02/26] iommu: add capability IOMMU_CAP_NOEXEC Antonios Motakis
2014-09-26 9:48 ` Joerg Roedel
2014-09-26 15:00 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 03/26] iommu/arm-smmu: add IOMMU_CAP_NOEXEC to the ARM SMMU driver Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 04/26] vfio/iommu_type1: support for platform bus devices on ARM Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag Antonios Motakis
2014-09-23 22:21 ` Alex Williamson
2014-09-23 22:30 ` Alex Williamson
2014-09-26 15:45 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 06/26] vfio/iommu_type1: implement " Antonios Motakis
2014-09-23 22:40 ` Alex Williamson
2014-09-26 15:39 ` Antonios Motakis
2014-09-26 20:27 ` Alex Williamson
2014-09-29 10:38 ` Antonios Motakis [this message]
2014-09-23 14:46 ` [PATCHv7 07/26] driver core: amba: add device binding path 'driver_override' Antonios Motakis
2014-09-23 22:45 ` Alex Williamson
2014-09-26 15:10 ` Antonios Motakis
2014-09-26 15:37 ` Russell King - ARM Linux
2014-09-29 10:14 ` Antonios Motakis
2014-10-08 12:18 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 08/26] driver core: amba: add documentation for " Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 09/26] vfio/platform: initial skeleton of VFIO support for platform devices Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus Antonios Motakis
2014-09-23 23:01 ` Alex Williamson
2014-09-26 15:30 ` Antonios Motakis
2014-09-26 20:18 ` Alex Williamson
2014-09-29 10:21 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 11/26] vfio: platform: add the VFIO PLATFORM module to Kconfig Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 12/26] vfio: amba: VFIO support for AMBA devices Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 13/26] vfio: amba: add the VFIO for AMBA devices module to Kconfig Antonios Motakis
2014-09-23 23:11 ` Alex Williamson
2014-09-26 15:06 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 14/26] vfio/platform: return info for bound device Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 15/26] vfio/platform: return info for device memory mapped IO regions Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 16/26] vfio/platform: read and write support for the device fd Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 17/26] vfio/platform: support MMAP of MMIO regions Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 18/26] vfio/platform: return IRQ info Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 19/26] vfio/platform: initial interrupts support code Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 20/26] vfio/platform: trigger an interrupt via eventfd Antonios Motakis
2014-09-24 17:00 ` Alex Williamson
2014-09-26 15:03 ` Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 21/26] vfio/platform: support for maskable and automasked interrupts Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 22/26] vfio: move eventfd support code for VFIO_PCI to a separate file Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 23/26] vfio: add local lock in virqfd instead of depending on VFIO PCI Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 24/26] vfio: pass an opaque pointer on virqfd initialization Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 25/26] vfio: initialize the virqfd workqueue in VFIO generic code Antonios Motakis
2014-09-23 14:46 ` [PATCHv7 26/26] vfio/platform: implement IRQ masking/unmasking via an eventfd Antonios Motakis
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=CAG8rG2ykooVtOo83hMrDvaeWvpvOpDi5X6XsHBf-BeD5WpG9Nw@mail.gmail.com \
--to=a.motakis@virtualopensystems.com \
--cc=alex.williamson@redhat.com \
--cc=christoffer.dall@linaro.org \
--cc=eric.auger@linaro.org \
--cc=iommu@lists.linux-foundation.org \
--cc=kim.phillips@freescale.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=tech@virtualopensystems.com \
--cc=will.deacon@arm.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).