diff mbox series

[Xen-devel,for-4.12] xen/arm: domain_build: Require the property "cpus" when building a domU

Message ID 20190218100345.30383-1-julien.grall@arm.com
State Superseded
Headers show
Series [Xen-devel,for-4.12] xen/arm: domain_build: Require the property "cpus" when building a domU | expand

Commit Message

Julien Grall Feb. 18, 2019, 10:03 a.m. UTC
The 3rd argument of function dt_property_read_u32() is only valid when
the call succeeded. So we cannot assume the value will not be modifed
in case of failure.

The documentation of Dom0less does not give a default value when the
property "cpus" is not set. So require the property in the configuration.

Coverity-ID: 1476825
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Andrew Cooper Feb. 18, 2019, 10:06 a.m. UTC | #1
On 18/02/2019 10:03, Julien Grall wrote:
> The 3rd argument of function dt_property_read_u32() is only valid when
> the call succeeded. So we cannot assume the value will not be modifed
> in case of failure.
>
> The documentation of Dom0less does not give a default value when the
> property "cpus" is not set. So require the property in the configuration.
>
> Coverity-ID: 1476825
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/domain_build.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 31af989e63..0d93042c5e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2069,7 +2069,6 @@ void __init create_domUs(void)
>              .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
>              .arch.nr_spis = 0,
>              .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap,
> -            .max_vcpus = 1,
>              .max_evtchn_port = -1,
>              .max_grant_frames = 64,
>              .max_maptrack_frames = 1024,
> @@ -2080,7 +2079,9 @@ void __init create_domUs(void)
>  
>          if ( dt_property_read_bool(node, "vpl011") )
>              d_cfg.arch.nr_spis = GUEST_VPL011_SPI - 32 + 1;
> -        dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus);
> +
> +        if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) )
> +            panic("Missing property 'cpus' for domain %s", dt_node_name(node));

Panic needs a \n at the end, now that I went and fixed up the 50/50
split we had of styles.

~Andrew

>  
>          d = domain_create(++max_init_domid, &d_cfg, false);
>          if ( IS_ERR(d) )
Julien Grall Feb. 18, 2019, 10:09 a.m. UTC | #2
Hi Andrew,

On 18/02/2019 10:06, Andrew Cooper wrote:
> On 18/02/2019 10:03, Julien Grall wrote:
>> The 3rd argument of function dt_property_read_u32() is only valid when
>> the call succeeded. So we cannot assume the value will not be modifed
>> in case of failure.
>>
>> The documentation of Dom0less does not give a default value when the
>> property "cpus" is not set. So require the property in the configuration.
>>
>> Coverity-ID: 1476825
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/domain_build.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 31af989e63..0d93042c5e 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2069,7 +2069,6 @@ void __init create_domUs(void)
>>               .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
>>               .arch.nr_spis = 0,
>>               .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap,
>> -            .max_vcpus = 1,
>>               .max_evtchn_port = -1,
>>               .max_grant_frames = 64,
>>               .max_maptrack_frames = 1024,
>> @@ -2080,7 +2079,9 @@ void __init create_domUs(void)
>>   
>>           if ( dt_property_read_bool(node, "vpl011") )
>>               d_cfg.arch.nr_spis = GUEST_VPL011_SPI - 32 + 1;
>> -        dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus);
>> +
>> +        if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) )
>> +            panic("Missing property 'cpus' for domain %s", dt_node_name(node));
> 
> Panic needs a \n at the end, now that I went and fixed up the 50/50
> split we had of styles.

I was not sure about it. It looks like we introduce new panic without the 
newline in this function. Hence the confusion.

I will send a patch for it.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 31af989e63..0d93042c5e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2069,7 +2069,6 @@  void __init create_domUs(void)
             .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
             .arch.nr_spis = 0,
             .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap,
-            .max_vcpus = 1,
             .max_evtchn_port = -1,
             .max_grant_frames = 64,
             .max_maptrack_frames = 1024,
@@ -2080,7 +2079,9 @@  void __init create_domUs(void)
 
         if ( dt_property_read_bool(node, "vpl011") )
             d_cfg.arch.nr_spis = GUEST_VPL011_SPI - 32 + 1;
-        dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus);
+
+        if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) )
+            panic("Missing property 'cpus' for domain %s", dt_node_name(node));
 
         d = domain_create(++max_init_domid, &d_cfg, false);
         if ( IS_ERR(d) )