On Wed, Sep 13, 2017 at 12:24:53AM +0200, Greg Kurz wrote: > When running with KVM PR, if a new HPT is allocated we need to inform > KVM about the HPT address and size. This is currently done with a hack > which is open-coded in several places. > > This patch consolidate the code in a dedicated helper that records > the HPT address and size in the sPAPR context, and then does the > magic for KVM PR. > > Note that ppc_spapr_reset() now resets all devices and CPUs before > allocating the HPT. This allows to drop the hack from spapr_cpu_reset(). > > Signed-off-by: Greg Kurz <gr...@kaod.org>
I like this more than the previous spin, but while discussing stuff
with SamB, I thought up a different approach, which I think will be
both cleaner and simpler.
It basically doesn't make sense to put the userspace HPT pointer into
env->spr[SDR1], we only do it to make kvmppc_put_books_sregs() do the
right thing.
Instead, we can have kvmppc_put_books_sregs() populate the "SDR1"
field in kvm_sregs from a vhyp hook. We already have the reverse side
in that kvmppc_get_books_sregs() doesn't update the internal SDR1
value if vhyp is set.
In any case the spapr hook would compute the correct value direct from
spapr->htab.
After incoming migration I'm not sure we need to do anything - I think
we already do a pretty thorough register resync with KVM.
> ---
> hw/ppc/spapr.c | 31 ++++++++++++++++++++++++++-----
> hw/ppc/spapr_cpu_core.c | 15 ---------------
> hw/ppc/spapr_hcall.c | 16 +---------------
> include/hw/ppc/spapr.h | 1 +
> 4 files changed, 28 insertions(+), 35 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f680f28a15ea..97f8afdbd7fe 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1309,6 +1309,25 @@ void spapr_free_hpt(sPAPRMachineState *spapr)
> close_htab_fd(spapr);
> }
>
> +void spapr_install_hpt(sPAPRMachineState *spapr, void *htab, uint32_t shift)
> +{
> + assert(htab);
> +
> + spapr->htab = htab;
> + spapr->htab_shift = shift;
> +
> + /*
> + * This is a hack for the benefit of KVM PR - it abuses the SDR1
> + * slot in kvm_sregs to communicate the userspace address of the
> + * HPT
> + */
> + if (kvm_enabled()) {
> + target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> + | (spapr->htab_shift - 18);
> + kvmppc_update_sdr1(sdr1);
> + }
> +}
> +
> void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
> Error **errp)
> {
> @@ -1339,16 +1358,17 @@ void spapr_reallocate_hpt(sPAPRMachineState *spapr,
> int shift,
> /* kernel-side HPT not needed, allocate in userspace instead */
> size_t size = 1ULL << shift;
> int i;
> + void *htab;
>
> - spapr->htab = qemu_memalign(size, size);
> - if (!spapr->htab) {
> + htab = qemu_memalign(size, size);
> + if (!htab) {
> error_setg_errno(errp, errno,
> "Could not allocate HPT of order %d", shift);
> return;
> }
>
> - memset(spapr->htab, 0, size);
> - spapr->htab_shift = shift;
> + memset(htab, 0, size);
> + spapr_install_hpt(spapr, htab, shift);
>
> for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
> DIRTY_HPTE(HPTE(spapr->htab, i));
> @@ -1405,6 +1425,8 @@ static void ppc_spapr_reset(void)
> /* Check for unknown sysbus devices */
> foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
>
> + qemu_devices_reset();
> +
> 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.
> @@ -1414,7 +1436,6 @@ static void ppc_spapr_reset(void)
> spapr_setup_hpt_and_vrma(spapr);
> }
>
> - qemu_devices_reset();
> spapr_clear_pending_events(spapr);
>
> /*
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index c08ee7571a50..c20b5c64b045 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -73,7 +73,6 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>
> static void spapr_cpu_reset(void *opaque)
> {
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> PowerPCCPU *cpu = opaque;
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -86,20 +85,6 @@ static void spapr_cpu_reset(void *opaque)
> cs->halted = 1;
>
> env->spr[SPR_HIOR] = 0;
> -
> - /*
> - * This is a hack for the benefit of KVM PR - it abuses the SDR1
> - * slot in kvm_sregs to communicate the userspace address of the
> - * HPT
> - */
> - if (kvm_enabled()) {
> - env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab
> - | (spapr->htab_shift - 18);
> - if (kvmppc_put_books_sregs(cpu) < 0) {
> - error_report("Unable to update SDR1 in KVM");
> - exit(1);
> - }
> - }
> }
>
> static void spapr_cpu_destroy(PowerPCCPU *cpu)
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 57bb411394ed..7892cd3e7ffa 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -730,15 +730,7 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
> pending->hpt, newsize);
> if (rc == H_SUCCESS) {
> qemu_vfree(spapr->htab);
> - spapr->htab = pending->hpt;
> - spapr->htab_shift = pending->shift;
> -
> - if (kvm_enabled()) {
> - /* For KVM PR, update the HPT pointer */
> - target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> - | (spapr->htab_shift - 18);
> - kvmppc_update_sdr1(sdr1);
> - }
> + spapr_install_hpt(spapr, pending->hpt, pending->shift);
>
> pending->hpt = NULL; /* so it's not free()d */
> }
> @@ -1564,12 +1556,6 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> * the point this is called, nothing should have been
> * entered into the existing HPT */
> spapr_reallocate_hpt(spapr, maxshift, &error_fatal);
> - if (kvm_enabled()) {
> - /* For KVM PR, update the HPT pointer */
> - target_ulong sdr1 = (target_ulong)(uintptr_t)spapr->htab
> - | (spapr->htab_shift - 18);
> - kvmppc_update_sdr1(sdr1);
> - }
> }
> }
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c1b365f56431..30e5805acca4 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -709,4 +709,5 @@ void spapr_do_system_reset_on_cpu(CPUState *cs,
> run_on_cpu_data arg);
> int spapr_vcpu_id(PowerPCCPU *cpu);
> PowerPCCPU *spapr_find_cpu(int vcpu_id);
>
> +void spapr_install_hpt(sPAPRMachineState *spapr, void *htab, uint32_t shift);
> #endif /* HW_SPAPR_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
