Re: [Qemu-devel] [PATCH 1/7] hw/mips_cmgcr: allow GCR base to be moved

[Leon Alrae]

On Fri, Aug 19, 2016 at 08:08:57PM +0100, Paul Burton wrote:
> Support moving the GCR base address & updating the CPU's CP0 CMGCRBase
> register appropriately. This is required if a platform needs to move its
> GCRs away from other memory, as the MIPS Boston development board does
> to avoid its flash memory.
> 
> Signed-off-by: Paul Burton <address@hidden>
> ---
>  hw/misc/mips_cmgcr.c         | 17 +++++++++++++++++
>  include/hw/misc/mips_cmgcr.h |  3 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/misc/mips_cmgcr.c b/hw/misc/mips_cmgcr.c
> index b3ba166..a1edb53 100644
> --- a/hw/misc/mips_cmgcr.c
> +++ b/hw/misc/mips_cmgcr.c
> @@ -29,6 +29,20 @@ static inline bool is_gic_connected(MIPSGCRState *s)
>      return s->gic_mr != NULL;
>  }
>  
> +static inline void update_gcr_base(MIPSGCRState *gcr, uint64_t val)
> +{
> +    CPUState *cpu;
> +    MIPSCPU *mips_cpu;
> +
> +    gcr->gcr_base = val & GCR_BASE_GCRBASE_MSK;
> +    memory_region_set_address(&gcr->iomem, gcr->gcr_base);
> +
> +    CPU_FOREACH(cpu) {
> +        mips_cpu = MIPS_CPU(cpu);
> +        mips_cpu->env.CP0_CMGCRBase = gcr->gcr_base >> 4;
> +    }
> +}
> +
>  static inline void update_cpc_base(MIPSGCRState *gcr, uint64_t val)
>  {
>      if (is_cpc_connected(gcr)) {
> @@ -117,6 +131,9 @@ static void gcr_write(void *opaque, hwaddr addr, uint64_t 
> data, unsigned size)
>      MIPSGCRVPState *other_vps = &gcr->vps[current_vps->other];
>  
>      switch (addr) {
> +    case GCR_BASE_OFS:
> +        update_gcr_base(gcr, data);
> +        break;
>      case GCR_GIC_BASE_OFS:
>          update_gic_base(gcr, data);
>          break;
> diff --git a/include/hw/misc/mips_cmgcr.h b/include/hw/misc/mips_cmgcr.h
> index a209d91..31bda6a 100644
> --- a/include/hw/misc/mips_cmgcr.h
> +++ b/include/hw/misc/mips_cmgcr.h
> @@ -41,6 +41,9 @@
>  #define GCR_L2_CONFIG_BYPASS_SHF    20
>  #define GCR_L2_CONFIG_BYPASS_MSK    ((0x1ULL) << GCR_L2_CONFIG_BYPASS_SHF)
>  
> +/* GCR_BASE register fields */
> +#define GCR_BASE_GCRBASE_MSK     0xffffffff8000ULL;

Unnecessary semicolon?

Otherwise

Reviewed-by: Leon Alrae <address@hidden>