Re: [PATCH 10/11] xen/arm: device assignment on 1:1 direct-map domain

[Julien Grall]

On 09/10/2021 10:40, Penny Zheng wrote:
Hi Julien

Hi Penny,


-----Original Message-----
From: Julien Grall <jul...@xen.org>
Sent: Thursday, September 23, 2021 7:27 PM
To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen
<wei.c...@arm.com>
Subject: Re: [PATCH 10/11] xen/arm: device assignment on 1:1 direct-map
domain

Hi,

On 23/09/2021 08:11, Penny Zheng wrote:
User could do device passthrough, with
"xen,force-assign-without-iommu" in the device tree snippet, on
trusted guest through 1:1 direct-map, if IOMMU absent or disabled on
hardware.

At the moment, it would be possible to passthrough a non-DMA capable
device with direct-mapping. After this patch, this is going to be forbidden.


In order to achieve that, this patch adds 1:1 direct-map check and
disables iommu-related action.

Signed-off-by: Penny Zheng <penny.zh...@arm.com>
---
   xen/arch/arm/domain_build.c | 12 ++++++++----
   1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c92e510ae7..9a9d2522b7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2070,14 +2070,18 @@ static int __init
handle_passthrough_prop(struct kernel_info *kinfo,
       if ( res < 0 )
           return res;

+    /*
+     * If xen_force, we allow assignment of devices without IOMMU
protection.
+     * And if IOMMU is disabled or absent, 1:1 direct-map is necessary > +
*/
+    if ( xen_force && is_domain_direct_mapped(kinfo->d) &&
+         !dt_device_is_protected(node) )

dt_device_is_protected() will be always false unless the device is protected
behing an SMMU using the legacy binding. So I don't think this is correct to
move this check ahead. In fact..

+        return 0;
+
       res = iommu_add_dt_device(node);

... the call should already be a NOP when the IOMMU is disabled or the
device is not behind an IOMMU. So can you explain what you are trying to
prevent here?


If the IOMMU is disabled, iommu_add_dt_device will return 1 as errno.
So we could not make it to the xen_force check...

I disagree. The check is:

if ( res < 0 )
  return res;

Given that res is 1, we wouldn't return and move to check whether the 
assignment can be done.


So I tried to move all IOMMU action behind xen_force check.

Now, device assignment without IOMMU protection is only
applicable on direct-map domains,

It is fine to assign a non-DMA capable device without direct-mapping. So 
why do you want to add this restriction?

Cheers,

--
Julien Grall