On 05/11/2014 07:13 PM, Alvise Rigo wrote: > The flag is mandatory for the ARM SMMU so we always add it if the MMIO > handles it.
Hi Alvise, Refering to the root problem explanation found in https://lkml.org/lkml/2014/2/8/176, I understand the problem is specific to devices that fetch instructions from executable memory region sections (XN =0). Typically this is not the case of Midway xgmac which does not need executable regions and hence does not need that change. in http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007095.html, Will says most IOMMU mappings should be XN. I am not knowledged enough on mem mapping settings to understand the consequences of always setting XN=0, even for devices that do not need request it. Does anyone have an opinion on this? Best Regards Eric > > Signed-off-by: Alvise Rigo <a.r...@virtualopensystems.com> > --- > hw/vfio/common.c | 9 +++++++++ > hw/vfio/vfio-common.h | 1 + > linux-headers/linux/vfio.h | 2 ++ > 3 files changed, 12 insertions(+) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 9d1f723..a805c5d 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -107,6 +107,11 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr > iova, > map.flags |= VFIO_DMA_MAP_FLAG_WRITE; > } > > + /* add exec flag */ > + if (container->iommu_data.has_exec_cap) { > + map.flags |= VFIO_DMA_MAP_FLAG_EXEC; > + } > + > /* > * Try the mapping, if it fails with EBUSY, unmap the region and try > * again. This shouldn't be necessary, but we sometimes see it in > @@ -352,6 +357,10 @@ static int vfio_connect_container(VFIOGroup *group) > return -errno; > } > > + if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_IOMMU_PROT_EXEC)) { > + container->iommu_data.has_exec_cap = true; > + } > + > container->iommu_data.type1.listener = vfio_memory_listener; > container->iommu_data.release = vfio_listener_release; > > diff --git a/hw/vfio/vfio-common.h b/hw/vfio/vfio-common.h > index 21148ef..1abbd1a 100644 > --- a/hw/vfio/vfio-common.h > +++ b/hw/vfio/vfio-common.h > @@ -35,6 +35,7 @@ typedef struct VFIOContainer { > union { > VFIOType1 type1; > }; > + bool has_exec_cap; /* support of exec capability by the IOMMU */ > void (*release)(struct VFIOContainer *); > } iommu_data; > QLIST_HEAD(, VFIOGroup) group_list; > diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h > index 17c58e0..95a02c5 100644 > --- a/linux-headers/linux/vfio.h > +++ b/linux-headers/linux/vfio.h > @@ -24,6 +24,7 @@ > #define VFIO_TYPE1_IOMMU 1 > #define VFIO_SPAPR_TCE_IOMMU 2 > > +#define VFIO_IOMMU_PROT_EXEC 5 > /* > * The IOCTL interface is designed for extensibility by embedding the > * structure length (argsz) and flags into structures passed between > @@ -392,6 +393,7 @@ struct vfio_iommu_type1_dma_map { > __u32 flags; > #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device > */ > #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */ > +#define VFIO_DMA_MAP_FLAG_EXEC (1 << 2) /* executable from device */ > __u64 vaddr; /* Process virtual address */ > __u64 iova; /* IO virtual address */ > __u64 size; /* Size of mapping (bytes) */ >
