Re: [PATCH v2 5/6] xen/arm: Remove processor specific setup in vcpu_initialise

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2014-03-05 at 12:46 +0800, Julien Grall wrote:
> This patch introduces the possibility to have specific processor callbacks
> that can be called in various place.
> 
> Currently VCPU initialisation code contains processor specific setup (for
> Cortex A7 and Cortex A15) for the ACTRL registers. It's possible to have
> processor with a different layout for this register.
> 
> Move this setup in a specific callback for ARM v7 processor.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: marc.ceeeee@gmail.com

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>     Changes in v2:
>         - Rename proc-v7-c.c in proc-caxx.c
>         - s/initialize/initialise/ to uniformize with the other name in
>         Xen
>         - define ACTLR_SMP in proc-caxx.c and remove ACTLR_V7_SMP in the
>         hearders
> Marc, IIIRC you said that Broadcom Brahma-15 is compatible Cortex-A15.
> Can you check this patch is valid for this processor?

I'm going to take silence as being golden when I come to apply this
(next time through my queue).

> ---
>  xen/arch/arm/Makefile          |  1 +
>  xen/arch/arm/arm32/Makefile    |  2 +-
>  xen/arch/arm/arm32/proc-caxx.c | 35 ++++++++++++++++++++++++++++++
>  xen/arch/arm/arm32/proc-v7.S   |  3 +++
>  xen/arch/arm/domain.c          |  8 ++-----
>  xen/arch/arm/processor.c       | 49 ++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/setup.c           |  3 +++
>  xen/include/asm-arm/procinfo.h | 17 +++++++++++++--
>  8 files changed, 109 insertions(+), 9 deletions(-)
>  create mode 100644 xen/arch/arm/arm32/proc-caxx.c
>  create mode 100644 xen/arch/arm/processor.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index d70f6d5..63e0460 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -32,6 +32,7 @@ obj-y += vuart.o
>  obj-y += hvm.o
>  obj-y += device.o
>  obj-y += decode.o
> +obj-y += processor.o
>  
>  #obj-bin-y += ....o
>  
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 65ecff4..df0e7de 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -1,7 +1,7 @@
>  subdir-y += lib
>  
>  obj-y += entry.o
> -obj-y += proc-v7.o
> +obj-y += proc-v7.o proc-caxx.o
>  
>  obj-y += traps.o
>  obj-y += domain.o
> diff --git a/xen/arch/arm/arm32/proc-caxx.c b/xen/arch/arm/arm32/proc-caxx.c
> new file mode 100644
> index 0000000..9166a1d
> --- /dev/null
> +++ b/xen/arch/arm/arm32/proc-caxx.c
> @@ -0,0 +1,35 @@
> +/*
> + * xen/arch/arm/arm32/proc-caxx.c
> + *
> + * arm V7 Cortex A15 and A7 initialisation
> + *
> + * Julien Grall <julien.grall@linaro.org>
> + * Copyright (c) 2014 Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <asm/procinfo.h>
> +#include <asm/processor.h>
> +
> +#define ACTLR_SMP (1 << 6)
> +
> +static void caxx_vcpu_initialise(struct vcpu *v)
> +{
> +    /* If the guest has more 1 VCPU, enable the SMP bit in ACTLR */
> +    if ( v->domain->max_vcpus > 1 )
> +        v->arch.actlr |= ACTLR_SMP;
> +    else
> +        v->arch.actlr &= ~ACTLR_SMP;
> +}
> +
> +const struct processor caxx_processor = {
> +    .vcpu_initialise = caxx_vcpu_initialise,
> +};
> diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
> index 8fb42db..b0d5343 100644
> --- a/xen/arch/arm/arm32/proc-v7.S
> +++ b/xen/arch/arm/arm32/proc-v7.S
> @@ -35,6 +35,7 @@ __v7_ca15mp_proc_info:
>          .long 0x410FC0F0             /* Cortex-A15 */
>          .long 0xFF0FFFF0             /* Mask */
>          .long ca15mp_init
> +        .long caxx_processor
>          .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
>  
>          .section ".init.proc.info", #alloc, #execinstr
> @@ -43,6 +44,7 @@ __v7_ca7mp_proc_info:
>          .long 0x410FC070             /* Cortex-A7 */
>          .long 0xFF0FFFF0             /* Mask */
>          .long ca7mp_init
> +        .long caxx_processor
>          .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
>  
>          .section ".init.proc.info", #alloc, #execinstr
> @@ -51,6 +53,7 @@ __v7_brahma15mp_proc_info:
>          .long 0x420F00F2             /* Broadcom Brahma-B15 */
>          .long 0xFF0FFFFF             /* Mask */
>          .long brahma15mp_init
> +        .long caxx_processor
>          .size __v7_brahma15mp_proc_info, . - __v7_brahma15mp_proc_info
>  
>  /*
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 8f20fdf..82a1e79 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -28,7 +28,7 @@
>  #include <asm/irq.h>
>  #include <asm/cpufeature.h>
>  #include <asm/vfp.h>
> -#include <asm/processor-ca15.h>
> +#include <asm/procinfo.h>
>  
>  #include <asm/gic.h>
>  #include <asm/platform.h>
> @@ -480,11 +480,7 @@ int vcpu_initialise(struct vcpu *v)
>  
>      v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
>  
> -    /* XXX: Handle other than CA15 cpus */
> -    if ( v->domain->max_vcpus > 1 )
> -        v->arch.actlr |= ACTLR_CA15_SMP;
> -    else
> -        v->arch.actlr &= ~ACTLR_CA15_SMP;
> +    processor_vcpu_initialise(v);
>  
>      if ( (rc = vcpu_vgic_init(v)) != 0 )
>          return rc;
> diff --git a/xen/arch/arm/processor.c b/xen/arch/arm/processor.c
> new file mode 100644
> index 0000000..8c425ce
> --- /dev/null
> +++ b/xen/arch/arm/processor.c
> @@ -0,0 +1,49 @@
> +/*
> + * xen/arch/arm/processor.c
> + *
> + * Helpers to execute processor specific code.
> + *
> + * Julien Grall <julien.grall@linaro.org>
> + * Copyright (C) 2014 Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <asm/procinfo.h>
> +
> +static const struct processor *processor = NULL;
> +
> +void __init processor_setup(void)
> +{
> +    const struct proc_info_list *procinfo;
> +
> +    procinfo = lookup_processor_type();
> +    if ( !procinfo )
> +        return;
> +
> +    processor = procinfo->processor;
> +}
> +
> +void processor_vcpu_initialise(struct vcpu *v)
> +{
> +    if ( !processor || !processor->vcpu_initialise )
> +        return;
> +
> +    processor->vcpu_initialise(v);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 1f6d713..5529cb1 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -42,6 +42,7 @@
>  #include <asm/gic.h>
>  #include <asm/cpufeature.h>
>  #include <asm/platform.h>
> +#include <asm/procinfo.h>
>  
>  struct cpuinfo_arm __read_mostly boot_cpu_data;
>  
> @@ -148,6 +149,8 @@ static void __init processor_id(void)
>      {
>          printk("32-bit Execution: Unsupported\n");
>      }
> +
> +    processor_setup();
>  }
>  
>  static void dt_unreserved_regions(paddr_t s, paddr_t e,
> diff --git a/xen/include/asm-arm/procinfo.h b/xen/include/asm-arm/procinfo.h
> index 9d3feb7..26306b3 100644
> --- a/xen/include/asm-arm/procinfo.h
> +++ b/xen/include/asm-arm/procinfo.h
> @@ -21,10 +21,23 @@
>  #ifndef __ASM_ARM_PROCINFO_H
>  #define __ASM_ARM_PROCINFO_H
>  
> +#include <xen/sched.h>
> +
> +struct processor {
> +    /* Initialize specific processor register for the new VPCU*/
> +    void (*vcpu_initialise)(struct vcpu *v);
> +};
> +
>  struct proc_info_list {
> -	unsigned int		cpu_val;
> -	unsigned int		cpu_mask;
> +    unsigned int        cpu_val;
> +    unsigned int        cpu_mask;
>      void                (*cpu_init)(void);
> +    struct processor    *processor;
>  };
>  
> +const __init struct proc_info_list *lookup_processor_type(void);
> +
> +void __init processor_setup(void);
> +void processor_vcpu_initialise(struct vcpu *v);
> +
>  #endif