[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");
