From: Julien Grall <jgrall@amazon.com>
The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools
like Eclair will report as a violation of Misra Rule 2.1.
Furthermore, the nested switch is not very easy to read. So move
out the nested switch in a separate function to improve the
readability and hopefully address the MISRA violation.
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
Only compiled tested. Waiting for the CI to confirm there is no
regression.
---
xen/arch/arm/arm64/domctl.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 14fc622e9956..8720d126c97d 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -33,27 +33,31 @@ static long switch_mode(struct domain *d, enum domain_type type)
return 0;
}
+static long set_address_size(struct domain *d, uint32_t address_size)
+{
+ switch ( address_size )
+ {
+ case 32:
+ if ( !cpu_has_el1_32 )
+ return -EINVAL;
+ /* SVE is not supported for 32 bit domain */
+ if ( is_sve_domain(d) )
+ return -EINVAL;
+ return switch_mode(d, DOMAIN_32BIT);
+ case 64:
+ return switch_mode(d, DOMAIN_64BIT);
+ default:
+ return -EINVAL;
+ }
+}
+
long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
switch ( domctl->cmd )
{
case XEN_DOMCTL_set_address_size:
- switch ( domctl->u.address_size.size )
- {
- case 32:
- if ( !cpu_has_el1_32 )
- return -EINVAL;
- /* SVE is not supported for 32 bit domain */
- if ( is_sve_domain(d) )
- return -EINVAL;
- return switch_mode(d, DOMAIN_32BIT);
- case 64:
- return switch_mode(d, DOMAIN_64BIT);
- default:
- return -EINVAL;
- }
- break;
+ return set_address_size(d, domctl->u.address_size.size);
default:
return -ENOSYS;
--
2.40.1
Hi Julien, > On 23 Oct 2023, at 19:52, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools > like Eclair will report as a violation of Misra Rule 2.1. > > Furthermore, the nested switch is not very easy to read. So move > out the nested switch in a separate function to improve the > readability and hopefully address the MISRA violation. > > Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > > --- > > Only compiled tested. Waiting for the CI to confirm there is no > regression. > --- > xen/arch/arm/arm64/domctl.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c > index 14fc622e9956..8720d126c97d 100644 > --- a/xen/arch/arm/arm64/domctl.c > +++ b/xen/arch/arm/arm64/domctl.c > @@ -33,27 +33,31 @@ static long switch_mode(struct domain *d, enum domain_type type) > return 0; > } > > +static long set_address_size(struct domain *d, uint32_t address_size) > +{ > + switch ( address_size ) > + { > + case 32: > + if ( !cpu_has_el1_32 ) > + return -EINVAL; > + /* SVE is not supported for 32 bit domain */ > + if ( is_sve_domain(d) ) > + return -EINVAL; > + return switch_mode(d, DOMAIN_32BIT); > + case 64: > + return switch_mode(d, DOMAIN_64BIT); > + default: > + return -EINVAL; > + } > +} > + > long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d, > XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > { > switch ( domctl->cmd ) > { > case XEN_DOMCTL_set_address_size: > - switch ( domctl->u.address_size.size ) > - { > - case 32: > - if ( !cpu_has_el1_32 ) > - return -EINVAL; > - /* SVE is not supported for 32 bit domain */ > - if ( is_sve_domain(d) ) > - return -EINVAL; > - return switch_mode(d, DOMAIN_32BIT); > - case 64: > - return switch_mode(d, DOMAIN_64BIT); > - default: > - return -EINVAL; > - } > - break; > + return set_address_size(d, domctl->u.address_size.size); > > default: > return -ENOSYS; > -- > 2.40.1 > >
On Tue, 24 Oct 2023, Bertrand Marquis wrote: > Hi Julien, > > > On 23 Oct 2023, at 19:52, Julien Grall <julien@xen.org> wrote: > > > > From: Julien Grall <jgrall@amazon.com> > > > > The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools > > like Eclair will report as a violation of Misra Rule 2.1. > > > > Furthermore, the nested switch is not very easy to read. So move > > out the nested switch in a separate function to improve the > > readability and hopefully address the MISRA violation. > > > > Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> I added the patch to my for-4.19 branch
Hi, I forgot to CC the maintainers. On 23/10/2023 18:52, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools > like Eclair will report as a violation of Misra Rule 2.1. > > Furthermore, the nested switch is not very easy to read. So move > out the nested switch in a separate function to improve the > readability and hopefully address the MISRA violation. > > Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > > Only compiled tested. Waiting for the CI to confirm there is no > regression. > --- > xen/arch/arm/arm64/domctl.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c > index 14fc622e9956..8720d126c97d 100644 > --- a/xen/arch/arm/arm64/domctl.c > +++ b/xen/arch/arm/arm64/domctl.c > @@ -33,27 +33,31 @@ static long switch_mode(struct domain *d, enum domain_type type) > return 0; > } > > +static long set_address_size(struct domain *d, uint32_t address_size) > +{ > + switch ( address_size ) > + { > + case 32: > + if ( !cpu_has_el1_32 ) > + return -EINVAL; > + /* SVE is not supported for 32 bit domain */ > + if ( is_sve_domain(d) ) > + return -EINVAL; > + return switch_mode(d, DOMAIN_32BIT); > + case 64: > + return switch_mode(d, DOMAIN_64BIT); > + default: > + return -EINVAL; > + } > +} > + > long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d, > XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > { > switch ( domctl->cmd ) > { > case XEN_DOMCTL_set_address_size: > - switch ( domctl->u.address_size.size ) > - { > - case 32: > - if ( !cpu_has_el1_32 ) > - return -EINVAL; > - /* SVE is not supported for 32 bit domain */ > - if ( is_sve_domain(d) ) > - return -EINVAL; > - return switch_mode(d, DOMAIN_32BIT); > - case 64: > - return switch_mode(d, DOMAIN_64BIT); > - default: > - return -EINVAL; > - } > - break; > + return set_address_size(d, domctl->u.address_size.size); > > default: > return -ENOSYS; -- Julien Grall
Hi Julien, (+Stefano and Bertrand) > On Oct 24, 2023, at 01:52, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools > like Eclair will report as a violation of Misra Rule 2.1. > > Furthermore, the nested switch is not very easy to read. So move > out the nested switch in a separate function to improve the > readability and hopefully address the MISRA violation. > > Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> > --- > Only compiled tested. Waiting for the CI to confirm there is no > regression. I also tested this patch on top of today’s staging in Arm’s internal CI, and this patch looks good. Tested-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
© 2016 - 2024 Red Hat, Inc.