Re: [Qemu-devel] [PATCH] ppc: Add/Re-introduce MMU model definitions needed by PR KVM

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 8222 bytes --]

On Tue, Nov 10, 2015 at 10:59:45AM +0530, Bharata B Rao wrote:
> On Tue, Nov 10, 2015 at 12:13:50PM +1100, Alexey Kardashevskiy wrote:
> > On 11/07/2015 08:12 AM, Benjamin Herrenschmidt wrote:
> > >On Fri, 2015-11-06 at 13:12 +0530, Bharata B Rao wrote:
> > >>Commit aa4bb5875231 (ppc: Add mmu_model defines for arch 2.03 and
> > >>2.07)
> > >>removed the mmu_model definition POWERPC_MMU_2_06a which is needed by
> > >>PR KVM. Reintroduce it and also add POWERPC_MMU_2_07a.
> > >>
> > >>This fixes QEMU crash (qemu: fatal: Unknown MMU model) during booting
> > >>of PR KVM guest.
> > >
> > >Hrm, I see... we clear the 1TSEG bit and that causes the switch/cases
> > >to no long work. Argh....
> > >
> > >We should clean up that junk. We are mixing up bit masks and an actual
> > >model "number" in the same field. We should make that cleaner, using
> > >a mask to extract the actual version and switch/case on *that*...
> > 
> > 
> > I like this and I wonder if Bharata is going to do this, if not, I will, I
> > just noticed this this patch made it to the dwg/spapr-next tree so we need
> > to hurry...
> > 
> > Bharata, got some time for this? Thanks.
> 
> I can only get to this tomorrow, so if it is urgent please feel free
> to work on this.
> 
> Meanwhile I have gotten till this point, very lightly tested though
> and patch description needs update.

This looks like a good start on a cleanup, but I'm disinclined to
merge it now, right before the hard freeze.  The mixing of model IDs
and feature bits is ugly, but it's been there for a long while - we
can cope with it for a bit longer.

> 
> Regards,
> Bharata.
> 
> ppc: Add/Re-introduce MMU model definitions needed by PR KVM
> 
> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> Commit aa4bb5875231 (ppc: Add mmu_model defines for arch 2.03 and 2.07)
> removed the mmu_model definition POWERPC_MMU_2_06a which is needed by
> PR KVM. Reintroduce it and also add POWERPC_MMU_2_07a.
> 
> This fixes QEMU crash (qemu: fatal: Unknown MMU model) during booting
> of PR KVM guest.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/cpu.h            |   25 +++++++++++++++----------
>  target-ppc/mmu_helper.c     |    8 ++++----
>  target-ppc/translate_init.c |   11 +++++++----
>  3 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index b34aed6..2c4a10a 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -88,6 +88,17 @@
>  
>  /*****************************************************************************/
>  /* MMU model                                                                 */
> +
> +#if defined(TARGET_PPC64)
> +#define POWERPC_MMU_64       0x00010000
> +#define POWERPC_MMU_1TSEG    0x00020000
> +#define POWERPC_MMU_AMR      0x00040000
> +#define POWERPC_MMU_MASK     ~(POWERPC_MMU_64 | POWERPC_MMU_1TSEG | \
> +                               POWERPC_MMU_AMR)
> +#else
> +#define POWERPC_MMU_MASK     ~0
> +#endif
> +
>  typedef enum powerpc_mmu_t powerpc_mmu_t;
>  enum powerpc_mmu_t {
>      POWERPC_MMU_UNKNOWN    = 0x00000000,
> @@ -112,19 +123,13 @@ enum powerpc_mmu_t {
>      /* PowerPC 601 MMU model (specific BATs format)            */
>      POWERPC_MMU_601        = 0x0000000A,
>  #if defined(TARGET_PPC64)
> -#define POWERPC_MMU_64       0x00010000
> -#define POWERPC_MMU_1TSEG    0x00020000
> -#define POWERPC_MMU_AMR      0x00040000
>      /* 64 bits PowerPC MMU                                     */
> -    POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
> -    /* Architecture 2.03 and later (has LPCR) */
> -    POWERPC_MMU_2_03       = POWERPC_MMU_64 | 0x00000002,
> +    POWERPC_MMU_64B        = 0x0000000B,
> +    POWERPC_MMU_2_03       = 0x0000000C,
>      /* Architecture 2.06 variant                               */
> -    POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> -                             | POWERPC_MMU_AMR | 0x00000003,
> +    POWERPC_MMU_2_06       = 0x0000000D,
>      /* Architecture 2.07 variant                               */
> -    POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> -                             | POWERPC_MMU_AMR | 0x00000004,
> +    POWERPC_MMU_2_07       = 0x0000000E,
>  #endif /* defined(TARGET_PPC64) */
>  };
>  
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index e52d0e5..9dead4b 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -1280,7 +1280,7 @@ static void mmu6xx_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
>  
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env)
>  {
> -    switch (env->mmu_model) {
> +    switch (env->mmu_model & POWERPC_MMU_MASK) {
>      case POWERPC_MMU_BOOKE:
>          mmubooke_dump_mmu(f, cpu_fprintf, env);
>          break;
> @@ -1430,7 +1430,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>      CPUPPCState *env = &cpu->env;
>      mmu_ctx_t ctx;
>  
> -    switch (env->mmu_model) {
> +    switch (env->mmu_model & POWERPC_MMU_MASK) {
>  #if defined(TARGET_PPC64)
>      case POWERPC_MMU_64B:
>      case POWERPC_MMU_2_03:
> @@ -1911,7 +1911,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
>  {
>      PowerPCCPU *cpu = ppc_env_get_cpu(env);
>  
> -    switch (env->mmu_model) {
> +    switch (env->mmu_model & POWERPC_MMU_MASK) {
>      case POWERPC_MMU_SOFT_6xx:
>      case POWERPC_MMU_SOFT_74xx:
>          ppc6xx_tlb_invalidate_all(env);
> @@ -1957,7 +1957,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
>      CPUState *cs;
>  
>      addr &= TARGET_PAGE_MASK;
> -    switch (env->mmu_model) {
> +    switch (env->mmu_model & POWERPC_MMU_MASK) {
>      case POWERPC_MMU_SOFT_6xx:
>      case POWERPC_MMU_SOFT_74xx:
>          ppc6xx_tlb_invalidate_virt(env, addr, 0);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 4934c80..a19aa32 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7967,7 +7967,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
>                      (1ull << MSR_DR) |
>                      (1ull << MSR_PMM) |
>                      (1ull << MSR_RI);
> -    pcc->mmu_model = POWERPC_MMU_64B;
> +    pcc->mmu_model = POWERPC_MMU_64B | POWERPC_MMU_64;
>  #if defined(CONFIG_SOFTMMU)
>      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>  #endif
> @@ -8020,7 +8020,8 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
>                      (1ull << MSR_DR) |
>                      (1ull << MSR_PMM) |
>                      (1ull << MSR_RI);
> -    pcc->mmu_model = POWERPC_MMU_2_03;
> +    /* Architecture 2.03 and later (has LPCR) */
> +    pcc->mmu_model = POWERPC_MMU_2_03 | POWERPC_MMU_64;
>  #if defined(CONFIG_SOFTMMU)
>      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>  #endif
> @@ -8164,7 +8165,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>                      (1ull << MSR_PMM) |
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
> -    pcc->mmu_model = POWERPC_MMU_2_06;
> +    pcc->mmu_model = POWERPC_MMU_2_06 | POWERPC_MMU_64 | POWERPC_MMU_1TSEG |
> +                     POWERPC_MMU_AMR;
>  #if defined(CONFIG_SOFTMMU)
>      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>  #endif
> @@ -8244,7 +8246,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                      (1ull << MSR_PMM) |
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
> -    pcc->mmu_model = POWERPC_MMU_2_07;
> +    pcc->mmu_model = POWERPC_MMU_2_07 | POWERPC_MMU_64 | POWERPC_MMU_1TSEG |
> +                     POWERPC_MMU_AMR;
>  #if defined(CONFIG_SOFTMMU)
>      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>  #endif
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]