Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Wei Liu]

On Sun, Sep 06, 2015 at 12:19:49PM +0800, Chen, Tiejun wrote:
[...]
> >>Tamas, do you actually mean to assign these to _different_
> guests, considering the log fragment above?)
> 
> 
> >>>No, I actually want to assign them to the same domain. The domain creation
> >>>fails with either of those devices specified for passthrough whether they
> >>>are to be attached to the same domain or not.
> >>>
> >>>
> >>Tamas, could you try this in your case?
> >>
> >
> >Took me a while to find the xl config option to set this flag (pci = [
> >'sbdf, rdm_policy=strict/relaxed' ]) but now it works as expected!
> >
> 
> I remember 'relaxed' is a default value so 'rdm_policy' can't be dropped
> here if you like this.
> 

For a specific PCI device, the default value is strict.

Wei.

> Thanks
> Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Chen, Tiejun]

Sorry for this delay response because I was on vacation.

On 9/5/2015 5:52 AM, Tamas K Lengyel wrote:

On Fri, Sep 4, 2015 at 2:17 AM, Jan Beulich  wrote:


>>> On 03.09.15 at 21:39,  wrote:
> So this change in 4.6 prevents me from passing through devices that have
> worked previously with VT-d:
>
> (XEN) [VT-D] cannot assign :00:1a.0 with shared RMRR at ae8a9000 for
> Dom30.
> (XEN) [VT-D] cannot assign :00:1d.0 with shared RMRR at ae8a9000 for
> Dom31.
>
> The devices are the USB 2.0 devices on a DQ67SW motherboard:
>
> 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> Family USB Enhanced Host Controller #2 (rev 04)
> 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> Family USB Enhanced Host Controller #1 (rev 04)

Please don't top post. And I'm also puzzled by you sending this to
Ian rather than the author.



Hm, I've just hit reply-all to the latest message I've found in the thread.




Technically - Tiejun, should this perhaps be permitted in relaxed
mode, at least until group assignment gets implemented? (Or


I agree. What about this?

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c

index 836aed5..038776a 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 ? "disallowed" : "risky",
seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
rmrr->base_address, d->domain_id);
-return -EPERM;
+if ( !relaxed )
+return -EPERM;
 }
 }



Tamas, do you actually mean to assign these to _different_
guests, considering the log fragment above?)



No, I actually want to assign them to the same domain. The domain creation
fails with either of those devices specified for passthrough whether they
are to be attached to the same domain or not.



Tamas, could you try this in your case?

Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Tamas K Lengyel]

>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 836aed5..038776a 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 ? "disallowed" : "risky",
>

This debug message is backwards?


> seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> rmrr->base_address, d->domain_id);
> -return -EPERM;
> +if ( !relaxed )
> +return -EPERM;
>  }
>  }
>
>
> Tamas, do you actually mean to assign these to _different_
>>> guests, considering the log fragment above?)
>>>
>>>
>> No, I actually want to assign them to the same domain. The domain creation
>> fails with either of those devices specified for passthrough whether they
>> are to be attached to the same domain or not.
>>
>>
> Tamas, could you try this in your case?
>

Took me a while to find the xl config option to set this flag (pci = [
'sbdf, rdm_policy=strict/relaxed' ]) but now it works as expected!

Thanks,
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Chen, Tiejun]

On 9/6/2015 11:19 AM, Tamas K Lengyel wrote:


diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..038776a 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 ? "disallowed" : "risky",



This debug message is backwards?


Yeah. Its indeed like this, relaxed ? "risky" : "disallowed"

But lets wait Jan's comment to step next.





seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
rmrr->base_address, d->domain_id);
-return -EPERM;
+if ( !relaxed )
+return -EPERM;
 }
 }


Tamas, do you actually mean to assign these to _different_

guests, considering the log fragment above?)



No, I actually want to assign them to the same domain. The domain creation
fails with either of those devices specified for passthrough whether they
are to be attached to the same domain or not.



Tamas, could you try this in your case?



Took me a while to find the xl config option to set this flag (pci = [
'sbdf, rdm_policy=strict/relaxed' ]) but now it works as expected!



I remember 'relaxed' is a default value so 'rdm_policy' can't be dropped 
here if you like this.


Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Tamas K Lengyel]

> Took me a while to find the xl config option to set this flag (pci = [
>> 'sbdf, rdm_policy=strict/relaxed' ]) but now it works as expected!
>>
>>
> I remember 'relaxed' is a default value so 'rdm_policy' can't be dropped
> here if you like this.
>

Without specifically setting rdm_policy=relaxed domain creation failed, so
it's definitely not the default.

Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Tamas K Lengyel]

On Fri, Sep 4, 2015 at 2:17 AM, Jan Beulich  wrote:

> >>> On 03.09.15 at 21:39,  wrote:
> > So this change in 4.6 prevents me from passing through devices that have
> > worked previously with VT-d:
> >
> > (XEN) [VT-D] cannot assign :00:1a.0 with shared RMRR at ae8a9000 for
> > Dom30.
> > (XEN) [VT-D] cannot assign :00:1d.0 with shared RMRR at ae8a9000 for
> > Dom31.
> >
> > The devices are the USB 2.0 devices on a DQ67SW motherboard:
> >
> > 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> > Family USB Enhanced Host Controller #2 (rev 04)
> > 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> > Family USB Enhanced Host Controller #1 (rev 04)
>
> Please don't top post. And I'm also puzzled by you sending this to
> Ian rather than the author.
>

Hm, I've just hit reply-all to the latest message I've found in the thread.


>
> Technically - Tiejun, should this perhaps be permitted in relaxed
> mode, at least until group assignment gets implemented? (Or
> Tamas, do you actually mean to assign these to _different_
> guests, considering the log fragment above?)
>

No, I actually want to assign them to the same domain. The domain creation
fails with either of those devices specified for passthrough whether they
are to be attached to the same domain or not.

Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Jan Beulich]

>>> On 03.09.15 at 21:39,  wrote:
> So this change in 4.6 prevents me from passing through devices that have
> worked previously with VT-d:
> 
> (XEN) [VT-D] cannot assign :00:1a.0 with shared RMRR at ae8a9000 for
> Dom30.
> (XEN) [VT-D] cannot assign :00:1d.0 with shared RMRR at ae8a9000 for
> Dom31.
> 
> The devices are the USB 2.0 devices on a DQ67SW motherboard:
> 
> 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> Family USB Enhanced Host Controller #2 (rev 04)
> 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
> Family USB Enhanced Host Controller #1 (rev 04)

Please don't top post. And I'm also puzzled by you sending this to
Ian rather than the author.

Technically - Tiejun, should this perhaps be permitted in relaxed
mode, at least until group assignment gets implemented? (Or
Tamas, do you actually mean to assign these to _different_
guests, considering the log fragment above?)

Jan

> On Wed, Jul 22, 2015 at 9:44 AM, Ian Jackson 
> wrote:
> 
>> From: Tiejun Chen 
>>
>> Currently we're intending to cover this kind of devices
>> with shared RMRR simply since the case of shared RMRR is
>> a rare case according to our previous experiences. But
>> late we can group these devices which shared rmrr, and
>> then allow all devices within a group to be assigned to
>> same domain.
>>
>> CC: Yang Zhang 
>> CC: Kevin Tian 
>> Signed-off-by: Tiejun Chen 
>> Acked-by: Kevin Tian 
>> ---
>>  xen/drivers/passthrough/vtd/iommu.c |   30 +++---
>>  1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/vtd/iommu.c
>> b/xen/drivers/passthrough/vtd/iommu.c
>> index 8a8d763..ce5c295 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> @@ -2293,13 +2293,37 @@ static int intel_iommu_assign_device(
>>  if ( list_empty(_drhd_units) )
>>  return -ENODEV;
>>
>> +seg = pdev->seg;
>> +bus = pdev->bus;
>> +/*
>> + * In rare cases one given rmrr is shared by multiple devices but
>> + * obviously this would put the security of a system at risk. So
>> + * we should prevent from this sort of device assignment.
>> + *
>> + * TODO: in the future we can introduce group device assignment
>> + * interface to make sure devices sharing RMRR are assigned to the
>> + * same domain together.
>> + */
>> +for_each_rmrr_device( rmrr, bdf, i )
>> +{
>> +if ( rmrr->segment == seg &&
>> + PCI_BUS(bdf) == bus &&
>> + PCI_DEVFN2(bdf) == devfn &&
>> + rmrr->scope.devices_cnt > 1 )
>> +{
>> +printk(XENLOG_G_ERR VTDPREFIX
>> +   " cannot assign %04x:%02x:%02x.%u"
>> +   " with shared RMRR at %"PRIx64" for Dom%d.\n",
>> +   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
>> +   rmrr->base_address, d->domain_id);
>> +return -EPERM;
>> +}
>> +}
>> +
>>  ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
>>  if ( ret )
>>  return ret;
>>
>> -seg = pdev->seg;
>> -bus = pdev->bus;
>> -
>>  /* Setup rmrr identity mapping */
>>  for_each_rmrr_device( rmrr, bdf, i )
>>  {
>> --
>> 1.7.10.4
>>
>>
>> ___
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org 
>> http://lists.xen.org/xen-devel 
>>




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Tamas K Lengyel]

So this change in 4.6 prevents me from passing through devices that have
worked previously with VT-d:

(XEN) [VT-D] cannot assign :00:1a.0 with shared RMRR at ae8a9000 for
Dom30.
(XEN) [VT-D] cannot assign :00:1d.0 with shared RMRR at ae8a9000 for
Dom31.

The devices are the USB 2.0 devices on a DQ67SW motherboard:

00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
Family USB Enhanced Host Controller #2 (rev 04)
00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset
Family USB Enhanced Host Controller #1 (rev 04)

Tamas


On Wed, Jul 22, 2015 at 9:44 AM, Ian Jackson 
wrote:

> From: Tiejun Chen 
>
> Currently we're intending to cover this kind of devices
> with shared RMRR simply since the case of shared RMRR is
> a rare case according to our previous experiences. But
> late we can group these devices which shared rmrr, and
> then allow all devices within a group to be assigned to
> same domain.
>
> CC: Yang Zhang 
> CC: Kevin Tian 
> Signed-off-by: Tiejun Chen 
> Acked-by: Kevin Tian 
> ---
>  xen/drivers/passthrough/vtd/iommu.c |   30 +++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 8a8d763..ce5c295 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2293,13 +2293,37 @@ static int intel_iommu_assign_device(
>  if ( list_empty(_drhd_units) )
>  return -ENODEV;
>
> +seg = pdev->seg;
> +bus = pdev->bus;
> +/*
> + * In rare cases one given rmrr is shared by multiple devices but
> + * obviously this would put the security of a system at risk. So
> + * we should prevent from this sort of device assignment.
> + *
> + * TODO: in the future we can introduce group device assignment
> + * interface to make sure devices sharing RMRR are assigned to the
> + * same domain together.
> + */
> +for_each_rmrr_device( rmrr, bdf, i )
> +{
> +if ( rmrr->segment == seg &&
> + PCI_BUS(bdf) == bus &&
> + PCI_DEVFN2(bdf) == devfn &&
> + rmrr->scope.devices_cnt > 1 )
> +{
> +printk(XENLOG_G_ERR VTDPREFIX
> +   " cannot assign %04x:%02x:%02x.%u"
> +   " with shared RMRR at %"PRIx64" for Dom%d.\n",
> +   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> +   rmrr->base_address, d->domain_id);
> +return -EPERM;
> +}
> +}
> +
>  ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
>  if ( ret )
>  return ret;
>
> -seg = pdev->seg;
> -bus = pdev->bus;
> -
>  /* Setup rmrr identity mapping */
>  for_each_rmrr_device( rmrr, bdf, i )
>  {
> --
> 1.7.10.4
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

[Ian Jackson]

From: Tiejun Chen tiejun.c...@intel.com

Currently we're intending to cover this kind of devices
with shared RMRR simply since the case of shared RMRR is
a rare case according to our previous experiences. But
late we can group these devices which shared rmrr, and
then allow all devices within a group to be assigned to
same domain.

CC: Yang Zhang yang.z.zh...@intel.com
CC: Kevin Tian kevin.t...@intel.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Acked-by: Kevin Tian kevin.t...@intel.com
---
 xen/drivers/passthrough/vtd/iommu.c |   30 +++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 8a8d763..ce5c295 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2293,13 +2293,37 @@ static int intel_iommu_assign_device(
 if ( list_empty(acpi_drhd_units) )
 return -ENODEV;
 
+seg = pdev-seg;
+bus = pdev-bus;
+/*
+ * In rare cases one given rmrr is shared by multiple devices but
+ * obviously this would put the security of a system at risk. So
+ * we should prevent from this sort of device assignment.
+ *
+ * TODO: in the future we can introduce group device assignment
+ * interface to make sure devices sharing RMRR are assigned to the
+ * same domain together.
+ */
+for_each_rmrr_device( rmrr, bdf, i )
+{
+if ( rmrr-segment == seg 
+ PCI_BUS(bdf) == bus 
+ PCI_DEVFN2(bdf) == devfn 
+ rmrr-scope.devices_cnt  1 )
+{
+printk(XENLOG_G_ERR VTDPREFIX
+cannot assign %04x:%02x:%02x.%u
+with shared RMRR at %PRIx64 for Dom%d.\n,
+   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+   rmrr-base_address, d-domain_id);
+return -EPERM;
+}
+}
+
 ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
 if ( ret )
 return ret;
 
-seg = pdev-seg;
-bus = pdev-bus;
-
 /* Setup rmrr identity mapping */
 for_each_rmrr_device( rmrr, bdf, i )
 {
-- 
1.7.10.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel