Re: [Xen-devel] [PATCH 2/2] xen/arm: domain_build: Don't expose IOMMU specific properties to the guest

[Oleksandr <olekstysh@xxxxxxxxx>]

On 01.10.19 22:07, Julien Grall wrote:
> Hi Oleksandr,
Hi Julien


> On 10/1/19 5:07 PM, Oleksandr wrote:
> > On 01.10.19 18:36, Julien Grall wrote:
> > > On 01/10/2019 16:25, Oleksandr wrote:
> > > > On 01.10.19 18:04, Julien Grall wrote:
> > > 1. Giving the IOMMU to Dom0 is a bad idea.
> Please to try expand your thoughts in the same e-mail when you say 
> "this is a bad idea".
Well, this was a conclusion I had got from the discussion [1]. Sorry for 
not being clear here.

> But, this is clearly what happen in current Xen setup if the driver is 
> not enabled. What I want to avoid is exposing an half complete 
> bindings to the guest (you don't know how it will behave).
> So we either remove all the properties and node related to the IOMMUs 
> or nothing.
I think, I got it. Our target is *not* to add a way for Dom0 to use 
IOMMU, but to be consistent in removing IOMMU node/master device 
properties. Now, we remove the IOMMU node if Xen identifies it (the 
IOMMU driver is present in Xen),
so looks like we have to remove master device properties only if this 
master device is behind the IOMMU which node is removed. This, I hope, 
will avoid exposing an half complete bindings to guest. Am I right?

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00858.html

----------

If you happy with that logic I will craft a proper patch.


diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 67021d9..6d45e55 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -480,10 +480,26 @@ static int __init write_properties(struct domain 
*d, struct kernel_info *kinfo,
     const struct dt_property *prop, *status = NULL;
     int res = 0;
     int had_dom0_bootargs = 0;
+    struct dt_device_node *iommu_node;

     if ( kinfo->cmdline && kinfo->cmdline[0] )
         bootargs = &kinfo->cmdline[0];

+    /*
+     * If we skip the IOMMU device when creating DT for Dom0 (even if
+     * the IOMMU device is not used by Xen), we should also skip the IOMMU
+     * specific properties of the master device behind it in order to avoid
+     * exposing an half complete IOMMU bindings to Dom0.
+     * Use "iommu_node" as an indicator of the master device which 
properties
+     * should be skipped.
+     */
+    iommu_node = dt_parse_phandle(node, "iommus", 0);
+    if ( iommu_node )
+    {
+        if ( device_get_class(iommu_node) != DEVICE_IOMMU )
+            iommu_node = NULL;
+    }
+
     dt_for_each_property_node (node, prop)
     {
         const void *prop_data = prop->value;
@@ -540,6 +556,19 @@ static int __init write_properties(struct domain 
*d, struct kernel_info *kinfo,
             continue;
         }

+        if ( iommu_node )
+        {
+            /* Don't expose IOMMU specific properties to Dom0 */
+            if ( dt_property_name_is_equal(prop, "iommus") )
+                continue;
+
+            if ( dt_property_name_is_equal(prop, "iommu-map") )
+                continue;
+
+            if ( dt_property_name_is_equal(prop, "iommu-map-mask") )
+                continue;
+        }
+
         res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);

         if ( res )


--
Regards,

Oleksandr Tyshchenko


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