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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 11/10/2015 04:29 PM, 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.


No, I am fine if you finish this :)


> Meanwhile I have gotten till this point, very lightly tested though
> and patch description needs update.

imho this looks worse than just adding POWERPC_MMU_2_06a and 
POWERPC_MMU_2_07a back...

I'd rather have "if (env->mmu_model & POWERPC_MMU_64){} else switch 
(env->mmu_model) {}" and remove POWERPC_MMU_64 cases from the switch'es.



>
> 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
>
>


-- 
Alexey