[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH v3 4/4] hw/arm/smmuv3: Add notificati
From: |
Auger Eric |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH v3 4/4] hw/arm/smmuv3: Add notifications on invalidation |
Date: |
Fri, 22 Jun 2018 09:24:25 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
Hello Jia,
On 06/22/2018 09:15 AM, Jia He wrote:
> 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 <address@hidden>
>> Reviewed-by: Peter Maydell <address@hidden>
>>
>> ---
>> 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?
Hum yes it is a bug. I will fix this asap!
Thanks
Eric
>
> 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 */
>>
>