From: Alex Williamson <alex.williamson@redhat.com> To: "GAUGUEY Rémy 228890" <remy.gauguey@cea.fr> Cc: Antonios Motakis <a.motakis@virtualopensystems.com>, "kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "open list:VFIO DRIVER" <kvm@vger.kernel.org>, "will.deacon@arm.com" <will.deacon@arm.com>, open list <linux-kernel@vger.kernel.org>, "tech@virtualopensystems.com" <tech@virtualopensystems.com> Subject: Re: [PATCH v3 4/6] vfio: type1: replace domain wide protection flags with supported capabilities Date: Thu, 29 Jan 2015 09:13:55 -0700 [thread overview] Message-ID: <1422548035.22865.235.camel@redhat.com> (raw) In-Reply-To: <022C7612790E20489F80A6F0D54B849F3B2C11AD@EXDAG0-B1.intra.cea.fr> On Thu, 2015-01-29 at 12:24 +0000, GAUGUEY Rémy 228890 wrote: > Hi Antonios, > > On Thu, 27 Nov 2014 18:22:53 Antonios Motakis wrote: > >VFIO_IOMMU_TYPE1 keeps track for each domain it knows a list of protection > >flags it always applies to all mappings in the domain. This is used for > >domains that support IOMMU_CAP_CACHE_COHERENCY. > > > >Refactor this slightly, by keeping track instead that a given domain > >supports the capability, and applying the IOMMU_CACHE protection flag when > >doing the actual DMA mappings. > > > >This will allow us to reuse the behavior for IOMMU_CAP_NOEXEC, which we > >also want to keep track of, but without applying it to all domains that > >support it unless the user explicitly requests it. > > > >Signed-off-by: Antonios Motakis <a.motakis@xxxxxxxxxxxxxxxxxxxxxx> > >--- > > drivers/vfio/vfio_iommu_type1.c | 25 +++++++++++++++++-------- > > 1 file changed, 17 insertions(+), 8 deletions(-) > > > >diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > >index 4a9d666..c54dab8 100644 > >--- a/drivers/vfio/vfio_iommu_type1.c > >+++ b/drivers/vfio/vfio_iommu_type1.c > > if (ret) > > return ret; > > > >@@ -731,7 +740,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > > } > > > > if (iommu_capable(bus, IOMMU_CAP_CACHE_COHERENCY)) > >- domain->prot |= IOMMU_CACHE; > >+ domain->caps |= IOMMU_CAP_CACHE_COHERENCY; > > IMHO this is not good since IOMMU_CAP_CACHE_COHERENCY is not a bitfield but an enum > See include/linux/iommu.h > enum iommu_cap { > IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA > transactions */ > IOMMU_CAP_INTR_REMAP, /* IOMMU supports interrupt isolation */ > IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */ > }; Good catch! > One possible fix would to redefine the enum with values with bitfiled values > enum iommu_cap { > IOMMU_CAP_CACHE_COHERENCY = 1, /* IOMMU can enforce cache coherent DMA > transactions */ > IOMMU_CAP_INTR_REMAP = 2, /* IOMMU supports interrupt isolation */ > IOMMU_CAP_NOEXEC = 4, /* IOMMU_NOEXEC flag */ > }; This seems like a large imposition on the IOMMU code to conform to our specific use case. Perhaps the method used by the original code to cache domain overriding mapping flags separately is the better approach. Thanks, Alex
WARNING: multiple messages have this Message-ID (diff)
From: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> To: "GAUGUEY Rémy 228890" <remy.gauguey-KCE40YydGKI@public.gmane.org> Cc: "open list:VFIO DRIVER" <kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "will.deacon-5wv7dgnIgG8@public.gmane.org" <will.deacon-5wv7dgnIgG8@public.gmane.org>, open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>, "tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org" <tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>, "kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org" <kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org> Subject: Re: [PATCH v3 4/6] vfio: type1: replace domain wide protection flags with supported capabilities Date: Thu, 29 Jan 2015 09:13:55 -0700 [thread overview] Message-ID: <1422548035.22865.235.camel@redhat.com> (raw) In-Reply-To: <022C7612790E20489F80A6F0D54B849F3B2C11AD-C/8XDKI8rPk8KqdCPk9DGyghONGMnRii@public.gmane.org> On Thu, 2015-01-29 at 12:24 +0000, GAUGUEY Rémy 228890 wrote: > Hi Antonios, > > On Thu, 27 Nov 2014 18:22:53 Antonios Motakis wrote: > >VFIO_IOMMU_TYPE1 keeps track for each domain it knows a list of protection > >flags it always applies to all mappings in the domain. This is used for > >domains that support IOMMU_CAP_CACHE_COHERENCY. > > > >Refactor this slightly, by keeping track instead that a given domain > >supports the capability, and applying the IOMMU_CACHE protection flag when > >doing the actual DMA mappings. > > > >This will allow us to reuse the behavior for IOMMU_CAP_NOEXEC, which we > >also want to keep track of, but without applying it to all domains that > >support it unless the user explicitly requests it. > > > >Signed-off-by: Antonios Motakis <a.motakis@xxxxxxxxxxxxxxxxxxxxxx> > >--- > > drivers/vfio/vfio_iommu_type1.c | 25 +++++++++++++++++-------- > > 1 file changed, 17 insertions(+), 8 deletions(-) > > > >diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > >index 4a9d666..c54dab8 100644 > >--- a/drivers/vfio/vfio_iommu_type1.c > >+++ b/drivers/vfio/vfio_iommu_type1.c > > if (ret) > > return ret; > > > >@@ -731,7 +740,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > > } > > > > if (iommu_capable(bus, IOMMU_CAP_CACHE_COHERENCY)) > >- domain->prot |= IOMMU_CACHE; > >+ domain->caps |= IOMMU_CAP_CACHE_COHERENCY; > > IMHO this is not good since IOMMU_CAP_CACHE_COHERENCY is not a bitfield but an enum > See include/linux/iommu.h > enum iommu_cap { > IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA > transactions */ > IOMMU_CAP_INTR_REMAP, /* IOMMU supports interrupt isolation */ > IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */ > }; Good catch! > One possible fix would to redefine the enum with values with bitfiled values > enum iommu_cap { > IOMMU_CAP_CACHE_COHERENCY = 1, /* IOMMU can enforce cache coherent DMA > transactions */ > IOMMU_CAP_INTR_REMAP = 2, /* IOMMU supports interrupt isolation */ > IOMMU_CAP_NOEXEC = 4, /* IOMMU_NOEXEC flag */ > }; This seems like a large imposition on the IOMMU code to conform to our specific use case. Perhaps the method used by the original code to cache domain overriding mapping flags separately is the better approach. Thanks, Alex _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2015-01-29 16:14 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-29 12:24 [PATCH v3 4/6] vfio: type1: replace domain wide protection flags with supported capabilities GAUGUEY Rémy 228890 2015-01-29 16:13 ` Alex Williamson [this message] 2015-01-29 16:13 ` Alex Williamson -- strict thread matches above, loose matches on Subject: below -- 2014-11-27 17:22 [PATCH v3 0/6] vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1 Antonios Motakis 2014-11-27 17:22 ` [PATCH v3 4/6] vfio: type1: replace domain wide protection flags with supported capabilities Antonios Motakis 2014-11-27 17:22 ` 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=1422548035.22865.235.camel@redhat.com \ --to=alex.williamson@redhat.com \ --cc=a.motakis@virtualopensystems.com \ --cc=iommu@lists.linux-foundation.org \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-kernel@vger.kernel.org \ --cc=remy.gauguey@cea.fr \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.