[Qemu-devel] [PATCH v2 1/2] spapr: introduce common helper to write HPT address to KVM PR
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 --- 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
Re: [Qemu-devel] [PATCH v2 1/2] spapr: introduce common helper to write HPT address to KVM PR
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 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");