[Qemu-devel] [PATCH 0/4] ppc: fix migration with KVM PR (nested)

[Qemu-devel] [PATCH 0/4] ppc: fix migration with KVM PR (nested)

[Greg Kurz]

A guest running with KVM PR ends up irresponsive after migration most of the
time. This happens because the HPT allocated by QEMU is likely to have a
different address on the destination than it had on the source, but we push
the source address to KVM.

This series does a little cleanup and fixes the issue. I could successfully
test it with a nested setup (KVM PR running in KVM HV).

However, this isn't enough to fix migration when using KVM PR on baremetal...
CPUs seem to end up looping on H_CEDE in the guest. I can't figure out what's
happening... Any suggestion would be appreciated.

Cheers,

--
Greg

---

Greg Kurz (4):
      spapr: only update SDR1 once per-cpu during CAS
      spapr: introduce a helper to compute the address of the HPT
      ppc: kvm: introduce a helper to update SDR1 for a single CPU
      ppc: kvm: update HPT pointer in KVM PR after migration


 hw/ppc/spapr.c          |   15 +++++++++++++++
 hw/ppc/spapr_cpu_core.c |    8 +++-----
 hw/ppc/spapr_hcall.c    |   20 +++++++++-----------
 include/hw/ppc/spapr.h  |    1 +
 target/ppc/cpu.h        |    1 +
 target/ppc/kvm.c        |   12 ++++++++----
 target/ppc/kvm_ppc.h    |    3 ++-
 target/ppc/machine.c    |    7 +++++++
 8 files changed, 46 insertions(+), 21 deletions(-)

[Qemu-devel] [PATCH 1/4] spapr: only update SDR1 once per-cpu during CAS

[Greg Kurz]

Commit b55d295e3ec9 added the possibility to support HPT resizing with KVM.
In the case of PR, we need to pass the userspace address of the HPT to KVM
using the SDR1 slot.
This is handled by kvmppc_update_sdr1() which uses CPU_FOREACH() to update
all CPUs. It is hence not needed to call kvmppc_update_sdr1() for each CPU.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_hcall.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8b3c0e17e75c..6ab8c188f381 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1559,20 +1559,16 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
         }
 
         if (spapr->htab_shift < maxshift) {
-            CPUState *cs;
-
             /* Guest doesn't know about HPT resizing, so we
              * pre-emptively resize for the maximum permitted RAM.  At
              * the point this is called, nothing should have been
              * entered into the existing HPT */
             spapr_reallocate_hpt(spapr, maxshift, &error_fatal);
-            CPU_FOREACH(cs) {
-                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);
-                }
+            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);
             }
         }
     }

[Qemu-devel] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[Greg Kurz]

The formula used to compute the address of the HPT allocated by QEMU is
open-coded in several places. This patch moves the magic to a dedicated
helper. While here, we also patch the callers to only pass the address
to KVM if we indeed have a userland HPT (ie, KVM PR).

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c          |    9 +++++++++
 hw/ppc/spapr_cpu_core.c |   12 +++++++-----
 hw/ppc/spapr_hcall.c    |   14 ++++++++------
 include/hw/ppc/spapr.h  |    1 +
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index caffa1276328..bf24c26b756d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1290,6 +1290,15 @@ static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
     }
 }
 
+target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
+{
+    if (!spapr->htab) {
+        return 0;
+    }
+
+    return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
+}
+
 int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
 {
     int shift;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 85037ef71e27..581eb4d92de9 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -93,11 +93,13 @@ static void spapr_cpu_reset(void *opaque)
      * 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);
+        target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
+        if (sdr1) {
+            env->spr[SPR_SDR1] = sdr1;
+            if (kvmppc_put_books_sregs(cpu) < 0) {
+                error_report("Unable to update SDR1 in KVM");
+                exit(1);
+            }
         }
     }
 }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6ab8c188f381..06059b44ab40 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -735,9 +735,10 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
 
         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);
+            target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
+            if (sdr1) {
+                kvmppc_update_sdr1(sdr1);
+            }
         }
 
         pending->hpt = NULL; /* so it's not free()d */
@@ -1566,9 +1567,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
             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);
+                target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
+                if (sdr1) {
+                    kvmppc_update_sdr1(sdr1);
+                }
             }
         }
     }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c1b365f56431..a1f5edc15018 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);
 
+target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr);
 #endif /* HW_SPAPR_H */

[Qemu-devel] [PATCH 3/4] ppc: kvm: introduce a helper to update SDR1 for a single CPU

[Greg Kurz]

When running with KVM PR, we hijack the SDR1 slot to pass the address of
the HPT allocated by QEMU to KVM.  On pseries virtual machines, we have to
do this when the guest calls the KVMPPC_H_CAS or the H_RESIZE_HPT_COMMIT
hypercalls. This is currently handled by kvmppc_update_sdr1() which updates
SDR1 for all CPUs. But we also need to update SDR1 at machine reset, and
this is currently open-coded in spapr_cpu_reset() on a per-CPU basis.

This patch renames kvmppc_update_sdr1() to kvmppc_update_sdr1_all() and
reuses the kvmppc_update_sdr1() function name to update a single CPU,
like we already do with in the CPU compat mode code.

It finally converts the sPAPR code to use the all CPUs or single CPU helpers
where appropriate.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_cpu_core.c |    6 +-----
 hw/ppc/spapr_hcall.c    |    4 ++--
 target/ppc/kvm.c        |   12 ++++++++----
 target/ppc/kvm_ppc.h    |    3 ++-
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 581eb4d92de9..da81688b0f4d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -95,11 +95,7 @@ static void spapr_cpu_reset(void *opaque)
     if (kvm_enabled()) {
         target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
         if (sdr1) {
-            env->spr[SPR_SDR1] = sdr1;
-            if (kvmppc_put_books_sregs(cpu) < 0) {
-                error_report("Unable to update SDR1 in KVM");
-                exit(1);
-            }
+            kvmppc_update_sdr1(cpu, sdr1);
         }
     }
 }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 06059b44ab40..e090b69efe7f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -737,7 +737,7 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
             /* For KVM PR, update the HPT pointer */
             target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
             if (sdr1) {
-                kvmppc_update_sdr1(sdr1);
+                kvmppc_update_sdr1_all(sdr1);
             }
         }
 
@@ -1569,7 +1569,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
                 /* For KVM PR, update the HPT pointer */
                 target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
                 if (sdr1) {
-                    kvmppc_update_sdr1(sdr1);
+                    kvmppc_update_sdr1_all(sdr1);
                 }
             }
         }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 6442dfcb95b3..e69366968f15 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2806,10 +2806,9 @@ int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift)
     return kvm_vm_ioctl(cs->kvm_state, KVM_PPC_RESIZE_HPT_COMMIT, &rhpt);
 }
 
-static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
+void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1)
 {
-    target_ulong sdr1 = arg.target_ptr;
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
 
     /* This is just for the benefit of PR KVM */
@@ -2821,7 +2820,12 @@ static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
     }
 }
 
-void kvmppc_update_sdr1(target_ulong sdr1)
+static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
+{
+    kvmppc_update_sdr1(POWERPC_CPU(cs), arg.target_ptr);
+}
+
+void kvmppc_update_sdr1_all(target_ulong sdr1)
 {
     CPUState *cs;
 
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index f780e6ec7b72..9524a7a0c21c 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -68,7 +68,8 @@ PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
 void kvmppc_check_papr_resize_hpt(Error **errp);
 int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int shift);
 int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
-void kvmppc_update_sdr1(target_ulong sdr1);
+void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1);
+void kvmppc_update_sdr1_all(target_ulong sdr1);
 bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
 
 bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);

[Qemu-devel] [PATCH 4/4] ppc: kvm: update HPT pointer in KVM PR after migration

[Greg Kurz]

When running with KVM PR, a pseries machine needs to allocate an HPT in
userspace and pass its address to KVM. This is done by hijacking the SDR1
slot.

It is very likely that the destination QEMU will allocate the HPT at
a different address, ie, the SDR1 value we get from the migration
stream is wrong and the guest ends up badly broken.

Let's fix this by re-computing the appropriate value for SDR1 and pushing
it to KVM at CPU post load. This is achieved by extending the PPC virtual
hypervisor interface.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c       |    6 ++++++
 target/ppc/cpu.h     |    1 +
 target/ppc/machine.c |    7 +++++++
 3 files changed, 14 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bf24c26b756d..11c65563bb6e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1299,6 +1299,11 @@ target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
     return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
 }
 
+static target_ulong get_spapr_hpt_pointer(PPCVirtualHypervisor *vhyp)
+{
+    return spapr_get_hpt_pointer(SPAPR_MACHINE(vhyp));
+}
+
 int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
 {
     int shift;
@@ -3613,6 +3618,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     vhc->unmap_hptes = spapr_unmap_hptes;
     vhc->store_hpte = spapr_store_hpte;
     vhc->get_patbe = spapr_get_patbe;
+    vhc->get_hpt_pointer = get_spapr_hpt_pointer;
     xic->ics_get = spapr_ics_get;
     xic->ics_resend = spapr_ics_resend;
     xic->icp_get = spapr_icp_get;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c9d3ffa89bcb..bb1d61c9358c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1243,6 +1243,7 @@ struct PPCVirtualHypervisorClass {
     void (*store_hpte)(PPCVirtualHypervisor *vhyp, hwaddr ptex,
                        uint64_t pte0, uint64_t pte1);
     uint64_t (*get_patbe)(PPCVirtualHypervisor *vhyp);
+    target_ulong (*get_hpt_pointer)(PPCVirtualHypervisor *vhyp);
 };
 
 #define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor"
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index e36b7100cb66..6ec4b3214a2d 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -294,6 +294,13 @@ static int cpu_post_load(void *opaque, int version_id)
 
     if (!cpu->vhyp) {
         ppc_store_sdr1(env, env->spr[SPR_SDR1]);
+    } else if (kvm_enabled()) {
+        PPCVirtualHypervisorClass *vhc =
+            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+        target_ulong sdr1 = vhc->get_hpt_pointer(cpu->vhyp);
+        if (sdr1) {
+            kvmppc_update_sdr1(cpu, sdr1);
+        }
     }
 
     /* Invalidate all msr bits except MSR_TGPR/MSR_HVB before restoring */

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/4] ppc: fix migration with KVM PR (nested)

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

I wanted to Cc Sam also but it was late and I forgot :)

On Mon, 04 Sep 2017 23:46:44 +0200
Greg Kurz <groug@kaod.org> wrote:

> A guest running with KVM PR ends up irresponsive after migration most of the
> time. This happens because the HPT allocated by QEMU is likely to have a
> different address on the destination than it had on the source, but we push
> the source address to KVM.
> 
> This series does a little cleanup and fixes the issue. I could successfully
> test it with a nested setup (KVM PR running in KVM HV).
> 
> However, this isn't enough to fix migration when using KVM PR on baremetal...
> CPUs seem to end up looping on H_CEDE in the guest. I can't figure out what's
> happening... Any suggestion would be appreciated.
> 
> Cheers,
> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (4):
>       spapr: only update SDR1 once per-cpu during CAS
>       spapr: introduce a helper to compute the address of the HPT
>       ppc: kvm: introduce a helper to update SDR1 for a single CPU
>       ppc: kvm: update HPT pointer in KVM PR after migration
> 
> 
>  hw/ppc/spapr.c          |   15 +++++++++++++++
>  hw/ppc/spapr_cpu_core.c |    8 +++-----
>  hw/ppc/spapr_hcall.c    |   20 +++++++++-----------
>  include/hw/ppc/spapr.h  |    1 +
>  target/ppc/cpu.h        |    1 +
>  target/ppc/kvm.c        |   12 ++++++++----
>  target/ppc/kvm_ppc.h    |    3 ++-
>  target/ppc/machine.c    |    7 +++++++
>  8 files changed, 46 insertions(+), 21 deletions(-)
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [Qemu-devel] [PATCH 1/4] spapr: only update SDR1 once per-cpu during CAS

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]

On Mon, Sep 04, 2017 at 11:46:55PM +0200, Greg Kurz wrote:
> Commit b55d295e3ec9 added the possibility to support HPT resizing with KVM.
> In the case of PR, we need to pass the userspace address of the HPT to KVM
> using the SDR1 slot.
> This is handled by kvmppc_update_sdr1() which uses CPU_FOREACH() to update
> all CPUs. It is hence not needed to call kvmppc_update_sdr1() for each CPU.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c |   14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8b3c0e17e75c..6ab8c188f381 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1559,20 +1559,16 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>          }
>  
>          if (spapr->htab_shift < maxshift) {
> -            CPUState *cs;
> -
>              /* Guest doesn't know about HPT resizing, so we
>               * pre-emptively resize for the maximum permitted RAM.  At
>               * the point this is called, nothing should have been
>               * entered into the existing HPT */
>              spapr_reallocate_hpt(spapr, maxshift, &error_fatal);
> -            CPU_FOREACH(cs) {
> -                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);
> -                }
> +            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);
>              }
>          }
>      }
> 

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4588 bytes --]


On Mon, Sep 04, 2017 at 11:47:05PM +0200, Greg Kurz wrote:
> The formula used to compute the address of the HPT allocated by QEMU is
> open-coded in several places. This patch moves the magic to a dedicated
> helper. While here, we also patch the callers to only pass the address
> to KVM if we indeed have a userland HPT (ie, KVM PR).

The helper function seems reasonable, though I'm not sure about the
name (a. it's not just a pointer, since it includes the encoded size
and b. the name doesn't indicate this is basically KVM PR specific).

THe "only pass the address to KVM if we indeed have a userland HPT
(ie, KVM PR)" bit doesn't really work.  You're doing it by testing for
(sdr1 != 0), but that can only be true if the HPT is minimum size,
which doesn't have much to do with anything meaningful.

> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr.c          |    9 +++++++++
>  hw/ppc/spapr_cpu_core.c |   12 +++++++-----
>  hw/ppc/spapr_hcall.c    |   14 ++++++++------
>  include/hw/ppc/spapr.h  |    1 +
>  4 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index caffa1276328..bf24c26b756d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1290,6 +1290,15 @@ static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
>      }
>  }
>  
> +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
> +{
> +    if (!spapr->htab) {
> +        return 0;
> +    }
> +
> +    return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> +}
> +
>  int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
>  {
>      int shift;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 85037ef71e27..581eb4d92de9 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -93,11 +93,13 @@ static void spapr_cpu_reset(void *opaque)
>       * 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);
> +        target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +        if (sdr1) {
> +            env->spr[SPR_SDR1] = sdr1;
> +            if (kvmppc_put_books_sregs(cpu) < 0) {
> +                error_report("Unable to update SDR1 in KVM");
> +                exit(1);
> +            }
>          }
>      }
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6ab8c188f381..06059b44ab40 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -735,9 +735,10 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>  
>          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);
> +            target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +            if (sdr1) {
> +                kvmppc_update_sdr1(sdr1);
> +            }
>          }
>  
>          pending->hpt = NULL; /* so it's not free()d */
> @@ -1566,9 +1567,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>              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);
> +                target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> +                if (sdr1) {
> +                    kvmppc_update_sdr1(sdr1);
> +                }
>              }
>          }
>      }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c1b365f56431..a1f5edc15018 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);
>  
> +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr);
>  #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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 3/4] ppc: kvm: introduce a helper to update SDR1 for a single CPU

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]

On Mon, Sep 04, 2017 at 11:47:14PM +0200, Greg Kurz wrote:
> When running with KVM PR, we hijack the SDR1 slot to pass the address of
> the HPT allocated by QEMU to KVM.  On pseries virtual machines, we have to
> do this when the guest calls the KVMPPC_H_CAS or the H_RESIZE_HPT_COMMIT
> hypercalls. This is currently handled by kvmppc_update_sdr1() which updates
> SDR1 for all CPUs. But we also need to update SDR1 at machine reset, and
> this is currently open-coded in spapr_cpu_reset() on a per-CPU basis.
> 
> This patch renames kvmppc_update_sdr1() to kvmppc_update_sdr1_all() and
> reuses the kvmppc_update_sdr1() function name to update a single CPU,
> like we already do with in the CPU compat mode code.
> 
> It finally converts the sPAPR code to use the all CPUs or single CPU helpers
> where appropriate.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Seems reasonable, but I'll wait for the next spin of the series, since
the previous patch wants some work.

> ---
>  hw/ppc/spapr_cpu_core.c |    6 +-----
>  hw/ppc/spapr_hcall.c    |    4 ++--
>  target/ppc/kvm.c        |   12 ++++++++----
>  target/ppc/kvm_ppc.h    |    3 ++-
>  4 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 581eb4d92de9..da81688b0f4d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -95,11 +95,7 @@ static void spapr_cpu_reset(void *opaque)
>      if (kvm_enabled()) {
>          target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>          if (sdr1) {
> -            env->spr[SPR_SDR1] = sdr1;
> -            if (kvmppc_put_books_sregs(cpu) < 0) {
> -                error_report("Unable to update SDR1 in KVM");
> -                exit(1);
> -            }
> +            kvmppc_update_sdr1(cpu, sdr1);
>          }
>      }
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 06059b44ab40..e090b69efe7f 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -737,7 +737,7 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>              /* For KVM PR, update the HPT pointer */
>              target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>              if (sdr1) {
> -                kvmppc_update_sdr1(sdr1);
> +                kvmppc_update_sdr1_all(sdr1);
>              }
>          }
>  
> @@ -1569,7 +1569,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>                  /* For KVM PR, update the HPT pointer */
>                  target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
>                  if (sdr1) {
> -                    kvmppc_update_sdr1(sdr1);
> +                    kvmppc_update_sdr1_all(sdr1);
>                  }
>              }
>          }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 6442dfcb95b3..e69366968f15 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2806,10 +2806,9 @@ int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift)
>      return kvm_vm_ioctl(cs->kvm_state, KVM_PPC_RESIZE_HPT_COMMIT, &rhpt);
>  }
>  
> -static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1)
>  {
> -    target_ulong sdr1 = arg.target_ptr;
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
>  
>      /* This is just for the benefit of PR KVM */
> @@ -2821,7 +2820,12 @@ static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
>      }
>  }
>  
> -void kvmppc_update_sdr1(target_ulong sdr1)
> +static void kvmppc_pivot_hpt_cpu(CPUState *cs, run_on_cpu_data arg)
> +{
> +    kvmppc_update_sdr1(POWERPC_CPU(cs), arg.target_ptr);
> +}
> +
> +void kvmppc_update_sdr1_all(target_ulong sdr1)
>  {
>      CPUState *cs;
>  
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index f780e6ec7b72..9524a7a0c21c 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -68,7 +68,8 @@ PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
>  void kvmppc_check_papr_resize_hpt(Error **errp);
>  int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int shift);
>  int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
> -void kvmppc_update_sdr1(target_ulong sdr1);
> +void kvmppc_update_sdr1(PowerPCCPU *cpu, target_ulong sdr1);
> +void kvmppc_update_sdr1_all(target_ulong sdr1);
>  bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
>  
>  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
> 

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 4911 bytes --]

On Sun, 10 Sep 2017 13:41:41 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Sep 04, 2017 at 11:47:05PM +0200, Greg Kurz wrote:
> > The formula used to compute the address of the HPT allocated by QEMU is
> > open-coded in several places. This patch moves the magic to a dedicated
> > helper. While here, we also patch the callers to only pass the address
> > to KVM if we indeed have a userland HPT (ie, KVM PR).  
> 
> The helper function seems reasonable, though I'm not sure about the
> name (a. it's not just a pointer, since it includes the encoded size
> and b. the name doesn't indicate this is basically KVM PR specific).
> 

Sure, I'll come up with a better name.

> THe "only pass the address to KVM if we indeed have a userland HPT
> (ie, KVM PR)" bit doesn't really work.  You're doing it by testing for
> (sdr1 != 0), but that can only be true if the HPT is minimum size,
> which doesn't have much to do with anything meaningful.
> 

Hmmm... if QEMU has allocated an HPT in userspace then the helper
necessarily returns a non-null value, no matter the HPT size. Am
I missing something ?

> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/ppc/spapr.c          |    9 +++++++++
> >  hw/ppc/spapr_cpu_core.c |   12 +++++++-----
> >  hw/ppc/spapr_hcall.c    |   14 ++++++++------
> >  include/hw/ppc/spapr.h  |    1 +
> >  4 files changed, 25 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index caffa1276328..bf24c26b756d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1290,6 +1290,15 @@ static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> >      }
> >  }
> >  
> > +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
> > +{
> > +    if (!spapr->htab) {
> > +        return 0;
> > +    }
> > +
> > +    return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> > +}
> > +
> >  int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
> >  {
> >      int shift;
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 85037ef71e27..581eb4d92de9 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -93,11 +93,13 @@ static void spapr_cpu_reset(void *opaque)
> >       * 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);
> > +        target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > +        if (sdr1) {
> > +            env->spr[SPR_SDR1] = sdr1;
> > +            if (kvmppc_put_books_sregs(cpu) < 0) {
> > +                error_report("Unable to update SDR1 in KVM");
> > +                exit(1);
> > +            }
> >          }
> >      }
> >  }
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 6ab8c188f381..06059b44ab40 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -735,9 +735,10 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
> >  
> >          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);
> > +            target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > +            if (sdr1) {
> > +                kvmppc_update_sdr1(sdr1);
> > +            }
> >          }
> >  
> >          pending->hpt = NULL; /* so it's not free()d */
> > @@ -1566,9 +1567,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> >              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);
> > +                target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > +                if (sdr1) {
> > +                    kvmppc_update_sdr1(sdr1);
> > +                }
> >              }
> >          }
> >      }
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index c1b365f56431..a1f5edc15018 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);
> >  
> > +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr);
> >  #endif /* HW_SPAPR_H */
> >   
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5648 bytes --]

On Mon, Sep 11, 2017 at 02:04:37PM +0200, Greg Kurz wrote:
> On Sun, 10 Sep 2017 13:41:41 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Sep 04, 2017 at 11:47:05PM +0200, Greg Kurz wrote:
> > > The formula used to compute the address of the HPT allocated by QEMU is
> > > open-coded in several places. This patch moves the magic to a dedicated
> > > helper. While here, we also patch the callers to only pass the address
> > > to KVM if we indeed have a userland HPT (ie, KVM PR).  
> > 
> > The helper function seems reasonable, though I'm not sure about the
> > name (a. it's not just a pointer, since it includes the encoded size
> > and b. the name doesn't indicate this is basically KVM PR specific).
> > 
> 
> Sure, I'll come up with a better name.
> 
> > THe "only pass the address to KVM if we indeed have a userland HPT
> > (ie, KVM PR)" bit doesn't really work.  You're doing it by testing for
> > (sdr1 != 0), but that can only be true if the HPT is minimum size,
> > which doesn't have much to do with anything meaningful.
> > 
> 
> Hmmm... if QEMU has allocated an HPT in userspace then the helper
> necessarily returns a non-null value, no matter the HPT size. Am
> I missing something ?

Yes, but the reverse is not true.  Even if qemu *hasn't* allocated an
HPT in userspace, it will usually return a non-zero value - the only
case it won't is when the HPT is minimum size.  That makes the test
pretty pointless.

> 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > >  hw/ppc/spapr.c          |    9 +++++++++
> > >  hw/ppc/spapr_cpu_core.c |   12 +++++++-----
> > >  hw/ppc/spapr_hcall.c    |   14 ++++++++------
> > >  include/hw/ppc/spapr.h  |    1 +
> > >  4 files changed, 25 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index caffa1276328..bf24c26b756d 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1290,6 +1290,15 @@ static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> > >      }
> > >  }
> > >  
> > > +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
> > > +{
> > > +    if (!spapr->htab) {
> > > +        return 0;
> > > +    }
> > > +
> > > +    return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> > > +}
> > > +
> > >  int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
> > >  {
> > >      int shift;
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index 85037ef71e27..581eb4d92de9 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -93,11 +93,13 @@ static void spapr_cpu_reset(void *opaque)
> > >       * 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);
> > > +        target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > > +        if (sdr1) {
> > > +            env->spr[SPR_SDR1] = sdr1;
> > > +            if (kvmppc_put_books_sregs(cpu) < 0) {
> > > +                error_report("Unable to update SDR1 in KVM");
> > > +                exit(1);
> > > +            }
> > >          }
> > >      }
> > >  }
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index 6ab8c188f381..06059b44ab40 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -735,9 +735,10 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
> > >  
> > >          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);
> > > +            target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > > +            if (sdr1) {
> > > +                kvmppc_update_sdr1(sdr1);
> > > +            }
> > >          }
> > >  
> > >          pending->hpt = NULL; /* so it's not free()d */
> > > @@ -1566,9 +1567,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> > >              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);
> > > +                target_ulong sdr1 = spapr_get_hpt_pointer(spapr);
> > > +                if (sdr1) {
> > > +                    kvmppc_update_sdr1(sdr1);
> > > +                }
> > >              }
> > >          }
> > >      }
> > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > > index c1b365f56431..a1f5edc15018 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);
> > >  
> > > +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr);
> > >  #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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] spapr: introduce a helper to compute the address of the HPT

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 2498 bytes --]

On Mon, 11 Sep 2017 22:50:45 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Sep 11, 2017 at 02:04:37PM +0200, Greg Kurz wrote:
> > On Sun, 10 Sep 2017 13:41:41 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Mon, Sep 04, 2017 at 11:47:05PM +0200, Greg Kurz wrote:  
> > > > The formula used to compute the address of the HPT allocated by QEMU is
> > > > open-coded in several places. This patch moves the magic to a dedicated
> > > > helper. While here, we also patch the callers to only pass the address
> > > > to KVM if we indeed have a userland HPT (ie, KVM PR).    
> > > 
> > > The helper function seems reasonable, though I'm not sure about the
> > > name (a. it's not just a pointer, since it includes the encoded size
> > > and b. the name doesn't indicate this is basically KVM PR specific).
> > >   
> > 
> > Sure, I'll come up with a better name.
> >   
> > > THe "only pass the address to KVM if we indeed have a userland HPT
> > > (ie, KVM PR)" bit doesn't really work.  You're doing it by testing for
> > > (sdr1 != 0), but that can only be true if the HPT is minimum size,
> > > which doesn't have much to do with anything meaningful.
> > >   
> > 
> > Hmmm... if QEMU has allocated an HPT in userspace then the helper
> > necessarily returns a non-null value, no matter the HPT size. Am
> > I missing something ?  
> 
> Yes, but the reverse is not true.  Even if qemu *hasn't* allocated an
> HPT in userspace, it will usually return a non-zero value - the only
> case it won't is when the HPT is minimum size.  That makes the test
> pretty pointless.
> 

The helper does return 0 if QEMU hasn't allocated an HPT in userspace.

[...]
> > > > +target_ulong spapr_get_hpt_pointer(sPAPRMachineState *spapr)
> > > > +{
> > > > +    if (!spapr->htab) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> > > > +}
> > > > +

but I agree the patch isn't good... for the comprehension at least. I'll
rework the patchset.

Also this causes a behavior change: we won't update SDR1 anymore with KVM HV,
which already handles it BTW.

void kvmppc_set_hpt(struct kvm *kvm, struct kvm_hpt_info *info)
{
	atomic64_set(&kvm->arch.mmio_update, 0);
	kvm->arch.hpt = *info;
	kvm->arch.sdr1 = __pa(info->virt) | (info->order - 18);

Maybe I should push this behavior change to a separate patch anyway...

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]