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

Re: [Xen-devel] [PATCH V5 7/8] iommu/arm: Introduce iommu_add_dt_device API




On 24.09.19 18:57, Julien Grall wrote:
Hi,

Hi Julien



On 9/24/19 4:30 PM, Oleksandr Tyshchenko wrote:
@@ -1263,15 +1264,22 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev,
      dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n",
                 dt_node_full_name(dev), need_mapping, nirq, naddr);
  -    if ( dt_device_is_protected(dev) && need_mapping )
+    if ( need_mapping )
      {
-        dt_dprintk("%s setup iommu\n", dt_node_full_name(dev));
-        res = iommu_assign_dt_device(d, dev);
-        if ( res )
-        {
-            printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n",
+        dt_dprintk("Check if %s is behind the IOMMU and add it\n",
                     dt_node_full_name(dev));
-            return res;
+
+        iommu_add_dt_device(dev);

Return value should always be checked or explain why this is not done.

Yes, I will add a check. The positive result for us is non-negative (either "device is protected" or "device doesn't need to be protected").




[...]

  int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
                         XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
  {
@@ -177,6 +241,13 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
              break;
          }
  +        iommu_add_dt_device(dev);

Same here.

Yes, I think, we don't need to check for return value, because the only one positive result "here" is the fact that "device is protected" (which is checked below).

What is more, if we add a check for the return value to be strictly 0, we will get an error after guest's reboot (since iommu_add_dt_device() will return -EEXIST).

So, I will add a comment explaining why we don't check. What do you think?



+        if ( !dt_device_is_protected(dev) )
+        {
+            ret = -EINVAL;
+            break;
+        }

This is already checked in iommu_assign_dt_device().

Will drop here.


--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.