[PATCH v2 0/2] xen/arm: domain_build: map_{dt_}irq_to_domain() fixes

[PATCH v2 0/2] xen/arm: domain_build: map_{dt_}irq_to_domain() fixes

[Michal Orzel]

Propagate return code correctly + fix the format specifiers.

Michal Orzel (2):
  xen/arm: domain_build: Propagate return code of map_irq_to_domain()
  xen/arm: domain_build: Fix format specifiers in
    map_{dt_}irq_to_domain()

 xen/arch/arm/domain_build.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] xen/arm: domain_build: Propagate return code of map_irq_to_domain()

[Michal Orzel]

From map_dt_irq_to_domain() we are assigning a return code of
map_irq_to_domain() to a variable without checking it for an error.
Fix it by propagating the return code directly since this is the last
call.

Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - split the patch so that a fix alone can be backported
---
 xen/arch/arm/domain_build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f80fdd1af206..9dee1bb8f21c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2320,7 +2320,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
 
     res = map_irq_to_domain(d, irq, !mr_data->skip_mapping, dt_node_name(dev));
 
-    return 0;
+    return res;
 }
 
 int __init map_range_to_domain(const struct dt_device_node *dev,
-- 
2.25.1

[PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in map_{dt_}irq_to_domain()

[Michal Orzel]

IRQ is of unsigned type so %u should be used. When printing domain id,
%pd should be the correct format to maintain the consistency.

Also, wherever possible, reduce the number of splitted lines for printk().

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - split the v1 patch so that the format specifiers are handled separately
 - also fix map_irq_to_domain()
---
 xen/arch/arm/domain_build.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9dee1bb8f21c..71f307a572e9 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2265,8 +2265,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
     res = irq_permit_access(d, irq);
     if ( res )
     {
-        printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
-               d->domain_id, irq);
+        printk(XENLOG_ERR "Unable to permit to %pd access to IRQ %u\n", d, irq);
         return res;
     }
 
@@ -2282,8 +2281,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
         res = route_irq_to_guest(d, irq, irq, devname);
         if ( res < 0 )
         {
-            printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
-                   irq, d->domain_id);
+            printk(XENLOG_ERR "Unable to map IRQ%u to %pd\n", irq, d);
             return res;
         }
     }
@@ -2303,8 +2301,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
 
     if ( irq < NR_LOCAL_IRQS )
     {
-        printk(XENLOG_ERR "%s: IRQ%"PRId32" is not a SPI\n",
-               dt_node_name(dev), irq);
+        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n", dt_node_name(dev), irq);
         return -EINVAL;
     }
 
@@ -2312,9 +2309,8 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
     res = irq_set_spi_type(irq, dt_irq->type);
     if ( res )
     {
-        printk(XENLOG_ERR
-               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
-               dt_node_name(dev), irq, d->domain_id);
+        printk(XENLOG_ERR "%s: Unable to setup IRQ%u to %pd\n",
+               dt_node_name(dev), irq, d);
         return res;
     }
 
-- 
2.25.1

Re: [PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in map_{dt_}irq_to_domain()

[Andrew Cooper]

On 11/05/2023 2:02 pm, Michal Orzel wrote:
> IRQ is of unsigned type so %u should be used. When printing domain id,
> %pd should be the correct format to maintain the consistency.
>
> Also, wherever possible, reduce the number of splitted lines for printk().

Very minor grammar note.  Just "split" here.

~Andrew

Re: [PATCH v2 1/2] xen/arm: domain_build: Propagate return code of map_irq_to_domain()

[Ayan Kumar Halder]

On 11/05/2023 14:02, Michal Orzel wrote:
>  From map_dt_irq_to_domain() we are assigning a return code of
> map_irq_to_domain() to a variable without checking it for an error.
> Fix it by propagating the return code directly since this is the last
> call.
>
> Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes in v2:
>   - split the patch so that a fix alone can be backported
> ---
>   xen/arch/arm/domain_build.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f80fdd1af206..9dee1bb8f21c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2320,7 +2320,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>   
>       res = map_irq_to_domain(d, irq, !mr_data->skip_mapping, dt_node_name(dev));
>   
> -    return 0;
> +    return res;
>   }
>   
>   int __init map_range_to_domain(const struct dt_device_node *dev,

Re: [PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in map_{dt_}irq_to_domain()

[Ayan Kumar Halder]

On 11/05/2023 14:02, Michal Orzel wrote:
> IRQ is of unsigned type so %u should be used. When printing domain id,
> %pd should be the correct format to maintain the consistency.
>
> Also, wherever possible, reduce the number of splitted lines for printk().
>
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes in v2:
>   - split the v1 patch so that the format specifiers are handled separately
>   - also fix map_irq_to_domain()
> ---
>   xen/arch/arm/domain_build.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9dee1bb8f21c..71f307a572e9 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2265,8 +2265,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
>       res = irq_permit_access(d, irq);
>       if ( res )
>       {
> -        printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -               d->domain_id, irq);
> +        printk(XENLOG_ERR "Unable to permit to %pd access to IRQ %u\n", d, irq);
>           return res;
>       }
>   
> @@ -2282,8 +2281,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
>           res = route_irq_to_guest(d, irq, irq, devname);
>           if ( res < 0 )
>           {
> -            printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> -                   irq, d->domain_id);
> +            printk(XENLOG_ERR "Unable to map IRQ%u to %pd\n", irq, d);
>               return res;
>           }
>       }
> @@ -2303,8 +2301,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>   
>       if ( irq < NR_LOCAL_IRQS )
>       {
> -        printk(XENLOG_ERR "%s: IRQ%"PRId32" is not a SPI\n",
> -               dt_node_name(dev), irq);
> +        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n", dt_node_name(dev), irq);
>           return -EINVAL;
>       }
>   
> @@ -2312,9 +2309,8 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>       res = irq_set_spi_type(irq, dt_irq->type);
>       if ( res )
>       {
> -        printk(XENLOG_ERR
> -               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
> -               dt_node_name(dev), irq, d->domain_id);
> +        printk(XENLOG_ERR "%s: Unable to setup IRQ%u to %pd\n",
> +               dt_node_name(dev), irq, d);
>           return res;
>       }
>   

RE: [PATCH v2 1/2] xen/arm: domain_build: Propagate return code of map_irq_to_domain()

[Henry Wang]

Hi Michal,

> -----Original Message-----
> Subject: [PATCH v2 1/2] xen/arm: domain_build: Propagate return code of
> map_irq_to_domain()
> 
> From map_dt_irq_to_domain() we are assigning a return code of
> map_irq_to_domain() to a variable without checking it for an error.
> Fix it by propagating the return code directly since this is the last
> call.
> 
> Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

RE: [PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in map_{dt_}irq_to_domain()

[Henry Wang]

Hi Michal,

> -----Original Message-----
> Subject: [PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in
> map_{dt_}irq_to_domain()
> 
> IRQ is of unsigned type so %u should be used. When printing domain id,
> %pd should be the correct format to maintain the consistency.
> 
> Also, wherever possible, reduce the number of splitted lines for printk().
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

Re: [PATCH v2 1/2] xen/arm: domain_build: Propagate return code of map_irq_to_domain()

[Julien Grall]

Hi Michal,

On 11/05/2023 14:02, Michal Orzel wrote:
>  From map_dt_irq_to_domain() we are assigning a return code of
> map_irq_to_domain() to a variable without checking it for an error.
> Fix it by propagating the return code directly since this is the last
> call.
> 
> Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

> ---
> Changes in v2:
>   - split the patch so that a fix alone can be backported
> ---
>   xen/arch/arm/domain_build.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f80fdd1af206..9dee1bb8f21c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2320,7 +2320,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>   
>       res = map_irq_to_domain(d, irq, !mr_data->skip_mapping, dt_node_name(dev));
>   
> -    return 0;
> +    return res;
>   }
>   
>   int __init map_range_to_domain(const struct dt_device_node *dev,

-- 
Julien Grall

Re: [PATCH v2 2/2] xen/arm: domain_build: Fix format specifiers in map_{dt_}irq_to_domain()

[Julien Grall]

Hi Michal,

On 11/05/2023 14:02, Michal Orzel wrote:
> IRQ is of unsigned type so %u should be used. When printing domain id,
> %pd should be the correct format to maintain the consistency.
> 
> Also, wherever possible, reduce the number of splitted lines for printk().

Reviewed-by: Julien Grall <jgrall@amazon.com>

I will fix the typo pointed out by Andrew whilst committing the series.

Cheers,

> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Changes in v2:
>   - split the v1 patch so that the format specifiers are handled separately
>   - also fix map_irq_to_domain()
> ---
>   xen/arch/arm/domain_build.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9dee1bb8f21c..71f307a572e9 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2265,8 +2265,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
>       res = irq_permit_access(d, irq);
>       if ( res )
>       {
> -        printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -               d->domain_id, irq);
> +        printk(XENLOG_ERR "Unable to permit to %pd access to IRQ %u\n", d, irq);
>           return res;
>       }
>   
> @@ -2282,8 +2281,7 @@ int __init map_irq_to_domain(struct domain *d, unsigned int irq,
>           res = route_irq_to_guest(d, irq, irq, devname);
>           if ( res < 0 )
>           {
> -            printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> -                   irq, d->domain_id);
> +            printk(XENLOG_ERR "Unable to map IRQ%u to %pd\n", irq, d);
>               return res;
>           }
>       }
> @@ -2303,8 +2301,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>   
>       if ( irq < NR_LOCAL_IRQS )
>       {
> -        printk(XENLOG_ERR "%s: IRQ%"PRId32" is not a SPI\n",
> -               dt_node_name(dev), irq);
> +        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n", dt_node_name(dev), irq);
>           return -EINVAL;
>       }
>   
> @@ -2312,9 +2309,8 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>       res = irq_set_spi_type(irq, dt_irq->type);
>       if ( res )
>       {
> -        printk(XENLOG_ERR
> -               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
> -               dt_node_name(dev), irq, d->domain_id);
> +        printk(XENLOG_ERR "%s: Unable to setup IRQ%u to %pd\n",
> +               dt_node_name(dev), irq, d);
>           return res;
>       }
>   

-- 
Julien Grall

Re: [PATCH v2 0/2] xen/arm: domain_build: map_{dt_}irq_to_domain() fixes

[Julien Grall]

Hi Michal,

On 11/05/2023 14:02, Michal Orzel wrote:
> Propagate return code correctly + fix the format specifiers.
> 
> Michal Orzel (2):
>    xen/arm: domain_build: Propagate return code of map_irq_to_domain()
>    xen/arm: domain_build: Fix format specifiers in
>      map_{dt_}irq_to_domain()

I have committed the series.

Cheers,

-- 
Julien Grall