Re: [Qemu-devel] [QEMU-PPC] [PATCH] target/ppc/spapr: Clear partition table entry when allocating hash table

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

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

On Tue, Mar 05, 2019 at 01:21:02PM +1100, Suraj Jitindar Singh wrote:
> If we allocate a hash page table then we know that the guest won't be
> using process tables, so set the partition table entry maintained for
> the guest to zero. If this isn't done, then the guest radix bit will
> remain set in the entry. This means that when the guest calls
> H_REGISTER_PROCESS_TABLE there will be a mismatch between then flags
> and the value in spapr->patb_entry, and the call will fail. The guest
> will then panic:
> 
> Failed to register process table (rc=-4)
> kernel BUG at arch/powerpc/platforms/pseries/lpar.c:959
> 
> The result being that it isn't possible to boot a hash guest on a P9
> system.
> 
> Also fix a bug in the flags parsing in h_register_process_table() which
> was introduced by the same patch, and simplify the handling to make it
> less likely that errors will be introduced in the future. The effect
> would have been setting the host radix bit LPCR_HR for a hash guest
> using process tables, which currently isn't supported and so couldn't
> have been triggered.
> 
> Fixes: 00fd075e18 "target/ppc/spapr: Set LPCR:HR when using Radix mode"
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied, thanks.

> ---
>  hw/ppc/spapr.c       |  1 +
>  hw/ppc/spapr_hcall.c | 12 ++++++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e39068578e..cf1ef9ebd4 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1632,6 +1632,7 @@ void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
>          }
>      }
>      /* We're setting up a hash table, so that means we're not radix */
> +    spapr->patb_entry = 0;
>      spapr_set_all_lpcrs(0, LPCR_HR | LPCR_UPRT);
>  }
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8bfdddc964..7016a09386 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1339,6 +1339,7 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
>      target_ulong proc_tbl = args[1];
>      target_ulong page_size = args[2];
>      target_ulong table_size = args[3];
> +    target_ulong update_lpcr = 0;
>      uint64_t cproc;
>  
>      if (flags & ~FLAGS_MASK) { /* Check no reserved bits are set */
> @@ -1394,10 +1395,13 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
>      spapr->patb_entry = cproc; /* Save new process table */
>  
>      /* Update the UPRT, HR and GTSE bits in the LPCR for all cpus */
> -    spapr_set_all_lpcrs(((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ?
> -                         (LPCR_UPRT | LPCR_HR) : 0) |
> -                        ((flags & FLAG_GTSE) ? LPCR_GTSE : 0),
> -                        LPCR_UPRT | LPCR_HR | LPCR_GTSE);
> +    if (flags & FLAG_RADIX)     /* Radix must use process tables, also set HR */
> +        update_lpcr |= (LPCR_UPRT | LPCR_HR);
> +    else if (flags & FLAG_HASH_PROC_TBL) /* Hash with process tables */
> +        update_lpcr |= LPCR_UPRT;
> +    if (flags & FLAG_GTSE)      /* Guest translation shootdown enable */
> +        update_lpcr |= FLAG_GTSE;
> +    spapr_set_all_lpcrs(update_lpcr, LPCR_UPRT | LPCR_HR | LPCR_GTSE);
>  
>      if (kvm_enabled()) {
>          return kvmppc_configure_v3_mmu(cpu, flags & FLAG_RADIX,

-- 
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: 833 bytes --]