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.

Take the opportunity to use the correct printk() format specifiers,
since both irq and domain id are of unsigned types.

Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/domain_build.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f80fdd1af206..2c14718bff87 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2303,7 +2303,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",
+        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n",
                dt_node_name(dev), irq);
         return -EINVAL;
     }
@@ -2313,14 +2313,14 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
     if ( res )
     {
         printk(XENLOG_ERR
-               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
+               "%s: Unable to setup IRQ%u to dom%u\n",
                dt_node_name(dev), irq, d->domain_id);
         return res;
     }
 
     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

Hi Michal,

On 11/05/2023 12:25, 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.
> 
> Take the opportunity to use the correct printk() format specifiers,
> since both irq and domain id are of unsigned types.

I would rather prefer if this is split in a separate patch because while 
we want to backport the first part, I don't think the latter wants to be.

> 
> Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
>   xen/arch/arm/domain_build.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f80fdd1af206..2c14718bff87 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2303,7 +2303,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",
> +        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n",
>                  dt_node_name(dev), irq);
>           return -EINVAL;
>       }
> @@ -2313,14 +2313,14 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>       if ( res )
>       {
>           printk(XENLOG_ERR
> -               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
> +               "%s: Unable to setup IRQ%u to dom%u\n",
>                  dt_node_name(dev), irq, d->domain_id);

Please switch %pd when printing the domain.

>           return res;
>       }
>   
>       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,

Cheers,

-- 
Julien Grall

Hi Julien,

On 11/05/2023 13:55, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 11/05/2023 12:25, 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.
>>
>> Take the opportunity to use the correct printk() format specifiers,
>> since both irq and domain id are of unsigned types.
> 
> I would rather prefer if this is split in a separate patch because while
> we want to backport the first part, I don't think the latter wants to be.
Sure. I will then fix specifiers in both map_dt_irq_to_domain and map_irq_to_domain.

> 
>>
>> Fixes: 467e5cbb2ffc ("xen: arm: consolidate mmio and irq mapping to dom0")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>>   xen/arch/arm/domain_build.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index f80fdd1af206..2c14718bff87 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2303,7 +2303,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",
>> +        printk(XENLOG_ERR "%s: IRQ%u is not a SPI\n",
>>                  dt_node_name(dev), irq);
>>           return -EINVAL;
>>       }
>> @@ -2313,14 +2313,14 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
>>       if ( res )
>>       {
>>           printk(XENLOG_ERR
>> -               "%s: Unable to setup IRQ%"PRId32" to dom%d\n",
>> +               "%s: Unable to setup IRQ%u to dom%u\n",
>>                  dt_node_name(dev), irq, d->domain_id);
> 
> Please switch %pd when printing the domain.
ok

~Michal