[Qemu-devel] [0/6] Pending pseries updates

[Qemu-devel] [0/6] Pending pseries updates

[David Gibson]

Hi Alex,

Here's another batch of updates for pseries, some of which affect
wider target-ppc code.  I have sent a few of these before, but I don't
believe any have made it into ppc-next so far.  5/6 is an important
bugfix we've discussed before, which I've CCed to qemu-stable.

[Qemu-devel] [PATCH 1/6] pseries: Set hash table size based on RAM size

[David Gibson]

Currently the pseries machine code always attempts to set the size of the
guests's hash page table to 16MB.  However, because of the way the POWER
MMU works, a suitable hash page table size should really depend on memory
size.  16MB will be excessive for guests with <1GB and RAM, and may not be
enough for guests with >2GB of RAM (depending on guest page size and
other factors).

The usual given rule of thumb is that the hash table should be 1/64 of
the size of memory, but in fact the Linux guests we are aiming at don't
really need that much.  This patch, therefore, changes the hash table
allocation code to aim for 1/128 of the size of RAM (rounding up).  When
using KVM, this size may still be adjusted by the host kernel if it is
unable to allocate a suitable (contiguous) table.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

---

This patch can only be safely applied after my earlier patch
"target-ppc: KVM: Fix some kernel version edge cases for
kvmppc_reset_htab()" (already in ppc-next), otherwise the kernel can
end up advertising to the guest a hash table size different from that
actually allocated by the kernel.
---
 hw/spapr.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index 1177efa..a8bd3c1 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -725,9 +725,16 @@ static void ppc_spapr_init(ram_addr_t ram_size,
     spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
     load_limit = spapr->fdt_addr - FW_OVERHEAD;
 
-    /* For now, always aim for a 16MB hash table */
-    /* FIXME: we should change this default based on RAM size */
-    spapr->htab_shift = 24;
+    /* We aim for a hash table of size 1/128 the size of RAM.  The
+     * normal rule of thumb is 1/64 the size of RAM, but that's much
+     * more than needed for the Linux guests we support. */
+    spapr->htab_shift = 18; /* Minimum architected size */
+    while (spapr->htab_shift <= 46) {
+        if ((1ULL << (spapr->htab_shift + 7)) >= ram_size) {
+            break;
+        }
+        spapr->htab_shift++;
+    }
 
     /* init CPUs */
     if (cpu_model == NULL) {
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/6] target-ppc: Remove unused power_mode field from cpu state

[David Gibson]

CPUPPCState includes a variable 'power_mode' which is used nowhere.  This
patch removes it.  This includes saving a dummy zero in its place during
vmsave, to avoid breaking the save format.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu.h     |    1 -
 target-ppc/machine.c |    4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index ca2fc21..faf4404 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1079,7 +1079,6 @@ struct CPUPPCState {
     int mmu_idx;         /* precomputed MMU index to speed up mem accesses */
 
     /* Power management */
-    int power_mode;
     int (*check_pow)(CPUPPCState *env);
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index d6c2ee4..21ce757 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -82,7 +82,7 @@ void cpu_save(QEMUFile *f, void *opaque)
     qemu_put_betls(f, &env->hflags);
     qemu_put_betls(f, &env->hflags_nmsr);
     qemu_put_sbe32s(f, &env->mmu_idx);
-    qemu_put_sbe32s(f, &env->power_mode);
+    qemu_put_sbe32(f, 0);
 }
 
 int cpu_load(QEMUFile *f, void *opaque, int version_id)
@@ -167,7 +167,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     qemu_get_betls(f, &env->hflags);
     qemu_get_betls(f, &env->hflags_nmsr);
     qemu_get_sbe32s(f, &env->mmu_idx);
-    qemu_get_sbe32s(f, &env->power_mode);
+    qemu_get_sbe32(f); /* Discard unused power_mode */
 
     return 0;
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/6] target-ppc: Extend FPU state for newer POWER CPUs

[David Gibson]

This patch adds some extra FPU state to CPUPPCState.  Specifically, fpscr
is extended to 64 bits, since some recent CPUs now have more status bits
than fit inside 64 bits, and we add the 32 VSR registers present on CPUs
with VSX (these extend the standard FP regs, which together with the
Altivec/VMX registers form a 64 x 128bit register file for VSX).

We don't actually support the instructions using these extra registers in
TCG yet, but we still a place to store the state so we can sync it with
KVM and savevm/loadvm it.  This patch updates the savevm code to not
fail on the extended state, but also does not actually save it - that's
a project for another patch.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu.h       |    4 +++-
 target-ppc/machine.c   |    8 ++++++--
 target-ppc/translate.c |    2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index faf4404..846778f 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -963,7 +963,7 @@ struct CPUPPCState {
     /* floating point registers */
     float64 fpr[32];
     /* floating point status and control register */
-    uint32_t fpscr;
+    uint64_t fpscr;
 
     /* Next instruction pointer */
     target_ulong nip;
@@ -1014,6 +1014,8 @@ struct CPUPPCState {
     /* Altivec registers */
     ppc_avr_t avr[32];
     uint32_t vscr;
+    /* VSX registers */
+    uint64_t vsr[32];
     /* SPE registers */
     uint64_t spe_acc;
     uint32_t spe_fscr;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 21ce757..5e7bc00 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -6,6 +6,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 {
     CPUPPCState *env = (CPUPPCState *)opaque;
     unsigned int i, j;
+    uint32_t fpscr;
 
     for (i = 0; i < 32; i++)
         qemu_put_betls(f, &env->gpr[i]);
@@ -30,7 +31,8 @@ void cpu_save(QEMUFile *f, void *opaque)
         u.d = env->fpr[i];
         qemu_put_be64(f, u.l);
     }
-    qemu_put_be32s(f, &env->fpscr);
+    fpscr = env->fpscr;
+    qemu_put_be32s(f, &fpscr);
     qemu_put_sbe32s(f, &env->access_type);
 #if defined(TARGET_PPC64)
     qemu_put_betls(f, &env->asr);
@@ -90,6 +92,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUPPCState *env = (CPUPPCState *)opaque;
     unsigned int i, j;
     target_ulong sdr1;
+    uint32_t fpscr;
 
     for (i = 0; i < 32; i++)
         qemu_get_betls(f, &env->gpr[i]);
@@ -114,7 +117,8 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
         u.l = qemu_get_be64(f);
         env->fpr[i] = u.d;
     }
-    qemu_get_be32s(f, &env->fpscr);
+    qemu_get_be32s(f, &fpscr);
+    env->fpscr = fpscr;
     qemu_get_sbe32s(f, &env->access_type);
 #if defined(TARGET_PPC64)
     qemu_get_betls(f, &env->asr);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index ac915cc..c8122b7 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9463,7 +9463,7 @@ void cpu_dump_state (CPUPPCState *env, FILE *f, fprintf_function cpu_fprintf,
         if ((i & (RFPL - 1)) == (RFPL - 1))
             cpu_fprintf(f, "\n");
     }
-    cpu_fprintf(f, "FPSCR %08x\n", env->fpscr);
+    cpu_fprintf(f, "FPSCR %08" PRIx64 "\n", env->fpscr);
 #if !defined(CONFIG_USER_ONLY)
     cpu_fprintf(f, " SRR0 " TARGET_FMT_lx "  SRR1 " TARGET_FMT_lx
                    "    PVR " TARGET_FMT_lx " VRSAVE " TARGET_FMT_lx "\n",
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 3/6] target-ppc: Extend FPU state for newer POWER CPUs

[Aurelien Jarno]

On Wed, Sep 26, 2012 at 01:12:18PM +1000, David Gibson wrote:
> This patch adds some extra FPU state to CPUPPCState.  Specifically, fpscr
> is extended to 64 bits, since some recent CPUs now have more status bits
> than fit inside 64 bits, and we add the 32 VSR registers present on CPUs
> with VSX (these extend the standard FP regs, which together with the
> Altivec/VMX registers form a 64 x 128bit register file for VSX).
> 
> We don't actually support the instructions using these extra registers in
> TCG yet, but we still a place to store the state so we can sync it with
> KVM and savevm/loadvm it.  This patch updates the savevm code to not
> fail on the extended state, but also does not actually save it - that's
> a project for another patch.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  target-ppc/cpu.h       |    4 +++-
>  target-ppc/machine.c   |    8 ++++++--
>  target-ppc/translate.c |    2 +-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index faf4404..846778f 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -963,7 +963,7 @@ struct CPUPPCState {
>      /* floating point registers */
>      float64 fpr[32];
>      /* floating point status and control register */
> -    uint32_t fpscr;
> +    uint64_t fpscr;

This will break the TCG code, as fpscr is mapped as an i32 in TCG. Also
if it is 64-bit only on PPC64 machines, it might be a good idea to
change it to target_ulong instead, and use _tl in the TCG code.

>      /* Next instruction pointer */
>      target_ulong nip;
> @@ -1014,6 +1014,8 @@ struct CPUPPCState {
>      /* Altivec registers */
>      ppc_avr_t avr[32];
>      uint32_t vscr;
> +    /* VSX registers */
> +    uint64_t vsr[32];
>      /* SPE registers */
>      uint64_t spe_acc;
>      uint32_t spe_fscr;
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index 21ce757..5e7bc00 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -6,6 +6,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>  {
>      CPUPPCState *env = (CPUPPCState *)opaque;
>      unsigned int i, j;
> +    uint32_t fpscr;
>  
>      for (i = 0; i < 32; i++)
>          qemu_put_betls(f, &env->gpr[i]);
> @@ -30,7 +31,8 @@ void cpu_save(QEMUFile *f, void *opaque)
>          u.d = env->fpr[i];
>          qemu_put_be64(f, u.l);
>      }
> -    qemu_put_be32s(f, &env->fpscr);
> +    fpscr = env->fpscr;
> +    qemu_put_be32s(f, &fpscr);
>      qemu_put_sbe32s(f, &env->access_type);
>  #if defined(TARGET_PPC64)
>      qemu_put_betls(f, &env->asr);
> @@ -90,6 +92,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>      CPUPPCState *env = (CPUPPCState *)opaque;
>      unsigned int i, j;
>      target_ulong sdr1;
> +    uint32_t fpscr;
>  
>      for (i = 0; i < 32; i++)
>          qemu_get_betls(f, &env->gpr[i]);
> @@ -114,7 +117,8 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>          u.l = qemu_get_be64(f);
>          env->fpr[i] = u.d;
>      }
> -    qemu_get_be32s(f, &env->fpscr);
> +    qemu_get_be32s(f, &fpscr);
> +    env->fpscr = fpscr;
>      qemu_get_sbe32s(f, &env->access_type);
>  #if defined(TARGET_PPC64)
>      qemu_get_betls(f, &env->asr);
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index ac915cc..c8122b7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -9463,7 +9463,7 @@ void cpu_dump_state (CPUPPCState *env, FILE *f, fprintf_function cpu_fprintf,
>          if ((i & (RFPL - 1)) == (RFPL - 1))
>              cpu_fprintf(f, "\n");
>      }
> -    cpu_fprintf(f, "FPSCR %08x\n", env->fpscr);
> +    cpu_fprintf(f, "FPSCR %08" PRIx64 "\n", env->fpscr);
>  #if !defined(CONFIG_USER_ONLY)
>      cpu_fprintf(f, " SRR0 " TARGET_FMT_lx "  SRR1 " TARGET_FMT_lx
>                     "    PVR " TARGET_FMT_lx " VRSAVE " TARGET_FMT_lx "\n",
> -- 
> 1.7.10.4
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/6] target-ppc: Extend FPU state for newer POWER CPUs

[David Gibson]

On Wed, Sep 26, 2012 at 08:36:45AM +0200, Aurelien Jarno wrote:
> On Wed, Sep 26, 2012 at 01:12:18PM +1000, David Gibson wrote:
> > This patch adds some extra FPU state to CPUPPCState.  Specifically, fpscr
> > is extended to 64 bits, since some recent CPUs now have more status bits
> > than fit inside 64 bits, and we add the 32 VSR registers present on CPUs
> > with VSX (these extend the standard FP regs, which together with the
> > Altivec/VMX registers form a 64 x 128bit register file for VSX).
> > 
> > We don't actually support the instructions using these extra registers in
> > TCG yet, but we still a place to store the state so we can sync it with
> > KVM and savevm/loadvm it.  This patch updates the savevm code to not
> > fail on the extended state, but also does not actually save it - that's
> > a project for another patch.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  target-ppc/cpu.h       |    4 +++-
> >  target-ppc/machine.c   |    8 ++++++--
> >  target-ppc/translate.c |    2 +-
> >  3 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> > index faf4404..846778f 100644
> > --- a/target-ppc/cpu.h
> > +++ b/target-ppc/cpu.h
> > @@ -963,7 +963,7 @@ struct CPUPPCState {
> >      /* floating point registers */
> >      float64 fpr[32];
> >      /* floating point status and control register */
> > -    uint32_t fpscr;
> > +    uint64_t fpscr;
> 
> This will break the TCG code, as fpscr is mapped as an i32 in TCG. Also
> if it is 64-bit only on PPC64 machines, it might be a good idea to
> change it to target_ulong instead, and use _tl in the TCG code.

Ah, good point, thanks for catching that.  Both changes made for the
next version.

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

[Qemu-devel] [PATCH 4/6] pseries: Implement PAPR NVRAM

[David Gibson]

The PAPR specification requires a certain amount of NVRAM, accessed via
RTAS, which we don't currently implement in qemu.  This patch addresses
this deficiency, implementing the NVRAM as a VIO device, with some glue to
instantiate it automatically based on a machine option.

The machine option specifies a drive id, which is used to back the NVRAM,
making it persistent.  If nothing is specified, the driver instead simply
allocates space for the NVRAM, which will not be persistent

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/Makefile.objs |    1 +
 hw/spapr.c           |    3 +
 hw/spapr.h           |    3 +
 hw/spapr_nvram.c     |  225 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-config.c        |    4 +
 5 files changed, 236 insertions(+)
 create mode 100644 hw/spapr_nvram.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 951e407..91cbe8c 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -11,6 +11,7 @@ obj-y += ppc_newworld.o
 obj-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
 obj-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
 obj-$(CONFIG_PSERIES) += spapr_pci.o pci-hotplug.o spapr_iommu.o
+obj-$(CONFIG_PSERIES) += spapr_nvram.o
 # PowerPC 4xx boards
 obj-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
 obj-y += ppc440_bamboo.o
diff --git a/hw/spapr.c b/hw/spapr.c
index a8bd3c1..079825a 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -804,6 +804,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
         }
     }
 
+    /* We always have at least the nvram device on VIO */
+    spapr_create_nvram(spapr);
+
     /* Set up PCI */
     spapr_pci_rtas_init();
 
diff --git a/hw/spapr.h b/hw/spapr.h
index e984e3f..d9c3b4a 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -6,11 +6,13 @@
 
 struct VIOsPAPRBus;
 struct sPAPRPHBState;
+struct sPAPRNVRAM;
 struct icp_state;
 
 typedef struct sPAPREnvironment {
     struct VIOsPAPRBus *vio_bus;
     QLIST_HEAD(, sPAPRPHBState) phbs;
+    struct sPAPRNVRAM *nvram;
     struct icp_state *icp;
 
     target_phys_addr_t ram_limit;
@@ -336,6 +338,7 @@ typedef struct sPAPRTCE {
 #define SPAPR_PCI_BASE_LIOBN    0x80000000
 
 void spapr_iommu_init(void);
+void spapr_create_nvram(sPAPREnvironment *spapr);
 DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size);
 void spapr_tce_free(DMAContext *dma);
 void spapr_tce_reset(DMAContext *dma);
diff --git a/hw/spapr_nvram.c b/hw/spapr_nvram.c
new file mode 100644
index 0000000..8cd8a53
--- /dev/null
+++ b/hw/spapr_nvram.c
@@ -0,0 +1,225 @@
+/*
+ * QEMU sPAPR NVRAM emulation
+ *
+ * Copyright (C) 2012 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include <sys/mman.h>
+#include <libfdt.h>
+
+#include "device_tree.h"
+#include "hw/sysbus.h"
+#include "hw/spapr.h"
+#include "hw/spapr_vio.h"
+
+typedef struct sPAPRNVRAM {
+    VIOsPAPRDevice sdev;
+    uint32_t size;
+    uint8_t *buf;
+    BlockDriverState *drive;
+} sPAPRNVRAM;
+
+#define MIN_NVRAM_SIZE 8192
+#define DEFAULT_NVRAM_SIZE 16384
+#define MAX_NVRAM_SIZE (UINT16_MAX * 16)
+
+static void rtas_nvram_fetch(sPAPREnvironment *spapr,
+                             uint32_t token, uint32_t nargs,
+                             target_ulong args,
+                             uint32_t nret, target_ulong rets)
+{
+    sPAPRNVRAM *nvram = spapr->nvram;
+    target_phys_addr_t offset, buffer, len;
+    int alen;
+    void *membuf;
+
+    if ((nargs != 3) || (nret != 2)) {
+        rtas_st(rets, 0, -3);
+        return;
+    }
+
+    if (!nvram) {
+        rtas_st(rets, 0, -1);
+        rtas_st(rets, 1, 0);
+        return;
+    }
+
+    offset = rtas_ld(args, 0);
+    buffer = rtas_ld(args, 1);
+    len = rtas_ld(args, 2);
+
+    if (((offset + len) < offset)
+        || ((offset + len) > nvram->size)) {
+        rtas_st(rets, 0, -3);
+        rtas_st(rets, 1, 0);
+        return;
+    }
+
+    membuf = cpu_physical_memory_map(buffer, &len, 1);
+    if (nvram->drive) {
+        alen = bdrv_pread(nvram->drive, offset, membuf, len);
+    } else {
+        assert(nvram->buf);
+
+        memcpy(membuf, nvram->buf + offset, len);
+        alen = len;
+    }
+    cpu_physical_memory_unmap(membuf, len, 1, len);
+
+    rtas_st(rets, 0, (alen < len) ? -1 : 0);
+    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
+}
+
+static void rtas_nvram_store(sPAPREnvironment *spapr,
+                             uint32_t token, uint32_t nargs,
+                             target_ulong args,
+                             uint32_t nret, target_ulong rets)
+{
+    sPAPRNVRAM *nvram = spapr->nvram;
+    target_phys_addr_t offset, buffer, len;
+    int alen;
+    void *membuf;
+
+    if ((nargs != 3) || (nret != 2)) {
+        rtas_st(rets, 0, -3);
+        return;
+    }
+
+    if (!nvram) {
+        rtas_st(rets, 0, -1);
+        return;
+    }
+
+    offset = rtas_ld(args, 0);
+    buffer = rtas_ld(args, 1);
+    len = rtas_ld(args, 2);
+
+    if (((offset + len) < offset)
+        || ((offset + len) > nvram->size)) {
+        rtas_st(rets, 0, -3);
+        return;
+    }
+
+    membuf = cpu_physical_memory_map(buffer, &len, 0);
+    if (nvram->drive) {
+        alen = bdrv_pwrite(nvram->drive, offset, membuf, len);
+    } else {
+        assert(nvram->buf);
+
+        memcpy(nvram->buf + offset, membuf, len);
+        alen = len;
+    }
+    cpu_physical_memory_unmap(membuf, len, 0, len);
+
+    rtas_st(rets, 0, (alen < len) ? -1 : 0);
+    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
+}
+
+static int spapr_nvram_init(VIOsPAPRDevice *dev)
+{
+    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
+
+    if (nvram->drive) {
+        nvram->size = bdrv_getlength(nvram->drive);
+    } else {
+        nvram->size = DEFAULT_NVRAM_SIZE;
+        nvram->buf = g_malloc0(nvram->size);
+    }
+
+    if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) {
+        fprintf(stderr, "spapr-nvram must be between %d and %d bytes in size\n",
+                MIN_NVRAM_SIZE, MAX_NVRAM_SIZE);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
+{
+    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
+
+    return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size);
+}
+
+static Property spapr_nvram_properties[] = {
+    DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev),
+    DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_nvram_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
+
+    k->init = spapr_nvram_init;
+    k->devnode = spapr_nvram_devnode;
+    k->dt_name = "nvram";
+    k->dt_type = "nvram";
+    k->dt_compatible = "qemu,spapr-nvram";
+    dc->props = spapr_nvram_properties;
+}
+
+static const TypeInfo spapr_nvram_type_info = {
+    .name          = "spapr-nvram",
+    .parent        = TYPE_VIO_SPAPR_DEVICE,
+    .instance_size = sizeof(sPAPRNVRAM),
+    .class_init    = spapr_nvram_class_init,
+};
+
+static void spapr_nvram_register_types(void)
+{
+    type_register_static(&spapr_nvram_type_info);
+}
+
+type_init(spapr_nvram_register_types)
+
+void spapr_create_nvram(sPAPREnvironment *spapr)
+{
+    QemuOpts *machine_opts;
+    DeviceState *dev;
+
+    dev = qdev_create(&spapr->vio_bus->bus, "spapr-nvram");
+
+    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+    if (machine_opts) {
+        const char *drivename;
+
+        drivename = qemu_opt_get(machine_opts, "nvram");
+        if (drivename) {
+            BlockDriverState *bs;
+
+            bs = bdrv_find(drivename);
+            if (!bs) {
+                fprintf(stderr, "No such block device \"%s\" for nvram\n",
+                        drivename);
+                exit(1);
+            }
+            qdev_prop_set_drive_nofail(dev, "drive", bs);
+        }
+    }
+
+    qdev_init_nofail(dev);
+
+    spapr->nvram = (sPAPRNVRAM *)dev;
+    spapr_rtas_register("nvram-fetch", rtas_nvram_fetch);
+    spapr_rtas_register("nvram-store", rtas_nvram_store);
+}
diff --git a/qemu-config.c b/qemu-config.c
index 12eafbb..1cd9a1b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -619,6 +619,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "mem-merge",
             .type = QEMU_OPT_BOOL,
             .help = "enable/disable memory merge support",
+        }, {
+            .name = "nvram",
+            .type = QEMU_OPT_STRING,
+            .help = "Drive backing persistent NVRAM",
         },
         { /* End of list */ }
     },
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 4/6] pseries: Implement PAPR NVRAM

[Alexander Graf]

On 26.09.2012, at 05:12, David Gibson wrote:

> The PAPR specification requires a certain amount of NVRAM, accessed via
> RTAS, which we don't currently implement in qemu.  This patch addresses
> this deficiency, implementing the NVRAM as a VIO device, with some glue to
> instantiate it automatically based on a machine option.
> 
> The machine option specifies a drive id, which is used to back the NVRAM,
> making it persistent.  If nothing is specified, the driver instead simply
> allocates space for the NVRAM, which will not be persistent
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/Makefile.objs |    1 +
> hw/spapr.c           |    3 +
> hw/spapr.h           |    3 +
> hw/spapr_nvram.c     |  225 ++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-config.c        |    4 +
> 5 files changed, 236 insertions(+)
> create mode 100644 hw/spapr_nvram.c
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 951e407..91cbe8c 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -11,6 +11,7 @@ obj-y += ppc_newworld.o
> obj-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
> obj-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
> obj-$(CONFIG_PSERIES) += spapr_pci.o pci-hotplug.o spapr_iommu.o
> +obj-$(CONFIG_PSERIES) += spapr_nvram.o
> # PowerPC 4xx boards
> obj-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
> obj-y += ppc440_bamboo.o
> diff --git a/hw/spapr.c b/hw/spapr.c
> index a8bd3c1..079825a 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -804,6 +804,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>         }
>     }
> 
> +    /* We always have at least the nvram device on VIO */
> +    spapr_create_nvram(spapr);
> +
>     /* Set up PCI */
>     spapr_pci_rtas_init();
> 
> diff --git a/hw/spapr.h b/hw/spapr.h
> index e984e3f..d9c3b4a 100644
> --- a/hw/spapr.h
> +++ b/hw/spapr.h
> @@ -6,11 +6,13 @@
> 
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> +struct sPAPRNVRAM;
> struct icp_state;
> 
> typedef struct sPAPREnvironment {
>     struct VIOsPAPRBus *vio_bus;
>     QLIST_HEAD(, sPAPRPHBState) phbs;
> +    struct sPAPRNVRAM *nvram;
>     struct icp_state *icp;
> 
>     target_phys_addr_t ram_limit;
> @@ -336,6 +338,7 @@ typedef struct sPAPRTCE {
> #define SPAPR_PCI_BASE_LIOBN    0x80000000
> 
> void spapr_iommu_init(void);
> +void spapr_create_nvram(sPAPREnvironment *spapr);
> DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size);
> void spapr_tce_free(DMAContext *dma);
> void spapr_tce_reset(DMAContext *dma);
> diff --git a/hw/spapr_nvram.c b/hw/spapr_nvram.c
> new file mode 100644
> index 0000000..8cd8a53
> --- /dev/null
> +++ b/hw/spapr_nvram.c
> @@ -0,0 +1,225 @@
> +/*
> + * QEMU sPAPR NVRAM emulation
> + *
> + * Copyright (C) 2012 David Gibson, IBM Corporation.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include <sys/mman.h>
> +#include <libfdt.h>
> +
> +#include "device_tree.h"
> +#include "hw/sysbus.h"
> +#include "hw/spapr.h"
> +#include "hw/spapr_vio.h"
> +
> +typedef struct sPAPRNVRAM {
> +    VIOsPAPRDevice sdev;
> +    uint32_t size;
> +    uint8_t *buf;
> +    BlockDriverState *drive;
> +} sPAPRNVRAM;
> +
> +#define MIN_NVRAM_SIZE 8192
> +#define DEFAULT_NVRAM_SIZE 16384
> +#define MAX_NVRAM_SIZE (UINT16_MAX * 16)
> +
> +static void rtas_nvram_fetch(sPAPREnvironment *spapr,
> +                             uint32_t token, uint32_t nargs,
> +                             target_ulong args,
> +                             uint32_t nret, target_ulong rets)
> +{
> +    sPAPRNVRAM *nvram = spapr->nvram;
> +    target_phys_addr_t offset, buffer, len;
> +    int alen;
> +    void *membuf;
> +
> +    if ((nargs != 3) || (nret != 2)) {
> +        rtas_st(rets, 0, -3);
> +        return;
> +    }
> +
> +    if (!nvram) {
> +        rtas_st(rets, 0, -1);
> +        rtas_st(rets, 1, 0);
> +        return;
> +    }
> +
> +    offset = rtas_ld(args, 0);
> +    buffer = rtas_ld(args, 1);
> +    len = rtas_ld(args, 2);
> +
> +    if (((offset + len) < offset)
> +        || ((offset + len) > nvram->size)) {
> +        rtas_st(rets, 0, -3);
> +        rtas_st(rets, 1, 0);
> +        return;
> +    }
> +
> +    membuf = cpu_physical_memory_map(buffer, &len, 1);
> +    if (nvram->drive) {
> +        alen = bdrv_pread(nvram->drive, offset, membuf, len);
> +    } else {
> +        assert(nvram->buf);
> +
> +        memcpy(membuf, nvram->buf + offset, len);
> +        alen = len;
> +    }
> +    cpu_physical_memory_unmap(membuf, len, 1, len);
> +
> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
> +}
> +
> +static void rtas_nvram_store(sPAPREnvironment *spapr,
> +                             uint32_t token, uint32_t nargs,
> +                             target_ulong args,
> +                             uint32_t nret, target_ulong rets)
> +{
> +    sPAPRNVRAM *nvram = spapr->nvram;
> +    target_phys_addr_t offset, buffer, len;
> +    int alen;
> +    void *membuf;
> +
> +    if ((nargs != 3) || (nret != 2)) {
> +        rtas_st(rets, 0, -3);
> +        return;
> +    }
> +
> +    if (!nvram) {
> +        rtas_st(rets, 0, -1);
> +        return;
> +    }
> +
> +    offset = rtas_ld(args, 0);
> +    buffer = rtas_ld(args, 1);
> +    len = rtas_ld(args, 2);
> +
> +    if (((offset + len) < offset)
> +        || ((offset + len) > nvram->size)) {
> +        rtas_st(rets, 0, -3);
> +        return;
> +    }
> +
> +    membuf = cpu_physical_memory_map(buffer, &len, 0);
> +    if (nvram->drive) {
> +        alen = bdrv_pwrite(nvram->drive, offset, membuf, len);
> +    } else {
> +        assert(nvram->buf);
> +
> +        memcpy(nvram->buf + offset, membuf, len);
> +        alen = len;
> +    }
> +    cpu_physical_memory_unmap(membuf, len, 0, len);
> +
> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
> +}
> +
> +static int spapr_nvram_init(VIOsPAPRDevice *dev)
> +{
> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
> +
> +    if (nvram->drive) {
> +        nvram->size = bdrv_getlength(nvram->drive);
> +    } else {
> +        nvram->size = DEFAULT_NVRAM_SIZE;
> +        nvram->buf = g_malloc0(nvram->size);
> +    }
> +
> +    if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) {
> +        fprintf(stderr, "spapr-nvram must be between %d and %d bytes in size\n",
> +                MIN_NVRAM_SIZE, MAX_NVRAM_SIZE);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
> +{
> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
> +
> +    return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size);
> +}
> +
> +static Property spapr_nvram_properties[] = {
> +    DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev),
> +    DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void spapr_nvram_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
> +
> +    k->init = spapr_nvram_init;
> +    k->devnode = spapr_nvram_devnode;
> +    k->dt_name = "nvram";
> +    k->dt_type = "nvram";
> +    k->dt_compatible = "qemu,spapr-nvram";
> +    dc->props = spapr_nvram_properties;
> +}
> +
> +static const TypeInfo spapr_nvram_type_info = {
> +    .name          = "spapr-nvram",
> +    .parent        = TYPE_VIO_SPAPR_DEVICE,
> +    .instance_size = sizeof(sPAPRNVRAM),
> +    .class_init    = spapr_nvram_class_init,
> +};
> +
> +static void spapr_nvram_register_types(void)
> +{
> +    type_register_static(&spapr_nvram_type_info);
> +}
> +
> +type_init(spapr_nvram_register_types)
> +
> +void spapr_create_nvram(sPAPREnvironment *spapr)

Could you please move this function into spapr.c?


Alex

> +{
> +    QemuOpts *machine_opts;
> +    DeviceState *dev;
> +
> +    dev = qdev_create(&spapr->vio_bus->bus, "spapr-nvram");
> +
> +    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> +    if (machine_opts) {
> +        const char *drivename;
> +
> +        drivename = qemu_opt_get(machine_opts, "nvram");
> +        if (drivename) {
> +            BlockDriverState *bs;
> +
> +            bs = bdrv_find(drivename);
> +            if (!bs) {
> +                fprintf(stderr, "No such block device \"%s\" for nvram\n",
> +                        drivename);
> +                exit(1);
> +            }
> +            qdev_prop_set_drive_nofail(dev, "drive", bs);
> +        }
> +    }
> +
> +    qdev_init_nofail(dev);
> +
> +    spapr->nvram = (sPAPRNVRAM *)dev;
> +    spapr_rtas_register("nvram-fetch", rtas_nvram_fetch);
> +    spapr_rtas_register("nvram-store", rtas_nvram_store);
> +}
> diff --git a/qemu-config.c b/qemu-config.c
> index 12eafbb..1cd9a1b 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -619,6 +619,10 @@ static QemuOptsList qemu_machine_opts = {
>             .name = "mem-merge",
>             .type = QEMU_OPT_BOOL,
>             .help = "enable/disable memory merge support",
> +        }, {
> +            .name = "nvram",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Drive backing persistent NVRAM",
>         },
>         { /* End of list */ }
>     },
> -- 
> 1.7.10.4
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/6] pseries: Implement PAPR NVRAM

[Alexander Graf]

On 26.09.2012, at 14:55, Alexander Graf wrote:

> 
> On 26.09.2012, at 05:12, David Gibson wrote:
> 
>> The PAPR specification requires a certain amount of NVRAM, accessed via
>> RTAS, which we don't currently implement in qemu.  This patch addresses
>> this deficiency, implementing the NVRAM as a VIO device, with some glue to
>> instantiate it automatically based on a machine option.
>> 
>> The machine option specifies a drive id, which is used to back the NVRAM,
>> making it persistent.  If nothing is specified, the driver instead simply
>> allocates space for the NVRAM, which will not be persistent
>> 
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>> hw/ppc/Makefile.objs |    1 +
>> hw/spapr.c           |    3 +
>> hw/spapr.h           |    3 +
>> hw/spapr_nvram.c     |  225 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> qemu-config.c        |    4 +
>> 5 files changed, 236 insertions(+)
>> create mode 100644 hw/spapr_nvram.c
>> 
>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>> index 951e407..91cbe8c 100644
>> --- a/hw/ppc/Makefile.objs
>> +++ b/hw/ppc/Makefile.objs
>> @@ -11,6 +11,7 @@ obj-y += ppc_newworld.o
>> obj-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
>> obj-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
>> obj-$(CONFIG_PSERIES) += spapr_pci.o pci-hotplug.o spapr_iommu.o
>> +obj-$(CONFIG_PSERIES) += spapr_nvram.o
>> # PowerPC 4xx boards
>> obj-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
>> obj-y += ppc440_bamboo.o
>> diff --git a/hw/spapr.c b/hw/spapr.c
>> index a8bd3c1..079825a 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -804,6 +804,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>        }
>>    }
>> 
>> +    /* We always have at least the nvram device on VIO */
>> +    spapr_create_nvram(spapr);
>> +
>>    /* Set up PCI */
>>    spapr_pci_rtas_init();
>> 
>> diff --git a/hw/spapr.h b/hw/spapr.h
>> index e984e3f..d9c3b4a 100644
>> --- a/hw/spapr.h
>> +++ b/hw/spapr.h
>> @@ -6,11 +6,13 @@
>> 
>> struct VIOsPAPRBus;
>> struct sPAPRPHBState;
>> +struct sPAPRNVRAM;
>> struct icp_state;
>> 
>> typedef struct sPAPREnvironment {
>>    struct VIOsPAPRBus *vio_bus;
>>    QLIST_HEAD(, sPAPRPHBState) phbs;
>> +    struct sPAPRNVRAM *nvram;
>>    struct icp_state *icp;
>> 
>>    target_phys_addr_t ram_limit;
>> @@ -336,6 +338,7 @@ typedef struct sPAPRTCE {
>> #define SPAPR_PCI_BASE_LIOBN    0x80000000
>> 
>> void spapr_iommu_init(void);
>> +void spapr_create_nvram(sPAPREnvironment *spapr);
>> DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size);
>> void spapr_tce_free(DMAContext *dma);
>> void spapr_tce_reset(DMAContext *dma);
>> diff --git a/hw/spapr_nvram.c b/hw/spapr_nvram.c
>> new file mode 100644
>> index 0000000..8cd8a53
>> --- /dev/null
>> +++ b/hw/spapr_nvram.c
>> @@ -0,0 +1,225 @@
>> +/*
>> + * QEMU sPAPR NVRAM emulation
>> + *
>> + * Copyright (C) 2012 David Gibson, IBM Corporation.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include <sys/mman.h>
>> +#include <libfdt.h>
>> +
>> +#include "device_tree.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/spapr.h"
>> +#include "hw/spapr_vio.h"
>> +
>> +typedef struct sPAPRNVRAM {
>> +    VIOsPAPRDevice sdev;
>> +    uint32_t size;
>> +    uint8_t *buf;
>> +    BlockDriverState *drive;
>> +} sPAPRNVRAM;
>> +
>> +#define MIN_NVRAM_SIZE 8192
>> +#define DEFAULT_NVRAM_SIZE 16384
>> +#define MAX_NVRAM_SIZE (UINT16_MAX * 16)
>> +
>> +static void rtas_nvram_fetch(sPAPREnvironment *spapr,
>> +                             uint32_t token, uint32_t nargs,
>> +                             target_ulong args,
>> +                             uint32_t nret, target_ulong rets)
>> +{
>> +    sPAPRNVRAM *nvram = spapr->nvram;
>> +    target_phys_addr_t offset, buffer, len;
>> +    int alen;
>> +    void *membuf;
>> +
>> +    if ((nargs != 3) || (nret != 2)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    if (!nvram) {
>> +        rtas_st(rets, 0, -1);
>> +        rtas_st(rets, 1, 0);
>> +        return;
>> +    }
>> +
>> +    offset = rtas_ld(args, 0);
>> +    buffer = rtas_ld(args, 1);
>> +    len = rtas_ld(args, 2);
>> +
>> +    if (((offset + len) < offset)
>> +        || ((offset + len) > nvram->size)) {
>> +        rtas_st(rets, 0, -3);
>> +        rtas_st(rets, 1, 0);
>> +        return;
>> +    }
>> +
>> +    membuf = cpu_physical_memory_map(buffer, &len, 1);
>> +    if (nvram->drive) {
>> +        alen = bdrv_pread(nvram->drive, offset, membuf, len);
>> +    } else {
>> +        assert(nvram->buf);
>> +
>> +        memcpy(membuf, nvram->buf + offset, len);
>> +        alen = len;
>> +    }
>> +    cpu_physical_memory_unmap(membuf, len, 1, len);
>> +
>> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
>> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
>> +}
>> +
>> +static void rtas_nvram_store(sPAPREnvironment *spapr,
>> +                             uint32_t token, uint32_t nargs,
>> +                             target_ulong args,
>> +                             uint32_t nret, target_ulong rets)
>> +{
>> +    sPAPRNVRAM *nvram = spapr->nvram;
>> +    target_phys_addr_t offset, buffer, len;
>> +    int alen;
>> +    void *membuf;
>> +
>> +    if ((nargs != 3) || (nret != 2)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    if (!nvram) {
>> +        rtas_st(rets, 0, -1);
>> +        return;
>> +    }
>> +
>> +    offset = rtas_ld(args, 0);
>> +    buffer = rtas_ld(args, 1);
>> +    len = rtas_ld(args, 2);
>> +
>> +    if (((offset + len) < offset)
>> +        || ((offset + len) > nvram->size)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    membuf = cpu_physical_memory_map(buffer, &len, 0);
>> +    if (nvram->drive) {
>> +        alen = bdrv_pwrite(nvram->drive, offset, membuf, len);
>> +    } else {
>> +        assert(nvram->buf);
>> +
>> +        memcpy(nvram->buf + offset, membuf, len);
>> +        alen = len;
>> +    }
>> +    cpu_physical_memory_unmap(membuf, len, 0, len);
>> +
>> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
>> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
>> +}
>> +
>> +static int spapr_nvram_init(VIOsPAPRDevice *dev)
>> +{
>> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
>> +
>> +    if (nvram->drive) {
>> +        nvram->size = bdrv_getlength(nvram->drive);
>> +    } else {
>> +        nvram->size = DEFAULT_NVRAM_SIZE;
>> +        nvram->buf = g_malloc0(nvram->size);
>> +    }
>> +
>> +    if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) {
>> +        fprintf(stderr, "spapr-nvram must be between %d and %d bytes in size\n",
>> +                MIN_NVRAM_SIZE, MAX_NVRAM_SIZE);
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
>> +{
>> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
>> +
>> +    return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size);
>> +}
>> +
>> +static Property spapr_nvram_properties[] = {
>> +    DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev),
>> +    DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void spapr_nvram_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
>> +
>> +    k->init = spapr_nvram_init;
>> +    k->devnode = spapr_nvram_devnode;
>> +    k->dt_name = "nvram";
>> +    k->dt_type = "nvram";
>> +    k->dt_compatible = "qemu,spapr-nvram";
>> +    dc->props = spapr_nvram_properties;
>> +}
>> +
>> +static const TypeInfo spapr_nvram_type_info = {
>> +    .name          = "spapr-nvram",
>> +    .parent        = TYPE_VIO_SPAPR_DEVICE,
>> +    .instance_size = sizeof(sPAPRNVRAM),
>> +    .class_init    = spapr_nvram_class_init,
>> +};
>> +
>> +static void spapr_nvram_register_types(void)
>> +{
>> +    type_register_static(&spapr_nvram_type_info);
>> +}
>> +
>> +type_init(spapr_nvram_register_types)
>> +
>> +void spapr_create_nvram(sPAPREnvironment *spapr)
> 
> Could you please move this function into spapr.c?

Reading this again I'm missing the explanation :).

I do understand the intention of having device initialization code for nvram in the nvram source code.

But semantically, it's the spapr machine creating the device. It should not have knowledge of any internals of the device, so the code can live anywhere anyways. However, by moving it into spapr.c, we can make it local to the spapr machine. If someone else wants to -device an spapr-nvram, they can do so (it will fail because there are no hypercall registrations there, but that's a different matter). But all the machine semantics of how to read the machine_opts live in the machine file then :).

I hope that explains things a bit better. I didn't realize how short my reply was - sorry :).


Alex

[Qemu-devel] [PATCH 5/6] pseries: Don't test for MSR_PR for hypercalls under KVM

[David Gibson]

PAPR hypercalls should only be invoked from the guest kernel, not guest
user programs, that is, with MSR[PR]=0.  Currently we check this in
spapr_hypercall, returning H_PRIVILEGE if MSR[PR]=1.

However, under KVM the state of MSR[PR] is already checked by the host
kernel before passing the hypercall to qemu, making this check redundant.
Worse, however, we don't generally synchronize KVM and qemu state on the
hypercall path, meaning that qemu could incorrectly reject a hypercall
because it has a stale MSR value.

This patch fixes the problem by moving the privilege test exclusively to
the TCG hypercall path.

Cc: qemu-stable@nongnu.org

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr.c       |    7 ++++++-
 hw/spapr_hcall.c |    5 -----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index 079825a..e6bf522 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -573,7 +573,12 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 
 static void emulate_spapr_hypercall(CPUPPCState *env)
 {
-    env->gpr[3] = spapr_hypercall(env, env->gpr[3], &env->gpr[4]);
+    if (msr_pr) {
+        hcall_dprintf("Hypercall made with MSR[PR]=1\n");
+        env->gpr[3] = H_PRIVILEGE;
+    } else {
+        env->gpr[3] = spapr_hypercall(env, env->gpr[3], &env->gpr[4]);
+    }
 }
 
 static void spapr_reset_htab(sPAPREnvironment *spapr)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 826ca67..194d9c2 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -681,11 +681,6 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn)
 target_ulong spapr_hypercall(CPUPPCState *env, target_ulong opcode,
                              target_ulong *args)
 {
-    if (msr_pr) {
-        hcall_dprintf("Hypercall made with MSR[PR]=1\n");
-        return H_PRIVILEGE;
-    }
-
     if ((opcode <= MAX_HCALL_OPCODE)
         && ((opcode & 0x3) == 0)) {
         spapr_hcall_fn fn = papr_hypercall_table[opcode / 4];
-- 
1.7.10.4

[Qemu-devel] [PATCH 6/6] ppc/pseries: Reset VPA registration on CPU reset

[David Gibson]

The ppc specific CPU state contains several variables which track the
VPA, SLB shadow and dispatch trace log.  These are structures shared
between OS and hypervisor that are used on the pseries machine to track
various per-CPU quantities.

The address of these structures needs to be registered by the guest on each
boot, however currently this registration is not cleared when we reset the
cpu.  This patch corrects this bug.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate_init.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index fba2b42..a972287 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10423,6 +10423,14 @@ static void ppc_cpu_reset(CPUState *s)
     env->pending_interrupts = 0;
     env->exception_index = POWERPC_EXCP_NONE;
     env->error_code = 0;
+
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+    env->vpa = 0;
+    env->slb_shadow = 0;
+    env->dispatch_trace_log = 0;
+    env->dtl_size = 0;
+#endif /* TARGET_PPC64 */
+
     /* Flush all TLBs */
     tlb_flush(env, 1);
 }
-- 
1.7.10.4

Re: [Qemu-devel] [0/6] Pending pseries updates

[Alexander Graf]

On 26.09.2012, at 05:12, David Gibson wrote:

> Hi Alex,
> 
> Here's another batch of updates for pseries, some of which affect
> wider target-ppc code.  I have sent a few of these before, but I don't
> believe any have made it into ppc-next so far.  5/6 is an important
> bugfix we've discussed before, which I've CCed to qemu-stable.

Thanks, applied 1/5, 2/6, 5/6, 6/6.

4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :).
5/6 still has a comment in flight.


Alex

Re: [Qemu-devel] [Qemu-ppc] [0/6] Pending pseries updates

[David Gibson]

On Wed, Sep 26, 2012 at 02:59:25PM +0200, Alexander Graf wrote:
> 
> On 26.09.2012, at 05:12, David Gibson wrote:
> 
> > Hi Alex,
> > 
> > Here's another batch of updates for pseries, some of which affect
> > wider target-ppc code.  I have sent a few of these before, but I don't
> > believe any have made it into ppc-next so far.  5/6 is an important
> > bugfix we've discussed before, which I've CCed to qemu-stable.
> 
> Thanks, applied 1/5, 2/6, 5/6, 6/6.
> 
> 4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :).
> 5/6 still has a comment in flight.

Uh, I assume you mean 3/6 and 4/6 here rather than 4/6 and 5/6.

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

Re: [Qemu-devel] [Qemu-ppc] [0/6] Pending pseries updates

[Alexander Graf]

On 27.09.2012, at 01:31, David Gibson <dwg@au1.ibm.com> wrote:

> On Wed, Sep 26, 2012 at 02:59:25PM +0200, Alexander Graf wrote:
>> 
>> On 26.09.2012, at 05:12, David Gibson wrote:
>> 
>>> Hi Alex,
>>> 
>>> Here's another batch of updates for pseries, some of which affect
>>> wider target-ppc code.  I have sent a few of these before, but I don't
>>> believe any have made it into ppc-next so far.  5/6 is an important
>>> bugfix we've discussed before, which I've CCed to qemu-stable.
>> 
>> Thanks, applied 1/5, 2/6, 5/6, 6/6.
>> 
>> 4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :).
>> 5/6 still has a comment in flight.
> 
> Uh, I assume you mean 3/6 and 4/6 here rather than 4/6 and 5/6.

Of course :). I'm just bad with numbers ;).

Btw, when running git format-patch to get patch files from your tree, try passing --cover-letter to it for a nice patch set summary :)

Alex

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

Re: [Qemu-devel] [Qemu-ppc] [0/6] Pending pseries updates

[David Gibson]

On Thu, Sep 27, 2012 at 10:05:48AM +0200, Alexander Graf wrote:
> 
> 
> On 27.09.2012, at 01:31, David Gibson <dwg@au1.ibm.com> wrote:
> 
> > On Wed, Sep 26, 2012 at 02:59:25PM +0200, Alexander Graf wrote:
> >> 
> >> On 26.09.2012, at 05:12, David Gibson wrote:
> >> 
> >>> Hi Alex,
> >>> 
> >>> Here's another batch of updates for pseries, some of which affect
> >>> wider target-ppc code.  I have sent a few of these before, but I don't
> >>> believe any have made it into ppc-next so far.  5/6 is an important
> >>> bugfix we've discussed before, which I've CCed to qemu-stable.
> >> 
> >> Thanks, applied 1/5, 2/6, 5/6, 6/6.
> >> 
> >> 4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :).
> >> 5/6 still has a comment in flight.
> > 
> > Uh, I assume you mean 3/6 and 4/6 here rather than 4/6 and 5/6.
> 
> Of course :). I'm just bad with numbers ;).
> 
> Btw, when running git format-patch to get patch files from your
> tree, try passing --cover-letter to it for a nice patch set summary
> :)

I don't generally use git format-patch directly, I use git sent-email
--compose.


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