Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-23 Thread Auger Eric 
Hi Geetha,

On 23/12/2016 14:33, Geetha Akula wrote:
> Hi Eric,
> 
> Seeing same issue reported by Diana on ThunderX with you
> v4.9-reserved-v4 branch.
> Vfio passthough work fine when allow_unsafe_interrupts is set.
Thank you for testing! I will fix the security assessment by better
studying flag propagation in domain hierarchy.

Best Regards

Eric

> 
> 
> Thank you,
> Geetha.
> 
> On Thu, Dec 22, 2016 at 6:32 PM, Auger Eric  > wrote:
> 
> Hi Diana,
> 
> On 22/12/2016 13:41, Diana Madalina Craciun wrote:
> > Hi Eric,
> >
> > On 12/13/2016 10:32 PM, Eric Auger wrote:
> >> In case the IOMMU does not bypass MSI transactions (typical
> >> case on ARM), we check all MSI controllers are IRQ remapping
> >> capable. If not the IRQ assignment may be unsafe.
> >>
> >> At this stage the arm-smmu-(v3) still advertise the
> >> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> >> removed in subsequent patches.
> >>
> >> Signed-off-by: Eric Auger  >
> >> ---
> >>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> >> index d07fe73..a05648b 100644
> >> --- a/drivers/vfio/vfio_iommu_type1.c
> >> +++ b/drivers/vfio/vfio_iommu_type1.c
> >> @@ -37,6 +37,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #define DRIVER_VERSION  "0.2"
> >>  #define DRIVER_AUTHOR   "Alex Williamson
> >"
> >> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  struct vfio_domain *domain, *d;
> >>  struct bus_type *bus = NULL;
> >>  int ret;
> >> -bool resv_msi;
> >> +bool resv_msi, msi_remap;
> >>  phys_addr_t resv_msi_base;
> >>
> >>  mutex_lock(>lock);
> >> @@ -818,8 +819,10 @@ static int
> vfio_iommu_type1_attach_group(void *iommu_data,
> >>  INIT_LIST_HEAD(>group_list);
> >>  list_add(>next, >group_list);
> >>
> >> -if (!allow_unsafe_interrupts &&
> >> -!iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> >> +msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> >> +   iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> >> +
> >> +if (!allow_unsafe_interrupts && !msi_remap) {
> >>  pr_warn("%s: No interrupt remapping support.  Use
> the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU
> support on this platform\n",
> >> __func__);
> >>  ret = -EPERM;
> >
> > I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
> > LS2080), so I did not set allow_unsafe_interrupts. It fails here
> > complaining that the there is no interrupt remapping support. The
> > irq_domain_check_msi_remap function returns false as none of the
> checked
> > domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
> > is that the flags are not propagated through the domain hierarchy when
> > the domain is created.
> 
> Hum OK. Please apologize for the inconvenience, all the more so this is
> the second time you report the same issue for different cause :-( At the
> moment I can't test on a GICv3 ITS based system. I will try to fix that
> though.
> 
> I would like to get the confirmation introducing this flag is the right
> direction though.
> 
> Thanks
> 
> Eric
> >
> > Thanks,
> >
> > Diana
> >
> >
> >
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> 
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-23 Thread Geetha Akula 
Hi Eric,

Seeing same issue reported by Diana on ThunderX with you v4.9-reserved-v4
branch.
Vfio passthough work fine when allow_unsafe_interrupts is set.


Thank you,
Geetha.

On Thu, Dec 22, 2016 at 6:32 PM, Auger Eric  wrote:

> Hi Diana,
>
> On 22/12/2016 13:41, Diana Madalina Craciun wrote:
> > Hi Eric,
> >
> > On 12/13/2016 10:32 PM, Eric Auger wrote:
> >> In case the IOMMU does not bypass MSI transactions (typical
> >> case on ARM), we check all MSI controllers are IRQ remapping
> >> capable. If not the IRQ assignment may be unsafe.
> >>
> >> At this stage the arm-smmu-(v3) still advertise the
> >> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> >> removed in subsequent patches.
> >>
> >> Signed-off-by: Eric Auger 
> >> ---
> >>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_
> type1.c
> >> index d07fe73..a05648b 100644
> >> --- a/drivers/vfio/vfio_iommu_type1.c
> >> +++ b/drivers/vfio/vfio_iommu_type1.c
> >> @@ -37,6 +37,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #define DRIVER_VERSION  "0.2"
> >>  #define DRIVER_AUTHOR   "Alex Williamson "
> >> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  struct vfio_domain *domain, *d;
> >>  struct bus_type *bus = NULL;
> >>  int ret;
> >> -bool resv_msi;
> >> +bool resv_msi, msi_remap;
> >>  phys_addr_t resv_msi_base;
> >>
> >>  mutex_lock(>lock);
> >> @@ -818,8 +819,10 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  INIT_LIST_HEAD(>group_list);
> >>  list_add(>next, >group_list);
> >>
> >> -if (!allow_unsafe_interrupts &&
> >> -!iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> >> +msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> >> +   iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> >> +
> >> +if (!allow_unsafe_interrupts && !msi_remap) {
> >>  pr_warn("%s: No interrupt remapping support.  Use the
> module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on
> this platform\n",
> >> __func__);
> >>  ret = -EPERM;
> >
> > I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
> > LS2080), so I did not set allow_unsafe_interrupts. It fails here
> > complaining that the there is no interrupt remapping support. The
> > irq_domain_check_msi_remap function returns false as none of the checked
> > domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
> > is that the flags are not propagated through the domain hierarchy when
> > the domain is created.
>
> Hum OK. Please apologize for the inconvenience, all the more so this is
> the second time you report the same issue for different cause :-( At the
> moment I can't test on a GICv3 ITS based system. I will try to fix that
> though.
>
> I would like to get the confirmation introducing this flag is the right
> direction though.
>
> Thanks
>
> Eric
> >
> > Thanks,
> >
> > Diana
> >
> >
> >
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-22 Thread Auger Eric 
Hi Diana,

On 22/12/2016 13:41, Diana Madalina Craciun wrote:
> Hi Eric,
> 
> On 12/13/2016 10:32 PM, Eric Auger wrote:
>> In case the IOMMU does not bypass MSI transactions (typical
>> case on ARM), we check all MSI controllers are IRQ remapping
>> capable. If not the IRQ assignment may be unsafe.
>>
>> At this stage the arm-smmu-(v3) still advertise the
>> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
>> removed in subsequent patches.
>>
>> Signed-off-by: Eric Auger 
>> ---
>>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
>> b/drivers/vfio/vfio_iommu_type1.c
>> index d07fe73..a05648b 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -37,6 +37,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #define DRIVER_VERSION  "0.2"
>>  #define DRIVER_AUTHOR   "Alex Williamson "
>> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void 
>> *iommu_data,
>>  struct vfio_domain *domain, *d;
>>  struct bus_type *bus = NULL;
>>  int ret;
>> -bool resv_msi;
>> +bool resv_msi, msi_remap;
>>  phys_addr_t resv_msi_base;
>>  
>>  mutex_lock(>lock);
>> @@ -818,8 +819,10 @@ static int vfio_iommu_type1_attach_group(void 
>> *iommu_data,
>>  INIT_LIST_HEAD(>group_list);
>>  list_add(>next, >group_list);
>>  
>> -if (!allow_unsafe_interrupts &&
>> -!iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
>> +msi_remap = resv_msi ? irq_domain_check_msi_remap() :
>> +   iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
>> +
>> +if (!allow_unsafe_interrupts && !msi_remap) {
>>  pr_warn("%s: No interrupt remapping support.  Use the module 
>> param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this 
>> platform\n",
>> __func__);
>>  ret = -EPERM;
> 
> I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
> LS2080), so I did not set allow_unsafe_interrupts. It fails here
> complaining that the there is no interrupt remapping support. The
> irq_domain_check_msi_remap function returns false as none of the checked
> domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
> is that the flags are not propagated through the domain hierarchy when
> the domain is created.

Hum OK. Please apologize for the inconvenience, all the more so this is
the second time you report the same issue for different cause :-( At the
moment I can't test on a GICv3 ITS based system. I will try to fix that
though.

I would like to get the confirmation introducing this flag is the right
direction though.

Thanks

Eric
> 
> Thanks,
> 
> Diana
> 
> 
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-22 Thread Diana Madalina Craciun 
Hi Eric,

On 12/13/2016 10:32 PM, Eric Auger wrote:
> In case the IOMMU does not bypass MSI transactions (typical
> case on ARM), we check all MSI controllers are IRQ remapping
> capable. If not the IRQ assignment may be unsafe.
>
> At this stage the arm-smmu-(v3) still advertise the
> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> removed in subsequent patches.
>
> Signed-off-by: Eric Auger 
> ---
>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index d07fe73..a05648b 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define DRIVER_VERSION  "0.2"
>  #define DRIVER_AUTHOR   "Alex Williamson "
> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>   struct vfio_domain *domain, *d;
>   struct bus_type *bus = NULL;
>   int ret;
> - bool resv_msi;
> + bool resv_msi, msi_remap;
>   phys_addr_t resv_msi_base;
>  
>   mutex_lock(>lock);
> @@ -818,8 +819,10 @@ static int vfio_iommu_type1_attach_group(void 
> *iommu_data,
>   INIT_LIST_HEAD(>group_list);
>   list_add(>next, >group_list);
>  
> - if (!allow_unsafe_interrupts &&
> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> + msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> +iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> +
> + if (!allow_unsafe_interrupts && !msi_remap) {
>   pr_warn("%s: No interrupt remapping support.  Use the module 
> param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this 
> platform\n",
>  __func__);
>   ret = -EPERM;

I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
LS2080), so I did not set allow_unsafe_interrupts. It fails here
complaining that the there is no interrupt remapping support. The
irq_domain_check_msi_remap function returns false as none of the checked
domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
is that the flags are not propagated through the domain hierarchy when
the domain is created.

Thanks,

Diana



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-13 Thread Eric Auger 
In case the IOMMU does not bypass MSI transactions (typical
case on ARM), we check all MSI controllers are IRQ remapping
capable. If not the IRQ assignment may be unsafe.

At this stage the arm-smmu-(v3) still advertise the
IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
removed in subsequent patches.

Signed-off-by: Eric Auger 
---
 drivers/vfio/vfio_iommu_type1.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index d07fe73..a05648b 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define DRIVER_VERSION  "0.2"
 #define DRIVER_AUTHOR   "Alex Williamson "
@@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
struct vfio_domain *domain, *d;
struct bus_type *bus = NULL;
int ret;
-   bool resv_msi;
+   bool resv_msi, msi_remap;
phys_addr_t resv_msi_base;
 
mutex_lock(>lock);
@@ -818,8 +819,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
INIT_LIST_HEAD(>group_list);
list_add(>next, >group_list);
 
-   if (!allow_unsafe_interrupts &&
-   !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
+   msi_remap = resv_msi ? irq_domain_check_msi_remap() :
+  iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
+
+   if (!allow_unsafe_interrupts && !msi_remap) {
pr_warn("%s: No interrupt remapping support.  Use the module 
param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this 
platform\n",
   __func__);
ret = -EPERM;
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu