Re: Question: KVM: Failed to bind vfio with PCI-e / SMMU on Juno-r2

2019-03-15 Thread Leo Yan
Hi Robin,

On Fri, Mar 15, 2019 at 12:54:10PM +, Robin Murphy wrote:
> Hi Leo,
> 
> Sorry for the delay - I'm on holiday this week, but since I've made the
> mistake of glancing at my inbox I should probably save you from wasting any
> more time...

Sorry for disturbing you in holiday and appreciate your help.  It's no
rush to reply.

> On 2019-03-15 11:03 am, Auger Eric wrote:
> > Hi Leo,
> > 
> > + Jean-Philippe
> > 
> > On 3/15/19 10:37 AM, Leo Yan wrote:
> > > Hi Eric, Robin,
> > > 
> > > On Wed, Mar 13, 2019 at 11:24:25AM +0100, Auger Eric wrote:
> > > 
> > > [...]
> > > 
> > > > > If the NIC supports MSIs they logically are used. This can be easily
> > > > > checked on host by issuing "cat /proc/interrupts | grep vfio". Can you
> > > > > check whether the guest received any interrupt? I remember that Robin
> > > > > said in the past that on Juno, the MSI doorbell was in the PCI host
> > > > > bridge window and possibly transactions towards the doorbell could not
> > > > > reach it since considered as peer to peer.
> > > > 
> > > > I found back Robin's explanation. It was not related to MSI IOVA being
> > > > within the PCI host bridge window but RAM GPA colliding with host PCI
> > > > config space?
> > > > 
> > > > "MSI doorbells integral to PCIe root complexes (and thus untranslatable)
> > > > typically have a programmable address, so could be anywhere. In the more
> > > > general category of "special hardware addresses", QEMU's default ARM
> > > > guest memory map puts RAM starting at 0x4000; on the ARM Juno
> > > > platform, that happens to be where PCI config space starts; as Juno's
> > > > PCIe doesn't support ACS, peer-to-peer or anything clever, if you assign
> > > > the PCI bus to a guest (all of it, given the lack of ACS), the root
> > > > complex just sees the guest's attempts to DMA to "memory" as the device
> > > > attempting to access config space and aborts them."
> > > 
> > > Below is some following investigation at my side:
> > > 
> > > Firstly, must admit that I don't understand well for up paragraph; so
> > > based on the description I am wandering if can use INTx mode and if
> > > it's lucky to avoid this hardware pitfall.
> > 
> > The problem above is that during the assignment process, the virtualizer
> > maps the whole guest RAM though the IOMMU (+ the MSI doorbell on ARM) to
> > allow the device, programmed in GPA to access the whole guest RAM.
> > Unfortunately if the device emits a DMA request with 0x4000 IOVA
> > address, this IOVA is interpreted by the Juno RC as a transaction
> > towards the PCIe config space. So this DMA request will not go beyond
> > the RC, will never reach the IOMMU and will never reach the guest RAM.
> > So globally the device is not able to reach part of the guest RAM.
> > That's how I interpret the above statement. Then I don't know the
> > details of the collision, I don't have access to this HW. I don't know
> > either if this problem still exists on the r2 HW.

Thanks a lot for rephrasing, Eric :)

> The short answer is that if you want PCI passthrough to work on Juno, the
> guest memory map has to look like a Juno.
> 
> The PCIe root complex uses an internal lookup table to generate appropriate
> AXI attributes for outgoing PCIe transactions; unfortunately this has no
> notion of 'default' attributes, so addresses *must* match one of the
> programmed windows in order to be valid. From memory, EDK2 sets up a 2GB
> window covering the lower DRAM bank, an 8GB window covering the upper DRAM
> bank, and a 1MB (or thereabouts) window covering the GICv2m region with
> Device attributes.

I checked kernel memory blocks info, it gives out below result:

root@debian:~# cat /sys/kernel/debug/memblock/memory
   0: 0x8000..0xfeff
   1: 0x00088000..0x0009

So I think the lower 2GB DRAM window is: [0x8000_..0xfeff_]
and the high DRAM window is [0x8_8000_..0x9__].

BTW, now I am using uboot rather than UEFI, so not sure if uboot has
programmed memory windows for PCIe.  Could you help give a point for
which registers should be set in UEFI thus I also can check related
configurations in uboot?

> Any PCIe transactions to addresses not within one of
> those windows will be aborted by the RC without ever going out to the AXI
> side where the SMMU lies (and I think anything matching the config space or
> I/O space windows or a region claimed by a BAR will be aborted even earlier
> as a peer-to-peer attempt regardless of the AXI Translation Table setup).
> 
> You could potentially modify the firmware to change the window
> configuration, but the alignment restrictions make it awkward. I've only
> ever tested passthrough on Juno using kvmtool, which IIRC already has guest
> RAM in an appropriate place (and is trivially easy to hack if not) - I don't
> remember if I ever actually tried guest MSI with that.

I did several tries with kvmtool to tweak memory regions but it's no
lucky.  Since the 

Re: [PATCH v5 05/22] iommu: Introduce cache_invalidate API

2019-03-15 Thread Jacob Pan
On Fri, 15 Mar 2019 17:08:49 +0100
Eric Auger  wrote:

> From: "Liu, Yi L" 
> 
> In any virtualization use case, when the first translation stage
> is "owned" by the guest OS, the host IOMMU driver has no knowledge
> of caching structure updates unless the guest invalidation activities
> are trapped by the virtualizer and passed down to the host.
> 
> Since the invalidation data are obtained from user space and will be
> written into physical IOMMU, we must allow security check at various
> layers. Therefore, generic invalidation data format are proposed here,
> model specific IOMMU drivers need to convert them into their own
> format.
> 
> Signed-off-by: Liu, Yi L 
> Signed-off-by: Jean-Philippe Brucker 
> Signed-off-by: Jacob Pan 
> Signed-off-by: Ashok Raj 
> Signed-off-by: Eric Auger 
> 
> ---
> v3 -> v4:
> - full reshape of the API following Alex' comments
> 
> v1 -> v2:
> - add arch_id field
> - renamed tlb_invalidate into cache_invalidate as this API allows
>   to invalidate context caches on top of IOTLBs
> 
> v1:
> renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in
> header. Commit message reworded.
> ---
>  drivers/iommu/iommu.c  | 14 
>  include/linux/iommu.h  | 21 +++
>  include/uapi/linux/iommu.h | 71
> ++ 3 files changed, 106
> insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 7d9285cea100..b72e326ddd41 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1544,6 +1544,20 @@ void iommu_detach_pasid_table(struct
> iommu_domain *domain) }
>  EXPORT_SYMBOL_GPL(iommu_detach_pasid_table);
>  
> +int iommu_cache_invalidate(struct iommu_domain *domain, struct
> device *dev,
> +struct iommu_cache_invalidate_info
> *inv_info) +{
> + int ret = 0;
> +
> + if (unlikely(!domain->ops->cache_invalidate))
> + return -ENODEV;
> +
> + ret = domain->ops->cache_invalidate(domain, dev, inv_info);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
> +
>  static void __iommu_detach_device(struct iommu_domain *domain,
> struct device *dev)
>  {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index fb9b7a8de25f..3d8e48876162 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -191,6 +191,7 @@ struct iommu_resv_region {
>   *  driver init to device driver init (default
> no)
>   * @attach_pasid_table: attach a pasid table
>   * @detach_pasid_table: detach the pasid table
> + * @cache_invalidate: invalidate translation caches
>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>   */
>  struct iommu_ops {
> @@ -239,6 +240,9 @@ struct iommu_ops {
> struct iommu_pasid_table_config
> *cfg); void (*detach_pasid_table)(struct iommu_domain *domain);
>  
> + int (*cache_invalidate)(struct iommu_domain *domain, struct
> device *dev,
> + struct iommu_cache_invalidate_info
> *inv_info); +
>   unsigned long pgsize_bitmap;
>  };
>  
> @@ -349,6 +353,9 @@ extern void iommu_detach_device(struct
> iommu_domain *domain, extern int iommu_attach_pasid_table(struct
> iommu_domain *domain, struct iommu_pasid_table_config *cfg);
>  extern void iommu_detach_pasid_table(struct iommu_domain *domain);
> +extern int iommu_cache_invalidate(struct iommu_domain *domain,
> +   struct device *dev,
> +   struct iommu_cache_invalidate_info
> *inv_info); extern struct iommu_domain
> *iommu_get_domain_for_dev(struct device *dev); extern struct
> iommu_domain *iommu_get_dma_domain(struct device *dev); extern int
> iommu_map(struct iommu_domain *domain, unsigned long iova, @@ -795,7
> +802,21 @@ int iommu_attach_pasid_table(struct iommu_domain *domain, }
>  
>  static inline
> +<<< HEAD
>  void iommu_detach_pasid_table(struct iommu_domain *domain) {}
> +===
> +void iommu_detach_pasid_table(struct iommu_domain *domain)
> +{
> + return -ENODEV;
> +}
> +static inline int
> +iommu_cache_invalidate(struct iommu_domain *domain,
> +struct device *dev,
> +struct iommu_cache_invalidate_info *inv_info)
> +{
> + return -ENODEV;
> +}
> +>>> 56df871916e5... iommu: Introduce cache_invalidate API  
forgot to merge :)
>  
>  #endif /* CONFIG_IOMMU_API */
>  
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 532a64075f23..e4c6a447e85a 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -159,4 +159,75 @@ struct iommu_pasid_table_config {
>   };
>  };
>  
> +/* defines the granularity of the invalidation */
> +enum iommu_inv_granularity {
> + IOMMU_INV_GRANU_DOMAIN, /* domain-selective
> invalidation */
> + IOMMU_INV_GRANU_PASID,  /* pasid-selective
> invalidation */
> + IOMMU_INV_GRANU_ADDR,   /* page-selective 

Re: [PATCH v2 0/3] KVM: Unify mmu_memory_cache functionality across architectures

2019-03-15 Thread Paolo Bonzini
On 31/01/19 10:52, Christoffer Dall wrote:
> We currently have duplicated functionality for the mmu_memory_cache used
> to pre-allocate memory for the page table manipulation code which cannot
> allocate memory while holding spinlocks.  This functionality is
> duplicated across x86, arm/arm64, and mips.
> 
> There were recently a debate of modifying the arm code to be more in
> line with the x86 code and some discussions around changing the page
> flags used for allocation.  This series should make it easier to take a
> uniform approach across architectures.
> 
> While there's not a huge amount of code sharing, we come out with a net
> gain.
> 
> Only tested on arm/arm64, and only compile-tested on x86 and mips.
> 
> Changes since v1:
>  - Split out rename from initial x86 patch to have separate patches to
>move the logic to common code and to rename.
>  - Introduce KVM_ARCH_WANT_MMU_MEMCACHE to avoid compile breakage on
>architectures that don't use this functionality.
>  - Rename KVM_NR_MEM_OBJS to KVM_MMU_NR_MEMCACHE_OBJS
> 
> ---
> 
> Christoffer Dall (4):
>   KVM: x86: Move mmu_memory_cache functions to common code
>   KVM: x86: Rename mmu_memory_cache to kvm_mmu_memcache
>   KVM: arm/arm64: Move to common kvm_mmu_memcache infrastructure
>   KVM: mips: Move to common kvm_mmu_memcache infrastructure
> 
>  arch/arm/include/asm/kvm_host.h  | 13 +---
>  arch/arm/include/asm/kvm_mmu.h   |  2 +-
>  arch/arm/include/asm/kvm_types.h | 12 
>  arch/arm64/include/asm/kvm_host.h| 13 +---
>  arch/arm64/include/asm/kvm_mmu.h |  2 +-
>  arch/arm64/include/asm/kvm_types.h   | 13 
>  arch/mips/include/asm/kvm_host.h | 15 +
>  arch/mips/include/asm/kvm_types.h| 12 
>  arch/mips/kvm/mips.c |  2 +-
>  arch/mips/kvm/mmu.c  | 54 +++-
>  arch/powerpc/include/asm/kvm_types.h |  5 ++
>  arch/s390/include/asm/kvm_types.h|  5 ++
>  arch/x86/include/asm/kvm_host.h  | 17 +
>  arch/x86/include/asm/kvm_types.h | 12 
>  arch/x86/kvm/mmu.c   | 97 ++--
>  arch/x86/kvm/paging_tmpl.h   |  4 +-
>  include/linux/kvm_host.h | 11 
>  include/linux/kvm_types.h| 13 
>  virt/kvm/arm/arm.c   |  2 +-
>  virt/kvm/arm/mmu.c   | 68 +--
>  virt/kvm/kvm_main.c  | 60 +
>  21 files changed, 202 insertions(+), 230 deletions(-)
>  create mode 100644 arch/arm/include/asm/kvm_types.h
>  create mode 100644 arch/arm64/include/asm/kvm_types.h
>  create mode 100644 arch/mips/include/asm/kvm_types.h
>  create mode 100644 arch/powerpc/include/asm/kvm_types.h
>  create mode 100644 arch/s390/include/asm/kvm_types.h
>  create mode 100644 arch/x86/include/asm/kvm_types.h
> 

Sorry for the belated review, this is a good idea and I would like to
include it at the beginning of the 5.2 cycle.

Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 3/4] KVM: arm/arm64: Move to common kvm_mmu_memcache infrastructure

2019-03-15 Thread Paolo Bonzini
On 31/01/19 10:52, Christoffer Dall wrote:
> +#define KVM_MMU_CACHE_GFPGFP_KERNEL
> +#define KVM_MMU_CACHE_PAGE_GFP   (GFP_KERNEL | __GFP_ZERO)

This should be GFP_KERNEL_ACCOUNT.  Can you leave GFP_KERNEL_ACCOUNT in
virt/kvm/kvm_main.c, and only use these #defines to add __GFP_ZERO?  I
don't think anyone should be using GFP_ATOMIC for instance.

Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 22/22] vfio: Document nested stage control

2019-03-15 Thread Eric Auger
New iotcls were introduced to pass information about guest stage1
to the host through VFIO. Let's document the nested stage control.

Signed-off-by: Eric Auger 

---

v2 -> v3:
- document the new fault API

v1 -> v2:
- use the new ioctl names
- add doc related to fault handling
---
 Documentation/vfio.txt | 83 ++
 1 file changed, 83 insertions(+)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index f1a4d3c3ba0b..aab59ddf5ebd 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -239,6 +239,89 @@ group and can access them as follows::
/* Gratuitous device reset and go... */
ioctl(device, VFIO_DEVICE_RESET);
 
+IOMMU Dual Stage Control
+
+
+Some IOMMUs support 2 stages/levels of translation. "Stage" corresponds to
+the ARM terminology while "level" corresponds to Intel's VTD terminology. In
+the following text we use either without distinction.
+
+This is useful when the guest is exposed with a virtual IOMMU and some
+devices are assigned to the guest through VFIO. Then the guest OS can use
+stage 1 (IOVA -> GPA), while the hypervisor uses stage 2 for VM isolation
+(GPA -> HPA).
+
+The guest gets ownership of the stage 1 page tables and also owns stage 1
+configuration structures. The hypervisor owns the root configuration structure
+(for security reason), including stage 2 configuration. This works as long
+configuration structures and page table format are compatible between the
+virtual IOMMU and the physical IOMMU.
+
+Assuming the HW supports it, this nested mode is selected by choosing the
+VFIO_TYPE1_NESTING_IOMMU type through:
+
+ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU);
+
+This forces the hypervisor to use the stage 2, leaving stage 1 available for
+guest usage.
+
+Once groups are attached to the container, the guest stage 1 translation
+configuration data can be passed to VFIO by using
+
+ioctl(container, VFIO_IOMMU_BIND_PASID_TABLE, _table_info);
+
+This allows to combine guest stage 1 configuration structure along with
+hypervisor stage 2 configuration structure. stage 1 configuration structures
+are dependent on the IOMMU type.
+
+As the stage 1 translation is fully delegated to the HW, physical events that
+may occur (especially translation faults), need to be propagated up to
+the virtualizer and re-injected into the guest.
+
+The userspace must be prepared to receive faults. The VFIO-PCI device
+exposes 2 regions dedicated to HW faults: one read-only "producer" fault
+region (kernel is the producer and writes into this region) and one
+write-only "consumer" fault region, type/subtype respectively:
+- VFIO_REGION_TYPE_NESTED/VFIO_REGION_SUBTYPE_NESTED_FAULT_PROD
+- VFIO_REGION_TYPE_NESTED/VFIO_REGION_SUBTYPE_NESTED_FAULT_CONS
+
+The producer fault region exposes a VFIO_REGION_INFO_CAP_PRODUCER_FAULT
+region capability that allows the userspace to retrieve the max fault
+ABI version supported by the kernel.
+
+The ABI version can be negotiated: the userspace writes the version it
+wants in the consumer region (greater or equal than 1). Once set, the
+ABI version cannot be changed.
+
+Then by using VFIO_DEVICE_SET_IRQS along with the VFIO_PCI_DMA_FAULT_IRQ_INDEX
+index, the virtualizer can register an eventfd signalled whenever a fault is
+observed at physical level.
+
+The kernel writes the fault records formatted according to the negotiated
+ABI version in the producer region fault queue. This part of the producer
+fault region can be mmapped (see VFIO_REGION_INFO_CAP_SPARSE_MMAP result).
+
+When the userspace consumes a fault in the queue, it should increment
+the consumer index to allow new fault records to replace the used ones.
+The queue size and the entry size can be retrieved in the producer region.
+The consumer index should never overshoot the producer index as in any
+other circular buffer scheme. Also it must be less than the queue size
+otherwise the change is ignored by the kernel.
+
+When the guest invalidates stage 1 related caches, invalidations must be
+forwarded to the host through
+ioctl(container, VFIO_IOMMU_CACHE_INVALIDATE, _data);
+Those invalidations can happen at various granularity levels, page, context, 
...
+
+The ARM SMMU specification introduces another challenge: MSIs are translated by
+both the virtual SMMU and the physical SMMU. To build a nested mapping for the
+IOVA programmed into the assigned device, the guest needs to pass its IOVA/MSI
+doorbell GPA binding to the host. Then the hypervisor can build a nested stage 
2
+binding eventually translating into the physical MSI doorbell.
+
+This is achieved by
+ioctl(container, VFIO_IOMMU_BIND_MSI, _binding);
+
 VFIO User API
 ---
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 21/22] vfio-pci: Add VFIO_PCI_DMA_FAULT_IRQ_INDEX

2019-03-15 Thread Eric Auger
Add a new VFIO_PCI_DMA_FAULT_IRQ_INDEX index. This allows to
set/unset an eventfd that will be triggered when DMA translation
faults are detected at physical level when the nested mode is used.

Signed-off-by: Eric Auger 
---
 drivers/vfio/pci/vfio_pci.c   |  3 +++
 drivers/vfio/pci/vfio_pci_intrs.c | 19 +++
 include/uapi/linux/vfio.h |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 8c895ece4750..36b57fe363d7 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -750,6 +750,8 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
*vdev, int irq_type)
return 1;
} else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
return 1;
+   } else if (irq_type == VFIO_PCI_DMA_FAULT_IRQ_INDEX) {
+   return 1;
}
 
return 0;
@@ -1086,6 +1088,7 @@ static long vfio_pci_ioctl(void *device_data,
switch (info.index) {
case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
case VFIO_PCI_REQ_IRQ_INDEX:
+   case VFIO_PCI_DMA_FAULT_IRQ_INDEX:
break;
case VFIO_PCI_ERR_IRQ_INDEX:
if (pci_is_pcie(vdev->pdev))
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c 
b/drivers/vfio/pci/vfio_pci_intrs.c
index 1c46045b0e7f..28a96117daf3 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -622,6 +622,18 @@ static int vfio_pci_set_req_trigger(struct vfio_pci_device 
*vdev,
   count, flags, data);
 }
 
+static int vfio_pci_set_dma_fault_trigger(struct vfio_pci_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags,
+ void *data)
+{
+   if (index != VFIO_PCI_DMA_FAULT_IRQ_INDEX || start != 0 || count > 1)
+   return -EINVAL;
+
+   return vfio_pci_set_ctx_trigger_single(>dma_fault_trigger,
+  count, flags, data);
+}
+
 int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags,
unsigned index, unsigned start, unsigned count,
void *data)
@@ -671,6 +683,13 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, 
uint32_t flags,
break;
}
break;
+   case VFIO_PCI_DMA_FAULT_IRQ_INDEX:
+   switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+   case VFIO_IRQ_SET_ACTION_TRIGGER:
+   func = vfio_pci_set_dma_fault_trigger;
+   break;
+   }
+   break;
}
 
if (!func)
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 40b7aec8fefa..b47f65df5b86 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -555,6 +555,7 @@ enum {
VFIO_PCI_MSIX_IRQ_INDEX,
VFIO_PCI_ERR_IRQ_INDEX,
VFIO_PCI_REQ_IRQ_INDEX,
+   VFIO_PCI_DMA_FAULT_IRQ_INDEX,
VFIO_PCI_NUM_IRQS
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 20/22] vfio_pci: Allow to mmap the fault queue

2019-03-15 Thread Eric Auger
The Producer Fault region contains the fault queue in the second page.
There is benefit to let the userspace mmap this area. So let's expose
this mmappable area through a sparse mmap entry and implement the mmap
operation.

Signed-off-by: Eric Auger 
---
 drivers/vfio/pci/vfio_pci.c | 61 +++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index cf12204486c3..8c895ece4750 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -274,15 +274,70 @@ static const struct vfio_pci_fault_abi 
fault_abi_versions[] = {
 
 #define NR_FAULT_ABIS ARRAY_SIZE(fault_abi_versions)
 
+static int vfio_pci_fault_mmap(struct vfio_pci_device *vdev,
+  struct vfio_pci_region *region,
+  struct vm_area_struct *vma)
+{
+   u64 phys_len, req_len, pgoff, req_start;
+   unsigned long long addr;
+   unsigned int index, ret;
+
+   index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
+
+   phys_len = region->size;
+
+   req_len = vma->vm_end - vma->vm_start;
+   pgoff = vma->vm_pgoff &
+   ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+   req_start = pgoff << PAGE_SHIFT;
+
+   /* only the second page of the producer fault region is mmappable */
+   if (req_start < PAGE_SIZE)
+   return -EINVAL;
+
+   if (req_start + req_len > phys_len)
+   return -EINVAL;
+
+   addr = virt_to_phys(vdev->fault_pages);
+   vma->vm_private_data = vdev;
+   vma->vm_pgoff = (addr >> PAGE_SHIFT) + pgoff;
+
+   ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ req_len, vma->vm_page_prot);
+   return ret;
+}
+
 static int vfio_pci_fault_prod_add_capability(struct vfio_pci_device *vdev,
struct vfio_pci_region *region, struct vfio_info_cap *caps)
 {
+   struct vfio_region_info_cap_sparse_mmap *sparse = NULL;
struct vfio_region_info_cap_fault cap = {
.header.id = VFIO_REGION_INFO_CAP_PRODUCER_FAULT,
.header.version = 1,
.version = NR_FAULT_ABIS,
};
-   return vfio_info_add_capability(caps, , sizeof(cap));
+   size_t size = sizeof(*sparse) + sizeof(*sparse->areas);
+   int ret;
+
+   ret = vfio_info_add_capability(caps, , sizeof(cap));
+   if (ret)
+   return ret;
+
+   sparse = kzalloc(size, GFP_KERNEL);
+   if (!sparse)
+   return -ENOMEM;
+
+   sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
+   sparse->header.version = 1;
+   sparse->nr_areas = 1;
+   sparse->areas[0].offset = PAGE_SIZE;
+   sparse->areas[0].size = PAGE_SIZE;
+
+   ret = vfio_info_add_capability(caps, >header, size);
+   if (ret)
+   kfree(sparse);
+
+   return ret;
 }
 
 static const struct vfio_pci_regops vfio_pci_fault_cons_regops = {
@@ -293,6 +348,7 @@ static const struct vfio_pci_regops 
vfio_pci_fault_cons_regops = {
 static const struct vfio_pci_regops vfio_pci_fault_prod_regops = {
.rw = vfio_pci_fault_prod_rw,
.release= vfio_pci_fault_release,
+   .mmap   = vfio_pci_fault_mmap,
.add_capability = vfio_pci_fault_prod_add_capability,
 };
 
@@ -351,7 +407,8 @@ static int vfio_pci_init_fault_region(struct 
vfio_pci_device *vdev)
VFIO_REGION_TYPE_NESTED,
VFIO_REGION_SUBTYPE_NESTED_FAULT_PROD,
_pci_fault_prod_regops, 2 * PAGE_SIZE,
-   VFIO_REGION_INFO_FLAG_READ, vdev->fault_pages);
+   VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_MMAP,
+   vdev->fault_pages);
if (ret)
goto out;
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 19/22] vfio-pci: Register an iommu fault handler

2019-03-15 Thread Eric Auger
This patch registers a fault handler which records faults in
a circular buffer and then signals an eventfd. This buffer is
exposed within the fault region.

Signed-off-by: Eric Auger 

---

v3 -> v4:
- move iommu_unregister_device_fault_handler to vfio_pci_release
---
 drivers/vfio/pci/vfio_pci.c | 49 +
 drivers/vfio/pci/vfio_pci_private.h |  1 +
 2 files changed, 50 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 01b1b4cb8349..cf12204486c3 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vfio_pci_private.h"
 
@@ -295,6 +296,46 @@ static const struct vfio_pci_regops 
vfio_pci_fault_prod_regops = {
.add_capability = vfio_pci_fault_prod_add_capability,
 };
 
+int vfio_pci_iommu_dev_fault_handler(struct iommu_fault_event *evt, void *data)
+{
+   struct vfio_pci_device *vdev = (struct vfio_pci_device *) data;
+   struct vfio_region_fault_prod *prod_region =
+   (struct vfio_region_fault_prod *)vdev->fault_pages;
+   struct vfio_region_fault_cons *cons_region =
+   (struct vfio_region_fault_cons *)(vdev->fault_pages + 2 * 
PAGE_SIZE);
+   struct iommu_fault *new =
+   (struct iommu_fault *)(vdev->fault_pages + prod_region->offset +
+   prod_region->prod * prod_region->entry_size);
+   int prod, cons, size;
+
+   mutex_lock(>fault_queue_lock);
+
+   if (!vdev->fault_abi)
+   goto unlock;
+
+   prod = prod_region->prod;
+   cons = cons_region->cons;
+   size = prod_region->nb_entries;
+
+   if (CIRC_SPACE(prod, cons, size) < 1)
+   goto unlock;
+
+   *new = evt->fault;
+   prod = (prod + 1) % size;
+   prod_region->prod = prod;
+   mutex_unlock(>fault_queue_lock);
+
+   mutex_lock(>igate);
+   if (vdev->dma_fault_trigger)
+   eventfd_signal(vdev->dma_fault_trigger, 1);
+   mutex_unlock(>igate);
+   return 0;
+
+unlock:
+   mutex_unlock(>fault_queue_lock);
+   return -EINVAL;
+}
+
 static int vfio_pci_init_fault_region(struct vfio_pci_device *vdev)
 {
struct vfio_region_fault_prod *header;
@@ -327,6 +368,13 @@ static int vfio_pci_init_fault_region(struct 
vfio_pci_device *vdev)
header = (struct vfio_region_fault_prod *)vdev->fault_pages;
header->version = -1;
header->offset = PAGE_SIZE;
+
+   ret = iommu_register_device_fault_handler(>pdev->dev,
+   vfio_pci_iommu_dev_fault_handler,
+   vdev);
+   if (ret)
+   goto out;
+
return 0;
 out:
kfree(vdev->fault_pages);
@@ -574,6 +622,7 @@ static void vfio_pci_release(void *device_data)
if (!(--vdev->refcnt)) {
vfio_spapr_pci_eeh_release(vdev->pdev);
vfio_pci_disable(vdev);
+   iommu_unregister_device_fault_handler(>pdev->dev);
}
 
mutex_unlock(>reflck->lock);
diff --git a/drivers/vfio/pci/vfio_pci_private.h 
b/drivers/vfio/pci/vfio_pci_private.h
index 8e0a55682d3f..a9276926f008 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -122,6 +122,7 @@ struct vfio_pci_device {
int ioeventfds_nr;
struct eventfd_ctx  *err_trigger;
struct eventfd_ctx  *req_trigger;
+   struct eventfd_ctx  *dma_fault_trigger;
struct mutexfault_queue_lock;
int fault_abi;
struct list_headdummy_resources_list;
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 18/22] vfio-pci: Add a new VFIO_REGION_TYPE_NESTED region type

2019-03-15 Thread Eric Auger
This patch adds two new regions aiming to handle nested mode
translation faults.

The first region (two host kernel pages) is read-only from the
user-space perspective. The first page contains an header
that provides information about the circular buffer located in the
second page. The circular buffer is put in a different page in
the prospect to be mmappable.

The max user API version supported by the kernel is returned
through a dedicated fault region capability.

The prod header contains
- the user API version in use (potentially inferior to the one
  returned in the capability),
- the offset of the queue within the region,
- the producer index relative to the start of the queue
- the max number of fault records,
- the size of each record.

The second region is write-only from the user perspective. It
contains the version of the requested fault ABI and the consumer
index that is updated by the userspace each time this latter has
consumed fault records.

The natural order of operation for the userspace is:
- retrieve the highest supported fault ABI version
- set the requested fault ABI version in the consumer region

Until the ABI version is not set by the userspace, the kernel
cannot return a comprehensive set of information inside the
prod header (entry size and number of entries in the fault queue).

Signed-off-by: Eric Auger 

---

v4 -> v5
- check cons is not null in vfio_pci_check_cons_fault

v3 -> v4:
- use 2 separate regions, respectively in read and write modes
- add the version capability
---
 drivers/vfio/pci/vfio_pci.c | 105 
 drivers/vfio/pci/vfio_pci_private.h |  17 +
 drivers/vfio/pci/vfio_pci_rdwr.c|  73 +++
 include/uapi/linux/vfio.h   |  42 +++
 4 files changed, 237 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index a25659b5a5d1..01b1b4cb8349 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -260,6 +260,106 @@ int vfio_pci_set_power_state(struct vfio_pci_device 
*vdev, pci_power_t state)
return ret;
 }
 
+void vfio_pci_fault_release(struct vfio_pci_device *vdev,
+   struct vfio_pci_region *region)
+{
+}
+
+static const struct vfio_pci_fault_abi fault_abi_versions[] = {
+   [0] = {
+   .entry_size = sizeof(struct iommu_fault),
+   },
+};
+
+#define NR_FAULT_ABIS ARRAY_SIZE(fault_abi_versions)
+
+static int vfio_pci_fault_prod_add_capability(struct vfio_pci_device *vdev,
+   struct vfio_pci_region *region, struct vfio_info_cap *caps)
+{
+   struct vfio_region_info_cap_fault cap = {
+   .header.id = VFIO_REGION_INFO_CAP_PRODUCER_FAULT,
+   .header.version = 1,
+   .version = NR_FAULT_ABIS,
+   };
+   return vfio_info_add_capability(caps, , sizeof(cap));
+}
+
+static const struct vfio_pci_regops vfio_pci_fault_cons_regops = {
+   .rw = vfio_pci_fault_cons_rw,
+   .release= vfio_pci_fault_release,
+};
+
+static const struct vfio_pci_regops vfio_pci_fault_prod_regops = {
+   .rw = vfio_pci_fault_prod_rw,
+   .release= vfio_pci_fault_release,
+   .add_capability = vfio_pci_fault_prod_add_capability,
+};
+
+static int vfio_pci_init_fault_region(struct vfio_pci_device *vdev)
+{
+   struct vfio_region_fault_prod *header;
+   int ret;
+
+   mutex_init(>fault_queue_lock);
+
+   vdev->fault_pages = kzalloc(3 * PAGE_SIZE, GFP_KERNEL);
+   if (!vdev->fault_pages)
+   return -ENOMEM;
+
+   ret = vfio_pci_register_dev_region(vdev,
+   VFIO_REGION_TYPE_NESTED,
+   VFIO_REGION_SUBTYPE_NESTED_FAULT_PROD,
+   _pci_fault_prod_regops, 2 * PAGE_SIZE,
+   VFIO_REGION_INFO_FLAG_READ, vdev->fault_pages);
+   if (ret)
+   goto out;
+
+   ret = vfio_pci_register_dev_region(vdev,
+   VFIO_REGION_TYPE_NESTED,
+   VFIO_REGION_SUBTYPE_NESTED_FAULT_CONS,
+   _pci_fault_cons_regops,
+   sizeof(struct vfio_region_fault_cons),
+   VFIO_REGION_INFO_FLAG_WRITE,
+   vdev->fault_pages + 2 * PAGE_SIZE);
+   if (ret)
+   goto out;
+
+   header = (struct vfio_region_fault_prod *)vdev->fault_pages;
+   header->version = -1;
+   header->offset = PAGE_SIZE;
+   return 0;
+out:
+   kfree(vdev->fault_pages);
+   return ret;
+}
+
+int vfio_pci_check_cons_fault(struct vfio_pci_device *vdev,
+struct vfio_region_fault_cons *cons_header)
+{
+   struct vfio_region_fault_prod *prod_header =
+   (struct vfio_region_fault_prod *)vdev->fault_pages;
+
+   if (cons_header->version > NR_FAULT_ABIS)
+   return -EINVAL;
+
+   if (!vdev->fault_abi) {
+   vdev->fault_abi = cons_header->version;
+   prod_header->entry_size =
+ 

[PATCH v5 17/22] iommu/smmuv3: Report non recoverable faults

2019-03-15 Thread Eric Auger
When a stage 1 related fault event is read from the event queue,
let's propagate it to potential external fault listeners, ie. users
who registered a fault handler.

Signed-off-by: Eric Auger 

---
v4 -> v5:
- s/IOMMU_FAULT_PERM_INST/IOMMU_FAULT_PERM_EXEC
---
 drivers/iommu/arm-smmu-v3.c | 169 +---
 1 file changed, 158 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index a4b82c520647..8b2160788c9b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -167,6 +167,26 @@
 #define ARM_SMMU_PRIQ_IRQ_CFG1 0xd8
 #define ARM_SMMU_PRIQ_IRQ_CFG2 0xdc
 
+/* Events */
+#define ARM_SMMU_EVT_F_UUT 0x01
+#define ARM_SMMU_EVT_C_BAD_STREAMID0x02
+#define ARM_SMMU_EVT_F_STE_FETCH   0x03
+#define ARM_SMMU_EVT_C_BAD_STE 0x04
+#define ARM_SMMU_EVT_F_BAD_ATS_TREQ0x05
+#define ARM_SMMU_EVT_F_STREAM_DISABLED 0x06
+#define ARM_SMMU_EVT_F_TRANSL_FORBIDDEN0x07
+#define ARM_SMMU_EVT_C_BAD_SUBSTREAMID 0x08
+#define ARM_SMMU_EVT_F_CD_FETCH0x09
+#define ARM_SMMU_EVT_C_BAD_CD  0x0a
+#define ARM_SMMU_EVT_F_WALK_EABT   0x0b
+#define ARM_SMMU_EVT_F_TRANSLATION 0x10
+#define ARM_SMMU_EVT_F_ADDR_SIZE   0x11
+#define ARM_SMMU_EVT_F_ACCESS  0x12
+#define ARM_SMMU_EVT_F_PERMISSION  0x13
+#define ARM_SMMU_EVT_F_TLB_CONFLICT0x20
+#define ARM_SMMU_EVT_F_CFG_CONFLICT0x21
+#define ARM_SMMU_EVT_E_PAGE_REQUEST0x24
+
 /* Common MSI config fields */
 #define MSI_CFG0_ADDR_MASK GENMASK_ULL(51, 2)
 #define MSI_CFG2_SHGENMASK(5, 4)
@@ -332,6 +352,15 @@
 #define EVTQ_MAX_SZ_SHIFT  7
 
 #define EVTQ_0_ID  GENMASK_ULL(7, 0)
+#define EVTQ_0_SSV GENMASK_ULL(11, 11)
+#define EVTQ_0_SUBSTREAMID GENMASK_ULL(31, 12)
+#define EVTQ_0_STREAMIDGENMASK_ULL(63, 32)
+#define EVTQ_1_PNU GENMASK_ULL(33, 33)
+#define EVTQ_1_IND GENMASK_ULL(34, 34)
+#define EVTQ_1_RNW GENMASK_ULL(35, 35)
+#define EVTQ_1_S2  GENMASK_ULL(39, 39)
+#define EVTQ_1_CLASS   GENMASK_ULL(40, 41)
+#define EVTQ_3_FETCH_ADDR  GENMASK_ULL(51, 3)
 
 /* PRI queue */
 #define PRIQ_ENT_DWORDS2
@@ -639,6 +668,64 @@ struct arm_smmu_domain {
spinlock_t  devices_lock;
 };
 
+/* fault propagation */
+
+#define IOMMU_FAULT_F_FIELDS   (IOMMU_FAULT_UNRECOV_PASID_VALID | \
+IOMMU_FAULT_UNRECOV_PERM_VALID | \
+IOMMU_FAULT_UNRECOV_ADDR_VALID)
+
+struct arm_smmu_fault_propagation_data {
+   enum iommu_fault_reason reason;
+   bool s1_check;
+   u32 fields; /* IOMMU_FAULT_UNRECOV_*_VALID bits */
+};
+
+/*
+ * Describes how SMMU faults translate into generic IOMMU faults
+ * and if they need to be reported externally
+ */
+static const struct arm_smmu_fault_propagation_data fault_propagation[] = {
+[ARM_SMMU_EVT_F_UUT]   = { },
+[ARM_SMMU_EVT_C_BAD_STREAMID]  = { },
+[ARM_SMMU_EVT_F_STE_FETCH] = { },
+[ARM_SMMU_EVT_C_BAD_STE]   = { },
+[ARM_SMMU_EVT_F_BAD_ATS_TREQ]  = { },
+[ARM_SMMU_EVT_F_STREAM_DISABLED]   = { },
+[ARM_SMMU_EVT_F_TRANSL_FORBIDDEN]  = { },
+[ARM_SMMU_EVT_C_BAD_SUBSTREAMID]   = {IOMMU_FAULT_REASON_PASID_INVALID,
+  false,
+  IOMMU_FAULT_UNRECOV_PASID_VALID
+ },
+[ARM_SMMU_EVT_F_CD_FETCH]  = {IOMMU_FAULT_REASON_PASID_FETCH,
+  false,
+  IOMMU_FAULT_UNRECOV_PASID_VALID |
+  IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID
+ },
+[ARM_SMMU_EVT_C_BAD_CD]= 
{IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
+  false,
+  IOMMU_FAULT_UNRECOV_PASID_VALID
+ },
+[ARM_SMMU_EVT_F_WALK_EABT] = {IOMMU_FAULT_REASON_WALK_EABT, true,
+  IOMMU_FAULT_F_FIELDS |
+  IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID
+ },
+[ARM_SMMU_EVT_F_TRANSLATION]   = {IOMMU_FAULT_REASON_PTE_FETCH, true,
+  IOMMU_FAULT_F_FIELDS
+ },
+[ARM_SMMU_EVT_F_ADDR_SIZE] = {IOMMU_FAULT_REASON_OOR_ADDRESS, true,
+  IOMMU_FAULT_F_FIELDS
+ },
+[ARM_SMMU_EVT_F_ACCESS]= {IOMMU_FAULT_REASON_ACCESS, 

[PATCH v5 16/22] iommu/smmuv3: Implement bind/unbind_guest_msi

2019-03-15 Thread Eric Auger
The bind/unbind_guest_msi() callbacks check the domain
is NESTED and redirect to the dma-iommu implementation.

Signed-off-by: Eric Auger 
---
 drivers/iommu/arm-smmu-v3.c | 44 +
 1 file changed, 44 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4a05dd66df74..a4b82c520647 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2207,6 +2207,48 @@ static void arm_smmu_put_resv_regions(struct device *dev,
kfree(entry);
 }
 
+static int
+arm_smmu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+   dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_device *smmu;
+   int ret = -EINVAL;
+
+   mutex_lock(_domain->init_mutex);
+   smmu = smmu_domain->smmu;
+   if (!smmu)
+   goto out;
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   goto out;
+
+   ret = iommu_dma_bind_guest_msi(domain, dev, giova, gpa, size);
+out:
+   mutex_unlock(_domain->init_mutex);
+   return ret;
+}
+
+static void
+arm_smmu_unbind_guest_msi(struct iommu_domain *domain,
+ struct device *dev, dma_addr_t giova)
+{
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_device *smmu;
+
+   mutex_lock(_domain->init_mutex);
+   smmu = smmu_domain->smmu;
+   if (!smmu)
+   goto unlock;
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   goto unlock;
+
+   iommu_dma_unbind_guest_msi(domain, dev, giova);
+unlock:
+   mutex_unlock(_domain->init_mutex);
+}
+
 static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
   struct iommu_pasid_table_config *cfg)
 {
@@ -2396,6 +2438,8 @@ static struct iommu_ops arm_smmu_ops = {
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
.cache_invalidate   = arm_smmu_cache_invalidate,
+   .bind_guest_msi = arm_smmu_bind_guest_msi,
+   .unbind_guest_msi   = arm_smmu_unbind_guest_msi,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 13/22] iommu/smmuv3: Implement attach/detach_pasid_table

2019-03-15 Thread Eric Auger
On attach_pasid_table() we program STE S1 related info set
by the guest into the actual physical STEs. At minimum
we need to program the context descriptor GPA and compute
whether the stage1 is translated/bypassed or aborted.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- adapt to changes in iommu_pasid_table_config
- different programming convention at s1_cfg/s2_cfg/ste.abort

v2 -> v3:
- callback now is named set_pasid_table and struct fields
  are laid out differently.

v1 -> v2:
- invalidate the STE before changing them
- hold init_mutex
- handle new fields
---
 drivers/iommu/arm-smmu-v3.c | 114 
 1 file changed, 114 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e22e944ffc05..e41f61844d78 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2207,6 +2207,118 @@ static void arm_smmu_put_resv_regions(struct device 
*dev,
kfree(entry);
 }
 
+static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
+  struct iommu_pasid_table_config *cfg)
+{
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_master_data *entry;
+   struct arm_smmu_s1_cfg *s1_cfg;
+   struct arm_smmu_device *smmu;
+   unsigned long flags;
+   int ret = -EINVAL;
+
+   if (cfg->format != IOMMU_PASID_FORMAT_SMMUV3)
+   return -EINVAL;
+
+   mutex_lock(_domain->init_mutex);
+
+   smmu = smmu_domain->smmu;
+
+   if (!smmu)
+   goto out;
+
+   if (!((smmu->features & ARM_SMMU_FEAT_TRANS_S1) &&
+ (smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+   dev_info(smmu_domain->smmu->dev,
+"does not implement two stages\n");
+   goto out;
+   }
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   goto out;
+
+   switch (cfg->config) {
+   case IOMMU_PASID_CONFIG_ABORT:
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_for_each_entry(entry, _domain->devices, list) {
+   entry->ste.s1_cfg = NULL;
+   entry->ste.abort = true;
+   arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+   }
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
+   ret = 0;
+   break;
+   case IOMMU_PASID_CONFIG_BYPASS:
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_for_each_entry(entry, _domain->devices, list) {
+   entry->ste.s1_cfg = NULL;
+   entry->ste.abort = false;
+   arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+   }
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
+   ret = 0;
+   break;
+   case IOMMU_PASID_CONFIG_TRANSLATE:
+   /* we currently support a single CD */
+   if (cfg->pasid_bits)
+   goto out;
+
+   s1_cfg = _domain->s1_cfg;
+   s1_cfg->cdptr_dma = cfg->base_ptr;
+
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_for_each_entry(entry, _domain->devices, list) {
+   entry->ste.s1_cfg = s1_cfg;
+   entry->ste.abort = false;
+   arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+   }
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
+   ret = 0;
+   break;
+   default:
+   break;
+   }
+out:
+   mutex_unlock(_domain->init_mutex);
+   return ret;
+}
+
+static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
+{
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_master_data *entry;
+   struct arm_smmu_device *smmu;
+   unsigned long flags;
+
+   mutex_lock(_domain->init_mutex);
+
+   smmu = smmu_domain->smmu;
+
+   if (!smmu)
+   return;
+
+   if (!((smmu->features & ARM_SMMU_FEAT_TRANS_S1) &&
+ (smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+   dev_info(smmu_domain->smmu->dev,
+"does not implement two stages\n");
+   return;
+   }
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   return;
+
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_for_each_entry(entry, _domain->devices, list) {
+   entry->ste.s1_cfg = NULL;
+   entry->ste.abort = true;
+   arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+   }
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
+
+   memset(_domain->s1_cfg, 0, sizeof(struct arm_smmu_s1_cfg));
+   mutex_unlock(_domain->init_mutex);
+}
+
 static struct 

[PATCH v5 15/22] dma-iommu: Implement NESTED_MSI cookie

2019-03-15 Thread Eric Auger
Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a range provided by the user.
This does not work in nested mode.

If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).

So we end up with 2 unrelated mappings, at S1 and S2:
 S1 S2
gIOVA-> gDB
   hIOVA->hDB

The PCI device would be programmed with hIOVA.

iommu_dma_bind_guest_msi allows to pass gIOVA/gDB
to the host so that gIOVA can be used by the host instead of
re-allocating a new hIOVA. The device handle also is passed
to garantee devices belonging to different stage1 domains record
distinguishable stage1 mappings. That way the host can create
the following nested
mapping:

 S1   S2
gIOVA->gDB->hDB

this time, the PCI device will be programmed with the gIOVA MSI
doorbell which is correctly mapped through the 2 stages.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- change function names; add unregister
- protect with msi_lock

v2 -> v3:
- also store the device handle on S1 mapping registration.
  This garantees we associate the associated S2 mapping binds
  to the correct physical MSI controller.

v1 -> v2:
- unmap stage2 on put()
---
 drivers/iommu/dma-iommu.c | 145 --
 include/linux/dma-iommu.h |  18 +
 2 files changed, 159 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 77aabe637a60..77ec3d35d41e 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -35,12 +35,16 @@
 struct iommu_dma_msi_page {
struct list_headlist;
dma_addr_t  iova;
+   dma_addr_t  gpa;
phys_addr_t phys;
+   size_t  s1_granule;
+   struct device   *dev;
 };
 
 enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
+   IOMMU_DMA_NESTED_MSI_COOKIE,
 };
 
 struct iommu_dma_cookie {
@@ -110,14 +114,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
  *
  * Users who manage their own IOVA allocation and do not want DMA API support,
  * but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
  * contiguous IOVA region, starting at @base, large enough to accommodate the
  * number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
+ * use case)
  */
 int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 {
struct iommu_dma_cookie *cookie;
+   int nesting, ret;
 
if (domain->type != IOMMU_DOMAIN_UNMANAGED)
return -EINVAL;
@@ -125,7 +132,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, 
dma_addr_t base)
if (domain->iova_cookie)
return -EEXIST;
 
-   cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+   ret =  iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, );
+   if (!ret && nesting)
+   cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+   else
+   cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
if (!cookie)
return -ENOMEM;
 
@@ -146,6 +158,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 {
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iommu_dma_msi_page *msi, *tmp;
+   bool s2_unmap = false;
 
if (!cookie)
return;
@@ -153,7 +166,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
put_iova_domain(>iovad);
 
+   if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
+   s2_unmap = true;
+
list_for_each_entry_safe(msi, tmp, >msi_page_list, list) {
+   if (s2_unmap && msi->phys) {
+   size_t size = cookie_msi_granule(cookie);
+
+   WARN_ON(iommu_unmap(domain, msi->gpa, size) != size);
+   }
list_del(>list);
kfree(msi);
}
@@ -162,6 +183,85 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 }
 EXPORT_SYMBOL(iommu_put_dma_cookie);
 
+/**
+ * iommu_dma_bind_guest_msi - Allows to pass the stage 1
+ * binding of a virtual MSI doorbell used by @dev.
+ *
+ * @domain: domain handle
+ * @dev: device handle
+ * @iova: guest iova
+ * @gpa: gpa of the virtual doorbell
+ * @size: size of the granule used for the stage1 mapping
+ *
+ * In 

[PATCH v5 11/22] iommu/arm-smmu-v3: Maintain a SID->device structure

2019-03-15 Thread Eric Auger
From: Jean-Philippe Brucker 

When handling faults from the event or PRI queue, we need to find the
struct device associated to a SID. Add a rb_tree to keep track of SIDs.

Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/arm-smmu-v3.c | 136 ++--
 1 file changed, 132 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index ff998c967a0a..21d027695181 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -588,6 +588,16 @@ struct arm_smmu_device {
 
/* IOMMU core code handle */
struct iommu_device iommu;
+
+   struct rb_root  streams;
+   struct mutexstreams_mutex;
+
+};
+
+struct arm_smmu_stream {
+   u32 id;
+   struct arm_smmu_master_data *master;
+   struct rb_node  node;
 };
 
 /* SMMU private data for each master */
@@ -597,6 +607,7 @@ struct arm_smmu_master_data {
 
struct arm_smmu_domain  *domain;
struct list_headlist; /* domain->devices */
+   struct arm_smmu_stream  *streams;
 
struct device   *dev;
 };
@@ -1243,6 +1254,32 @@ static int arm_smmu_init_l2_strtab(struct 
arm_smmu_device *smmu, u32 sid)
return 0;
 }
 
+__maybe_unused
+static struct arm_smmu_master_data *
+arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
+{
+   struct rb_node *node;
+   struct arm_smmu_stream *stream;
+   struct arm_smmu_master_data *master = NULL;
+
+   mutex_lock(>streams_mutex);
+   node = smmu->streams.rb_node;
+   while (node) {
+   stream = rb_entry(node, struct arm_smmu_stream, node);
+   if (stream->id < sid) {
+   node = node->rb_right;
+   } else if (stream->id > sid) {
+   node = node->rb_left;
+   } else {
+   master = stream->master;
+   break;
+   }
+   }
+   mutex_unlock(>streams_mutex);
+
+   return master;
+}
+
 /* IRQ and event handlers */
 static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
 {
@@ -1881,6 +1918,71 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device 
*smmu, u32 sid)
return sid < limit;
 }
 
+static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_data *master)
+{
+   int i;
+   int ret = 0;
+   struct arm_smmu_stream *new_stream, *cur_stream;
+   struct rb_node **new_node, *parent_node = NULL;
+   struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+   master->streams = kcalloc(fwspec->num_ids,
+ sizeof(struct arm_smmu_stream), GFP_KERNEL);
+   if (!master->streams)
+   return -ENOMEM;
+
+   mutex_lock(>streams_mutex);
+   for (i = 0; i < fwspec->num_ids && !ret; i++) {
+   new_stream = >streams[i];
+   new_stream->id = fwspec->ids[i];
+   new_stream->master = master;
+
+   new_node = &(smmu->streams.rb_node);
+   while (*new_node) {
+   cur_stream = rb_entry(*new_node, struct arm_smmu_stream,
+ node);
+   parent_node = *new_node;
+   if (cur_stream->id > new_stream->id) {
+   new_node = &((*new_node)->rb_left);
+   } else if (cur_stream->id < new_stream->id) {
+   new_node = &((*new_node)->rb_right);
+   } else {
+   dev_warn(master->dev,
+"stream %u already in tree\n",
+cur_stream->id);
+   ret = -EINVAL;
+   break;
+   }
+   }
+
+   if (!ret) {
+   rb_link_node(_stream->node, parent_node, new_node);
+   rb_insert_color(_stream->node, >streams);
+   }
+   }
+   mutex_unlock(>streams_mutex);
+
+   return ret;
+}
+
+static void arm_smmu_remove_master(struct arm_smmu_device *smmu,
+  struct arm_smmu_master_data *master)
+{
+   int i;
+   struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+   if (!master->streams)
+   return;
+
+   mutex_lock(>streams_mutex);
+   for (i = 0; i < fwspec->num_ids; i++)
+   rb_erase(>streams[i].node, >streams);
+   mutex_unlock(>streams_mutex);
+
+   kfree(master->streams);
+}
+
 static struct iommu_ops arm_smmu_ops;
 
 static int arm_smmu_add_device(struct device *dev)
@@ -1929,13 +2031,35 @@ static int arm_smmu_add_device(struct device *dev)
}
 

[PATCH v5 14/22] iommu/smmuv3: Implement cache_invalidate

2019-03-15 Thread Eric Auger
Implement domain-selective and page-selective IOTLB invalidations.

Signed-off-by: Eric Auger 

---

v3 -> v4:
- adapt to changes in the uapi
- add support for leaf parameter
- do not use arm_smmu_tlb_inv_range_nosync or arm_smmu_tlb_inv_context
  anymore

v2 -> v3:
- replace __arm_smmu_tlb_sync by arm_smmu_cmdq_issue_sync

v1 -> v2:
- properly pass the asid
---
 drivers/iommu/arm-smmu-v3.c | 57 +
 1 file changed, 57 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e41f61844d78..4a05dd66df74 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2319,6 +2319,62 @@ static void arm_smmu_detach_pasid_table(struct 
iommu_domain *domain)
mutex_unlock(_domain->init_mutex);
 }
 
+static int
+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   return -EINVAL;
+
+   if (!smmu)
+   return -EINVAL;
+
+   if (inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB) {
+   if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
+   struct arm_smmu_cmdq_ent cmd = {
+   .opcode = CMDQ_OP_TLBI_NH_ASID,
+   .tlbi = {
+   .vmid = smmu_domain->s2_cfg.vmid,
+   .asid = inv_info->pasid,
+   },
+   };
+
+   arm_smmu_cmdq_issue_cmd(smmu, );
+   arm_smmu_cmdq_issue_sync(smmu);
+
+   } else if (inv_info->granularity == IOMMU_INV_GRANU_ADDR) {
+   struct iommu_inv_addr_info *info = _info->addr_info;
+   size_t size = info->nb_granules * info->granule_size;
+   bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
+   struct arm_smmu_cmdq_ent cmd = {
+   .opcode = CMDQ_OP_TLBI_NH_VA,
+   .tlbi = {
+   .addr = info->addr,
+   .vmid = smmu_domain->s2_cfg.vmid,
+   .asid = info->pasid,
+   .leaf = leaf,
+   },
+   };
+
+   do {
+   arm_smmu_cmdq_issue_cmd(smmu, );
+   cmd.tlbi.addr += info->granule_size;
+   } while (size -= info->granule_size);
+   arm_smmu_cmdq_issue_sync(smmu);
+   } else {
+   return -EINVAL;
+   }
+   }
+   if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID ||
+   inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+   return -ENOENT;
+   }
+   return 0;
+}
+
 static struct iommu_ops arm_smmu_ops = {
.capable= arm_smmu_capable,
.domain_alloc   = arm_smmu_domain_alloc,
@@ -2339,6 +2395,7 @@ static struct iommu_ops arm_smmu_ops = {
.put_resv_regions   = arm_smmu_put_resv_regions,
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
+   .cache_invalidate   = arm_smmu_cache_invalidate,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 08/22] vfio: VFIO_IOMMU_CACHE_INVALIDATE

2019-03-15 Thread Eric Auger
From: "Liu, Yi L" 

When the guest "owns" the stage 1 translation structures,  the host
IOMMU driver has no knowledge of caching structure updates unless
the guest invalidation requests are trapped and passed down to the
host.

This patch adds the VFIO_IOMMU_CACHE_INVALIDATE ioctl with aims
at propagating guest stage1 IOMMU cache invalidations to the host.

Signed-off-by: Liu, Yi L 
Signed-off-by: Eric Auger 

---

v2 -> v3:
- introduce vfio_iommu_for_each_dev back in this patch

v1 -> v2:
- s/TLB/CACHE
- remove vfio_iommu_task usage
- commit message rewording
---
 drivers/vfio/vfio_iommu_type1.c | 47 +
 include/uapi/linux/vfio.h   | 13 +
 2 files changed, 60 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 222e9199edbf..12a40b9db6aa 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -113,6 +113,26 @@ struct vfio_regions {
 #define IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu)\
(!list_empty(>domain_list))
 
+/* iommu->lock must be held */
+static int
+vfio_iommu_for_each_dev(struct vfio_iommu *iommu, void *data,
+   int (*fn)(struct device *, void *))
+{
+   struct vfio_domain *d;
+   struct vfio_group *g;
+   int ret = 0;
+
+   list_for_each_entry(d, >domain_list, next) {
+   list_for_each_entry(g, >group_list, next) {
+   ret = iommu_group_for_each_dev(g->iommu_group,
+  data, fn);
+   if (ret)
+   break;
+   }
+   }
+   return ret;
+}
+
 static int put_pfn(unsigned long pfn, int prot);
 
 /*
@@ -1681,6 +1701,15 @@ vfio_attach_pasid_table(struct vfio_iommu *iommu,
return ret;
 }
 
+static int vfio_cache_inv_fn(struct device *dev, void *data)
+{
+   struct vfio_iommu_type1_cache_invalidate *ustruct =
+   (struct vfio_iommu_type1_cache_invalidate *)data;
+   struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+
+   return iommu_cache_invalidate(d, dev, >info);
+}
+
 static long vfio_iommu_type1_ioctl(void *iommu_data,
   unsigned int cmd, unsigned long arg)
 {
@@ -1767,6 +1796,24 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
} else if (cmd == VFIO_IOMMU_DETACH_PASID_TABLE) {
vfio_detach_pasid_table(iommu);
return 0;
+   } else if (cmd == VFIO_IOMMU_CACHE_INVALIDATE) {
+   struct vfio_iommu_type1_cache_invalidate ustruct;
+   int ret;
+
+   minsz = offsetofend(struct vfio_iommu_type1_cache_invalidate,
+   info);
+
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (ustruct.argsz < minsz || ustruct.flags)
+   return -EINVAL;
+
+   mutex_lock(>lock);
+   ret = vfio_iommu_for_each_dev(iommu, ,
+ vfio_cache_inv_fn);
+   mutex_unlock(>lock);
+   return ret;
}
 
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 329d378565d9..29f0ef2d805d 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -776,6 +776,19 @@ struct vfio_iommu_type1_attach_pasid_table {
 #define VFIO_IOMMU_ATTACH_PASID_TABLE  _IO(VFIO_TYPE, VFIO_BASE + 22)
 #define VFIO_IOMMU_DETACH_PASID_TABLE  _IO(VFIO_TYPE, VFIO_BASE + 23)
 
+/**
+ * VFIO_IOMMU_CACHE_INVALIDATE - _IOWR(VFIO_TYPE, VFIO_BASE + 24,
+ * struct vfio_iommu_type1_cache_invalidate)
+ *
+ * Propagate guest IOMMU cache invalidation to the host.
+ */
+struct vfio_iommu_type1_cache_invalidate {
+   __u32   argsz;
+   __u32   flags;
+   struct iommu_cache_invalidate_info info;
+};
+#define VFIO_IOMMU_CACHE_INVALIDATE  _IO(VFIO_TYPE, VFIO_BASE + 24)
+
 /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
 
 /*
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 12/22] iommu/smmuv3: Get prepared for nested stage support

2019-03-15 Thread Eric Auger
To allow nested stage support, we need to store both
stage 1 and stage 2 configurations (and remove the former
union).

A nested setup is characterized by both s1_cfg and s2_cfg
set.

We introduce a new ste.abort field that will be set upon
guest stage1 configuration passing. If s1_cfg is NULL and
ste.abort is set, traffic can't pass. If ste.abort is not set,
S1 is bypassed.

arm_smmu_write_strtab_ent() is modified to write both stage
fields in the STE and deal with the abort field.

In nested mode, only stage 2 is "finalized" as the host does
not own/configure the stage 1 context descriptor, guest does.

Signed-off-by: Eric Auger 

---

v4 -> v5:
- reset ste.abort on detach

v3 -> v4:
- s1_cfg.nested_abort and nested_bypass removed.
- s/ste.nested/ste.abort
- arm_smmu_write_strtab_ent modifications with introduction
  of local abort, bypass and translate local variables
- comment updated

v1 -> v2:
- invalidate the STE before moving from a live STE config to another
- add the nested_abort and nested_bypass fields
---
 drivers/iommu/arm-smmu-v3.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 21d027695181..e22e944ffc05 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -211,6 +211,7 @@
 #define STRTAB_STE_0_CFG_BYPASS4
 #define STRTAB_STE_0_CFG_S1_TRANS  5
 #define STRTAB_STE_0_CFG_S2_TRANS  6
+#define STRTAB_STE_0_CFG_NESTED7
 
 #define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
 #define STRTAB_STE_0_S1FMT_LINEAR  0
@@ -514,6 +515,7 @@ struct arm_smmu_strtab_ent {
 * configured according to the domain type.
 */
boolassigned;
+   boolabort;
struct arm_smmu_s1_cfg  *s1_cfg;
struct arm_smmu_s2_cfg  *s2_cfg;
 };
@@ -628,10 +630,8 @@ struct arm_smmu_domain {
boolnon_strict;
 
enum arm_smmu_domain_stage  stage;
-   union {
-   struct arm_smmu_s1_cfg  s1_cfg;
-   struct arm_smmu_s2_cfg  s2_cfg;
-   };
+   struct arm_smmu_s1_cfg  s1_cfg;
+   struct arm_smmu_s2_cfg  s2_cfg;
 
struct iommu_domain domain;
 
@@ -1108,12 +1108,13 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
  __le64 *dst, struct arm_smmu_strtab_ent 
*ste)
 {
/*
-* This is hideously complicated, but we only really care about
-* three cases at the moment:
+* We care about the following transitions:
 *
 * 1. Invalid (all zero) -> bypass/fault (init)
-* 2. Bypass/fault -> translation/bypass (attach)
-* 3. Translation/bypass -> bypass/fault (detach)
+* 2. Bypass/fault -> single stage translation/bypass (attach)
+* 3. single stage Translation/bypass -> bypass/fault (detach)
+* 4. S2 -> S1 + S2 (attach_pasid_table)
+* 5. S1 + S2 -> S2 (detach_pasid_table)
 *
 * Given that we can't update the STE atomically and the SMMU
 * doesn't read the thing in a defined order, that leaves us
@@ -1124,7 +1125,7 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
 * 3. Update Config, sync
 */
u64 val = le64_to_cpu(dst[0]);
-   bool ste_live = false;
+   bool abort, bypass, translate, ste_live = false;
struct arm_smmu_cmdq_ent prefetch_cmd = {
.opcode = CMDQ_OP_PREFETCH_CFG,
.prefetch   = {
@@ -1138,11 +1139,11 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
break;
case STRTAB_STE_0_CFG_S1_TRANS:
case STRTAB_STE_0_CFG_S2_TRANS:
+   case STRTAB_STE_0_CFG_NESTED:
ste_live = true;
break;
case STRTAB_STE_0_CFG_ABORT:
-   if (disable_bypass)
-   break;
+   break;
default:
BUG(); /* STE corruption */
}
@@ -1152,8 +1153,13 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
val = STRTAB_STE_0_V;
 
/* Bypass/fault */
-   if (!ste->assigned || !(ste->s1_cfg || ste->s2_cfg)) {
-   if (!ste->assigned && disable_bypass)
+
+   abort = (!ste->assigned && disable_bypass) || ste->abort;
+   translate = ste->s1_cfg || ste->s2_cfg;
+   bypass = !abort && !translate;
+
+   if (abort || bypass) {
+   if (abort)
val |= FIELD_PREP(STRTAB_STE_0_CFG, 
STRTAB_STE_0_CFG_ABORT);
else
val |= FIELD_PREP(STRTAB_STE_0_CFG, 
STRTAB_STE_0_CFG_BYPASS);
@@ 

[PATCH v5 10/22] iommu/arm-smmu-v3: Link domains and devices

2019-03-15 Thread Eric Auger
From: Jean-Philippe Brucker 

When removing a mapping from a domain, we need to send an invalidation to
all devices that might have stored it in their Address Translation Cache
(ATC). In addition with SVM, we'll need to invalidate context descriptors
of all devices attached to a live domain.

Maintain a list of devices in each domain, protected by a spinlock. It is
updated every time we attach or detach devices to and from domains.

It needs to be a spinlock because we'll invalidate ATC entries from
within hardirq-safe contexts, but it may be possible to relax the read
side with RCU later.

Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/arm-smmu-v3.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d3880010c6cf..ff998c967a0a 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -594,6 +594,11 @@ struct arm_smmu_device {
 struct arm_smmu_master_data {
struct arm_smmu_device  *smmu;
struct arm_smmu_strtab_ent  ste;
+
+   struct arm_smmu_domain  *domain;
+   struct list_headlist; /* domain->devices */
+
+   struct device   *dev;
 };
 
 /* SMMU private data for an IOMMU domain */
@@ -618,6 +623,9 @@ struct arm_smmu_domain {
};
 
struct iommu_domain domain;
+
+   struct list_headdevices;
+   spinlock_t  devices_lock;
 };
 
 struct arm_smmu_option_prop {
@@ -1493,6 +1501,9 @@ static struct iommu_domain 
*arm_smmu_domain_alloc(unsigned type)
}
 
mutex_init(_domain->init_mutex);
+   INIT_LIST_HEAD(_domain->devices);
+   spin_lock_init(_domain->devices_lock);
+
return _domain->domain;
 }
 
@@ -1713,6 +1724,16 @@ static void arm_smmu_detach_dev(struct device *dev)
 {
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct arm_smmu_master_data *master = fwspec->iommu_priv;
+   unsigned long flags;
+   struct arm_smmu_domain *smmu_domain = master->domain;
+
+   if (smmu_domain) {
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_del(>list);
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
+
+   master->domain = NULL;
+   }
 
master->ste.assigned = false;
arm_smmu_install_ste_for_dev(fwspec);
@@ -1722,6 +1743,7 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
 {
int ret = 0;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+   unsigned long flags;
struct arm_smmu_device *smmu;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_master_data *master;
@@ -1757,6 +1779,11 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
}
 
ste->assigned = true;
+   master->domain = smmu_domain;
+
+   spin_lock_irqsave(_domain->devices_lock, flags);
+   list_add(>list, _domain->devices);
+   spin_unlock_irqrestore(_domain->devices_lock, flags);
 
if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
ste->s1_cfg = NULL;
@@ -1883,6 +1910,7 @@ static int arm_smmu_add_device(struct device *dev)
return -ENOMEM;
 
master->smmu = smmu;
+   master->dev = dev;
fwspec->iommu_priv = master;
}
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 09/22] vfio: VFIO_IOMMU_BIND/UNBIND_MSI

2019-03-15 Thread Eric Auger
This patch adds the VFIO_IOMMU_BIND/UNBIND_MSI ioctl which aim
to pass/withdraw the guest MSI binding to/from the host.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- add UNBIND
- unwind on BIND error

v2 -> v3:
- adapt to new proto of bind_guest_msi
- directly use vfio_iommu_for_each_dev

v1 -> v2:
- s/vfio_iommu_type1_guest_msi_binding/vfio_iommu_type1_bind_guest_msi
---
 drivers/vfio/vfio_iommu_type1.c | 58 +
 include/uapi/linux/vfio.h   | 29 +
 2 files changed, 87 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 12a40b9db6aa..66513679081b 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1710,6 +1710,25 @@ static int vfio_cache_inv_fn(struct device *dev, void 
*data)
return iommu_cache_invalidate(d, dev, >info);
 }
 
+static int vfio_bind_msi_fn(struct device *dev, void *data)
+{
+   struct vfio_iommu_type1_bind_msi *ustruct =
+   (struct vfio_iommu_type1_bind_msi *)data;
+   struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+
+   return iommu_bind_guest_msi(d, dev, ustruct->iova,
+   ustruct->gpa, ustruct->size);
+}
+
+static int vfio_unbind_msi_fn(struct device *dev, void *data)
+{
+   dma_addr_t *iova = (dma_addr_t *)data;
+   struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+
+   iommu_unbind_guest_msi(d, dev, *iova);
+   return 0;
+}
+
 static long vfio_iommu_type1_ioctl(void *iommu_data,
   unsigned int cmd, unsigned long arg)
 {
@@ -1814,6 +1833,45 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
  vfio_cache_inv_fn);
mutex_unlock(>lock);
return ret;
+   } else if (cmd == VFIO_IOMMU_BIND_MSI) {
+   struct vfio_iommu_type1_bind_msi ustruct;
+   int ret;
+
+   minsz = offsetofend(struct vfio_iommu_type1_bind_msi,
+   size);
+
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (ustruct.argsz < minsz || ustruct.flags)
+   return -EINVAL;
+
+   mutex_lock(>lock);
+   ret = vfio_iommu_for_each_dev(iommu, ,
+ vfio_bind_msi_fn);
+   if (ret)
+   vfio_iommu_for_each_dev(iommu, ,
+   vfio_unbind_msi_fn);
+   mutex_unlock(>lock);
+   return ret;
+   } else if (cmd == VFIO_IOMMU_UNBIND_MSI) {
+   struct vfio_iommu_type1_unbind_msi ustruct;
+   int ret;
+
+   minsz = offsetofend(struct vfio_iommu_type1_unbind_msi,
+   iova);
+
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (ustruct.argsz < minsz || ustruct.flags)
+   return -EINVAL;
+
+   mutex_lock(>lock);
+   ret = vfio_iommu_for_each_dev(iommu, ,
+ vfio_unbind_msi_fn);
+   mutex_unlock(>lock);
+   return ret;
}
 
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 29f0ef2d805d..6763389b6adc 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -789,6 +789,35 @@ struct vfio_iommu_type1_cache_invalidate {
 };
 #define VFIO_IOMMU_CACHE_INVALIDATE  _IO(VFIO_TYPE, VFIO_BASE + 24)
 
+/**
+ * VFIO_IOMMU_BIND_MSI - _IOWR(VFIO_TYPE, VFIO_BASE + 25,
+ * struct vfio_iommu_type1_bind_msi)
+ *
+ * Pass a stage 1 MSI doorbell mapping to the host so that this
+ * latter can build a nested stage2 mapping
+ */
+struct vfio_iommu_type1_bind_msi {
+   __u32   argsz;
+   __u32   flags;
+   __u64   iova;
+   __u64   gpa;
+   __u64   size;
+};
+#define VFIO_IOMMU_BIND_MSI  _IO(VFIO_TYPE, VFIO_BASE + 25)
+
+/**
+ * VFIO_IOMMU_UNBIND_MSI - _IOWR(VFIO_TYPE, VFIO_BASE + 26,
+ * struct vfio_iommu_type1_unbind_msi)
+ *
+ * Unregister an MSI mapping
+ */
+struct vfio_iommu_type1_unbind_msi {
+   __u32   argsz;
+   __u32   flags;
+   __u64   iova;
+};
+#define VFIO_IOMMU_UNBIND_MSI  _IO(VFIO_TYPE, VFIO_BASE + 26)
+
 /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
 
 /*
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 07/22] vfio: VFIO_IOMMU_ATTACH/DETACH_PASID_TABLE

2019-03-15 Thread Eric Auger
From: "Liu, Yi L" 

This patch adds VFIO_IOMMU_ATTACH/DETACH_PASID_TABLE ioctl
which aims to pass/withdraw the virtual iommu guest configuration
to/from the VFIO driver downto to the iommu subsystem.

Signed-off-by: Jacob Pan 
Signed-off-by: Liu, Yi L 
Signed-off-by: Eric Auger 

---
v3 -> v4:
- restore ATTACH/DETACH
- add unwind on failure

v2 -> v3:
- s/BIND_PASID_TABLE/SET_PASID_TABLE

v1 -> v2:
- s/BIND_GUEST_STAGE/BIND_PASID_TABLE
- remove the struct device arg
---
 drivers/vfio/vfio_iommu_type1.c | 53 +
 include/uapi/linux/vfio.h   | 17 +++
 2 files changed, 70 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 73652e21efec..222e9199edbf 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1644,6 +1644,43 @@ static int vfio_domains_have_iommu_cache(struct 
vfio_iommu *iommu)
return ret;
 }
 
+static void
+vfio_detach_pasid_table(struct vfio_iommu *iommu)
+{
+   struct vfio_domain *d;
+
+   mutex_lock(>lock);
+
+   list_for_each_entry(d, >domain_list, next) {
+   iommu_detach_pasid_table(d->domain);
+   }
+   mutex_unlock(>lock);
+}
+
+static int
+vfio_attach_pasid_table(struct vfio_iommu *iommu,
+   struct vfio_iommu_type1_attach_pasid_table *ustruct)
+{
+   struct vfio_domain *d;
+   int ret = 0;
+
+   mutex_lock(>lock);
+
+   list_for_each_entry(d, >domain_list, next) {
+   ret = iommu_attach_pasid_table(d->domain, >config);
+   if (ret)
+   goto unwind;
+   }
+   goto unlock;
+unwind:
+   list_for_each_entry_continue_reverse(d, >domain_list, next) {
+   iommu_detach_pasid_table(d->domain);
+   }
+unlock:
+   mutex_unlock(>lock);
+   return ret;
+}
+
 static long vfio_iommu_type1_ioctl(void *iommu_data,
   unsigned int cmd, unsigned long arg)
 {
@@ -1714,6 +1751,22 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 
return copy_to_user((void __user *)arg, , minsz) ?
-EFAULT : 0;
+   } else if (cmd == VFIO_IOMMU_ATTACH_PASID_TABLE) {
+   struct vfio_iommu_type1_attach_pasid_table ustruct;
+
+   minsz = offsetofend(struct vfio_iommu_type1_attach_pasid_table,
+   config);
+
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (ustruct.argsz < minsz || ustruct.flags)
+   return -EINVAL;
+
+   return vfio_attach_pasid_table(iommu, );
+   } else if (cmd == VFIO_IOMMU_DETACH_PASID_TABLE) {
+   vfio_detach_pasid_table(iommu);
+   return 0;
}
 
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 02bb7ad6e986..329d378565d9 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #define VFIO_API_VERSION   0
 
@@ -759,6 +760,22 @@ struct vfio_iommu_type1_dma_unmap {
 #define VFIO_IOMMU_ENABLE  _IO(VFIO_TYPE, VFIO_BASE + 15)
 #define VFIO_IOMMU_DISABLE _IO(VFIO_TYPE, VFIO_BASE + 16)
 
+/**
+ * VFIO_IOMMU_ATTACH_PASID_TABLE - _IOWR(VFIO_TYPE, VFIO_BASE + 22,
+ * struct vfio_iommu_type1_attach_pasid_table)
+ *
+ * Passes the PASID table to the host. Calling ATTACH_PASID_TABLE
+ * while a table is already installed is allowed: it replaces the old
+ * table. DETACH does a comprehensive tear down of the nested mode.
+ */
+struct vfio_iommu_type1_attach_pasid_table {
+   __u32   argsz;
+   __u32   flags;
+   struct iommu_pasid_table_config config;
+};
+#define VFIO_IOMMU_ATTACH_PASID_TABLE  _IO(VFIO_TYPE, VFIO_BASE + 22)
+#define VFIO_IOMMU_DETACH_PASID_TABLE  _IO(VFIO_TYPE, VFIO_BASE + 23)
+
 /*  Additional API for SPAPR TCE (Server POWERPC) IOMMU  */
 
 /*
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 05/22] iommu: Introduce cache_invalidate API

2019-03-15 Thread Eric Auger
From: "Liu, Yi L" 

In any virtualization use case, when the first translation stage
is "owned" by the guest OS, the host IOMMU driver has no knowledge
of caching structure updates unless the guest invalidation activities
are trapped by the virtualizer and passed down to the host.

Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
Signed-off-by: Eric Auger 

---
v3 -> v4:
- full reshape of the API following Alex' comments

v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows
  to invalidate context caches on top of IOTLBs

v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in
header. Commit message reworded.
---
 drivers/iommu/iommu.c  | 14 
 include/linux/iommu.h  | 21 +++
 include/uapi/linux/iommu.h | 71 ++
 3 files changed, 106 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 7d9285cea100..b72e326ddd41 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1544,6 +1544,20 @@ void iommu_detach_pasid_table(struct iommu_domain 
*domain)
 }
 EXPORT_SYMBOL_GPL(iommu_detach_pasid_table);
 
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+  struct iommu_cache_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->cache_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
  struct device *dev)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fb9b7a8de25f..3d8e48876162 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -191,6 +191,7 @@ struct iommu_resv_region {
  *  driver init to device driver init (default no)
  * @attach_pasid_table: attach a pasid table
  * @detach_pasid_table: detach the pasid table
+ * @cache_invalidate: invalidate translation caches
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -239,6 +240,9 @@ struct iommu_ops {
  struct iommu_pasid_table_config *cfg);
void (*detach_pasid_table)(struct iommu_domain *domain);
 
+   int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+   struct iommu_cache_invalidate_info *inv_info);
+
unsigned long pgsize_bitmap;
 };
 
@@ -349,6 +353,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 extern int iommu_attach_pasid_table(struct iommu_domain *domain,
struct iommu_pasid_table_config *cfg);
 extern void iommu_detach_pasid_table(struct iommu_domain *domain);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -795,7 +802,21 @@ int iommu_attach_pasid_table(struct iommu_domain *domain,
 }
 
 static inline
+<<< HEAD
 void iommu_detach_pasid_table(struct iommu_domain *domain) {}
+===
+void iommu_detach_pasid_table(struct iommu_domain *domain)
+{
+   return -ENODEV;
+}
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+  struct device *dev,
+  struct iommu_cache_invalidate_info *inv_info)
+{
+   return -ENODEV;
+}
+>>> 56df871916e5... iommu: Introduce cache_invalidate API
 
 #endif /* CONFIG_IOMMU_API */
 
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 532a64075f23..e4c6a447e85a 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -159,4 +159,75 @@ struct iommu_pasid_table_config {
};
 };
 
+/* defines the granularity of the invalidation */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN, /* domain-selective invalidation */
+   IOMMU_INV_GRANU_PASID,  /* pasid-selective invalidation */
+   IOMMU_INV_GRANU_ADDR,   /* page-selective invalidation */
+};
+
+/**
+ * Address Selective Invalidation Structure
+ *
+ * @flags indicates the granularity of the address-selective invalidation
+ * - if PASID bit is set, @pasid field is populated and the invalidation
+ *   relates to cache entries 

[PATCH v5 04/22] iommu: Introduce attach/detach_pasid_table API

2019-03-15 Thread Eric Auger
From: Jacob Pan 

In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest,
the physical IOMMU must be programmed to be consistent with
the guest mappings. If the physical IOMMU supports two
translation stages it makes sense to program guest mappings
onto the first stage/level (ARM/Intel terminology) while the host
owns the stage/level 2.

In that case, it is mandated to trap on guest configuration
settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows
to set/unset the pasid table information.

A generic iommu_pasid_table_config struct is introduced in
a new iommu.h uapi header. This is going to be used by the VFIO
user API.

Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Liu, Yi L 
Signed-off-by: Ashok Raj 
Signed-off-by: Jacob Pan 
Signed-off-by: Eric Auger 
Reviewed-by: Jean-Philippe Brucker 

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v4 -> v5:
- no returned valued for dummy definition of iommu_detach_pasid_table
- fix order in comment
- added Jean's R-b

v3 -> v4:
- s/set_pasid_table/attach_pasid_table
- restore detach_pasid_table. Detach can be used on unwind path.
- add padding
- remove @abort
- signature used for config and format
- add comments for fields in the SMMU struct

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3
---
 drivers/iommu/iommu.c  | 19 +++
 include/linux/iommu.h  | 19 +++
 include/uapi/linux/iommu.h | 47 ++
 3 files changed, 85 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 56d5bf68de53..7d9285cea100 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1525,6 +1525,25 @@ int iommu_attach_device(struct iommu_domain *domain, 
struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_attach_pasid_table(struct iommu_domain *domain,
+struct iommu_pasid_table_config *cfg)
+{
+   if (unlikely(!domain->ops->attach_pasid_table))
+   return -ENODEV;
+
+   return domain->ops->attach_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_attach_pasid_table);
+
+void iommu_detach_pasid_table(struct iommu_domain *domain)
+{
+   if (unlikely(!domain->ops->detach_pasid_table))
+   return;
+
+   domain->ops->detach_pasid_table(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
  struct device *dev)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index aeb4b615cb44..fb9b7a8de25f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -189,6 +189,8 @@ struct iommu_resv_region {
  * @of_xlate: add OF master IDs to iommu grouping
  * @is_attach_deferred: Check if domain attach should be deferred from iommu
  *  driver init to device driver init (default no)
+ * @attach_pasid_table: attach a pasid table
+ * @detach_pasid_table: detach the pasid table
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -233,6 +235,10 @@ struct iommu_ops {
int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
bool (*is_attach_deferred)(struct iommu_domain *domain, struct device 
*dev);
 
+   int (*attach_pasid_table)(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+   void (*detach_pasid_table)(struct iommu_domain *domain);
+
unsigned long pgsize_bitmap;
 };
 
@@ -340,6 +346,9 @@ extern int iommu_attach_device(struct iommu_domain *domain,
   struct device *dev);
 extern void iommu_detach_device(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_attach_pasid_table(struct iommu_domain *domain,
+   struct iommu_pasid_table_config *cfg);
+extern void iommu_detach_pasid_table(struct iommu_domain *domain);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -778,6 +787,16 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct 
fwnode_handle *fwnode)
return NULL;
 }
 
+static inline
+int iommu_attach_pasid_table(struct iommu_domain *domain,
+struct iommu_pasid_table_config *cfg)
+{
+   return -ENODEV;
+}
+
+static inline
+void iommu_detach_pasid_table(struct iommu_domain *domain) {}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS
diff 

[PATCH v5 01/22] driver core: add per device iommu param

2019-03-15 Thread Eric Auger
From: Jacob Pan 

DMA faults can be detected by IOMMU at device level. Adding a pointer
to struct device allows IOMMU subsystem to report relevant faults
back to the device driver for further handling.
For direct assigned device (or user space drivers), guest OS holds
responsibility to handle and respond per device IOMMU fault.
Therefore we need fault reporting mechanism to propagate faults beyond
IOMMU subsystem.

There are two other IOMMU data pointers under struct device today, here
we introduce iommu_param as a parent pointer such that all device IOMMU
data can be consolidated here. The idea was suggested here by Greg KH
and Joerg. The name iommu_param is chosen here since iommu_data has been used.

Suggested-by: Greg Kroah-Hartman 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Jacob Pan 
Link: https://lkml.org/lkml/2017/10/6/81
---
 include/linux/device.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index b425a7ee04ce..39b4dd1b01f5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -42,6 +42,7 @@ struct iommu_ops;
 struct iommu_group;
 struct iommu_fwspec;
 struct dev_pin_info;
+struct iommu_param;
 
 struct bus_attribute {
struct attributeattr;
@@ -961,6 +962,7 @@ struct dev_links_info {
  * device (i.e. the bus driver that discovered the device).
  * @iommu_group: IOMMU group the device belongs to.
  * @iommu_fwspec: IOMMU-specific properties supplied by firmware.
+ * @iommu_param: Per device generic IOMMU runtime data
  *
  * @offline_disabled: If set, the device is permanently online.
  * @offline:   Set after successful invocation of bus type's .offline().
@@ -1054,6 +1056,7 @@ struct device {
void(*release)(struct device *dev);
struct iommu_group  *iommu_group;
struct iommu_fwspec *iommu_fwspec;
+   struct iommu_param  *iommu_param;
 
booloffline_disabled:1;
booloffline:1;
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v5 02/22] iommu: introduce device fault data

2019-03-15 Thread Eric Auger
From: Jacob Pan 

Device faults detected by IOMMU can be reported outside the IOMMU
subsystem for further processing. This patch introduces
a generic device fault data structure.

The fault can be either an unrecoverable fault or a page request,
also referred to as a recoverable fault.

We only care about non internal faults that are likely to be reported
to an external subsystem.

Signed-off-by: Jacob Pan 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Liu, Yi L 
Signed-off-by: Ashok Raj 
Signed-off-by: Eric Auger 

---
v4 -> v5:
- simplified struct iommu_fault_event comment
- Moved IOMMU_FAULT_PERM outside of the struct
- Removed IOMMU_FAULT_PERM_INST
- s/IOMMU_FAULT_PAGE_REQUEST_PASID_PRESENT/
  IOMMU_FAULT_PAGE_REQUEST_PASID_VALID

v3 -> v4:
- use a union containing aither an unrecoverable fault or a page
  request message. Move the device private data in the page request
  structure. Reshuffle the fields and use flags.
- move fault perm attributes to the uapi
- remove a bunch of iommu_fault_reason enum values that were related
  to internal errors
---
 include/linux/iommu.h  |  44 ++
 include/uapi/linux/iommu.h | 115 +
 2 files changed, 159 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ffbbc7e39cee..c6f398f7e6e0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define IOMMU_READ (1 << 0)
 #define IOMMU_WRITE(1 << 1)
@@ -48,6 +49,7 @@ struct bus_type;
 struct device;
 struct iommu_domain;
 struct notifier_block;
+struct iommu_fault_event;
 
 /* iommu fault flags */
 #define IOMMU_FAULT_READ   0x0
@@ -55,6 +57,7 @@ struct notifier_block;
 
 typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
struct device *, unsigned long, int, void *);
+typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *, void *);
 
 struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped*/
@@ -247,6 +250,46 @@ struct iommu_device {
struct device *dev;
 };
 
+/**
+ * struct iommu_fault_event - Generic fault event
+ *
+ * Can represent recoverable faults such as a page requests or
+ * unrecoverable faults such as DMA or IRQ remapping faults.
+ *
+ * @fault: fault descriptor
+ * @iommu_private: used by the IOMMU driver for storing fault-specific
+ * data. Users should not modify this field before
+ * sending the fault response.
+ */
+struct iommu_fault_event {
+   struct iommu_fault fault;
+   u64 iommu_private;
+};
+
+/**
+ * struct iommu_fault_param - per-device IOMMU fault data
+ * @dev_fault_handler: Callback function to handle IOMMU faults at device level
+ * @data: handler private data
+ *
+ */
+struct iommu_fault_param {
+   iommu_dev_fault_handler_t handler;
+   void *data;
+};
+
+/**
+ * struct iommu_param - collection of per-device IOMMU data
+ *
+ * @fault_param: IOMMU detected device fault reporting data
+ *
+ * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
+ * struct iommu_group  *iommu_group;
+ * struct iommu_fwspec *iommu_fwspec;
+ */
+struct iommu_param {
+   struct iommu_fault_param *fault_param;
+};
+
 int  iommu_device_register(struct iommu_device *iommu);
 void iommu_device_unregister(struct iommu_device *iommu);
 int  iommu_device_sysfs_add(struct iommu_device *iommu,
@@ -422,6 +465,7 @@ struct iommu_ops {};
 struct iommu_group {};
 struct iommu_fwspec {};
 struct iommu_device {};
+struct iommu_fault_param {};
 
 static inline bool iommu_present(struct bus_type *bus)
 {
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
new file mode 100644
index ..edcc0dda7993
--- /dev/null
+++ b/include/uapi/linux/iommu.h
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * IOMMU user API definitions
+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include 
+
+#define IOMMU_FAULT_PERM_WRITE (1 << 0) /* write */
+#define IOMMU_FAULT_PERM_EXEC  (1 << 1) /* exec */
+#define IOMMU_FAULT_PERM_PRIV  (1 << 2) /* privileged */
+
+/*  Generic fault types, can be expanded IRQ remapping fault */
+enum iommu_fault_type {
+   IOMMU_FAULT_DMA_UNRECOV = 1,/* unrecoverable fault */
+   IOMMU_FAULT_PAGE_REQ,   /* page request fault */
+};
+
+enum iommu_fault_reason {
+   IOMMU_FAULT_REASON_UNKNOWN = 0,
+
+   /* Could not access the PASID table (fetch caused external abort) */
+   IOMMU_FAULT_REASON_PASID_FETCH,
+
+   /* pasid entry is invalid or has configuration errors */
+   IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
+
+   /*
+* PASID is out of range (e.g. exceeds the maximum PASID
+* supported by the IOMMU) or disabled.
+*/
+   IOMMU_FAULT_REASON_PASID_INVALID,
+
+   /*
+* 

[PATCH v5 00/22] SMMUv3 Nested Stage Setup

2019-03-15 Thread Eric Auger
This series allows a virtualizer to program the nested stage mode.
This is useful when both the host and the guest are exposed with
an SMMUv3 and a PCI device is assigned to the guest using VFIO.

In this mode, the physical IOMMU must be programmed to translate
the two stages: the one set up by the guest (IOVA -> GPA) and the
one set up by the host VFIO driver as part of the assignment process
(GPA -> HPA).

On Intel, this is traditionnaly achieved by combining the 2 stages
into a single physical stage. However this relies on the capability
to trap on each guest translation structure update. This is possible
by using the VTD Caching Mode. Unfortunately the ARM SMMUv3 does
not offer a similar mechanism.

However, the ARM SMMUv3 architecture supports 2 physical stages! Those
were devised exactly with that use case in mind. Assuming the HW
implements both stages (optional), the guest now can use stage 1
while the host uses stage 2.

This assumes the virtualizer has means to propagate guest settings
to the host SMMUv3 driver. This series brings this VFIO/IOMMU
infrastructure.  Those services are:
- bind the guest stage 1 configuration to the stream table entry
- propagate guest TLB invalidations
- bind MSI IOVAs
- propagate faults collected at physical level up to the virtualizer

This series largely reuses the user API and infrastructure originally
devised for SVA/SVM and patches submitted by Jacob, Yi Liu, Tianyu in
[1-2] and Jean-Philippe [3-4].

Best Regards

Eric

This series can be found at:
https://github.com/eauger/linux/tree/v5.0-2stage-v5

References:
[1] [PATCH v5 00/23] IOMMU and VT-d driver support for Shared Virtual
Address (SVA)
https://lwn.net/Articles/754331/
[2] [RFC PATCH 0/8] Shared Virtual Memory virtualization for VT-d
(VFIO part)
https://lists.linuxfoundation.org/pipermail/iommu/2017-April/021475.html
[3] [v2,00/40] Shared Virtual Addressing for the IOMMU
https://patchwork.ozlabs.org/cover/912129/
[4] [PATCH v3 00/10] Shared Virtual Addressing for the IOMMU
https://patchwork.kernel.org/cover/10608299/

History:
v4 -> v5:
- fix bug reported by Vincent: fault handler unregistration now happens in
  vfio_pci_release
- IOMMU_FAULT_PERM_* moved outside of struct definition + small
  uapi changes suggested by Kean-Philippe (except fetch_addr)
- iommu: introduce device fault report API: removed the PRI part.
- see individual logs for more details
- reset the ste abort flag on detach

v3 -> v4:
- took into account Alex, jean-Philippe and Robin's comments on v3
- rework of the smmuv3 driver integration
- add tear down ops for msi binding and PASID table binding
- fix S1 fault propagation
- put fault reporting patches at the beginning of the series following
  Jean-Philippe's request
- update of the cache invalidate and fault API uapis
- VFIO fault reporting rework with 2 separate regions and one mmappable
  segment for the fault queue
- moved to PATCH

v2 -> v3:
- When registering the S1 MSI binding we now store the device handle. This
  addresses Robin's comment about discimination of devices beonging to
  different S1 groups and using different physical MSI doorbells.
- Change the fault reporting API: use VFIO_PCI_DMA_FAULT_IRQ_INDEX to
  set the eventfd and expose the faults through an mmappable fault region

v1 -> v2:
- Added the fault reporting capability
- asid properly passed on invalidation (fix assignment of multiple
  devices)
- see individual change logs for more info


Eric Auger (13):
  iommu: Introduce bind/unbind_guest_msi
  vfio: VFIO_IOMMU_BIND/UNBIND_MSI
  iommu/smmuv3: Get prepared for nested stage support
  iommu/smmuv3: Implement attach/detach_pasid_table
  iommu/smmuv3: Implement cache_invalidate
  dma-iommu: Implement NESTED_MSI cookie
  iommu/smmuv3: Implement bind/unbind_guest_msi
  iommu/smmuv3: Report non recoverable faults
  vfio-pci: Add a new VFIO_REGION_TYPE_NESTED region type
  vfio-pci: Register an iommu fault handler
  vfio_pci: Allow to mmap the fault queue
  vfio-pci: Add VFIO_PCI_DMA_FAULT_IRQ_INDEX
  vfio: Document nested stage control

Jacob Pan (4):
  driver core: add per device iommu param
  iommu: introduce device fault data
  iommu: introduce device fault report API
  iommu: Introduce attach/detach_pasid_table API

Jean-Philippe Brucker (2):
  iommu/arm-smmu-v3: Link domains and devices
  iommu/arm-smmu-v3: Maintain a SID->device structure

Liu, Yi L (3):
  iommu: Introduce cache_invalidate API
  vfio: VFIO_IOMMU_ATTACH/DETACH_PASID_TABLE
  vfio: VFIO_IOMMU_CACHE_INVALIDATE

 Documentation/vfio.txt  |  83 
 drivers/iommu/arm-smmu-v3.c | 581 ++--
 drivers/iommu/dma-iommu.c   | 145 ++-
 drivers/iommu/iommu.c   | 201 +-
 drivers/vfio/pci/vfio_pci.c | 214 ++
 drivers/vfio/pci/vfio_pci_intrs.c   |  19 +
 drivers/vfio/pci/vfio_pci_private.h |  18 +
 drivers/vfio/pci/vfio_pci_rdwr.c|  73 
 drivers/vfio/vfio_iommu_type1.c | 158 

Re: [RFC] Question about TLB flush while set Stage-2 huge pages

2019-03-15 Thread Suzuki K Poulose

Hi Zhengui,

On 15/03/2019 08:21, Zheng Xiang wrote:

Hi Suzuki,

I have tested this patch, VM doesn't hang and we get expected WARNING log:


Thanks for the quick testing !


However, we also get the following unexpected log:

[  908.329900] BUG: Bad page state in process qemu-kvm  pfn:a2fb41cf
[  908.339415] page:7e28bed073c0 count:-4 mapcount:0 
mapping: index:0x0
[  908.339416] flags: 0x4e00()
[  908.339418] raw: 4e00 dead0100 dead0200 

[  908.339419] raw:   fffc 

[  908.339420] page dumped because: nonzero _refcount
[  908.339437] CPU: 32 PID: 72599 Comm: qemu-kvm Kdump: loaded Tainted: GB  
W5.0.0+ #1
[  908.339438] Call trace:
[  908.339439]  dump_backtrace+0x0/0x188
[  908.339441]  show_stack+0x24/0x30
[  908.339442]  dump_stack+0xa8/0xcc
[  908.339443]  bad_page+0xf0/0x150
[  908.339445]  free_pages_check_bad+0x84/0xa0
[  908.339446]  free_pcppages_bulk+0x4b8/0x750
[  908.339448]  free_unref_page_commit+0x13c/0x198
[  908.339449]  free_unref_page+0x84/0xa0
[  908.339451]  __free_pages+0x58/0x68
[  908.339452]  zap_huge_pmd+0x290/0x2d8
[  908.339454]  unmap_page_range+0x2b4/0x470
[  908.339455]  unmap_single_vma+0x94/0xe8
[  908.339457]  unmap_vmas+0x8c/0x108
[  908.339458]  exit_mmap+0xd4/0x178
[  908.339459]  mmput+0x74/0x180
[  908.339460]  do_exit+0x2b4/0x5b0
[  908.339462]  do_group_exit+0x3c/0xe0
[  908.339463]  __arm64_sys_exit_group+0x24/0x28
[  908.339465]  el0_svc_common+0xa0/0x180
[  908.339466]  el0_svc_handler+0x38/0x78
[  908.339467]  el0_svc+0x8/0xc


Thats bad, we seem to be making upto 4 unbalanced put_page().


---
   virt/kvm/arm/mmu.c | 51 +++
   1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 66e0fbb5..04b0f9b 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1076,24 +1076,38 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct 
kvm_mmu_memory_cache
    * Skip updating the page table if the entry is
    * unchanged.
    */
-    if (pmd_val(old_pmd) == pmd_val(*new_pmd))
+    if (pmd_val(old_pmd) == pmd_val(*new_pmd)) {
   return 0;
-
+    } else if (WARN_ON_ONCE(!pmd_thp_or_huge(old_pmd))) {
   /*
- * Mapping in huge pages should only happen through a
- * fault.  If a page is merged into a transparent huge
- * page, the individual subpages of that huge page
- * should be unmapped through MMU notifiers before we
- * get here.
- *
- * Merging of CompoundPages is not supported; they
- * should become splitting first, unmapped, merged,
- * and mapped back in on-demand.
+ * If we have PTE level mapping for this block,
+ * we must unmap it to avoid inconsistent TLB
+ * state. We could end up in this situation if
+ * the memory slot was marked for dirty logging
+ * and was reverted, leaving PTE level mappings
+ * for the pages accessed during the period.
+ * Normal THP split/merge follows mmu_notifier
+ * callbacks and do get handled accordingly.
    */
-    VM_BUG_ON(pmd_pfn(old_pmd) != pmd_pfn(*new_pmd));
+    unmap_stage2_range(kvm, (addr & S2_PMD_MASK), S2_PMD_SIZE);


It seems that kvm decreases the _refcount of the page twice in 
transparent_hugepage_adjust()
and unmap_stage2_range().


But I thought we should be doing that on the head_page already, as this is THP.
I will take a look and get back to you on this. Btw, is it possible for you
to turn on CONFIG_DEBUG_VM and re-run with the above patch ?

Kind regards
Suzuki


___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH kvmtool v1 2/2] vfio-pci: Fallback to INTx mode when disable MSI/MSIX

2019-03-15 Thread Jean-Philippe Brucker
On 15/03/2019 08:33, Leo Yan wrote:
> Since PCI forbids enabling INTx, MSI or MSIX at the same time, it's by
> default to disable INTx mode when enable MSI/MSIX mode; but this logic is
> easily broken if the guest PCI driver detects the MSI/MSIX cannot work as
> expected and tries to rollback to use INTx mode.  The INTx mode has been
> disabled and it has no chance to be enabled again, thus both INTx mode
> and MSI/MSIX mode will not be enabled in vfio for this case.
> 
> Below shows the detailed flow for introducing this issue:
> 
>   vfio_pci_configure_dev_irqs()
> `-> vfio_pci_enable_intx()
> 
>   vfio_pci_enable_msis()
> `-> vfio_pci_disable_intx()
> 
>   vfio_pci_disable_msis()   => Guest PCI driver disables MSI
> 
> To fix this issue, when disable MSI/MSIX we need to check if INTx mode
> is available for this device or not; if the device can support INTx then
> we need to re-enable it so the device can fallback to use it.
> 
> Signed-off-by: Leo Yan 
> ---
>  vfio/pci.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/vfio/pci.c b/vfio/pci.c
> index c0683f6..44727bb 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -28,6 +28,7 @@ struct vfio_irq_eventfd {
>   msi_update_state(state, val, VFIO_PCI_MSI_STATE_EMPTY)
>  
>  static void vfio_pci_disable_intx(struct kvm *kvm, struct vfio_device *vdev);
> +static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev);
>  
>  static int vfio_pci_enable_msis(struct kvm *kvm, struct vfio_device *vdev,
>   bool msix)
> @@ -50,7 +51,7 @@ static int vfio_pci_enable_msis(struct kvm *kvm, struct 
> vfio_device *vdev,
>   if (!msi_is_enabled(msis->virt_state))
>   return 0;
>  
> - if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX) {
> + if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX)
>   /*
>* PCI (and VFIO) forbids enabling INTx, MSI or MSIX at the same
>* time. Since INTx has to be enabled from the start (we don't
> @@ -58,9 +59,6 @@ static int vfio_pci_enable_msis(struct kvm *kvm, struct 
> vfio_device *vdev,
>* disable it now.
>*/
>   vfio_pci_disable_intx(kvm, vdev);
> - /* Permanently disable INTx */
> - pdev->irq_modes &= ~VFIO_PCI_IRQ_MODE_INTX;

As a result vfio_pci_disable_intx() may be called multiple times (each
time the guest enables one MSI vector). Could you make
vfio_pci_disable_intx() safe against that (maybe use intx_fd == -1 to
denote the INTx state)?

> - }
>  
>   eventfds = (void *)msis->irq_set + sizeof(struct vfio_irq_set);
>  
> @@ -162,7 +160,16 @@ static int vfio_pci_disable_msis(struct kvm *kvm, struct 
> vfio_device *vdev,
>   msi_set_enabled(msis->phys_state, false);
>   msi_set_empty(msis->phys_state, true);
>  
> - return 0;
> + if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX)
> + /*
> +  * When MSI or MSIX is disabled, this might be called when
> +  * PCI driver detects the MSI interrupt failure and wants to
> +  * rollback to INTx mode.  Thus enable INTx if the device
> +  * supports INTx mode in this case.
> +  */
> + ret = vfio_pci_enable_intx(kvm, vdev);

Let's remove vfio_pci_reserve_irq_fds(2) from vfio_pci_enable_intx(), it
should only called once per run, and isn't particularly useful here
since INTx only uses 2 fds. It's used to bump the fd rlimit when a
device needs ~2048 file descriptors for MSI-X.

Thanks,
Jean

> +
> + return ret >= 0 ? 0 : ret;
>  }
>  
>  static int vfio_pci_update_msi_entry(struct kvm *kvm, struct vfio_device 
> *vdev,
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH kvmtool v1 1/2] vfio-pci: Release INTx's guest to host eventfd properly

2019-03-15 Thread Jean-Philippe Brucker
Hi,

On 15/03/2019 08:33, Leo Yan wrote:
> The PCI device INTx uses event fd 'unmask_fd' to signal the deassertion
> of the line from guest to host; but this eventfd isn't released properly
> when disable INTx.
> 
> When disable INTx this patch firstly unbinds interrupt signal by calling
> ioctl VFIO_DEVICE_SET_IRQS and then it uses the new added field
> 'unmask_fd' in struct vfio_pci_device to close event fd.
> 
> Signed-off-by: Leo Yan 
> ---
>  include/kvm/vfio.h |  1 +
>  vfio/pci.c | 15 ---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
> index 60e6c54..28223cf 100644
> --- a/include/kvm/vfio.h
> +++ b/include/kvm/vfio.h
> @@ -74,6 +74,7 @@ struct vfio_pci_device {
>  
>   unsigned long   irq_modes;
>   int intx_fd;
> + int unmask_fd;
>   unsigned intintx_gsi;
>   struct vfio_pci_msi_common  msi;
>   struct vfio_pci_msi_common  msix;
> diff --git a/vfio/pci.c b/vfio/pci.c
> index 03de3c1..c0683f6 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -996,18 +996,26 @@ static void vfio_pci_disable_intx(struct kvm *kvm, 
> struct vfio_device *vdev)
>  {
>   struct vfio_pci_device *pdev = >pci;
>   int gsi = pdev->intx_gsi;
> - struct vfio_irq_set irq_set = {
> - .argsz  = sizeof(irq_set),
> + struct vfio_irq_set trigger_irq = {
> + .argsz  = sizeof(trigger_irq),
>   .flags  = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER,
>   .index  = VFIO_PCI_INTX_IRQ_INDEX,
>   };
>  
> + struct vfio_irq_set unmask_irq = {
> + .argsz  = sizeof(unmask_irq),
> + .flags  = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_UNMASK,
> + .index  = VFIO_PCI_INTX_IRQ_INDEX,
> + };
> +
>   pr_debug("user requested MSI, disabling INTx %d", gsi);
>  
> - ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _set);
> + ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _irq);
> + ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _irq);

The patch makes sense, we do need to close unmask_fd, but I don't think
we need the additional ioctl. VFIO removes the unmask trigger when we
disable INTx in the first ioctl, so an additional ioctl to remove the
unmask trigger will return EINVAL.

Thanks,
Jean

>   irq__del_irqfd(kvm, gsi, pdev->intx_fd);
>  
>   close(pdev->intx_fd);
> + close(pdev->unmask_fd);
>  }
>  
>  static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
> @@ -1095,6 +1103,7 @@ static int vfio_pci_enable_intx(struct kvm *kvm, struct 
> vfio_device *vdev)
>   }
>  
>   pdev->intx_fd = trigger_fd;
> + pdev->unmask_fd = unmask_fd;
>   /* Guest is going to ovewrite our irq_line... */
>   pdev->intx_gsi = gsi;
>  
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: Question: KVM: Failed to bind vfio with PCI-e / SMMU on Juno-r2

2019-03-15 Thread Robin Murphy

Hi Leo,

Sorry for the delay - I'm on holiday this week, but since I've made the 
mistake of glancing at my inbox I should probably save you from wasting 
any more time...


On 2019-03-15 11:03 am, Auger Eric wrote:

Hi Leo,

+ Jean-Philippe

On 3/15/19 10:37 AM, Leo Yan wrote:

Hi Eric, Robin,

On Wed, Mar 13, 2019 at 11:24:25AM +0100, Auger Eric wrote:

[...]


If the NIC supports MSIs they logically are used. This can be easily
checked on host by issuing "cat /proc/interrupts | grep vfio". Can you
check whether the guest received any interrupt? I remember that Robin
said in the past that on Juno, the MSI doorbell was in the PCI host
bridge window and possibly transactions towards the doorbell could not
reach it since considered as peer to peer.


I found back Robin's explanation. It was not related to MSI IOVA being
within the PCI host bridge window but RAM GPA colliding with host PCI
config space?

"MSI doorbells integral to PCIe root complexes (and thus untranslatable)
typically have a programmable address, so could be anywhere. In the more
general category of "special hardware addresses", QEMU's default ARM
guest memory map puts RAM starting at 0x4000; on the ARM Juno
platform, that happens to be where PCI config space starts; as Juno's
PCIe doesn't support ACS, peer-to-peer or anything clever, if you assign
the PCI bus to a guest (all of it, given the lack of ACS), the root
complex just sees the guest's attempts to DMA to "memory" as the device
attempting to access config space and aborts them."


Below is some following investigation at my side:

Firstly, must admit that I don't understand well for up paragraph; so
based on the description I am wandering if can use INTx mode and if
it's lucky to avoid this hardware pitfall.


The problem above is that during the assignment process, the virtualizer
maps the whole guest RAM though the IOMMU (+ the MSI doorbell on ARM) to
allow the device, programmed in GPA to access the whole guest RAM.
Unfortunately if the device emits a DMA request with 0x4000 IOVA
address, this IOVA is interpreted by the Juno RC as a transaction
towards the PCIe config space. So this DMA request will not go beyond
the RC, will never reach the IOMMU and will never reach the guest RAM.
So globally the device is not able to reach part of the guest RAM.
That's how I interpret the above statement. Then I don't know the
details of the collision, I don't have access to this HW. I don't know
either if this problem still exists on the r2 HW.


The short answer is that if you want PCI passthrough to work on Juno, 
the guest memory map has to look like a Juno.


The PCIe root complex uses an internal lookup table to generate 
appropriate AXI attributes for outgoing PCIe transactions; unfortunately 
this has no notion of 'default' attributes, so addresses *must* match 
one of the programmed windows in order to be valid. From memory, EDK2 
sets up a 2GB window covering the lower DRAM bank, an 8GB window 
covering the upper DRAM bank, and a 1MB (or thereabouts) window covering 
the GICv2m region with Device attributes. Any PCIe transactions to 
addresses not within one of those windows will be aborted by the RC 
without ever going out to the AXI side where the SMMU lies (and I think 
anything matching the config space or I/O space windows or a region 
claimed by a BAR will be aborted even earlier as a peer-to-peer attempt 
regardless of the AXI Translation Table setup).


You could potentially modify the firmware to change the window 
configuration, but the alignment restrictions make it awkward. I've only 
ever tested passthrough on Juno using kvmtool, which IIRC already has 
guest RAM in an appropriate place (and is trivially easy to hack if not) 
- I don't remember if I ever actually tried guest MSI with that.


Robin.


But when I want to rollback to use INTx mode I found there have issue
for kvmtool to support INTx mode, so this is why I wrote the patch [1]
to fix the issue.  Alternatively, we also can set the NIC driver
module parameter 'sky2.disable_msi=1' thus can totally disable msi and
only use INTx mode.

Anyway, finally I can get INTx mode enabled and I can see the
interrupt will be registered successfully on both host and guest:

Host side:

CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
  41:  0  0  0  0  0  0 
GICv2  54 Level arm-pmu
  42:  0  0  0  0  0  0 
GICv2  58 Level arm-pmu
  43:  0  0  0  0  0  0 
GICv2  62 Level arm-pmu
  45:772  0  0  0  0  0 
GICv2 171 Level vfio-intx(:08:00.0)

Guest side:

# cat /proc/interrupts
CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
  12:  0  0  0  0  0  0 
GIC-0  96 Level eth1

So you could see the 

Re: Question: KVM: Failed to bind vfio with PCI-e / SMMU on Juno-r2

2019-03-15 Thread Auger Eric
Hi Leo,

+ Jean-Philippe

On 3/15/19 10:37 AM, Leo Yan wrote:
> Hi Eric, Robin,
> 
> On Wed, Mar 13, 2019 at 11:24:25AM +0100, Auger Eric wrote:
> 
> [...]
> 
>>> If the NIC supports MSIs they logically are used. This can be easily
>>> checked on host by issuing "cat /proc/interrupts | grep vfio". Can you
>>> check whether the guest received any interrupt? I remember that Robin
>>> said in the past that on Juno, the MSI doorbell was in the PCI host
>>> bridge window and possibly transactions towards the doorbell could not
>>> reach it since considered as peer to peer.
>>
>> I found back Robin's explanation. It was not related to MSI IOVA being
>> within the PCI host bridge window but RAM GPA colliding with host PCI
>> config space?
>>
>> "MSI doorbells integral to PCIe root complexes (and thus untranslatable)
>> typically have a programmable address, so could be anywhere. In the more
>> general category of "special hardware addresses", QEMU's default ARM
>> guest memory map puts RAM starting at 0x4000; on the ARM Juno
>> platform, that happens to be where PCI config space starts; as Juno's
>> PCIe doesn't support ACS, peer-to-peer or anything clever, if you assign
>> the PCI bus to a guest (all of it, given the lack of ACS), the root
>> complex just sees the guest's attempts to DMA to "memory" as the device
>> attempting to access config space and aborts them."
> 
> Below is some following investigation at my side:
> 
> Firstly, must admit that I don't understand well for up paragraph; so
> based on the description I am wandering if can use INTx mode and if
> it's lucky to avoid this hardware pitfall.

The problem above is that during the assignment process, the virtualizer
maps the whole guest RAM though the IOMMU (+ the MSI doorbell on ARM) to
allow the device, programmed in GPA to access the whole guest RAM.
Unfortunately if the device emits a DMA request with 0x4000 IOVA
address, this IOVA is interpreted by the Juno RC as a transaction
towards the PCIe config space. So this DMA request will not go beyond
the RC, will never reach the IOMMU and will never reach the guest RAM.
So globally the device is not able to reach part of the guest RAM.
That's how I interpret the above statement. Then I don't know the
details of the collision, I don't have access to this HW. I don't know
either if this problem still exists on the r2 HW.
> 
> But when I want to rollback to use INTx mode I found there have issue
> for kvmtool to support INTx mode, so this is why I wrote the patch [1]
> to fix the issue.  Alternatively, we also can set the NIC driver
> module parameter 'sky2.disable_msi=1' thus can totally disable msi and
> only use INTx mode.
> 
> Anyway, finally I can get INTx mode enabled and I can see the
> interrupt will be registered successfully on both host and guest:
> 
> Host side:
> 
>CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
>  41:  0  0  0  0  0  0 
> GICv2  54 Level arm-pmu
>  42:  0  0  0  0  0  0 
> GICv2  58 Level arm-pmu
>  43:  0  0  0  0  0  0 
> GICv2  62 Level arm-pmu
>  45:772  0  0  0  0  0 
> GICv2 171 Level vfio-intx(:08:00.0)
> 
> Guest side:
> 
> # cat /proc/interrupts
>CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
>  12:  0  0  0  0  0  0 
> GIC-0  96 Level eth1
> 
> So you could see the host can receive the interrupts, but these
> interrupts are mainly triggered before binding vfio-pci driver.  But
> seems now after launch kvm I can see there have very small mount
> interrupts are triggered in host and the guest kernel also can receive
> the virtual interrupts, e.g. if use 'dhclient eth1' command in guest
> OS, this command stalls for long time (> 1 minute) after return back,
> I can see both the host OS and guest OS can receive 5~6 interrupts.
> Based on this, I guess the flow for interrupts forwarding has been
> enabled.  But seems the data packet will not really output and I use
> wireshark to capture packets, but cannot find any packet output from
> the NIC.
> 
> I did another testing is to shrink the memory space/io/bus region to
> less than 0x4000, so this can avoid to put guest memory IPA into
> 0x4000.  But this doesn't work.

What is worth to try is to move the base address of the guest RAM. I
think there were some recent works on this on kvmtool. Adding
Jean-Philippe in the loop.

Thanks

Eric
> 
> @Robin, could you help explain for the hardware issue and review my
> methods are feasible on Juno board?  Thanks a lot for suggestions.
> 
> I will dig more for the memory mapping and post at here.
> 
> Thanks,
> Leo Yan
> 
> [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2019-March/035055.html
> 

Re: Question: KVM: Failed to bind vfio with PCI-e / SMMU on Juno-r2

2019-03-15 Thread Leo Yan
Hi Eric, Robin,

On Wed, Mar 13, 2019 at 11:24:25AM +0100, Auger Eric wrote:

[...]

> > If the NIC supports MSIs they logically are used. This can be easily
> > checked on host by issuing "cat /proc/interrupts | grep vfio". Can you
> > check whether the guest received any interrupt? I remember that Robin
> > said in the past that on Juno, the MSI doorbell was in the PCI host
> > bridge window and possibly transactions towards the doorbell could not
> > reach it since considered as peer to peer.
> 
> I found back Robin's explanation. It was not related to MSI IOVA being
> within the PCI host bridge window but RAM GPA colliding with host PCI
> config space?
> 
> "MSI doorbells integral to PCIe root complexes (and thus untranslatable)
> typically have a programmable address, so could be anywhere. In the more
> general category of "special hardware addresses", QEMU's default ARM
> guest memory map puts RAM starting at 0x4000; on the ARM Juno
> platform, that happens to be where PCI config space starts; as Juno's
> PCIe doesn't support ACS, peer-to-peer or anything clever, if you assign
> the PCI bus to a guest (all of it, given the lack of ACS), the root
> complex just sees the guest's attempts to DMA to "memory" as the device
> attempting to access config space and aborts them."

Below is some following investigation at my side:

Firstly, must admit that I don't understand well for up paragraph; so
based on the description I am wandering if can use INTx mode and if
it's lucky to avoid this hardware pitfall.

But when I want to rollback to use INTx mode I found there have issue
for kvmtool to support INTx mode, so this is why I wrote the patch [1]
to fix the issue.  Alternatively, we also can set the NIC driver
module parameter 'sky2.disable_msi=1' thus can totally disable msi and
only use INTx mode.

Anyway, finally I can get INTx mode enabled and I can see the
interrupt will be registered successfully on both host and guest:

Host side:

   CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
 41:  0  0  0  0  0  0 
GICv2  54 Level arm-pmu
 42:  0  0  0  0  0  0 
GICv2  58 Level arm-pmu
 43:  0  0  0  0  0  0 
GICv2  62 Level arm-pmu
 45:772  0  0  0  0  0 
GICv2 171 Level vfio-intx(:08:00.0)

Guest side:

# cat /proc/interrupts
   CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
 12:  0  0  0  0  0  0 
GIC-0  96 Level eth1

So you could see the host can receive the interrupts, but these
interrupts are mainly triggered before binding vfio-pci driver.  But
seems now after launch kvm I can see there have very small mount
interrupts are triggered in host and the guest kernel also can receive
the virtual interrupts, e.g. if use 'dhclient eth1' command in guest
OS, this command stalls for long time (> 1 minute) after return back,
I can see both the host OS and guest OS can receive 5~6 interrupts.
Based on this, I guess the flow for interrupts forwarding has been
enabled.  But seems the data packet will not really output and I use
wireshark to capture packets, but cannot find any packet output from
the NIC.

I did another testing is to shrink the memory space/io/bus region to
less than 0x4000, so this can avoid to put guest memory IPA into
0x4000.  But this doesn't work.

@Robin, could you help explain for the hardware issue and review my
methods are feasible on Juno board?  Thanks a lot for suggestions.

I will dig more for the memory mapping and post at here.

Thanks,
Leo Yan

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2019-March/035055.html
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [RFC] arm/cpu: fix soft lockup panic after resuming from stop

2019-03-15 Thread Christoffer Dall
Hi Steve,

On Wed, Mar 13, 2019 at 10:11:30AM +, Steven Price wrote:
> 
> Personally I think what we need is:
> 
> * Either a patch like the one from Heyi Guo (save/restore CNTVCT_EL0) or
> alternatively hooking up KVM_KVMCLOCK_CTRL to prevent the watchdog
> firing when user space explicitly stops scheduling the guest for a while.

If we save/restore CNTVCT_EL0 and the warning goes away, does the guest
wall clock timekeeping get all confused and does it figure this out
automagically somehow?

Does KVM_KVMCLOCK_CTRL solve that problem?

> 
> * KVM itself saving/restoring CNTVCT_EL0 during suspend/resume so the
> guest doesn't see time pass during a suspend.

This smells like policy to me so I'd much prefer keeping as much
functionality in user space as possible.  If we already have the APIs we
need from KVM, let's use them.

> 
> * Something equivalent to MSR_KVM_WALL_CLOCK_NEW for arm which allows
> the guest to query the wall clock time from the host and provides an
> offset between CNTVCT_EL0 to wall clock time which the KVM can update
> during suspend/resume. This means that during a suspend/resume the guest
> can observe that wall clock time has passed, without having to be
> bothered about CNTVCT_EL0 jumping forwards.
> 

Isn't the proper Arm architectural solution for this to read the
physical counter for wall clock time keeping ?

(Yes that will require a trap on physical counter reads after migration
on current systems, but migration sucks in terms of timekeeping
already.)


Thanks,

Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: Kick cpu when WFI in single-threaded kvm integration

2019-03-15 Thread Christoffer Dall
Hi Jan,

On Thu, Mar 14, 2019 at 12:19:02PM +, Jan Bolke wrote:
> Hi all,
> 
> Currently I am working on a SystemC integration of kvm on arm.
> Therefore, I use the kvm api and of course SystemC (library to simulate 
> hardware platforms with C++).
> 
> As I need the virtual cpu to interrupt its execution loop from time to time 
> to let the rest of the SystemC simulation execute,
> I use a perf_event and let the kernel send a signal on overflow to the 
> simulation thread which kicks the virtual cpu (suggested by this mailing 
> list, thanks again).
> Thus I am able to simulate a quantum mechanism for the virtual cpu.
> 
> As I am running benchmarks (e.g. Coremark) on my virtual platform this works 
> fine.
> 
> I also get to boot Linux until it spawns the terminal and then wait for 
> interrupts from my virtual uart.
> Here comes the problem:
> The perf event counting mechanism does increment its counted instructions 
> very very slowly when the virtual cpu executes wfi.
> Thus my whole simulation starts to hang.
> As my simulation is single threaded I need the signal from the kernel to kick 
> my cpu to let the virtual uart deliver its interrupt to react to my input.
> I tried to use the request_interrupt_window flag but this does not seem to 
> work.
> 
> Is there a way to kick the virtual cpu when it is waiting for interrupts? Or 
> do I have to patch my kvm code?
> 

Let me see if I understand your question properly; you are running a KVM
virtual CPU which executes WFI in the guest, and then you are not
receiving interrupts in the guest nor getting events back from KVM which
you somehow use to run a backend simulation in userspace?

KVM/Arm can do two things for WFI:

 1. Let the guest directly execute it without trapping to the hypervisor
(the physical CPU will NOT exit the guest until there's a physical
interrupt on the CPU).

 2. Trap WFI to KVM.  KVM asks Linux to schedule another process until
there's a virtual interrupt for the VCPU.  This is what mainline
KVM/Arm does.


I suspect what's happening is that you are using a normal kernel
configured as (2), and therefore you only count cycles for the perf
event while the guest runs the timer ISR which obviously is much much
less than if you had a constant running VCPU.  Am I on the right track?

If so, there are a couple of things you could try.

First, you can try disabling the trap on WFI (note that this changes
pretty fundamental behavior of KVM, and this is not recommended for
production use or for systems level performance investigations where
more than one workload contends for a single physical CPU):

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 7f9d2bfcf82e..b38a5a134fef 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -84,7 +84,7 @@
  * FMO:Override CPSR.F and enable signaling with VF
  * SWIO:   Turn set/way invalidates into set/way clean+invalidate
  */
-#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
+#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_VM | \
 HCR_TVM | HCR_BSU_IS | HCR_FB | HCR_TAC | \
 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
 HCR_FMO | HCR_IMO)


Note that I'm not sure how the performance counter counts on your
particular platform when the CPU is in WFI, so this may not help at all.


Second, and possibly preferred, you can hook up your simulation event to
a timer event in the case of trapping on a WFI.  See kvm_handle_wfx() in
arch/arm64/kvm/handle_exit.c and follow kvm_vcpu_block() from there to
see how KVM/Arm handles this event.


Hope this helps,

Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH kvmtool v1 1/2] vfio-pci: Release INTx's guest to host eventfd properly

2019-03-15 Thread Leo Yan
The PCI device INTx uses event fd 'unmask_fd' to signal the deassertion
of the line from guest to host; but this eventfd isn't released properly
when disable INTx.

When disable INTx this patch firstly unbinds interrupt signal by calling
ioctl VFIO_DEVICE_SET_IRQS and then it uses the new added field
'unmask_fd' in struct vfio_pci_device to close event fd.

Signed-off-by: Leo Yan 
---
 include/kvm/vfio.h |  1 +
 vfio/pci.c | 15 ---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
index 60e6c54..28223cf 100644
--- a/include/kvm/vfio.h
+++ b/include/kvm/vfio.h
@@ -74,6 +74,7 @@ struct vfio_pci_device {
 
unsigned long   irq_modes;
int intx_fd;
+   int unmask_fd;
unsigned intintx_gsi;
struct vfio_pci_msi_common  msi;
struct vfio_pci_msi_common  msix;
diff --git a/vfio/pci.c b/vfio/pci.c
index 03de3c1..c0683f6 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -996,18 +996,26 @@ static void vfio_pci_disable_intx(struct kvm *kvm, struct 
vfio_device *vdev)
 {
struct vfio_pci_device *pdev = >pci;
int gsi = pdev->intx_gsi;
-   struct vfio_irq_set irq_set = {
-   .argsz  = sizeof(irq_set),
+   struct vfio_irq_set trigger_irq = {
+   .argsz  = sizeof(trigger_irq),
.flags  = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER,
.index  = VFIO_PCI_INTX_IRQ_INDEX,
};
 
+   struct vfio_irq_set unmask_irq = {
+   .argsz  = sizeof(unmask_irq),
+   .flags  = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_UNMASK,
+   .index  = VFIO_PCI_INTX_IRQ_INDEX,
+   };
+
pr_debug("user requested MSI, disabling INTx %d", gsi);
 
-   ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _set);
+   ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _irq);
+   ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, _irq);
irq__del_irqfd(kvm, gsi, pdev->intx_fd);
 
close(pdev->intx_fd);
+   close(pdev->unmask_fd);
 }
 
 static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
@@ -1095,6 +1103,7 @@ static int vfio_pci_enable_intx(struct kvm *kvm, struct 
vfio_device *vdev)
}
 
pdev->intx_fd = trigger_fd;
+   pdev->unmask_fd = unmask_fd;
/* Guest is going to ovewrite our irq_line... */
pdev->intx_gsi = gsi;
 
-- 
2.19.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH kvmtool v1 2/2] vfio-pci: Fallback to INTx mode when disable MSI/MSIX

2019-03-15 Thread Leo Yan
Since PCI forbids enabling INTx, MSI or MSIX at the same time, it's by
default to disable INTx mode when enable MSI/MSIX mode; but this logic is
easily broken if the guest PCI driver detects the MSI/MSIX cannot work as
expected and tries to rollback to use INTx mode.  The INTx mode has been
disabled and it has no chance to be enabled again, thus both INTx mode
and MSI/MSIX mode will not be enabled in vfio for this case.

Below shows the detailed flow for introducing this issue:

  vfio_pci_configure_dev_irqs()
`-> vfio_pci_enable_intx()

  vfio_pci_enable_msis()
`-> vfio_pci_disable_intx()

  vfio_pci_disable_msis()   => Guest PCI driver disables MSI

To fix this issue, when disable MSI/MSIX we need to check if INTx mode
is available for this device or not; if the device can support INTx then
we need to re-enable it so the device can fallback to use it.

Signed-off-by: Leo Yan 
---
 vfio/pci.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/vfio/pci.c b/vfio/pci.c
index c0683f6..44727bb 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -28,6 +28,7 @@ struct vfio_irq_eventfd {
msi_update_state(state, val, VFIO_PCI_MSI_STATE_EMPTY)
 
 static void vfio_pci_disable_intx(struct kvm *kvm, struct vfio_device *vdev);
+static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev);
 
 static int vfio_pci_enable_msis(struct kvm *kvm, struct vfio_device *vdev,
bool msix)
@@ -50,7 +51,7 @@ static int vfio_pci_enable_msis(struct kvm *kvm, struct 
vfio_device *vdev,
if (!msi_is_enabled(msis->virt_state))
return 0;
 
-   if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX) {
+   if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX)
/*
 * PCI (and VFIO) forbids enabling INTx, MSI or MSIX at the same
 * time. Since INTx has to be enabled from the start (we don't
@@ -58,9 +59,6 @@ static int vfio_pci_enable_msis(struct kvm *kvm, struct 
vfio_device *vdev,
 * disable it now.
 */
vfio_pci_disable_intx(kvm, vdev);
-   /* Permanently disable INTx */
-   pdev->irq_modes &= ~VFIO_PCI_IRQ_MODE_INTX;
-   }
 
eventfds = (void *)msis->irq_set + sizeof(struct vfio_irq_set);
 
@@ -162,7 +160,16 @@ static int vfio_pci_disable_msis(struct kvm *kvm, struct 
vfio_device *vdev,
msi_set_enabled(msis->phys_state, false);
msi_set_empty(msis->phys_state, true);
 
-   return 0;
+   if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_INTX)
+   /*
+* When MSI or MSIX is disabled, this might be called when
+* PCI driver detects the MSI interrupt failure and wants to
+* rollback to INTx mode.  Thus enable INTx if the device
+* supports INTx mode in this case.
+*/
+   ret = vfio_pci_enable_intx(kvm, vdev);
+
+   return ret >= 0 ? 0 : ret;
 }
 
 static int vfio_pci_update_msi_entry(struct kvm *kvm, struct vfio_device *vdev,
-- 
2.19.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [RFC] Question about TLB flush while set Stage-2 huge pages

2019-03-15 Thread Zheng Xiang
Hi Suzuki,

I have tested this patch, VM doesn't hang and we get expected WARNING log:

[  526.184452] pstate: 2049 (nzCv daif +PAN -UAO)
[  526.184454] pc : user_mem_abort+0x484/0x9e0
[  526.184455] lr : user_mem_abort+0x478/0x9e0
[  526.184456] sp : 84a038e0
[  526.184457] x29: 84a038e0 x28: 00012f60
[  526.184458] x27: 8a2fa27ae918 x26: 0020
[  526.184460] x25:  x24: 
[  526.184461] x23: 00400a269d0007fd x22: 849cd000
[  526.184462] x21: 1181d000 x20: 0a26eef72003
[  526.184463] x19: 8a2fb41d4bd8 x18: 4fffb8b22000
[  526.184465] x17:  x16: 
[  526.184466] x15: 0001 x14: 08dd12a8
[  526.184467] x13: 0041 x12: 8a26eeca6e30
[  526.184468] x11: 8000fe4af800 x10: 0040
[  526.184469] x9 : 097c46c0 x8 : 8000ff400248
[  526.184471] x7 : 0010 x6 : 21f8
[  526.184472] x5 : a269d000 x4 : 0018
[  526.184473] x3 : 000a x2 : 0004
[  526.184474] x1 :  x0 : 
[  526.184476] Call trace:
[  526.184477]  user_mem_abort+0x484/0x9e0
[  526.184479]  kvm_handle_guest_abort+0x11c/0x478
[  526.184480]  handle_exit+0x14c/0x1c8
[  526.184482]  kvm_arch_vcpu_ioctl_run+0x280/0x898
[  526.184483]  kvm_vcpu_ioctl+0x488/0x8a8
[  526.184485]  do_vfs_ioctl+0xc4/0x8c0
[  526.184486]  ksys_ioctl+0x8c/0xa0
[  526.184487]  __arm64_sys_ioctl+0x28/0x38
[  526.184489]  el0_svc_common+0xa0/0x180
[  526.184491]  el0_svc_handler+0x38/0x78
[  526.184492]  el0_svc+0x8/0xc

However, we also get the following unexpected log:

[  908.329900] BUG: Bad page state in process qemu-kvm  pfn:a2fb41cf
[  908.339415] page:7e28bed073c0 count:-4 mapcount:0 
mapping: index:0x0
[  908.339416] flags: 0x4e00()
[  908.339418] raw: 4e00 dead0100 dead0200 

[  908.339419] raw:   fffc 

[  908.339420] page dumped because: nonzero _refcount
[  908.339437] CPU: 32 PID: 72599 Comm: qemu-kvm Kdump: loaded Tainted: GB  
W5.0.0+ #1
[  908.339438] Call trace:
[  908.339439]  dump_backtrace+0x0/0x188
[  908.339441]  show_stack+0x24/0x30
[  908.339442]  dump_stack+0xa8/0xcc
[  908.339443]  bad_page+0xf0/0x150
[  908.339445]  free_pages_check_bad+0x84/0xa0
[  908.339446]  free_pcppages_bulk+0x4b8/0x750
[  908.339448]  free_unref_page_commit+0x13c/0x198
[  908.339449]  free_unref_page+0x84/0xa0
[  908.339451]  __free_pages+0x58/0x68
[  908.339452]  zap_huge_pmd+0x290/0x2d8
[  908.339454]  unmap_page_range+0x2b4/0x470
[  908.339455]  unmap_single_vma+0x94/0xe8
[  908.339457]  unmap_vmas+0x8c/0x108
[  908.339458]  exit_mmap+0xd4/0x178
[  908.339459]  mmput+0x74/0x180
[  908.339460]  do_exit+0x2b4/0x5b0
[  908.339462]  do_group_exit+0x3c/0xe0
[  908.339463]  __arm64_sys_exit_group+0x24/0x28
[  908.339465]  el0_svc_common+0xa0/0x180
[  908.339466]  el0_svc_handler+0x38/0x78
[  908.339467]  el0_svc+0x8/0xc

>> Marc and I had a discussion about this and it looks like we may have an
>> issue here. So with the cancellation of logging, we do not trigger the
>> mmu_notifiers (as the userspace memory mapping hasn't changed) and thus
>> have memory leaks while trying to install a huge mapping. Would it be
>> possible for you to try the patch below ? It will trigger a WARNING
>> to confirm our theory, but should not cause the hang. As we unmap
>> the PMD/PUD range of PTE mappings before reinstalling a block map.
> 
> Thanks for the reply. And I think this is alomst what Zheng Xiang wanted to 
> say! We will test this patch tomorrow and give you some feedback.
> 
> BTW, we have noticed that X86 had also suffered from the similar issue. You 
> may want to look into commit 3ea3b7fa9af0 ("kvm: mmu: lazy collapse small 
> sptes into large sptes" 2015) :-)
> 
> 
> thanks,
> 
> zenghui
> 
>>
>>
>> ---8>---
>>
>> test: kvm: arm: Fix handling of stage2 huge mappings
>>
>> We rely on the mmu_notifier call backs to handle the split/merging
>> of huge pages and thus we are guaranteed that while creating a
>> block mapping, the entire block is unmapped at stage2. However,
>> we miss a case where the block mapping is split for dirty logging
>> case and then could later be made block mapping, if we cancel the
>> dirty logging. This not only creates inconsistent TLB entries for
>> the pages in the the block, but also leakes the table pages for
>> PMD level.
>>
>> Handle these corner cases for the huge mappings at stage2.
>>
>> Signed-off-by: Suzuki K Poulose 
>> ---
>>   virt/kvm/arm/mmu.c | 51 +++
>>   1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index 66e0fbb5..04b0f9b 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -1076,24 +1076,38