[PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

[PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

[Tiejun Chen]

Currently we don't allow passing through any group devices which are
sharing same RMRR entry since it would break security among VMs. And
indeed, we expect we can figure out a better way to handle this kind
of case completely.

But before the group assignment gets implemented, we might make this
permission dependent on our RMRR policy. So, now it would be allowed
in the relaxed mode.

CC: Yang Zhang <yang.z.zhang@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 xen/drivers/passthrough/vtd/iommu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..4249cfa 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2310,12 +2310,16 @@ static int intel_iommu_assign_device(
              PCI_DEVFN2(bdf) == devfn &&
              rmrr->scope.devices_cnt > 1 )
         {
+            u32 relaxed = flag & XEN_DOMCTL_DEV_RDM_RELAXED;
+
             printk(XENLOG_G_ERR VTDPREFIX
-                   " cannot assign %04x:%02x:%02x.%u"
+                   " Currently its %s to assign %04x:%02x:%02x.%u"
                    " with shared RMRR at %"PRIx64" for Dom%d.\n",
+                   relaxed ? "risky" : "disallowed",
                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                    rmrr->base_address, d->domain_id);
-            return -EPERM;
+            if ( !relaxed )
+                return -EPERM;
         }
     }
 
-- 
1.9.1

Re: [PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

[Wei Liu]

On Tue, Sep 08, 2015 at 08:57:14AM +0800, Tiejun Chen wrote:
> Currently we don't allow passing through any group devices which are
> sharing same RMRR entry since it would break security among VMs. And
> indeed, we expect we can figure out a better way to handle this kind
> of case completely.
> 
> But before the group assignment gets implemented, we might make this
> permission dependent on our RMRR policy. So, now it would be allowed
> in the relaxed mode.
> 

Is this targeting 4.6? I think so.

> CC: Yang Zhang <yang.z.zhang@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> CC: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 836aed5..4249cfa 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2310,12 +2310,16 @@ static int intel_iommu_assign_device(


FWIW the comment before this hunk also needs to be updated.

>               PCI_DEVFN2(bdf) == devfn &&
>               rmrr->scope.devices_cnt > 1 )
>          {
> +            u32 relaxed = flag & XEN_DOMCTL_DEV_RDM_RELAXED;
> +
>              printk(XENLOG_G_ERR VTDPREFIX
> -                   " cannot assign %04x:%02x:%02x.%u"
> +                   " Currently its %s to assign %04x:%02x:%02x.%u"

"it's" or "it is"

>                     " with shared RMRR at %"PRIx64" for Dom%d.\n",
> +                   relaxed ? "risky" : "disallowed",
>                     seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
>                     rmrr->base_address, d->domain_id);
> -            return -EPERM;
> +            if ( !relaxed )
> +                return -EPERM;
>          }
>      }
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

[Jan Beulich]

>>> On 08.09.15 at 02:57, <tiejun.chen@intel.com> wrote:
> Currently we don't allow passing through any group devices which are
> sharing same RMRR entry since it would break security among VMs. And
> indeed, we expect we can figure out a better way to handle this kind
> of case completely.
> 
> But before the group assignment gets implemented, we might make this
> permission dependent on our RMRR policy. So, now it would be allowed
> in the relaxed mode.
> 
> CC: Yang Zhang <yang.z.zhang@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> CC: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
> b/xen/drivers/passthrough/vtd/iommu.c
> index 836aed5..4249cfa 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2310,12 +2310,16 @@ static int intel_iommu_assign_device(
>               PCI_DEVFN2(bdf) == devfn &&
>               rmrr->scope.devices_cnt > 1 )
>          {
> +            u32 relaxed = flag & XEN_DOMCTL_DEV_RDM_RELAXED;

Conventionally this ought to be a bool_t.

>              printk(XENLOG_G_ERR VTDPREFIX
> -                   " cannot assign %04x:%02x:%02x.%u"
> +                   " Currently its %s to assign %04x:%02x:%02x.%u"

In addition to what Wei said - what meaning is "currently" supposed
to have here? To a reader of the resulting log it might look as if
(s)he'd need to reconfigure the hypervisor (via some command line
option perhaps), while acceptance / refusal solely depends on the
arguments the caller provides.

Apart from that in the relaxed case the message level should be
XENLOG_G_WARNING, not XENLOG_G_ERR.

Jan