Thanks for your comments. A v5 is ongoing, with the removal of domain->caps, instead domain->domain->ops->capable(cap) is tested.
Regards,On Tue, Mar 3, 2015 at 7:01 PM, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:
On Tue, 2015-03-03 at 18:46 +0100, Eric Auger wrote:
> Hi Baptiste,
>
> In "vfio: type1: implement the VFIO_DMA_MAP_FLAG_NOEXEC flag" you still
> kept domain->caps |= IOMMU_CAP_NOEXEC so potentially overwriting 1<<
> IOMMU_CAP_CACHE_COHERENCY I guess.
>
> Sorry I do not have this 4th patch file in my mailbox.
>
> Best Regards
>
> Eric
>
> if (iommu_capable(bus, IOMMU_CAP_CACHE_COHERENCY))
> domain->caps |= (1 << IOMMU_CAP_CACHE_COHERENCY);
>
> if (iommu_capable(bus, IOMMU_CAP_NOEXEC))
> domain->caps |= IOMMU_CAP_NOEXEC;
Patch 4/5 has problems too, vfio_domains_have_iommu_cap() is called with
IOMMU_CAP_CACHE_COHERENCY, but nobody is shifting that into a bitmap
before doing the comparison.
TBH, I don't see the point of creating this artificial bitmap out of the
capabilities. Why can't we keep everything in the domain of actual
flags passed to iommu_ops functions? It's just silly to test for cached
capability and re-invent the mapping flags on every mapping call and
it's just as easy to generalize a test using the actual flags as to use
the capabilities, perhaps easier. Thanks,
Alex
> _______________________________________________
> On 03/02/2015 05:58 PM, Baptiste Reynal wrote:
> > This patch series makes the VFIO_IOMMU_TYPE1 driver buildable on ARM, so it
> > may be used with ARM SMMUs. It also adds support for the IOMMU_NOEXEC flag
> > supported by SMMUs adhering to the ARM SMMU specification so the VFIO user can
> > specify whether the target memory can be executed by the device behind the
> > SMMU.
> >
> > Changes from v3:
> > - Rebased on linux v4.0-rc1
> > - Use bit shifting for domain->caps
> > - Baptiste Reynal is the new maintainer of this serie
> > Changes from v2:
> > - Rebased on latest iommu/next branch by Joerg Roedel
> > Changes from v1:
> > - Bugfixes and corrected some typos
> > - Use enum for VFIO IOMMU driver capabilities
> >
> > Antonios Motakis (5):
> > vfio: implement iommu driver capabilities with an enum
> > vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag
> > vfio: type1: replace domain wide protection flags with supported
> > capabilities
> > vfio: type1: replace vfio_domains_have_iommu_cache with generic
> > function
> > vfio: type1: implement the VFIO_DMA_MAP_FLAG_NOEXEC flag
> >
> > drivers/vfio/vfio_iommu_type1.c | 91 +++++++++++++++++++++++++++++------------
> > include/uapi/linux/vfio.h | 30 ++++++++------
> > 2 files changed, 81 insertions(+), 40 deletions(-)
> >
>
> iommu mailing list
> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
