H
On 6/21/2018 7:16 PM, Eric Auger Wrote:
> On TLB invalidation commands, let's call registered
> IOMMU notifiers. Those can only be UNMAP notifiers.
> SMMUv3 does not support notification on MAP (VFIO).
>
> This patch allows vhost use case where IOTLB API is notified
> on each guest IOTLB invalidation.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>
> ---
> v2 -> v3:
> - added Peter's R-b
> ---
> hw/arm/smmu-common.c | 34 +++++++++++++++
> hw/arm/smmuv3.c | 99
> +++++++++++++++++++++++++++++++++++++++++++-
> hw/arm/trace-events | 5 +++
> include/hw/arm/smmu-common.h | 6 +++
> 4 files changed, 142 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index f66e444..3098915 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -385,6 +385,40 @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1,
> gconstpointer v2)
> return (k1->asid == k2->asid) && (k1->iova == k2->iova);
> }
>
> +/* Unmap the whole notifier's range */
> +static void smmu_unmap_notifier_range(IOMMUNotifier *n)
> +{
> + IOMMUTLBEntry entry;
> +
> + entry.target_as = &address_space_memory;
> + entry.iova = n->start;
> + entry.perm = IOMMU_NONE;
> + entry.addr_mask = n->end - n->start;
> +
> + memory_region_notify_one(n, &entry);
> +}
> +
> +/* Unmap all notifiers attached to @mr */
> +inline void smmu_inv_notifiers_mr(IOMMUMemoryRegion *mr)
> +{
> + IOMMUNotifier *n;
> +
> + trace_smmu_inv_notifiers_mr(mr->parent_obj.name);
> + IOMMU_NOTIFIER_FOREACH(n, mr) {
> + smmu_unmap_notifier_range(n);
> + }
> +}
> +
> +/* Unmap all notifiers of all mr's */
> +void smmu_inv_notifiers_all(SMMUState *s)
> +{
> + SMMUNotifierNode *node;
> +
> + QLIST_FOREACH(node, &s->notifiers_list, next) {
> + smmu_inv_notifiers_mr(&node->sdev->iommu);
> + }
> +}
> +
> static void smmu_base_realize(DeviceState *dev, Error **errp)
> {
> SMMUState *s = ARM_SMMU(dev);
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 853975a..c58e596 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -780,6 +780,68 @@ epilogue:
> return entry;
> }
>
> +/**
> + * smmuv3_notify_iova - call the notifier @n for a given
> + * @asid and @iova tuple.
> + *
> + * @mr: IOMMU mr region handle
> + * @n: notifier to be called
> + * @asid: address space ID or negative value if we don't care
> + * @iova: iova
> + */
> +static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
> + IOMMUNotifier *n,
> + int asid,
> + dma_addr_t iova)
> +{
> + SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
> + SMMUEventInfo event = {};
> + SMMUTransTableInfo *tt;
> + SMMUTransCfg *cfg;
> + IOMMUTLBEntry entry;
> +
> + cfg = smmuv3_get_config(sdev, &event);
> + if (!cfg) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s error decoding the configuration for iommu
> mr=%s\n",
> + __func__, mr->parent_obj.name);
> + return;
> + }
> +
> + if (asid >= 0 && cfg->asid != asid) {
> + return;
> + }
> +
> + tt = select_tt(cfg, iova);
> + if (!tt) {
> + return;
> + }
> +
> + entry.target_as = &address_space_memory;
> + entry.iova = iova;
> + entry.addr_mask = (1 << tt->granule_sz) - 1;
> + entry.perm = IOMMU_NONE;
> +
> + memory_region_notify_one(n, &entry);
> +}
> +
> +/* invalidate an asid/iova tuple in all mr's */
> +static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t
> iova)
> +{
> + SMMUNotifierNode *node;
> +
> + QLIST_FOREACH(node, &s->notifiers_list, next) {
> + IOMMUMemoryRegion *mr = &node->sdev->iommu;
> + IOMMUNotifier *n;
> +
> + trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, iova);
> +
> + IOMMU_NOTIFIER_FOREACH(n, mr) {
> + smmuv3_notify_iova(mr, n, asid, iova);
> + }
> + }
> +}
> +
> static int smmuv3_cmdq_consume(SMMUv3State *s)
> {
> SMMUState *bs = ARM_SMMU(s);
> @@ -899,12 +961,14 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> uint16_t asid = CMD_ASID(&cmd);
>
> trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> + smmu_inv_notifiers_all(&s->smmu_state);
> smmu_iotlb_inv_asid(bs, asid);
> break;
> }
> case SMMU_CMD_TLBI_NH_ALL:
> case SMMU_CMD_TLBI_NSNH_ALL:
> trace_smmuv3_cmdq_tlbi_nh();
> + smmu_inv_notifiers_all(&s->smmu_state);
> smmu_iotlb_inv_all(bs);
> break;
> case SMMU_CMD_TLBI_NH_VAA:
> @@ -913,6 +977,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> uint16_t vmid = CMD_VMID(&cmd);
>
> trace_smmuv3_cmdq_tlbi_nh_vaa(vmid, addr);
> + smmuv3_inv_notifiers_iova(bs, -1, addr);
> smmu_iotlb_inv_all(bs);
> break;
> }
> @@ -924,6 +989,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> bool leaf = CMD_LEAF(&cmd);
>
> trace_smmuv3_cmdq_tlbi_nh_va(vmid, asid, addr, leaf);
> + smmuv3_inv_notifiers_iova(bs, asid, addr);
> smmu_iotlb_inv_iova(bs, asid, addr);
> break;
> }
> @@ -1402,9 +1468,38 @@ static void
> smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> IOMMUNotifierFlag old,
> IOMMUNotifierFlag new)
> {
> + SMMUDevice *sdev = container_of(iommu, SMMUDevice, iommu);
> + SMMUv3State *s3 = sdev->smmu;
> + SMMUState *s = &(s3->smmu_state);
> + SMMUNotifierNode *node = NULL;
> + SMMUNotifierNode *next_node = NULL;
> +
> + if (new == IOMMU_NOTIFIER_MAP) {
> + int bus_num = pci_bus_num(sdev->bus);
> + PCIDevice *pcidev = pci_find_device(sdev->bus, bus_num, sdev->devfn);
> +
> + warn_report("SMMUv3 does not support notification on MAP: "
> + "device %s will not function properly", pcidev->name);
> + }
> +
ah, I know why there is no such warning in my testing server.
the IOMMUNotifierFlag is initialized with IOMMU_NOTIFIER_ALL=(IOMMU_NOTIFIER_MAP
| IOMMU_NOTIFIER_UNMAP). Should this condition be considerred?
Cheers,
Jia
> if (old == IOMMU_NOTIFIER_NONE) {
> - warn_report("SMMUV3 does not support vhost/vfio integration yet: "
> - "devices of those types will not function properly");
> + trace_smmuv3_notify_flag_add(iommu->parent_obj.name);
> + node = g_malloc0(sizeof(*node));
> + node->sdev = sdev;
> + 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->sdev == sdev) {
> + if (new == IOMMU_NOTIFIER_NONE) {
> + trace_smmuv3_notify_flag_del(iommu->parent_obj.name);
> + QLIST_REMOVE(node, next);
> + g_free(node);
> + }
> + return;
> + }
> }
> }
>
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index be69c5d..27b11d6 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -17,6 +17,7 @@ smmu_iotlb_cache_miss(uint16_t asid, uint64_t addr,
> uint32_t hit, uint32_t miss,
> smmu_iotlb_inv_all(void) "IOTLB invalidate all"
> smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
> smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d
> addr=0x%"PRIx64
> +smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
>
> #hw/arm/smmuv3.c
> smmuv3_read_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r)
> "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)"
> @@ -55,3 +56,7 @@ smmuv3_cmdq_tlbi_nh_vaa(int vmid, uint64_t addr) "vmid =%d
> addr=0x%"PRIx64
> smmuv3_cmdq_tlbi_nh(void) ""
> smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d"
> smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid %d"
> +smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu
> mr=%s"
> +smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu
> mr=%s"
> +smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova)
> "iommu mr=%s asid=%d iova=0x%"PRIx64
> +
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index d173806..50e2912 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -160,4 +160,10 @@ void smmu_iotlb_inv_all(SMMUState *s);
> void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);
> void smmu_iotlb_inv_iova(SMMUState *s, uint16_t asid, dma_addr_t iova);
>
> +/* Unmap the range of all the notifiers registered to any IOMMU mr */
> +void smmu_inv_notifiers_all(SMMUState *s);
> +
> +/* Unmap the range of all the notifiers registered to @mr */
> +void smmu_inv_notifiers_mr(IOMMUMemoryRegion *mr);
> +
> #endif /* HW_ARM_SMMU_COMMON */
>
