Patchwork [2/2] pseries: Implement hcall-bulk hypervisor interface

login
register
mail settings
Submitter David Gibson
Date Sept. 1, 2011, 1:50 a.m.
Message ID <20110901015050.GI11906@yookeroo.fritz.box>
Download mbox | patch
Permalink /patch/112792/
State New
Headers show

Comments

David Gibson - Sept. 1, 2011, 1:50 a.m.
On Wed, Aug 31, 2011 at 11:22:18AM +0200, Alexander Graf wrote:
> On 11.08.2011, at 04:36, David Gibson wrote:
[snip]
> > +#define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
> > +#define   H_BULK_REMOVE_REQUEST        0x4000000000000000ULL
> > +#define   H_BULK_REMOVE_RESPONSE       0x8000000000000000ULL
> > +#define   H_BULK_REMOVE_END            0xc000000000000000ULL
> > +#define H_BULK_REMOVE_CODE             0x3000000000000000ULL
> > +#define   H_BULK_REMOVE_SUCCESS        0x0000000000000000ULL
> > +#define   H_BULK_REMOVE_NOT_FOUND      0x1000000000000000ULL
> > +#define   H_BULK_REMOVE_PARM           0x2000000000000000ULL
> > +#define   H_BULK_REMOVE_HW             0x3000000000000000ULL
> > +#define H_BULK_REMOVE_RC               0x0c00000000000000ULL
> > +#define H_BULK_REMOVE_FLAGS            0x0300000000000000ULL
> > +#define   H_BULK_REMOVE_ABSOLUTE       0x0000000000000000ULL
> > +#define   H_BULK_REMOVE_ANDCOND        0x0100000000000000ULL
> > +#define   H_BULK_REMOVE_AVPN           0x0200000000000000ULL
> > +#define H_BULK_REMOVE_PTEX             0x00ffffffffffffffULL
> 
> indenting looks broken.

Nope, that's intended.  The unindented ones are the field masks in the
parameter, the intended ones are the defined values for the
corresponding field.

> > +
> > +static target_ulong h_bulk_remove(CPUState *env, sPAPREnvironment *spapr,
> > +                                  target_ulong opcode, target_ulong *args)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < 4; i++) {
> > +        target_ulong *tsh = &args[i*2];
> > +        target_ulong tsl = args[i*2 + 1];
> 
> Mind to replace all those magic numbers by something more verbose?

So, "all those" == 2.  I can replace the 4 with something I guess, but
changing the 2 would just be silly - that code is just taking the
arguments a pair at a time.

Revised patch below.

From e1da8cc05498d471908e8461346c403874496427 Mon Sep 17 00:00:00 2001
From: David Gibson <david@gibson.dropbear.id.au>
Date: Mon, 8 Aug 2011 14:07:12 +1000
Subject: [PATCH] pseries: Implement hcall-bulk hypervisor interface

This patch adds support for the H_REMOVE_BULK hypercall on the pseries
machine.  Strictly speaking this isn't necessarym since the kernel will
only attempt to use this if hcall-bulk is advertised in the device tree,
which previously it was not.

Adding this support may give a marginal performance increase, but more
importantly it reduces the differences between the emulated machine and
an existing PowerVM or kvm system, both of which already implement
hcall-bulk.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr.c       |    2 +-
 hw/spapr_hcall.c |  124 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 113 insertions(+), 13 deletions(-)
Alexander Graf - Sept. 2, 2011, 1:26 p.m.
On 09/01/2011 03:50 AM, David Gibson wrote:
> On Wed, Aug 31, 2011 at 11:22:18AM +0200, Alexander Graf wrote:
>> On 11.08.2011, at 04:36, David Gibson wrote:
> [snip]
>>> +#define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
>>> +#define   H_BULK_REMOVE_REQUEST        0x4000000000000000ULL
>>> +#define   H_BULK_REMOVE_RESPONSE       0x8000000000000000ULL
>>> +#define   H_BULK_REMOVE_END            0xc000000000000000ULL
>>> +#define H_BULK_REMOVE_CODE             0x3000000000000000ULL
>>> +#define   H_BULK_REMOVE_SUCCESS        0x0000000000000000ULL
>>> +#define   H_BULK_REMOVE_NOT_FOUND      0x1000000000000000ULL
>>> +#define   H_BULK_REMOVE_PARM           0x2000000000000000ULL
>>> +#define   H_BULK_REMOVE_HW             0x3000000000000000ULL
>>> +#define H_BULK_REMOVE_RC               0x0c00000000000000ULL
>>> +#define H_BULK_REMOVE_FLAGS            0x0300000000000000ULL
>>> +#define   H_BULK_REMOVE_ABSOLUTE       0x0000000000000000ULL
>>> +#define   H_BULK_REMOVE_ANDCOND        0x0100000000000000ULL
>>> +#define   H_BULK_REMOVE_AVPN           0x0200000000000000ULL
>>> +#define H_BULK_REMOVE_PTEX             0x00ffffffffffffffULL
>> indenting looks broken.
> Nope, that's intended.  The unindented ones are the field masks in the
> parameter, the intended ones are the defined values for the
> corresponding field.

Interesting :). Makes sense now that you explain it - looked awkward at 
first.

>>> +
>>> +static target_ulong h_bulk_remove(CPUState *env, sPAPREnvironment *spapr,
>>> +                                  target_ulong opcode, target_ulong *args)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i<  4; i++) {
>>> +        target_ulong *tsh =&args[i*2];
>>> +        target_ulong tsl = args[i*2 + 1];
>> Mind to replace all those magic numbers by something more verbose?
> So, "all those" == 2.  I can replace the 4 with something I guess, but
> changing the 2 would just be silly - that code is just taking the
> arguments a pair at a time.

Yup. Thanks! Applied.


Alex

Patch

diff --git a/hw/spapr.c b/hw/spapr.c
index de45487..2e71b58 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -72,7 +72,7 @@  static void *spapr_create_fdt_skel(const char *cpu_model,
     uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
     uint32_t pft_size_prop[] = {0, cpu_to_be32(hash_shift)};
     char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
-        "\0hcall-tce\0hcall-vio\0hcall-splpar";
+        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
     uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
     int i;
     char *modelname;
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 0c61c10..f578f08 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -174,20 +174,25 @@  static target_ulong h_enter(CPUState *env, sPAPREnvironment *spapr,
     return H_SUCCESS;
 }
 
-static target_ulong h_remove(CPUState *env, sPAPREnvironment *spapr,
-                             target_ulong opcode, target_ulong *args)
+enum {
+    REMOVE_SUCCESS = 0,
+    REMOVE_NOT_FOUND = 1,
+    REMOVE_PARM = 2,
+    REMOVE_HW = 3,
+};
+
+static target_ulong remove_hpte(CPUState *env, target_ulong ptex, target_ulong avpn,
+                                target_ulong flags,
+                                target_ulong *vp, target_ulong *rp)
 {
-    target_ulong flags = args[0];
-    target_ulong pte_index = args[1];
-    target_ulong avpn = args[2];
     uint8_t *hpte;
     target_ulong v, r, rb;
 
-    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
-        return H_PARAMETER;
+    if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+        return REMOVE_PARM;
     }
 
-    hpte = env->external_htab + (pte_index * HASH_PTE_SIZE_64);
+    hpte = env->external_htab + (ptex * HASH_PTE_SIZE_64);
     while (!lock_hpte(hpte, HPTE_V_HVLOCK)) {
         /* We have no real concurrency in qemu soft-emulation, so we
          * will never actually have a contested lock */
@@ -202,14 +207,106 @@  static target_ulong h_remove(CPUState *env, sPAPREnvironment *spapr,
         ((flags & H_ANDCOND) && (v & avpn) != 0)) {
         stq_p(hpte, v & ~HPTE_V_HVLOCK);
         assert(!(ldq_p(hpte) & HPTE_V_HVLOCK));
-        return H_NOT_FOUND;
+        return REMOVE_NOT_FOUND;
     }
-    args[0] = v & ~HPTE_V_HVLOCK;
-    args[1] = r;
+    *vp = v & ~HPTE_V_HVLOCK;
+    *rp = r;
     stq_p(hpte, 0);
-    rb = compute_tlbie_rb(v, r, pte_index);
+    rb = compute_tlbie_rb(v, r, ptex);
     ppc_tlb_invalidate_one(env, rb);
     assert(!(ldq_p(hpte) & HPTE_V_HVLOCK));
+    return REMOVE_SUCCESS;
+}
+
+static target_ulong h_remove(CPUState *env, sPAPREnvironment *spapr,
+                             target_ulong opcode, target_ulong *args)
+{
+    target_ulong flags = args[0];
+    target_ulong pte_index = args[1];
+    target_ulong avpn = args[2];
+    int ret;
+
+    ret = remove_hpte(env, pte_index, avpn, flags,
+                      &args[0], &args[1]);
+
+    switch (ret) {
+    case REMOVE_SUCCESS:
+        return H_SUCCESS;
+
+    case REMOVE_NOT_FOUND:
+        return H_NOT_FOUND;
+
+    case REMOVE_PARM:
+        return H_PARAMETER;
+
+    case REMOVE_HW:
+        return H_HARDWARE;
+    }
+
+    assert(0);
+}
+
+#define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
+#define   H_BULK_REMOVE_REQUEST        0x4000000000000000ULL
+#define   H_BULK_REMOVE_RESPONSE       0x8000000000000000ULL
+#define   H_BULK_REMOVE_END            0xc000000000000000ULL
+#define H_BULK_REMOVE_CODE             0x3000000000000000ULL
+#define   H_BULK_REMOVE_SUCCESS        0x0000000000000000ULL
+#define   H_BULK_REMOVE_NOT_FOUND      0x1000000000000000ULL
+#define   H_BULK_REMOVE_PARM           0x2000000000000000ULL
+#define   H_BULK_REMOVE_HW             0x3000000000000000ULL
+#define H_BULK_REMOVE_RC               0x0c00000000000000ULL
+#define H_BULK_REMOVE_FLAGS            0x0300000000000000ULL
+#define   H_BULK_REMOVE_ABSOLUTE       0x0000000000000000ULL
+#define   H_BULK_REMOVE_ANDCOND        0x0100000000000000ULL
+#define   H_BULK_REMOVE_AVPN           0x0200000000000000ULL
+#define H_BULK_REMOVE_PTEX             0x00ffffffffffffffULL
+
+#define H_BULK_REMOVE_MAX_BATCH        4
+
+static target_ulong h_bulk_remove(CPUState *env, sPAPREnvironment *spapr,
+                                  target_ulong opcode, target_ulong *args)
+{
+    int i;
+
+    for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
+        target_ulong *tsh = &args[i*2];
+        target_ulong tsl = args[i*2 + 1];
+        target_ulong v, r, ret;
+
+        if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
+            break;
+        } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
+            return H_PARAMETER;
+        }
+
+        *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
+        *tsh |= H_BULK_REMOVE_RESPONSE;
+
+        if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
+            *tsh |= H_BULK_REMOVE_PARM;
+            return H_PARAMETER;
+        }
+
+        ret = remove_hpte(env, *tsh & H_BULK_REMOVE_PTEX, tsl,
+                          (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
+                          &v, &r);
+
+        *tsh |= ret << 60;
+
+        switch (ret) {
+        case REMOVE_SUCCESS:
+            *tsh |= (r & (HPTE_R_C | HPTE_R_R)) << 43;
+            break;
+
+        case REMOVE_PARM:
+            return H_PARAMETER;
+
+        case REMOVE_HW:
+            return H_HARDWARE;
+        }
+    }
+
     return H_SUCCESS;
 }
 
@@ -581,6 +678,9 @@  static void hypercall_init(void)
     spapr_register_hypercall(H_REMOVE, h_remove);
     spapr_register_hypercall(H_PROTECT, h_protect);
 
+    /* hcall-bulk */
+    spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
+
     /* hcall-dabr */
     spapr_register_hypercall(H_SET_DABR, h_set_dabr);