Re: [Qemu-ppc] [PATCH V2 10/10] target/ppc: Enable RADIX for pseries TCG guest

[David Gibson]

On Wed, Mar 01, 2017 at 06:13:01PM +1100, Suraj Jitindar Singh wrote:
> Now that we have added all the infrastructure we can enable a pseries TCG
> guest to use radix.
> 
> We make the assumption that since we can support radix, a guest will choose
> radix, so set the guest radix bit in the partition table entry
> unconditionally. This also represents that we haven't allocated an hpt.
> 
> We must set spapr->htab_shift even if we aren't going to allocate an hpt as
> we used this to set the pft-size device tree entry which the guest uses to
> determine the pte_index mask in the event it decides to use a hash mmu.
> 
> During machine reset if we're on kvm and the hypervisor doesn't have radix
> capability then we have to allocate a hpt, under TCG we can delay this
> until CAS time.
> NOTE: Allocating an hpt clears the guest radix bit in patb_entry.
> 
> During machine init we unconditionally allow GTSE on radix in TCG and
> on KVM if the hypervisor supports radix.
> 
> When creating the device tree ibm,arch-vec-5-platform-support node we set
> both hash and radix supported if we're on a ISAV3.00 capable processor,
> otherwise we set hash only.
> 
> During CAS we check if we previously assumed radix (Guest Radix bit set in
> patb_entry) and the guest decided to do hash at which point we have to
> allocate a hpt.
> 
> Finally we call the radix fault handler if we're a radix guest instead of
> exiting enabling address translation for a radix guest.

Not sure about the patch itself, but looks as if these comments at
least cover some of the same ground as the CAS stuff in Sam's KVM
series - so looks like a little more polish / comment updating is
necessary to make the series completely work together.

> 
> Signed-off-by: Suraj Jitindar Singh <address@hidden>
> 
> ---
> 
> V1 -> V2:
>  - Added patch to series
> ---
>  hw/ppc/spapr.c             | 39 +++++++++++++++++++++++++++------------
>  hw/ppc/spapr_hcall.c       | 13 +++++++------
>  target/ppc/mmu-book3s-v3.c |  6 ++----
>  3 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f6155a6..adc107e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -850,6 +850,8 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void 
> *fdt)
>   * option vector 5: */
>  static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
>  {
> +    PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> +
>      char val[2 * 3] = {
>          24, 0x00, /* Hash/Radix, filled in below. */
>          25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
> @@ -866,8 +868,12 @@ static void spapr_dt_ov5_platform_support(void *fdt, int 
> chosen)
>              val[1] = 0x00; /* Hash */
>          }
>      } else {
> -        /* TODO: TCG case, hash */
> -        val[1] = 0x00;
> +        if (first_ppc_cpu->env.mmu_model & POWERPC_MMU_V3) {
> +            /* Either - TCG supports both so let the guest choose */
> +            val[1] = 0x80;
> +        } else {
> +            val[0] = 0x00; /* Hash */
> +        }
>      }
>      _FDT(fdt_setprop(fdt, chosen, "ibm,arch-vec-5-platform-support",
>                       val, sizeof(val)));
> @@ -1301,15 +1307,24 @@ static void ppc_spapr_reset(void)
>      /* Check for unknown sysbus devices */
>      foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
>  
> -    if (kvm_enabled()) { /* We assume Radix under KVM */
> -        spapr->patb_entry = PATBE1_GR;
> -    } else {
> -        spapr->patb_entry = 0;
> -    }
> +    /*
> +     * Assume we're radix, this is fine since in real mode we don't care.
> +     * Either we were right and the guest registers a radix process table,
> +     * the host doesn't support radix so we allocate a hpt directly below,
> +     * or finally the guest can choose hash at H_CAS or H_REGISTER_PROC_TBL
> +     * time in which case we allocate an hpt in the H_CALL handler if one
> +     * hasn't already been allocated previously.
> +     */
> +    spapr->patb_entry = PATBE1_GR;
>  
> -    /* If using KVM with radix mode available, VCPUs can be started
> -     * without a HPT because KVM will start them in radix mode. */
> -    if (!(kvm_enabled() && kvmppc_has_cap_mmu_radix())) {
> +    /*
> +     * If using KVM with radix mode available, VCPUs can be started
> +     * without a HPT because KVM will start them in radix mode.
> +     * In tcg we can always do radix, so don't allocate an hpt
> +     */
> +    spapr->htab_shift = spapr_hpt_shift_for_ramsize(MACHINE(
> +                                qdev_get_machine())->maxram_size);
> +    if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> @@ -2093,8 +2108,8 @@ static void ppc_spapr_init(MachineState *machine)
>      }
>  
>      spapr_ovec_set(spapr->ov5, OV5_FORM1_AFFINITY);
> -    if (kvmppc_has_cap_mmu_radix()) {
> -        /* KVM always allows GTSE with radix... */
> +    if (!kvm_enabled() || kvmppc_has_cap_mmu_radix()) {
> +        /* KVM always allows GTSE with radix, so does TCG */

Comment here is unclear: Does TCG *always* allow GTSE, or always allow
GTSE when in radix mode?

>          spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);

So.. where's the TCG implementation of GTSE?

>      }
>      /* ... but not with hash (currently). */
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index d86e553..9416ab4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1142,8 +1142,11 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>      ov5_updates = spapr_ovec_new();
>      spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
>                                          ov5_cas_old, spapr->ov5_cas);
> -    /* Now that processing is finished, set the radix/hash bit for the
> -     * guest if it requested a valid mode; otherwise terminate the boot. */
> +    /*
> +     * Now that processing is finished, set the radix/hash bit for the
> +     * guest if it requested a valid mode; otherwise terminate the boot.
> +     * NOTE: TCG supports both, so the boot is never terminated
> +     */
>      if (guest_radix) {
>          if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {
>              error_report("qemu: Guest requested radix MMU mode when it is 
> not available.");
> @@ -1169,10 +1172,8 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>      } else {
>          /* If ppc_spapr_reset() did not set up a HPT but one is necessary
>           * (because the guest isn't going to use radix) then set it up here. 
> */
> -        if (kvm_enabled()) {
> -            if (kvmppc_has_cap_mmu_radix() && !guest_radix) {
> -                spapr_setup_hpt_and_vrma(spapr);
> -            }
> +        if (ppc64_radix_guest(cpu) && !guest_radix) {
> +            spapr_setup_hpt_and_vrma(spapr);
>          }
>      }
>  
> diff --git a/target/ppc/mmu-book3s-v3.c b/target/ppc/mmu-book3s-v3.c
> index 005c963..e7798b3 100644
> --- a/target/ppc/mmu-book3s-v3.c
> +++ b/target/ppc/mmu-book3s-v3.c
> @@ -22,15 +22,13 @@
>  #include "cpu.h"
>  #include "mmu-hash64.h"
>  #include "mmu-book3s-v3.h"
> -#include "qemu/error-report.h"
> +#include "mmu-radix64.h"
>  
>  int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>                                int mmu_idx)
>  {
>      if (ppc64_radix_guest(cpu)) { /* Guest uses radix */
> -        /* TODO - Unsupported */
> -        error_report("Guest Radix Support Unimplemented");
> -        exit(1);
> +        return ppc_radix64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
>      } else { /* Guest uses hash */
>          return ppc_hash64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
>      }

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

signature.asc
Description: PGP signature