Re: [Xen-devel] [RFC PATCH] xen/arm: check on domain type against hardware support

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-09-18 at 17:43 +0530, vijay.kilari@xxxxxxxxx wrote:
> @@ -35,7 +36,10 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct 
> domain *d,
>          switch ( domctl->u.address_size.size )
>          {
>          case 32:
> -            return switch_mode(d, DOMAIN_32BIT);
> +            if ( cpu_has_el1_32 )
> +                return switch_mode(d, DOMAIN_32BIT);
> +            else
> +                return -EINVAL;

Please can you separates the error checking from the actual work, e.g.:
                        if ( !cpu_has_el1_32 )
                                return -EINVAL;
                        return switch_mode(...)


> @@ -1274,6 +1275,12 @@ int construct_dom0(struct domain *d)
>          return rc;
>  
>  #ifdef CONFIG_ARM_64
> +    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
> +    if ( (!(cpu_has_el1_32)) && kinfo.type == DOMAIN_32BIT )

You can drop the outer brackets in "(!(cpu_has_el1_32))"

> +    {
> +        printk("Platform does not support 32-bit domain\n");
> +        return -EINVAL;
> +    }
>      d->arch.type = kinfo.type;
>  #endif
>  
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 2e80b5a..9fbd315 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -883,8 +883,11 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
>      snprintf(s, sizeof(s), "xen-%d.%d-aarch64 ", major, minor);
>      safe_strcat(*info, s);
>  #endif
> -    snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
> -    safe_strcat(*info, s);
> +    if ( cpu_has_aarch32 )

This is a bit subtle.

On a v8 processor do we need to check that the cpu has 32-bit support
before we look at this register (i.e. is it guaranteed to be
available/sane on a v8 core which has no 32-bit stuff?)

Secondly, the way cpu_has_aarch32 is currently coded it is only actually
checking for the A32 instruction set, not the T32 one.

arm32 Xen runs in A32 so we could possibly ignore that distinction. But
what about on v8, is T32 without A32 allowed? I suspect not, but the way
the docs are worded makes it a bit unclear: they say "Not support in
ARMv8", but I know you are allowed to build a v8 with no AArch32
support, so I suspect they mean "if you do AArch32 you must do both T32
and A32" (which takes me back to the first question...).

> +    {
> +        snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
> +        safe_strcat(*info, s);
> +    }
>  }
>  
>  /*



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel