1. Patchew
  2. Xen
  3. View series

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

Amit Singh Tomar posted 1 patch 4 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/1556902928-18682-1-git-send-email-amittomer25@gmail.com
There is a newer version of this series
xen/arch/arm/domain_build.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
[Xen-devel] [PATCH] xen/arm: Black list everything with a PPI
Posted by Amit Singh Tomar 4 years, 11 months ago 
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: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI
Posted by Julien Grall 4 years, 11 months ago 
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
Posted by Amit Tomer 4 years, 11 months ago 
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
Posted by Oleksandr 4 years, 11 months ago 
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
Posted by Amit Tomer 4 years, 11 months ago 
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
Posted by Andre Przywara 4 years, 11 months ago 
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
Posted by Julien Grall 4 years, 11 months ago 

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.