qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v7 0/5] IOMMU: intel_iommu support map and unmap notifications
@ 2016-11-28 15:51 Aviv B.D
  2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 1/5] IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to guest Aviv B.D
                   ` (7 more replies)
  0 siblings, 8 replies; 45+ messages in thread
From: Aviv B.D @ 2016-11-28 15:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Alex Williamson, Peter Xu, Jan Kiszka,
	Jason Wang, Aviv Ben-David

From: "Aviv Ben-David" <bd.aviv@gmail.com>

* Advertize Cache Mode capability in iommu cap register. 
  This capability is controlled by "cache-mode" property of intel-iommu device.
  To enable this option call QEMU with "-device intel-iommu,cache-mode=true".

* On page cache invalidation in intel vIOMMU, check if the domain belong to
  registered notifier, and notify accordingly.

Currently this patch still doesn't enabling VFIO devices support with vIOMMU 
present. Current problems:
* vfio_iommu_map_notify is not aware about memory range belong to specific 
  VFIOGuestIOMMU.
* intel_iommu's replay op is not implemented yet (May come in different patch 
  set).
  The replay function is required for hotplug vfio device and to move devices 
  between existing domains.

Changes from v1 to v2:
* remove assumption that the cache do not clears
* fix lockup on high load.

Changes from v2 to v3:
* remove debug leftovers
* split to sepearate commits
* change is_write to flags in vtd_do_iommu_translate, add IOMMU_NO_FAIL 
  to suppress error propagating to guest.

Changes from v3 to v4:
* Add property to intel_iommu device to control the CM capability, 
  default to False.
* Use s->iommu_ops.notify_flag_changed to register notifiers.

Changes from v4 to v4 RESEND:
* Fix codding style pointed by checkpatch.pl script.

Changes from v4 to v5:
* Reduce the number of changes in patch 2 and make flags real bitfield.
* Revert deleted debug prints.
* Fix memory leak in patch 3.

Changes from v5 to v6:
* fix prototype of iommu_translate function for more IOMMU types.
* VFIO will be notified only on the difference, without unmap 
  before change to maps.

Changes from v6 to v7:
* Add replay op to iommu_ops, with current behavior as default implementation
  (Patch 4).
* Add stub to disable VFIO with intel_iommu support (Patch 5).
* Cosmetic changes to other patches.

Aviv Ben-David (5):
  IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to
    guest
  IOMMU: change iommu_op->translate's is_write to flags, add support to
    NO_FAIL flag mode
  IOMMU: enable intel_iommu map and unmap notifiers
  IOMMU: add specific replay function with default implemenation
  IOMMU: add specific null implementation of iommu_replay to intel_iommu

 exec.c                         |   3 +-
 hw/alpha/typhoon.c             |   2 +-
 hw/i386/amd_iommu.c            |   4 +-
 hw/i386/intel_iommu.c          | 165 ++++++++++++++++++++++++++++++++++-------
 hw/i386/intel_iommu_internal.h |   3 +
 hw/pci-host/apb.c              |   2 +-
 hw/ppc/spapr_iommu.c           |   2 +-
 hw/s390x/s390-pci-bus.c        |   2 +-
 include/exec/memory.h          |  10 ++-
 include/hw/i386/intel_iommu.h  |  11 +++
 memory.c                       |  11 ++-
 11 files changed, 177 insertions(+), 38 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH v7 3/5] IOMMU: enable intel_iommu map and unmap notifiers
@ 2016-12-16  9:12 Liu, Yi L
  2016-12-19 10:00 ` Peter Xu
  2016-12-19 11:54 ` Liu, Yi L
  0 siblings, 2 replies; 45+ messages in thread
From: Liu, Yi L @ 2016-12-16  9:12 UTC (permalink / raw)
  To: bd.aviv@gmail.com, qemu-devel@nongnu.org
  Cc: Michael S. Tsirkin, , Jan Kiszka, , Peter Xu, , Alex Williamson,
	, Jason Wang, Lan, Tianyu, Tian, Kevin, Liu, Yi L

> From: "Aviv Ben-David" <address@hidden>
> 
> Adds a list of registered vtd_as's to intel iommu state to save
> iteration over each PCI device in a search of the corrosponding domain.
> 
> Signed-off-by: Aviv Ben-David <address@hidden>
> ---
>  hw/i386/intel_iommu.c          | 94 ++++++++++++++++++++++++++++++++++++++----
>  hw/i386/intel_iommu_internal.h |  2 +
>  include/hw/i386/intel_iommu.h  |  9 ++++
>  3 files changed, 98 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 05973b9..d872969 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -679,7 +679,7 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
> gpa,
>          }
>          *reads = (*reads) && (slpte & VTD_SL_R);
>          *writes = (*writes) && (slpte & VTD_SL_W);
> -        if (!(slpte & access_right_check)) {
> +        if (!(slpte & access_right_check) && !(flags & IOMMU_NO_FAIL)) {
>              VTD_DPRINTF(GENERAL, "error: lack of %s permission for "
>                          "gpa 0x%"PRIx64 " slpte 0x%"PRIx64,
>                          (flags & IOMMU_WO ? "write" : "read"), gpa, slpte);
> @@ -978,6 +978,23 @@ static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState 
> *s, uint8_t bus_num)
>      return vtd_bus;
>  }
>  
> +static int vtd_get_did_dev(IntelIOMMUState *s, uint8_t bus_num, uint8_t devfn,
> +                           uint16_t *domain_id)
> +{
> +    VTDContextEntry ce;
> +    int ret_fr;
> +
> +    assert(domain_id);
> +
> +    ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, &ce);
> +    if (ret_fr) {
> +        return -1;
> +    }
> +
> +    *domain_id =  VTD_CONTEXT_ENTRY_DID(ce.hi);
> +    return 0;
> +}
> +
>  /* Do a context-cache device-selective invalidation.
>   * @func_mask: FM field after shifting
>   */
> @@ -1064,6 +1081,45 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState 
> *s, uint16_t domain_id)
>                                  &domain_id);
>  }
>  
> +static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
> +                                           uint16_t domain_id, hwaddr addr,
> +                                           uint8_t am)
> +{
> +    IntelIOMMUNotifierNode *node;
> +
> +    QLIST_FOREACH(node, &(s->notifiers_list), next) {
Aviv,

Regards to the s->notifiers_list, I didn't see the init op to it. Does it happen
in another patch? If so, it may be better to move it in this patch since this 
patch introduces both the definition and usage of notifiers_list.

If it is already clarified, then ignore it.

Thanks,
Yi L
> +        VTDAddressSpace *vtd_as = node->vtd_as;
> +        uint16_t vfio_domain_id;
> +        int ret = vtd_get_did_dev(s, pci_bus_num(vtd_as->bus), vtd_as->devfn,
> +                                  &vfio_domain_id);
> +
> +        if (!ret && domain_id == vfio_domain_id) {
> +            hwaddr original_addr = addr;
> +
> +            while (addr < original_addr + (1 << am) * VTD_PAGE_SIZE) {
> +                IOMMUTLBEntry entry = s->iommu_ops.translate(
> +                                                         &node->vtd_as->iommu,
> +                                                         addr,
> +                                                         IOMMU_NO_FAIL);
> +
> +                if (entry.perm == IOMMU_NONE &&
> +                        node->notifier_flag & IOMMU_NOTIFIER_UNMAP) {
> +                    entry.target_as = &address_space_memory;
> +                    entry.iova = addr & VTD_PAGE_MASK_4K;
> +                    entry.translated_addr = 0;
> +                    entry.addr_mask = ~VTD_PAGE_MASK(VTD_PAGE_SHIFT);
> +                    memory_region_notify_iommu(&node->vtd_as->iommu, entry);
> +                    addr += VTD_PAGE_SIZE;
> +                } else if (node->notifier_flag & IOMMU_NOTIFIER_MAP) {
> +                        memory_region_notify_iommu(&node->vtd_as->iommu, 
> entry);
> +                        addr += entry.addr_mask + 1;
> +                }
> +            }
> +        }
> +    }
> +}
> +
>  static void vtd_iotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>                                        hwaddr addr, uint8_t am)
>  {
> @@ -1074,6 +1130,8 @@ static void vtd_iotlb_page_invalidate(IntelIOMMUState *s, 
> uint16_t domain_id,
>      info.addr = addr;
>      info.mask = ~((1 << am) - 1);
>      g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page, &info);
> +
> +    vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am);
>  }
>  
>  /* Flush IOTLB
> @@ -1999,15 +2057,37 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion 
> *iommu,
>                                            IOMMUNotifierFlag new)
>  {
>      VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
> +    IntelIOMMUState *s = vtd_as->iommu_state;
> +    IntelIOMMUNotifierNode *node = NULL;
> +    IntelIOMMUNotifierNode *next_node = NULL;
>  
> -    if (new & IOMMU_NOTIFIER_MAP) {
> -        error_report("Device at bus %s addr %02x.%d requires iommu "
> -                     "notifier which is currently not supported by "
> -                     "intel-iommu emulation",
> -                     vtd_as->bus->qbus.name, PCI_SLOT(vtd_as->devfn),
> -                     PCI_FUNC(vtd_as->devfn));
> +    if (!s->cache_mode_enabled && new & IOMMU_NOTIFIER_MAP) {
> +        error_report("We need to set cache_mode=1 for intel-iommu to enable "
> +                     "device assignment with IOMMU protection.");
>          exit(1);
>      }
> +
> +    /* Add new ndoe if no mapping was exising before this call */
> +    if (old == IOMMU_NOTIFIER_NONE) {
> +        node = g_malloc0(sizeof(*node));
> +        node->vtd_as = vtd_as;
> +        node->notifier_flag = new;
> +        QLIST_INSERT_HEAD(&s->notifiers_list, node, next);
> +        return;
> +    }
> +
> +    /* update notifier node with new flags */
> +    QLIST_FOREACH_SAFE(node, &s->notifiers_list, next, next_node) {
> +        if (node->vtd_as == vtd_as) {
> +            if (new == IOMMU_NOTIFIER_NONE) {
> +                QLIST_REMOVE(node, next);
> +                g_free(node);
> +            } else {
> +                node->notifier_flag = new;
> +            }
> +            return;
> +        }
> +    }
>  }
>  
>  static const VMStateDescription vtd_vmstate = {
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 00cbe0d..c1b9cfc 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -381,6 +381,8 @@ typedef struct VTDIOTLBPageInvInfo VTDIOTLBPageInvInfo;
>  #define VTD_PAGE_SHIFT_1G           30
>  #define VTD_PAGE_MASK_1G            (~((1ULL << VTD_PAGE_SHIFT_1G) - 1))
>  
> +#define VTD_PAGE_MASK(shift)        (~((1ULL << (shift)) - 1))
> +
>  struct VTDRootEntry {
>      uint64_t val;
>      uint64_t rsvd;
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 749eef9..1a355c1 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -63,6 +63,7 @@ typedef union VTD_IR_TableEntry VTD_IR_TableEntry;
>  typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress;
>  typedef struct VTDIrq VTDIrq;
>  typedef struct VTD_MSIMessage VTD_MSIMessage;
> +typedef struct IntelIOMMUNotifierNode IntelIOMMUNotifierNode;
>  
>  /* Context-Entry */
>  struct VTDContextEntry {
> @@ -247,6 +248,12 @@ struct VTD_MSIMessage {
>  /* When IR is enabled, all MSI/MSI-X data bits should be zero */
>  #define VTD_IR_MSI_DATA          (0)
>  
> +struct IntelIOMMUNotifierNode {
> +    VTDAddressSpace *vtd_as;
> +    IOMMUNotifierFlag notifier_flag;
> +    QLIST_ENTRY(IntelIOMMUNotifierNode) next;
> +};
> +
>  /* The iommu (DMAR) device state struct */
>  struct IntelIOMMUState {
>      X86IOMMUState x86_iommu;
> @@ -284,6 +291,8 @@ struct IntelIOMMUState {
>      MemoryRegionIOMMUOps iommu_ops;
>      GHashTable *vtd_as_by_busptr;   /* VTDBus objects indexed by PCIBus* 
> reference */
>      VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed by 
> bus number */
> +    /* list of registered notifiers */
> +    QLIST_HEAD(, IntelIOMMUNotifierNode) notifiers_list;
>  
>      /* interrupt remapping */
>      bool intr_enabled;              /* Whether guest enabled IR */
> -- 
> 1.9.1
>

^ permalink raw reply	[flat|nested] 45+ messages in thread
end of thread, other threads:[~2016-12-19 13:27 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-28 15:51 [Qemu-devel] [PATCH v7 0/5] IOMMU: intel_iommu support map and unmap notifications Aviv B.D
2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 1/5] IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to guest Aviv B.D
2016-12-01  4:25   ` Tian, Kevin
2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 2/5] IOMMU: change iommu_op->translate's is_write to flags, add support to NO_FAIL flag mode Aviv B.D
2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 3/5] IOMMU: enable intel_iommu map and unmap notifiers Aviv B.D
2016-11-29  3:23   ` 蓝天宇
2016-11-29  7:57     ` Aviv B.D.
2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 4/5] IOMMU: add specific replay function with default implemenation Aviv B.D
2016-11-28 15:51 ` [Qemu-devel] [PATCH v7 5/5] IOMMU: add specific null implementation of iommu_replay to intel_iommu Aviv B.D
2016-11-28 16:36   ` Alex Williamson
2016-11-28 18:57     ` Aviv B.D.
2016-11-30  9:23 ` [Qemu-devel] [PATCH v7 0/5] IOMMU: intel_iommu support map and unmap notifications Peter Xu
2016-12-01  4:21   ` Tian, Kevin
2016-12-01  8:13     ` Lan Tianyu
2016-12-02  5:59     ` Peter Xu
2016-12-02  6:23       ` Tian, Kevin
2016-12-02  6:58         ` Peter Xu
2016-12-02 17:26       ` Alex Williamson
2016-12-01  8:27   ` Lan Tianyu
2016-12-02  6:08     ` Peter Xu
2016-12-02 17:30       ` Alex Williamson
2016-12-06  2:03         ` Lan, Tianyu
2016-12-06  2:18         ` Peter Xu
2016-12-01 15:42   ` Alex Williamson
2016-12-02  6:17     ` Peter Xu
2016-12-01  3:26 ` Tian, Kevin
2016-12-01  6:44 ` Lan Tianyu
2016-12-02  6:52   ` Peter Xu
2016-12-06  6:30     ` Lan Tianyu
2016-12-06  6:51       ` Peter Xu
2016-12-06  7:06         ` Lan Tianyu
2016-12-06  7:22           ` Peter Xu
2016-12-06  8:27             ` Lan Tianyu
2016-12-06 10:59               ` Peter Xu
2016-12-06 16:58                 ` Alex Williamson
2016-12-07  6:09                 ` Lan Tianyu
2016-12-07  6:43                   ` Peter Xu
2016-12-07 14:04                     ` Lan Tianyu
2016-12-08  2:39                       ` Peter Xu
2016-12-08  5:41                         ` Lan Tianyu
  -- strict thread matches above, loose matches on Subject: below --
2016-12-16  9:12 [Qemu-devel] [PATCH v7 3/5] IOMMU: enable intel_iommu map and unmap notifiers Liu, Yi L
2016-12-19 10:00 ` Peter Xu
2016-12-19 11:53   ` Liu, Yi L
2016-12-19 13:26     ` Peter Xu
2016-12-19 11:54 ` Liu, Yi L

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).