[Qemu-devel] [QEMU-PPC] [PATCH 1/2] ppc/spapr: Add implementation of hcall H_PURR

[Qemu-devel] [QEMU-PPC] [PATCH 1/2] ppc/spapr: Add implementation of hcall H_PURR

[Suraj Jitindar Singh]

The hcall H_PURR is used by a guest to read the PURR (processor
utilisation of resources register). A guest expects that this register
will count at a rate of timebase scaled by the number of guest vcpus
present in the vcore. That is the per vcpu purr will count at a rate of
timebase / # vcpus per vcore.

Implement a handler for the H_PURR hcall and return the purr value
divided by smp_threads so that the sum of the purr deltas across the
vcpus of a vcore equals the timebase delta

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index aae9fd2b3e..88b3343f04 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1819,6 +1819,27 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_purr(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUPPCState *env = &cpu->env;
+    target_ulong purr;
+
+    if (kvm_enabled()) {
+        cpu_synchronize_state(CPU(cpu));
+        /*
+         * Divide by smp_threads so that the sum of the purr deltas across the
+         * vcpus of a vcore equal the timebase delta.
+         */
+        purr = env->spr[SPR_PURR] / smp_threads;
+    } else {
+        purr = cpu_ppc_load_purr(env);
+    }
+    args[0] = purr;
+
+    return H_SUCCESS;
+}
+
 static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
 static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
 
@@ -1915,6 +1936,9 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(H_LOGICAL_DCBF, h_logical_dcbf);
     spapr_register_hypercall(KVMPPC_H_LOGICAL_MEMOP, h_logical_memop);
 
+    /* hcall-purr */
+    spapr_register_hypercall(H_PURR, h_purr);
+
     /* qemu/KVM-PPC specific hcalls */
     spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
 
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH 2/2] ppc/spapr: Enable H_PURR in-kernel handling

[Suraj Jitindar Singh]

Enable in-kernel handling of the hcall H_PURR. This means that if the
kernel implements an hcall handler for H_PURR, it will be used.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr.c       | 3 +++
 target/ppc/kvm.c     | 5 +++++
 target/ppc/kvm_ppc.h | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 39e698e9b0..b62dce8876 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2851,6 +2851,9 @@ static void spapr_machine_init(MachineState *machine)
 
         /* Enable H_PAGE_INIT */
         kvmppc_enable_h_page_init();
+
+        /* Enable H_PURR */
+        kvmppc_enable_h_purr();
     }
 
     /* allocate RAM */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 3bf0a46c33..0fd3ef4d48 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2075,6 +2075,11 @@ void kvmppc_enable_h_page_init(void)
     kvmppc_enable_hcall(kvm_state, H_PAGE_INIT);
 }
 
+void kvmppc_enable_h_purr(void)
+{
+    kvmppc_enable_hcall(kvm_state, H_PURR);
+}
+
 void kvmppc_set_papr(PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 45776cad79..b5d2feab22 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
 void kvmppc_enable_set_mode_hcall(void);
 void kvmppc_enable_clear_ref_mod_hcalls(void);
 void kvmppc_enable_h_page_init(void);
+void kvmppc_enable_h_purr(void);
 void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
@@ -145,6 +146,10 @@ static inline void kvmppc_enable_h_page_init(void)
 {
 }
 
+static inline void kvmppc_enable_h_purr(void)
+{
+}
+
 static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
-- 
2.13.6

Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/2] ppc/spapr: Add implementation of hcall H_PURR

[David Gibson]

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

On Mon, Jun 24, 2019 at 03:58:11PM +1000, Suraj Jitindar Singh wrote:
> The hcall H_PURR is used by a guest to read the PURR (processor
> utilisation of resources register). A guest expects that this register
> will count at a rate of timebase scaled by the number of guest vcpus
> present in the vcore. That is the per vcpu purr will count at a rate of
> timebase / # vcpus per vcore.
> 
> Implement a handler for the H_PURR hcall and return the purr value
> divided by smp_threads so that the sum of the purr deltas across the
> vcpus of a vcore equals the timebase delta
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Does this need something new advertised in the hypertas DT entry?

> ---
>  hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aae9fd2b3e..88b3343f04 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1819,6 +1819,27 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +static target_ulong h_purr(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    target_ulong purr;
> +
> +    if (kvm_enabled()) {
> +        cpu_synchronize_state(CPU(cpu));
> +        /*
> +         * Divide by smp_threads so that the sum of the purr deltas across the
> +         * vcpus of a vcore equal the timebase delta.
> +         */
> +        purr = env->spr[SPR_PURR] / smp_threads;
> +    } else {
> +        purr = cpu_ppc_load_purr(env);
> +    }
> +    args[0] = purr;
> +
> +    return H_SUCCESS;
> +}
> +
>  static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
>  static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
>  
> @@ -1915,6 +1936,9 @@ static void hypercall_register_types(void)
>      spapr_register_hypercall(H_LOGICAL_DCBF, h_logical_dcbf);
>      spapr_register_hypercall(KVMPPC_H_LOGICAL_MEMOP, h_logical_memop);
>  
> +    /* hcall-purr */
> +    spapr_register_hypercall(H_PURR, h_purr);
> +
>      /* qemu/KVM-PPC specific hcalls */
>      spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
>  

-- 
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 1/2] ppc/spapr: Add implementation of hcall H_PURR

[Suraj Jitindar Singh]

On Fri, 2019-06-28 at 19:29 +1000, David Gibson wrote:
> On Mon, Jun 24, 2019 at 03:58:11PM +1000, Suraj Jitindar Singh wrote:
> > The hcall H_PURR is used by a guest to read the PURR (processor
> > utilisation of resources register). A guest expects that this
> > register
> > will count at a rate of timebase scaled by the number of guest
> > vcpus
> > present in the vcore. That is the per vcpu purr will count at a
> > rate of
> > timebase / # vcpus per vcore.
> > 
> > Implement a handler for the H_PURR hcall and return the purr value
> > divided by smp_threads so that the sum of the purr deltas across
> > the
> > vcpus of a vcore equals the timebase delta
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> 
> Does this need something new advertised in the hypertas DT entry?

Hi David,

There doesn't seem to be a concensus on what the return value from the
H_PURR hcall should be, whether it just returns the hardware value or
does some adjusting of the value based on guest smt mode as I've
implemented in the patch below.

As such please drop this patch series.

The guest can just read the purr register directly anyway and then
interpret the values as it pleases.

Kind Regards,
Suraj

> 
> > ---
> >  hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index aae9fd2b3e..88b3343f04 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1819,6 +1819,27 @@ static target_ulong h_update_dt(PowerPCCPU
> > *cpu, SpaprMachineState *spapr,
> >      return H_SUCCESS;
> >  }
> >  
> > +static target_ulong h_purr(PowerPCCPU *cpu, SpaprMachineState
> > *spapr,
> > +                           target_ulong opcode, target_ulong
> > *args)
> > +{
> > +    CPUPPCState *env = &cpu->env;
> > +    target_ulong purr;
> > +
> > +    if (kvm_enabled()) {
> > +        cpu_synchronize_state(CPU(cpu));
> > +        /*
> > +         * Divide by smp_threads so that the sum of the purr
> > deltas across the
> > +         * vcpus of a vcore equal the timebase delta.
> > +         */
> > +        purr = env->spr[SPR_PURR] / smp_threads;
> > +    } else {
> > +        purr = cpu_ppc_load_purr(env);
> > +    }
> > +    args[0] = purr;
> > +
> > +    return H_SUCCESS;
> > +}
> > +
> >  static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4)
> > + 1];
> >  static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX -
> > KVMPPC_HCALL_BASE + 1];
> >  
> > @@ -1915,6 +1936,9 @@ static void hypercall_register_types(void)
> >      spapr_register_hypercall(H_LOGICAL_DCBF, h_logical_dcbf);
> >      spapr_register_hypercall(KVMPPC_H_LOGICAL_MEMOP,
> > h_logical_memop);
> >  
> > +    /* hcall-purr */
> > +    spapr_register_hypercall(H_PURR, h_purr);
> > +
> >      /* qemu/KVM-PPC specific hcalls */
> >      spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
> >  
> 
> 

Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/2] ppc/spapr: Add implementation of hcall H_PURR

[David Gibson]

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

On Mon, Jul 01, 2019 at 02:23:21PM +1000, Suraj Jitindar Singh wrote:
> On Fri, 2019-06-28 at 19:29 +1000, David Gibson wrote:
> > On Mon, Jun 24, 2019 at 03:58:11PM +1000, Suraj Jitindar Singh wrote:
> > > The hcall H_PURR is used by a guest to read the PURR (processor
> > > utilisation of resources register). A guest expects that this
> > > register
> > > will count at a rate of timebase scaled by the number of guest
> > > vcpus
> > > present in the vcore. That is the per vcpu purr will count at a
> > > rate of
> > > timebase / # vcpus per vcore.
> > > 
> > > Implement a handler for the H_PURR hcall and return the purr value
> > > divided by smp_threads so that the sum of the purr deltas across
> > > the
> > > vcpus of a vcore equals the timebase delta
> > > 
> > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > 
> > Does this need something new advertised in the hypertas DT entry?
> 
> Hi David,
> 
> There doesn't seem to be a concensus on what the return value from the
> H_PURR hcall should be, whether it just returns the hardware value or
> does some adjusting of the value based on guest smt mode as I've
> implemented in the patch below.

*eyeroll*.  Lack of forethought in PAPR strikes again.

> As such please drop this patch series.

Ok, will do.

> 
> The guest can just read the purr register directly anyway and then
> interpret the values as it pleases.
> 
> Kind Regards,
> Suraj
> 
> > 
> > > ---
> > >  hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index aae9fd2b3e..88b3343f04 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -1819,6 +1819,27 @@ static target_ulong h_update_dt(PowerPCCPU
> > > *cpu, SpaprMachineState *spapr,
> > >      return H_SUCCESS;
> > >  }
> > >  
> > > +static target_ulong h_purr(PowerPCCPU *cpu, SpaprMachineState
> > > *spapr,
> > > +                           target_ulong opcode, target_ulong
> > > *args)
> > > +{
> > > +    CPUPPCState *env = &cpu->env;
> > > +    target_ulong purr;
> > > +
> > > +    if (kvm_enabled()) {
> > > +        cpu_synchronize_state(CPU(cpu));
> > > +        /*
> > > +         * Divide by smp_threads so that the sum of the purr
> > > deltas across the
> > > +         * vcpus of a vcore equal the timebase delta.
> > > +         */
> > > +        purr = env->spr[SPR_PURR] / smp_threads;
> > > +    } else {
> > > +        purr = cpu_ppc_load_purr(env);
> > > +    }
> > > +    args[0] = purr;
> > > +
> > > +    return H_SUCCESS;
> > > +}
> > > +
> > >  static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4)
> > > + 1];
> > >  static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX -
> > > KVMPPC_HCALL_BASE + 1];
> > >  
> > > @@ -1915,6 +1936,9 @@ static void hypercall_register_types(void)
> > >      spapr_register_hypercall(H_LOGICAL_DCBF, h_logical_dcbf);
> > >      spapr_register_hypercall(KVMPPC_H_LOGICAL_MEMOP,
> > > h_logical_memop);
> > >  
> > > +    /* hcall-purr */
> > > +    spapr_register_hypercall(H_PURR, h_purr);
> > > +
> > >      /* qemu/KVM-PPC specific hcalls */
> > >      spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
> > >  
> > 
> > 
> 

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