qemu-ppc
[Top][All Lists]
Advanced

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

Re: [Qemu-ppc] [PATCH v3 07/12] spapr: Only setup HTP if necessary.


From: David Gibson
Subject: Re: [Qemu-ppc] [PATCH v3 07/12] spapr: Only setup HTP if necessary.
Date: Wed, 15 Mar 2017 16:47:21 +1100
User-agent: Mutt/1.7.1 (2016-10-04)

On Tue, Mar 14, 2017 at 04:43:41PM +1100, Sam Bobroff wrote:
> On Fri, Mar 03, 2017 at 02:39:47PM +1100, David Gibson wrote:
> > On Thu, Mar 02, 2017 at 04:39:02PM +1100, Sam Bobroff wrote:
> > > If QEMU is using KVM, and KVM is capable of running in radix mode,
> > > guests can be run in real-mode without allocating a HPT (because KVM
> > > will use a minimal RPT). So in this case, we avoid creating the HPT
> > > at reset time and later (during CAS) create it if it is necessary.
> > 
> > Your subject line still has a typo.
> > 
> > > Signed-off-by: Sam Bobroff <address@hidden>
> > > ---
> > > Changes in v3:
> > > * in spapr_setup_hpt_and_vrma() replaced MACHINE(qdev_get_machine()) with 
> > > MACHINE(spapr).
> > > 
> > >  hw/ppc/spapr.c         | 24 +++++++++++++++---------
> > >  hw/ppc/spapr_hcall.c   | 10 ++++++++++
> > >  include/hw/ppc/spapr.h |  1 +
> > >  3 files changed, 26 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index d154fa2112..0b266e96f1 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1207,6 +1207,17 @@ static void spapr_reallocate_hpt(sPAPRMachineState 
> > > *spapr, int shift,
> > >      }
> > >  }
> > >  
> > > +void spapr_setup_hpt_and_vrma(sPAPRMachineState *spapr)
> > > +{
> > > +    spapr_reallocate_hpt(spapr,
> > > +                     
> > > spapr_hpt_shift_for_ramsize(MACHINE(spapr)->maxram_size),
> > > +                     &error_fatal);
> > > +    if (spapr->vrma_adjust) {
> > > +        spapr->rma_size = kvmppc_rma_size(spapr_node0_size(),
> > > +                                          spapr->htab_shift);
> > > +    }
> > > +}
> > > +
> > >  static void find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
> > >  {
> > >      bool matched = false;
> > > @@ -1237,15 +1248,10 @@ static void ppc_spapr_reset(void)
> > >  
> > >      spapr->patb_entry = 0;
> > >  
> > > -    /* 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) {
> > > -        spapr->rma_size = kvmppc_rma_size(spapr_node0_size(),
> > > -                                          spapr->htab_shift);
> > > +    /* 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())) {
> > > +        spapr_setup_hpt_and_vrma(spapr);
> > >      }
> > >  
> > >      qemu_devices_reset();
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index f05a90ed2c..0c1b0326ad 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -999,6 +999,16 @@ 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()) {
> > 
> > I'd still like to see this changed so that it is based on whether an
> > HPT *has* been set up, rather than on whether we expect it would have
> > been set up.  That will be less fragile against future changes.
> 
> I'm not particularly happy with this either but I'm not sure exactly
> what you're suggesting.
> 
> Do you mean adding some new VM IOCTL to allow allow QEMU to query KVM
> and ask what sort of page table has been set up, or would be for a new
> VM? I think it would be easy to add that separately so I could follow
> this series with one to do it.

No, a new ioctl() shouldn't be necessary.  We just need to track in
qemu if we constructed an HPT - either by allocating it ourselves, or
by calling kvmppc_reset_htab().

> 
> > > +            /* 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);
> > 
> > > +            }
> > > +        }
> > > +    }
> > >  
> > >      if (!spapr->cas_reboot) {
> > >          spapr->cas_reboot =
> > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > > index 2975967c07..24da5dc27e 100644
> > > --- a/include/hw/ppc/spapr.h
> > > +++ b/include/hw/ppc/spapr.h
> > > @@ -591,6 +591,7 @@ void spapr_dt_events(sPAPRMachineState *sm, void 
> > > *fdt);
> > >  int spapr_h_cas_compose_response(sPAPRMachineState *sm,
> > >                                   target_ulong addr, target_ulong size,
> > >                                   sPAPROptionVector *ov5_updates);
> > > +void spapr_setup_hpt_and_vrma(sPAPRMachineState *spapr);
> > >  sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn);
> > >  void spapr_tce_table_enable(sPAPRTCETable *tcet,
> > >                              uint32_t page_shift, uint64_t bus_offset,
> > 
> 
> 

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