Re: [Qemu-devel] [RFC PATCH 07/17] target/ppc/POWER9: Add partition table pointer to sPAPRMachineState

[David Gibson]

On Thu, Feb 09, 2017 at 01:57:58PM +1100, Suraj Jitindar Singh wrote:
> On Wed, 2017-02-01 at 15:04 +1100, David Gibson wrote:
> > On Fri, Jan 13, 2017 at 05:28:13PM +1100, Suraj Jitindar Singh wrote:
> > > 
> > > POWER9 uses a partition table to store information relating to how
> > > address translation is performed on a per partition basis.
> > > 
> > > Add a data area for this to the sPAPRMachineState struct and
> > > (re)allocate
> > > it on machine reset.
> > > 
> > > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> > Hm.  I'm having trouble understanding this one.
> > 
> > IIUC, for the "pseries" machine type this partition table entry is
> > essentially a dummy one, since there's actually only one partition.
> > For KVM the machine would be represented by one of many partition
> > table entries on the real host - so the entry here isn't really
> > relevant.  For TCG, I'm not sure what would be looking at it.  We
> > haven't implemented the partition->host level translation in TCG
> > anyway, and in any case pseries (rather than powernv) should be
> > bypassing that - in which case I'd expect it to bypass looking at the
> > dummy partition table entry as well.
> As discussed elsewhere it seems having a whole partition table entry is
> unnecessary. This will be replaced by a single process table pointer to
> be accessed via the vhyp.
> 
> In fact this won't be necessary for the legacy case at all and so will
> drop this patch from the series in favour of moving it into the radix
> enablement series.

Ok, makes sense.

> > 
> > > 
> > > ---
> > >  hw/ppc/spapr.c         | 38 ++++++++++++++++++++++++++++++++------
> > >  include/hw/ppc/spapr.h |  1 +
> > >  target/ppc/mmu.h       | 13 +++++++++++++
> > >  3 files changed, 46 insertions(+), 6 deletions(-)
> > >  create mode 100644 target/ppc/mmu.h
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 208ef7b..45bd2de 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -41,6 +41,7 @@
> > >  #include "migration/migration.h"
> > >  #include "mmu-hash64.h"
> > >  #include "qom/cpu.h"
> > > +#include "mmu.h"
> > >  
> > >  #include "hw/boards.h"
> > >  #include "hw/ppc/ppc.h"
> > > @@ -1115,6 +1116,26 @@ static void
> > > spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
> > >      }
> > >  }
> > >  
> > > +static void spapr_reallocate_patb(sPAPRMachineState *spapr, Error
> > > **errp)
> > > +{
> > > +    g_free(spapr->patb);
> > > +    spapr->patb = NULL;
> > > +
> > > +    if (!kvm_enabled()) {
> > > +        /* We need to allocate a partition table entry */
> > > +        size_t size = sizeof(struct patb_entry);
> > > +
> > > +        spapr->patb = qemu_memalign(size, size);
> > > +        if (!spapr->patb) {
> > > +            error_setg_errno(errp, errno, "Could not allocate
> > > memory for "
> > > +                                          "partition table
> > > entry");
> > > +            return;
> > > +        }
> > > +
> > > +        memset(spapr->patb, 0, size);
> > > +    }
> > > +}
> > > +
> > >  static void find_unknown_sysbus_device(SysBusDevice *sbdev, void
> > > *opaque)
> > >  {
> > >      bool matched = false;
> > > @@ -1134,7 +1155,7 @@ static void ppc_spapr_reset(void)
> > >  {
> > >      MachineState *machine = MACHINE(qdev_get_machine());
> > >      sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> > > -    PowerPCCPU *first_ppc_cpu;
> > > +    PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> > >      uint32_t rtas_limit;
> > >      hwaddr rtas_addr, fdt_addr;
> > >      void *fdt;
> > > @@ -1143,10 +1164,16 @@ static void ppc_spapr_reset(void)
> > >      /* Check for unknown sysbus devices */
> > >      foreach_dynamic_sysbus_device(find_unknown_sysbus_device,
> > > NULL);
> > >  
> > > -    /* Allocate and/or reset the hash page table */
> > > -    spapr_reallocate_hpt(spapr,
> > > -                         spapr_hpt_shift_for_ramsize(machine-
> > > >maxram_size),
> > > -                         &error_fatal);
> > > +    switch (first_ppc_cpu->env.mmu_model) {
> > > +    case POWERPC_MMU_3_00:
> > > +        /* Allocate the partition table */
> > > +        spapr_reallocate_patb(spapr, &error_fatal);
> > > +    default:
> > > +        /* Allocate and/or reset the hash page table */
> > > +        spapr_reallocate_hpt(spapr,
> > > +                             spapr_hpt_shift_for_ramsize(machine-
> > > >maxram_size),
> > > +                             &error_fatal);
> > > +    }
> > >  
> > >      /* Update the RMA size if necessary */
> > >      if (spapr->vrma_adjust) {
> > > @@ -1193,7 +1220,6 @@ static void ppc_spapr_reset(void)
> > >      g_free(fdt);
> > >  
> > >      /* Set up the entry state */
> > > -    first_ppc_cpu = POWERPC_CPU(first_cpu);
> > >      first_ppc_cpu->env.gpr[3] = fdt_addr;
> > >      first_ppc_cpu->env.gpr[5] = 0;
> > >      first_cpu->halted = 0;
> > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > > index bd5bcf7..b654773 100644
> > > --- a/include/hw/ppc/spapr.h
> > > +++ b/include/hw/ppc/spapr.h
> > > @@ -63,6 +63,7 @@ struct sPAPRMachineState {
> > >  
> > >      void *htab;
> > >      uint32_t htab_shift;
> > > +    void *patb;
> > >      hwaddr rma_size;
> > >      int vrma_adjust;
> > >      ssize_t rtas_size;
> > > diff --git a/target/ppc/mmu.h b/target/ppc/mmu.h
> > > new file mode 100644
> > > index 0000000..67b9707
> > > --- /dev/null
> > > +++ b/target/ppc/mmu.h
> > > @@ -0,0 +1,13 @@
> > > +#ifndef MMU_H
> > > +#define MMU_H
> > > +
> > > +#ifndef CONFIG_USER_ONLY
> > > +
> > > +/* Partition Table Entry */
> > > +struct patb_entry {
> > > +    uint64_t patbe0, patbe1;
> > > +};
> > > +
> > > +#endif /* CONFIG_USER_ONLY */
> > > +
> > > +#endif /* MMU_H */
> 

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