[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb desc

From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor
Date: Tue, 8 Nov 2016 14:54:19 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 2016年11月08日 07:35, Peter Xu wrote:
On Mon, Nov 07, 2016 at 03:09:50PM +0800, Jason Wang wrote:


+static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
+                                          VTDInvDesc *inv_desc)
+    VTDAddressSpace *vtd_dev_as;
+    IOMMUTLBEntry entry;
Since "entry" is allocated on the stack...


+    entry.target_as = &vtd_dev_as->as;
+    entry.addr_mask = sz - 1;
+    entry.iova = addr;
+    memory_region_notify_iommu(entry.target_as->root, entry);
... here we need to assign entry.perm explicitly to IOMMU_NONE, right?

Also I think it'll be nice that we set all the fields even not used,
to avoid rubbish from the stack passed down to notifier handlers.


This is better, if no other comments on the series I will post a patch on top to fix this.

+static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp)
+    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
+    return s->dt_supported;
+static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error 
+    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
+    s->dt_supported = value;
  static void x86_iommu_instance_init(Object *o)
      X86IOMMUState *s = X86_IOMMU_DEVICE(o);
@@ -114,6 +126,11 @@ static void x86_iommu_instance_init(Object *o)
      s->intr_supported = false;
      object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get,
                               x86_iommu_intremap_prop_set, NULL);
+    s->dt_supported = false;
+    object_property_add_bool(o, "device-iotlb",
+                             x86_iommu_device_iotlb_prop_get,
+                             x86_iommu_device_iotlb_prop_set,
+                             NULL);
Again, a nit-pick here is to use Property for "device-iotlb":

     static Property vtd_properties[] = {
         DEFINE_PROP_UINT32("device-iotlb", X86IOMMUState, dt_supported, false),

However not worth a repost.


-- peterx

We may want to share this with AMD IOMMU. (Looking at AMD IOMMU codes, its device-iotlb support is buggy).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]