[PATCH] xen/arm: Black list everything with a PPI

[PATCH] xen/arm: Black list everything with a PPI

[Amit Singh Tomar]

XEN should not forward PPIs to Dom0 as it only support SPIs.
One of solution to this problem is to skip any device that
uses PPI source completely while building domain itself.

This patch goes through all the interrupt sources of device and skip it
if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
skipping PMU node.

Suggested-by:  Julien Grall <julien.grall@arm.com>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
    * This replaces following patch.
      https://patchwork.kernel.org/patch/10899881/
---
 xen/arch/arm/domain_build.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d983677..8f54472 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         { /* sentinel */ },
     };
     struct dt_device_node *child;
-    int res;
+    int res, i, nirq, irq_id;
     const char *name;
     const char *path;
 
@@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         return 0;
     }
 
+    /* Skip the node, using PPI source */
+    nirq = dt_number_of_irq(node);
+
+    for ( i = 0 ; i < nirq ; i++ )
+    {
+        irq_id = platform_get_irq(node, i);
+
+        if ( irq_id >= 16 && irq_id < 32 )
+        {
+            dt_dprintk(" Skip node with (PPI source)\n");
+            return 0;
+        }
+    }
+
     /*
      * Xen is using some path for its own purpose. Warn if a node
      * already exists with the same path.
-- 
2.7.4


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

[Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Amit Singh Tomar]

XEN should not forward PPIs to Dom0 as it only support SPIs.
One of solution to this problem is to skip any device that
uses PPI source completely while building domain itself.

This patch goes through all the interrupt sources of device and skip it
if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
skipping PMU node.

Suggested-by:  Julien Grall <julien.grall@arm.com>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
    * This replaces following patch.
      https://patchwork.kernel.org/patch/10899881/
---
 xen/arch/arm/domain_build.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d983677..8f54472 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         { /* sentinel */ },
     };
     struct dt_device_node *child;
-    int res;
+    int res, i, nirq, irq_id;
     const char *name;
     const char *path;
 
@@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         return 0;
     }
 
+    /* Skip the node, using PPI source */
+    nirq = dt_number_of_irq(node);
+
+    for ( i = 0 ; i < nirq ; i++ )
+    {
+        irq_id = platform_get_irq(node, i);
+
+        if ( irq_id >= 16 && irq_id < 32 )
+        {
+            dt_dprintk(" Skip node with (PPI source)\n");
+            return 0;
+        }
+    }
+
     /*
      * Xen is using some path for its own purpose. Warn if a node
      * already exists with the same path.
-- 
2.7.4


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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Oleksandr]

On 03.05.19 20:02, Amit Singh Tomar wrote:

Hi, Amit

> XEN should not forward PPIs to Dom0 as it only support SPIs.
> One of solution to this problem is to skip any device that
> uses PPI source completely while building domain itself.
>
> This patch goes through all the interrupt sources of device and skip it
> if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> skipping PMU node.
>
> Suggested-by:  Julien Grall <julien.grall@arm.com>
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>      * This replaces following patch.
>        https://patchwork.kernel.org/patch/10899881/
> ---
>   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d983677..8f54472 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           { /* sentinel */ },
>       };
>       struct dt_device_node *child;
> -    int res;
> +    int res, i, nirq, irq_id;
>       const char *name;
>       const char *path;
>   
> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           return 0;
>       }
>   
> +    /* Skip the node, using PPI source */
> +    nirq = dt_number_of_irq(node);
> +
> +    for ( i = 0 ; i < nirq ; i++ )
> +    {
> +        irq_id = platform_get_irq(node, i);

Do we need to do something here if platform_get_irq() returns -1?

> +
> +        if ( irq_id >= 16 && irq_id < 32 )
> +        {
> +            dt_dprintk(" Skip node with (PPI source)\n");
> +            return 0;
> +        }
> +    }
> +
>       /*
>        * Xen is using some path for its own purpose. Warn if a node
>        * already exists with the same path.

Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I 
have tested your patch just to be sure it wouldn't skip anything by a 
mistake)


-- 
Regards,

Oleksandr Tyshchenko


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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Oleksandr]

On 03.05.19 20:02, Amit Singh Tomar wrote:

Hi, Amit

> XEN should not forward PPIs to Dom0 as it only support SPIs.
> One of solution to this problem is to skip any device that
> uses PPI source completely while building domain itself.
>
> This patch goes through all the interrupt sources of device and skip it
> if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> skipping PMU node.
>
> Suggested-by:  Julien Grall <julien.grall@arm.com>
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>      * This replaces following patch.
>        https://patchwork.kernel.org/patch/10899881/
> ---
>   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d983677..8f54472 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           { /* sentinel */ },
>       };
>       struct dt_device_node *child;
> -    int res;
> +    int res, i, nirq, irq_id;
>       const char *name;
>       const char *path;
>   
> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           return 0;
>       }
>   
> +    /* Skip the node, using PPI source */
> +    nirq = dt_number_of_irq(node);
> +
> +    for ( i = 0 ; i < nirq ; i++ )
> +    {
> +        irq_id = platform_get_irq(node, i);

Do we need to do something here if platform_get_irq() returns -1?

> +
> +        if ( irq_id >= 16 && irq_id < 32 )
> +        {
> +            dt_dprintk(" Skip node with (PPI source)\n");
> +            return 0;
> +        }
> +    }
> +
>       /*
>        * Xen is using some path for its own purpose. Warn if a node
>        * already exists with the same path.

Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I 
have tested your patch just to be sure it wouldn't skip anything by a 
mistake)


-- 
Regards,

Oleksandr Tyshchenko


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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Amit Tomer]

Hello,

Thanks for having a look at it.

On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
>
>
> On 03.05.19 20:02, Amit Singh Tomar wrote:
>
> Hi, Amit
>
> > XEN should not forward PPIs to Dom0 as it only support SPIs.
> > One of solution to this problem is to skip any device that
> > uses PPI source completely while building domain itself.
> >
> > This patch goes through all the interrupt sources of device and skip it
> > if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> > skipping PMU node.
> >
> > Suggested-by:  Julien Grall <julien.grall@arm.com>
> > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > ---
> >      * This replaces following patch.
> >        https://patchwork.kernel.org/patch/10899881/
> > ---
> >   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index d983677..8f54472 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> >           { /* sentinel */ },
> >       };
> >       struct dt_device_node *child;
> > -    int res;
> > +    int res, i, nirq, irq_id;
> >       const char *name;
> >       const char *path;
> >
> > @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> >           return 0;
> >       }
> >
> > +    /* Skip the node, using PPI source */
> > +    nirq = dt_number_of_irq(node);
> > +
> > +    for ( i = 0 ; i < nirq ; i++ )
> > +    {
> > +        irq_id = platform_get_irq(node, i);
>
> Do we need to do something here if platform_get_irq() returns -1?

Yeah, I should have done it. some think like following:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284

> > +
> > +        if ( irq_id >= 16 && irq_id < 32 )
> > +        {
> > +            dt_dprintk(" Skip node with (PPI source)\n");
> > +            return 0;
> > +        }
> > +    }
> > +
> >       /*
> >        * Xen is using some path for its own purpose. Warn if a node
> >        * already exists with the same path.
>
> Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I
> have tested your patch just to be sure it wouldn't skip anything by a
> mistake)

Ok, Thanks for testing it. I would resend it with error condition check.

-Thanks
Amit.

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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Amit Tomer]

Hello,

Thanks for having a look at it.

On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
>
>
> On 03.05.19 20:02, Amit Singh Tomar wrote:
>
> Hi, Amit
>
> > XEN should not forward PPIs to Dom0 as it only support SPIs.
> > One of solution to this problem is to skip any device that
> > uses PPI source completely while building domain itself.
> >
> > This patch goes through all the interrupt sources of device and skip it
> > if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> > skipping PMU node.
> >
> > Suggested-by:  Julien Grall <julien.grall@arm.com>
> > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > ---
> >      * This replaces following patch.
> >        https://patchwork.kernel.org/patch/10899881/
> > ---
> >   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index d983677..8f54472 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> >           { /* sentinel */ },
> >       };
> >       struct dt_device_node *child;
> > -    int res;
> > +    int res, i, nirq, irq_id;
> >       const char *name;
> >       const char *path;
> >
> > @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> >           return 0;
> >       }
> >
> > +    /* Skip the node, using PPI source */
> > +    nirq = dt_number_of_irq(node);
> > +
> > +    for ( i = 0 ; i < nirq ; i++ )
> > +    {
> > +        irq_id = platform_get_irq(node, i);
>
> Do we need to do something here if platform_get_irq() returns -1?

Yeah, I should have done it. some think like following:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284

> > +
> > +        if ( irq_id >= 16 && irq_id < 32 )
> > +        {
> > +            dt_dprintk(" Skip node with (PPI source)\n");
> > +            return 0;
> > +        }
> > +    }
> > +
> >       /*
> >        * Xen is using some path for its own purpose. Warn if a node
> >        * already exists with the same path.
>
> Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I
> have tested your patch just to be sure it wouldn't skip anything by a
> mistake)

Ok, Thanks for testing it. I would resend it with error condition check.

-Thanks
Amit.

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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Andre Przywara]

On Thu, 16 May 2019 17:15:36 +0530
Amit Tomer <amittomer25@gmail.com> wrote:

Hi,

> Thanks for having a look at it.
> 
> On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
> >
> >
> > On 03.05.19 20:02, Amit Singh Tomar wrote:
> >
> > Hi, Amit
> >  
> > > XEN should not forward PPIs to Dom0 as it only support SPIs.
> > > One of solution to this problem is to skip any device that
> > > uses PPI source completely while building domain itself.
> > >
> > > This patch goes through all the interrupt sources of device and skip it
> > > if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> > > skipping PMU node.
> > >
> > > Suggested-by:  Julien Grall <julien.grall@arm.com>
> > > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > > ---
> > >      * This replaces following patch.
> > >        https://patchwork.kernel.org/patch/10899881/
> > > ---
> > >   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
> > >   1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index d983677..8f54472 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> > >           { /* sentinel */ },
> > >       };
> > >       struct dt_device_node *child;
> > > -    int res;
> > > +    int res, i, nirq, irq_id;
> > >       const char *name;
> > >       const char *path;
> > >
> > > @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> > >           return 0;
> > >       }
> > >
> > > +    /* Skip the node, using PPI source */
> > > +    nirq = dt_number_of_irq(node);
> > > +
> > > +    for ( i = 0 ; i < nirq ; i++ )
> > > +    {
> > > +        irq_id = platform_get_irq(node, i);  
> >
> > Do we need to do something here if platform_get_irq() returns -1?  
> 
> Yeah, I should have done it. some think like following:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284

Why would that (or any actual check against -1) be necessary?
If irq_id is < 0, then it would surely not match the condition below and
*nothing* would happen. So I'd say: Keep it like it is, no action necessary.

> > > +
> > > +        if ( irq_id >= 16 && irq_id < 32 )

Any chance you can put names there? Or at least add a comment that PPIs range from 16 to 31?

> > > +        {
> > > +            dt_dprintk(" Skip node with (PPI source)\n");
> > > +            return 0;
> > > +        }
> > > +    }
> > > +
> > >       /*
> > >        * Xen is using some path for its own purpose. Warn if a node
> > >        * already exists with the same path.  
> >
> > Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I
> > have tested your patch just to be sure it wouldn't skip anything by a
> > mistake)  
> 
> Ok, Thanks for testing it. I would resend it with error condition check.

Please don't ;-)

Cheers,
Andre

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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Andre Przywara]

On Thu, 16 May 2019 17:15:36 +0530
Amit Tomer <amittomer25@gmail.com> wrote:

Hi,

> Thanks for having a look at it.
> 
> On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
> >
> >
> > On 03.05.19 20:02, Amit Singh Tomar wrote:
> >
> > Hi, Amit
> >  
> > > XEN should not forward PPIs to Dom0 as it only support SPIs.
> > > One of solution to this problem is to skip any device that
> > > uses PPI source completely while building domain itself.
> > >
> > > This patch goes through all the interrupt sources of device and skip it
> > > if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> > > skipping PMU node.
> > >
> > > Suggested-by:  Julien Grall <julien.grall@arm.com>
> > > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > > ---
> > >      * This replaces following patch.
> > >        https://patchwork.kernel.org/patch/10899881/
> > > ---
> > >   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
> > >   1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index d983677..8f54472 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> > >           { /* sentinel */ },
> > >       };
> > >       struct dt_device_node *child;
> > > -    int res;
> > > +    int res, i, nirq, irq_id;
> > >       const char *name;
> > >       const char *path;
> > >
> > > @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
> > >           return 0;
> > >       }
> > >
> > > +    /* Skip the node, using PPI source */
> > > +    nirq = dt_number_of_irq(node);
> > > +
> > > +    for ( i = 0 ; i < nirq ; i++ )
> > > +    {
> > > +        irq_id = platform_get_irq(node, i);  
> >
> > Do we need to do something here if platform_get_irq() returns -1?  
> 
> Yeah, I should have done it. some think like following:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284

Why would that (or any actual check against -1) be necessary?
If irq_id is < 0, then it would surely not match the condition below and
*nothing* would happen. So I'd say: Keep it like it is, no action necessary.

> > > +
> > > +        if ( irq_id >= 16 && irq_id < 32 )

Any chance you can put names there? Or at least add a comment that PPIs range from 16 to 31?

> > > +        {
> > > +            dt_dprintk(" Skip node with (PPI source)\n");
> > > +            return 0;
> > > +        }
> > > +    }
> > > +
> > >       /*
> > >        * Xen is using some path for its own purpose. Warn if a node
> > >        * already exists with the same path.  
> >
> > Patch looks good. Although R-Car H3 seems to not use PPIs for PMU, I
> > have tested your patch just to be sure it wouldn't skip anything by a
> > mistake)  
> 
> Ok, Thanks for testing it. I would resend it with error condition check.

Please don't ;-)

Cheers,
Andre

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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Julien Grall]

On 16/05/2019 14:17, Andre Przywara wrote:
> On Thu, 16 May 2019 17:15:36 +0530
> Amit Tomer <amittomer25@gmail.com> wrote:
>> On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
>>> On 03.05.19 20:02, Amit Singh Tomar wrote:
>>>> Suggested-by:  Julien Grall <julien.grall@arm.com>
>>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>>> ---
>>>>       * This replaces following patch.
>>>>         https://patchwork.kernel.org/patch/10899881/
>>>> ---
>>>>    xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>>>>    1 file changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index d983677..8f54472 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>>>>            { /* sentinel */ },
>>>>        };
>>>>        struct dt_device_node *child;
>>>> -    int res;
>>>> +    int res, i, nirq, irq_id;
>>>>        const char *name;
>>>>        const char *path;
>>>>
>>>> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>>>>            return 0;
>>>>        }
>>>>
>>>> +    /* Skip the node, using PPI source */
>>>> +    nirq = dt_number_of_irq(node);
>>>> +
>>>> +    for ( i = 0 ; i < nirq ; i++ )
>>>> +    {
>>>> +        irq_id = platform_get_irq(node, i);
>>>
>>> Do we need to do something here if platform_get_irq() returns -1?
>>
>> Yeah, I should have done it. some think like following:
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284
> 
> Why would that (or any actual check against -1) be necessary?
> If irq_id is < 0, then it would surely not match the condition below and
> *nothing* would happen. So I'd say: Keep it like it is, no action necessary.

Worst, depending on the action done with check, you could actively break support 
for platform with multiple interrupt controllers. That's why in handle_device(), 
the interrupt controller is checked before calling platform_get_irq().

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Julien Grall]

On 16/05/2019 14:17, Andre Przywara wrote:
> On Thu, 16 May 2019 17:15:36 +0530
> Amit Tomer <amittomer25@gmail.com> wrote:
>> On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@gmail.com> wrote:
>>> On 03.05.19 20:02, Amit Singh Tomar wrote:
>>>> Suggested-by:  Julien Grall <julien.grall@arm.com>
>>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>>> ---
>>>>       * This replaces following patch.
>>>>         https://patchwork.kernel.org/patch/10899881/
>>>> ---
>>>>    xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>>>>    1 file changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index d983677..8f54472 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>>>>            { /* sentinel */ },
>>>>        };
>>>>        struct dt_device_node *child;
>>>> -    int res;
>>>> +    int res, i, nirq, irq_id;
>>>>        const char *name;
>>>>        const char *path;
>>>>
>>>> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>>>>            return 0;
>>>>        }
>>>>
>>>> +    /* Skip the node, using PPI source */
>>>> +    nirq = dt_number_of_irq(node);
>>>> +
>>>> +    for ( i = 0 ; i < nirq ; i++ )
>>>> +    {
>>>> +        irq_id = platform_get_irq(node, i);
>>>
>>> Do we need to do something here if platform_get_irq() returns -1?
>>
>> Yeah, I should have done it. some think like following:
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284
> 
> Why would that (or any actual check against -1) be necessary?
> If irq_id is < 0, then it would surely not match the condition below and
> *nothing* would happen. So I'd say: Keep it like it is, no action necessary.

Worst, depending on the action done with check, you could actively break support 
for platform with multiple interrupt controllers. That's why in handle_device(), 
the interrupt controller is checked before calling platform_get_irq().

Cheers,

-- 
Julien Grall

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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Julien Grall]

Hi Amit,

Just a quick follow-up. Is there any plan to send a new version of this patch?

Cheers,

On 03/05/2019 18:02, Amit Singh Tomar wrote:
> XEN should not forward PPIs to Dom0 as it only support SPIs.
> One of solution to this problem is to skip any device that
> uses PPI source completely while building domain itself.
> 
> This patch goes through all the interrupt sources of device and skip it
> if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> skipping PMU node.
> 
> Suggested-by:  Julien Grall <julien.grall@arm.com>
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>      * This replaces following patch.
>        https://patchwork.kernel.org/patch/10899881/
> ---
>   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d983677..8f54472 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           { /* sentinel */ },
>       };
>       struct dt_device_node *child;
> -    int res;
> +    int res, i, nirq, irq_id;
>       const char *name;
>       const char *path;
>   
> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           return 0;
>       }
>   
> +    /* Skip the node, using PPI source */
> +    nirq = dt_number_of_irq(node);
> +
> +    for ( i = 0 ; i < nirq ; i++ )
> +    {
> +        irq_id = platform_get_irq(node, i);
> +
> +        if ( irq_id >= 16 && irq_id < 32 )
> +        {
> +            dt_dprintk(" Skip node with (PPI source)\n");
> +            return 0;
> +        }
> +    }
> +
>       /*
>        * Xen is using some path for its own purpose. Warn if a node
>        * already exists with the same path.
> 

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Julien Grall]

Hi Amit,

Just a quick follow-up. Is there any plan to send a new version of this patch?

Cheers,

On 03/05/2019 18:02, Amit Singh Tomar wrote:
> XEN should not forward PPIs to Dom0 as it only support SPIs.
> One of solution to this problem is to skip any device that
> uses PPI source completely while building domain itself.
> 
> This patch goes through all the interrupt sources of device and skip it
> if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> skipping PMU node.
> 
> Suggested-by:  Julien Grall <julien.grall@arm.com>
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>      * This replaces following patch.
>        https://patchwork.kernel.org/patch/10899881/
> ---
>   xen/arch/arm/domain_build.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d983677..8f54472 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           { /* sentinel */ },
>       };
>       struct dt_device_node *child;
> -    int res;
> +    int res, i, nirq, irq_id;
>       const char *name;
>       const char *path;
>   
> @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>           return 0;
>       }
>   
> +    /* Skip the node, using PPI source */
> +    nirq = dt_number_of_irq(node);
> +
> +    for ( i = 0 ; i < nirq ; i++ )
> +    {
> +        irq_id = platform_get_irq(node, i);
> +
> +        if ( irq_id >= 16 && irq_id < 32 )
> +        {
> +            dt_dprintk(" Skip node with (PPI source)\n");
> +            return 0;
> +        }
> +    }
> +
>       /*
>        * Xen is using some path for its own purpose. Warn if a node
>        * already exists with the same path.
> 

-- 
Julien Grall

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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Amit Tomer]

Hi,

On Wed, May 29, 2019 at 11:17 PM Julien Grall <julien.grall@arm.com> wrote:
>
> Hi Amit,
>
> Just a quick follow-up. Is there any plan to send a new version of this patch?
>
Sorry for late on this, I would be able to send it(with a comment for
PPI's range) on coming weekend.

Thanks,
-Amit

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

Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Amit Tomer]

Hi,

On Wed, May 29, 2019 at 11:17 PM Julien Grall <julien.grall@arm.com> wrote:
>
> Hi Amit,
>
> Just a quick follow-up. Is there any plan to send a new version of this patch?
>
Sorry for late on this, I would be able to send it(with a comment for
PPI's range) on coming weekend.

Thanks,
-Amit

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

Re: [PATCH] xen/arm: Black list everything with a PPI

[Amit Tomer]

Sorry Just sent the wrong patch , Please ignore this.

On Fri, May 3, 2019 at 10:13 PM Amit Singh Tomar <amittomer25@gmail.com> wrote:
>
> XEN should not forward PPIs to Dom0 as it only support SPIs.
> One of solution to this problem is to skip any device that
> uses PPI source completely while building domain itself.
>
> This patch goes through all the interrupt sources of device and skip it
> if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
> skipping PMU node.
>
> Suggested-by:  Julien Grall <julien.grall@arm.com>
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
>     * This replaces following patch.
>       https://patchwork.kernel.org/patch/10899881/
> ---
>  xen/arch/arm/domain_build.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d983677..0ae54db 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1334,6 +1334,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>          DT_MATCH_COMPATIBLE("arm,cortex-a15-pmu"),
>          DT_MATCH_COMPATIBLE("arm,cortex-a53-edac"),
>          DT_MATCH_COMPATIBLE("arm,armv8-pmuv3"),
> +        DT_MATCH_COMPATIBLE("arm,cortex-a53-pmu"),
>          DT_MATCH_PATH("/cpus"),
>          DT_MATCH_TYPE("memory"),
>          /* The memory mapped timer is not supported by Xen. */
> @@ -1353,7 +1354,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>          { /* sentinel */ },
>      };
>      struct dt_device_node *child;
> -    int res;
> +    int res, i, nirq, irq_id;
>      const char *name;
>      const char *path;
>
> @@ -1399,6 +1400,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>          return 0;
>      }
>
> +    /* Skip the node, using PPI source */
> +    nirq = dt_number_of_irq(node);
> +
> +    for ( i = 0 ; i < nirq ; i++ )
> +    {
> +        irq_id = platform_get_irq(node, i);
> +
> +        if ( irq_id >= 16 && irq_id < 32 )
> +        {
> +            dt_dprintk(" Skip node with (PPI source)\n");
> +            return 0;
> +        }
> +    }
> +
>      /*
>       * Xen is using some path for its own purpose. Warn if a node
>       * already exists with the same path.
> --
> 2.7.4
>

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

[PATCH] xen/arm: Black list everything with a PPI

[Amit Singh Tomar]

XEN should not forward PPIs to Dom0 as it only support SPIs.
One of solution to this problem is to skip any device that
uses PPI source completely while building domain itself.

This patch goes through all the interrupt sources of device and skip it
if one of interrupt source is PPI. It fixes XEN boot on i.MX8MQ by
skipping PMU node.

Suggested-by:  Julien Grall <julien.grall@arm.com>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
    * This replaces following patch.
      https://patchwork.kernel.org/patch/10899881/
---
 xen/arch/arm/domain_build.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d983677..0ae54db 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1334,6 +1334,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         DT_MATCH_COMPATIBLE("arm,cortex-a15-pmu"),
         DT_MATCH_COMPATIBLE("arm,cortex-a53-edac"),
         DT_MATCH_COMPATIBLE("arm,armv8-pmuv3"),
+        DT_MATCH_COMPATIBLE("arm,cortex-a53-pmu"),
         DT_MATCH_PATH("/cpus"),
         DT_MATCH_TYPE("memory"),
         /* The memory mapped timer is not supported by Xen. */
@@ -1353,7 +1354,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         { /* sentinel */ },
     };
     struct dt_device_node *child;
-    int res;
+    int res, i, nirq, irq_id;
     const char *name;
     const char *path;
 
@@ -1399,6 +1400,20 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         return 0;
     }
 
+    /* Skip the node, using PPI source */
+    nirq = dt_number_of_irq(node);
+
+    for ( i = 0 ; i < nirq ; i++ )
+    {
+        irq_id = platform_get_irq(node, i);
+
+        if ( irq_id >= 16 && irq_id < 32 )
+        {
+            dt_dprintk(" Skip node with (PPI source)\n");
+            return 0;
+        }
+    }
+
     /*
      * Xen is using some path for its own purpose. Warn if a node
      * already exists with the same path.
-- 
2.7.4


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