qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH v3 11/12] spapr: Enable ISA 3.0 MMU mode selection


From: David Gibson
Subject: Re: [Qemu-ppc] [PATCH v3 11/12] spapr: Enable ISA 3.0 MMU mode selection via CAS
Date: Wed, 15 Mar 2017 16:19:25 +1100
User-agent: Mutt/1.7.1 (2016-10-04)

On Wed, Mar 15, 2017 at 11:37:42AM +1100, Sam Bobroff wrote:
> On Fri, Mar 03, 2017 at 03:14:00PM +1100, David Gibson wrote:
> > On Thu, Mar 02, 2017 at 04:39:06PM +1100, Sam Bobroff wrote:
> > > Add the new node, /chosen/ibm,arch-vec-5-platform-support to the
> > > device tree. This allows the guest to determine which modes are
> > > supported by the hypervisor.
> > > 
> > > Update the option vector processing in h_client_architecture_support()
> > > to handle the new MMU bits. This allows guests to request hash or
> > > radix mode and QEMU to create the guest's HPT at this time if it is
> > > necessary but hasn't yet been done.  QEMU will terminate the guest if
> > > it requests an unavailable mode, as required by the architecture.
> > > 
> > > Extend the ibm,pa-features node with the new ISA 3.0 values
> > > and set the radix bit if KVM supports radix mode. This probably won't
> > > be used directly by guests to determine the availability of radix mode
> > > (that is indicated by the new node added above) but the architecture
> > > requires that it be set when the hardware supports it.
> > > 
> > > ISA 3.0 guests will now begin to call h_register_process_table(),
> > > which has been added previously.
> > > 
> > > Signed-off-by: Sam Bobroff <address@hidden>
> > > ---
> > > Changes in v3:
> > > * Added some terse documentation for the ibm,pa_features bits for ISA 
> > > 3.00.
> > > * Shortened the error reports for guest termination a little.
> > > 
> > >  hw/ppc/spapr.c              | 72 
> > > +++++++++++++++++++++++++++++++++++++--------
> > >  hw/ppc/spapr_hcall.c        | 36 ++++++++++++++++++-----
> > >  include/hw/ppc/spapr_ovec.h |  5 ++++
> > >  3 files changed, 92 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index c488826405..486ed9c735 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -205,20 +205,31 @@ static void spapr_populate_pa_features(CPUPPCState 
> > > *env, void *fdt, int offset)
> > >          0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> > >          0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> > >          0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
> > > -    /* Currently we don't advertise any of the "new" ISAv3.00 
> > > functionality */
> > > -    uint8_t pa_features_300[] = { 64, 0,
> > > -        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /*  0 -  5 */
> > > -        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /*  6 - 11 */
> > > +    uint8_t pa_features_300[66 + 2] = { 66, 0,
> > 
> > Putting explicit lengths in static arrays like this is usually not a
> > good idea.
> 
> Agreed, but here the length has to appear in the data and I wanted to
> make that clear (the length is "66 + 2" so that it matches the 66 in the
> first byte).

Ah.  Nice thought, but I don't know that the C compiler actually
checks that terribly hard in any case. Better might be an explicit
        QEMU_BUILD_BUG_ON(sizeof(pa_features) != pa_features[0] + 2);
> 
> I don't mind what we do tho. Leave it this way, add a comment or remove
> the length specifier?
> 
> > > +        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: 
> > > fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> > > +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, SSO, 5: 
> > > LE|CFAR|EB|LSQ */
> > > +        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /* 0 - 5 */
> > > +        /* 6: DS207 */
> > > +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> > > +        /* 16: Vector */
> > >          0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> > > -        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 24 - 29 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 - 35 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 36 - 41 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 42 - 47 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 48 - 53 */
> > > -        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 54 - 59 */
> > > -        0x00, 0x00, 0x00, 0x00           }; /* 60 - 63 */
> > > -
> > > +        /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
> > > +        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> > > +        /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
> > > +        0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> > > +        /* 36: SPR SO, 38: Copy/Paste, 40: Radix MMU */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
> > > +        /* 42: PM, 44: PC RA, 46: SC vec'd */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> > > +        /* 48: SIMD, 50: QP BFP, 52: String */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> > > +        /* 54: DecFP, 56: DecI, 58: SHA */
> > > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> > > +        /* 60: NM atomic, 62: RNG */
> > > +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> > > +    };
> > >      uint8_t *pa_features;
> > >      size_t pa_size;
> > >  
> > > @@ -823,6 +834,34 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, 
> > > void *fdt)
> > >      spapr_dt_rtas_tokens(fdt, rtas);
> > >  }
> > >  
> > > +/* Prepare ibm,arch-vec-5-platform-support, which indicates the MMU 
> > > features
> > > + * that the guest may request and thus the valid values for bytes 24..26 
> > > of
> > > + * option vector 5: */
> > > +static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
> > > +{
> > > +    char val[2 * 3] = {
> > > +        24, 0x00, /* Hash/Radix, filled in below. */
> > > +        25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
> > > +        26, 0x40, /* Radix options: GTSE == yes. */
> > > +    };
> > > +
> > > +    if (kvm_enabled()) {
> > > +        if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) {
> > > +            val[1] = 0x80; /* OV5_MMU_BOTH */
> > > +        } else if (kvmppc_has_cap_mmu_radix()) {
> > > +            val[1] = 0x40; /* OV5_MMU_RADIX_300 */
> > > +        } else {
> > > +            assert(kvmppc_has_cap_mmu_hash_v3());
> > > +            val[1] = 0x00; /* Hash */
> > > +        }
> > > +    } else {
> > > +        /* TODO: TCG case, hash */
> > > +        val[1] = 0x00;
> > > +    }
> > > +    _FDT(fdt_setprop(fdt, chosen, "ibm,arch-vec-5-platform-support",
> > > +                     val, sizeof(val)));
> > > +}
> > > +
> > >  static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
> > >  {
> > >      MachineState *machine = MACHINE(spapr);
> > > @@ -876,6 +915,8 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, 
> > > void *fdt)
> > >          _FDT(fdt_setprop_string(fdt, chosen, "linux,stdout-path", 
> > > stdout_path));
> > >      }
> > >  
> > > +    spapr_dt_ov5_platform_support(fdt, chosen);
> > > +
> > >      g_free(stdout_path);
> > >      g_free(bootlist);
> > >  }
> > > @@ -2041,6 +2082,11 @@ 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... */
> > > +        spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
> > > +    }
> > > +    /* ... but not with hash (currently). */
> > >  
> > >      /* advertise support for dedicated HP event source to guests */
> > >      if (spapr->use_hotplug_event_source) {
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index 2bf794d9cd..094f105fd4 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -1064,6 +1064,7 @@ static target_ulong 
> > > h_client_architecture_support(PowerPCCPU *cpu,
> > >      uint32_t best_compat = 0;
> > >      int i;
> > >      sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates;
> > > +    bool guest_radix;
> > >  
> > >      /*
> > >       * We scan the supplied table of PVRs looking for two things
> > > @@ -1115,6 +1116,13 @@ static target_ulong 
> > > h_client_architecture_support(PowerPCCPU *cpu,
> > >      ov_table = list;
> > >  
> > >      ov5_guest = spapr_ovec_parse_vector(ov_table, 5);
> > > +    if (spapr_ovec_test(ov5_guest, OV5_MMU_BOTH)) {
> > > +        error_report("qemu: guest requested hash and radix MMU, which is 
> > > invalid.");
> > > +        exit(EXIT_FAILURE);
> > > +    }
> > > +    /* The radix/hash bit in byte 24 requires special handling: */
> > > +    guest_radix = spapr_ovec_test(ov5_guest, OV5_MMU_RADIX_300);
> > > +    spapr_ovec_clear(ov5_guest, OV5_MMU_RADIX_300);
> > >  
> > >      /* NOTE: there are actually a number of ov5 bits where input from the
> > >       * guest is always zero, and the platform/QEMU enables them 
> > > independently
> > > @@ -1133,14 +1141,18 @@ 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);
> > > -    if (kvm_enabled()) {
> > > -        if (kvmppc_has_cap_mmu_radix()) {
> > > -            /* If the HPT hasn't yet been set up (see
> > > -             * ppc_spapr_reset()), and it's needed, do it now: */
> > > -            if (!spapr_ovec_test(ov5_updates, OV5_MMU_RADIX)) {
> > > -                /* legacy hash or new hash: */
> > > -                spapr_setup_hpt_and_vrma(spapr);
> > > -            }
> > > +    /* Now that processing is finished, set the radix/hash bit for the
> > > +     * guest if it requested a valid mode; otherwise terminate the boot. 
> > > */
> > > +    if (guest_radix) {
> > > +        if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {
> > > +            error_report("qemu: Guest requested unavailable MMU mode 
> > > (radix).");
> > > +            exit(EXIT_FAILURE);
> > > +        }
> > > +        spapr_ovec_set(spapr->ov5_cas, OV5_MMU_RADIX_300);
> > > +    } else {
> > > +        if (kvm_enabled() && !kvmppc_has_cap_mmu_hash_v3()) {
> > > +            error_report("qemu: Guest requested unavailable MMU mode 
> > > (hash).");
> > > +            exit(EXIT_FAILURE);
> > >          }
> > >      }
> > >  
> > > @@ -1153,6 +1165,14 @@ static target_ulong 
> > > h_client_architecture_support(PowerPCCPU *cpu,
> > >  
> > >      if (spapr->cas_reboot) {
> > >          qemu_system_reset_request();
> > > +    } 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);
> > > +            }
> > > +        }
> > >      }
> > >  
> > >      return H_SUCCESS;
> > > diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> > > index 355a34411f..e2dfbac558 100644
> > > --- a/include/hw/ppc/spapr_ovec.h
> > > +++ b/include/hw/ppc/spapr_ovec.h
> > > @@ -48,6 +48,11 @@ typedef struct sPAPROptionVector sPAPROptionVector;
> > >  #define OV5_FORM1_AFFINITY      OV_BIT(5, 0)
> > >  #define OV5_HP_EVT              OV_BIT(6, 5)
> > >  
> > > +/* ISA 3.00 MMU features: */
> > > +#define OV5_MMU_BOTH            OV_BIT(24, 0) /* Radix and hash */
> > > +#define OV5_MMU_RADIX_300       OV_BIT(24, 1) /* 1 => Radix only, 0 => 
> > > Hash only */
> > > +#define OV5_MMU_RADIX_GTSE      OV_BIT(26, 1) /* Radix GTSE */
> > > +
> > >  /* interfaces */
> > >  sPAPROptionVector *spapr_ovec_new(void);
> > >  sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig);
> > 
> 
> 

-- 
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: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]