| Message ID | 1404736043-22900-9-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 7 July 2014 13:27, Eric Auger <eric.auger@linaro.org> wrote: > From: Alvise Rigo <a.rigo@virtualopensystems.com> > > The flag is mandatory for the ARM SMMU so we always add it if the MMIO > handles it. > > Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com> > --- > hw/vfio/common.c | 9 +++++++++ > include/hw/vfio/vfio-common.h | 1 + > linux-headers/linux/vfio.h | 2 ++ > 3 files changed, 12 insertions(+) > diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h > index 26c218e..b13f7d3 100644 > --- a/linux-headers/linux/vfio.h > +++ b/linux-headers/linux/vfio.h > @@ -30,6 +30,7 @@ > */ > #define VFIO_DMA_CC_IOMMU 4 > > +#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 > @@ -398,6 +399,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) */ You shouldn't change linux-headers/ files except by syncing them from a kernel tree using scripts/update-linux-headers.sh. Those changes should always be in a separate commit that includes the kernel tree and commit hash synced against in its commit message. For an RFC patchseries where the equivalent kernel changes haven't been accepted upstream yet it's ok to sync against a local tree (and clearly note in the commit message that it's not for committing to upstream qemu), but the changes should still be in their own patch. thanks -- PMM
On Mon, Jul 07, 2014 at 01:27:18PM +0100, Eric Auger wrote: > From: Alvise Rigo <a.rigo@virtualopensystems.com> > > The flag is mandatory for the ARM SMMU so we always add it if the MMIO > handles it. I though the logic of this flag was changing (so that you request an NX mapping instead), so I'd hold off on this change until the kernel has decided what it's doing. Also, the ARM SMMU doesn't mandate any flags, you probably need this as a result of using a PL330, which is an odd case of a DMA master that spits out EXEC transactions (instruction fetch). Will
Il 07/07/2014 14:49, Will Deacon ha scritto: > On Mon, Jul 07, 2014 at 01:27:18PM +0100, Eric Auger wrote: >> From: Alvise Rigo <a.rigo@virtualopensystems.com> >> >> The flag is mandatory for the ARM SMMU so we always add it if the MMIO >> handles it. > > I though the logic of this flag was changing (so that you request an NX > mapping instead), so I'd hold off on this change until the kernel has > decided what it's doing. Yes, you are right. This patch is not needed anymore, in fact it was dropped in my last patch series. It should not be here, please ignore it. Regards, alvise > > Also, the ARM SMMU doesn't mandate any flags, you probably need this as > a result of using a PL330, which is an odd case of a DMA master that > spits out EXEC transactions (instruction fetch). > > Will >
On 07/07/2014 03:25 PM, Alvise Rigo wrote: > Il 07/07/2014 14:49, Will Deacon ha scritto: >> On Mon, Jul 07, 2014 at 01:27:18PM +0100, Eric Auger wrote: >>> From: Alvise Rigo <a.rigo@virtualopensystems.com> >>> >>> The flag is mandatory for the ARM SMMU so we always add it if the MMIO >>> handles it. >> >> I though the logic of this flag was changing (so that you request an NX >> mapping instead), so I'd hold off on this change until the kernel has >> decided what it's doing. > > Yes, you are right. > This patch is not needed anymore, in fact it was dropped in my last > patch series. > It should not be here, please ignore it. OK. My apologies. Best Regards Eric > > Regards, > alvise > >> >> Also, the ARM SMMU doesn't mandate any flags, you probably need this as >> a result of using a PL330, which is an odd case of a DMA master that >> spits out EXEC transactions (instruction fetch). >> >> Will >>
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ed93cf3..e22f326 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -233,6 +233,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 @@ -688,6 +693,10 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as) goto free_container_exit; } + 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/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index d19622b..e670ae3 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -76,6 +76,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(, VFIOGuestIOMMU) giommu_list; diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h index 26c218e..b13f7d3 100644 --- a/linux-headers/linux/vfio.h +++ b/linux-headers/linux/vfio.h @@ -30,6 +30,7 @@ */ #define VFIO_DMA_CC_IOMMU 4 +#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 @@ -398,6 +399,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) */