All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] Add PowerNV skeleton
@ 2016-07-25 14:24 Cédric Le Goater
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file Cédric Le Goater
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-25 14:24 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-devel, qemu-ppc, Cédric Le Goater

The patchset starts with two small cleanups which are used in the
PowerNV skeleton. The PowerNV platform does not provide enough support
yet to be useful but it is the first step to add the required chiplets
missing in the model.

This version only adds a few minor cleanups to the initial patch that
was sent by Ben last year and we might want to do some more rework
before merging the code. The PnvChip struct could become a Qemu Object
for instance, or not. Feedback welcomed !

I guess it will take some time to get it right. Hopefully we will find
a minimal base to merge and move on with the other pnv chiplets. If
you want to look at what comes after, here is a port on v2.7.0-rc0 :

	https://github.com/legoater/qemu/commits/powernv-ipmi-2.7

which boots smoothly a xenial.

Cheers,

C. 


Benjamin Herrenschmidt (1):
  ppc/pnv: Add skeleton PowerNV platform

Cédric Le Goater (2):
  hw/ppc: include fdt helper routine in a common file
  hw/ppc: use error_report instead of fprintf

 default-configs/ppc64-softmmu.mak |   1 +
 hw/ppc/Makefile.objs              |   2 +
 hw/ppc/pnv.c                      | 593 ++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr.c                    |  23 +-
 hw/ppc/spapr_drc.c                |   8 +-
 hw/ppc/spapr_events.c             |  11 +-
 hw/ppc/spapr_iommu.c              |   4 +-
 hw/ppc/spapr_rtas.c               |  13 +-
 hw/ppc/spapr_vio.c                |   3 +-
 include/hw/ppc/fdt.h              |  23 ++
 include/hw/ppc/pnv.h              |  35 +++
 11 files changed, 677 insertions(+), 39 deletions(-)
 create mode 100644 hw/ppc/pnv.c
 create mode 100644 include/hw/ppc/fdt.h
 create mode 100644 include/hw/ppc/pnv.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file
  2016-07-25 14:24 [Qemu-devel] [PATCH 0/3] Add PowerNV skeleton Cédric Le Goater
@ 2016-07-25 14:24 ` Cédric Le Goater
  2016-07-26  5:57   ` David Gibson
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf Cédric Le Goater
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform Cédric Le Goater
  2 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-25 14:24 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-devel, qemu-ppc, Cédric Le Goater

spapr_pci would also be a good candidate but the macro _FDT is
slightly different. It returns and does not exit.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c        | 11 +----------
 hw/ppc/spapr_events.c | 11 +----------
 include/hw/ppc/fdt.h  | 23 +++++++++++++++++++++++
 3 files changed, 25 insertions(+), 20 deletions(-)
 create mode 100644 include/hw/ppc/fdt.h

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9193ac2c122b..538ff5a46768 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -47,6 +47,7 @@
 #include "hw/ppc/ppc.h"
 #include "hw/loader.h"
 
+#include "hw/ppc/fdt.h"
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
 #include "hw/pci-host/spapr.h"
@@ -299,16 +300,6 @@ static hwaddr spapr_node0_size(void)
     return machine->ram_size;
 }
 
-#define _FDT(exp) \
-    do { \
-        int ret = (exp);                                           \
-        if (ret < 0) {                                             \
-            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
-                    #exp, fdt_strerror(ret));                      \
-            exit(1);                                               \
-        }                                                          \
-    } while (0)
-
 static void add_str(GString *s, const gchar *s1)
 {
     g_string_append_len(s, s1, strlen(s1) + 1);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index b0668b34a927..4c7b6aeab630 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -32,6 +32,7 @@
 #include "hw/qdev.h"
 #include "sysemu/device_tree.h"
 
+#include "hw/ppc/fdt.h"
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
 #include "hw/pci/pci.h"
@@ -210,16 +211,6 @@ struct hp_log_full {
 #define EVENT_MASK_HOTPLUG                   0x10000000
 #define EVENT_MASK_IO                        0x08000000
 
-#define _FDT(exp) \
-    do { \
-        int ret = (exp);                                           \
-        if (ret < 0) {                                             \
-            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
-                    #exp, fdt_strerror(ret));                      \
-            exit(1);                                               \
-        }                                                          \
-    } while (0)
-
 void spapr_events_fdt_skel(void *fdt, uint32_t check_exception_irq)
 {
     uint32_t irq_ranges[] = {cpu_to_be32(check_exception_irq), cpu_to_be32(1)};
diff --git a/include/hw/ppc/fdt.h b/include/hw/ppc/fdt.h
new file mode 100644
index 000000000000..fff3e1b57763
--- /dev/null
+++ b/include/hw/ppc/fdt.h
@@ -0,0 +1,23 @@
+/*
+ * QEMU PowerPC helper routines for the device tree.
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef PPC_FDT_H
+#define PPC_FDT_H
+
+#define _FDT(exp)                               \
+    do { \
+        int ret = (exp);                                           \
+        if (ret < 0) {                                             \
+            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
+                    #exp, fdt_strerror(ret));                      \
+            exit(1);                                               \
+        }                                                          \
+    } while (0)
+
+#endif /* PPC_FDT_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf
  2016-07-25 14:24 [Qemu-devel] [PATCH 0/3] Add PowerNV skeleton Cédric Le Goater
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file Cédric Le Goater
@ 2016-07-25 14:24 ` Cédric Le Goater
  2016-07-26  5:58   ` David Gibson
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform Cédric Le Goater
  2 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-25 14:24 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-devel, qemu-ppc, Cédric Le Goater

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c       | 12 ++++++------
 hw/ppc/spapr_drc.c   |  8 ++++----
 hw/ppc/spapr_iommu.c |  4 ++--
 hw/ppc/spapr_rtas.c  | 13 +++++++------
 hw/ppc/spapr_vio.c   |  3 ++-
 5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 538ff5a46768..4d073ce39e87 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -647,13 +647,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
         _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
                                pcc->l1_dcache_size)));
     } else {
-        fprintf(stderr, "Warning: Unknown L1 dcache size for cpu\n");
+        error_report("Warning: Unknown L1 dcache size for cpu");
     }
     if (pcc->l1_icache_size) {
         _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
                                pcc->l1_icache_size)));
     } else {
-        fprintf(stderr, "Warning: Unknown L1 icache size for cpu\n");
+        error_report("Warning: Unknown L1 icache size for cpu");
     }
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
@@ -944,20 +944,20 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 
     ret = spapr_populate_memory(spapr, fdt);
     if (ret < 0) {
-        fprintf(stderr, "couldn't setup memory nodes in fdt\n");
+        error_report("couldn't setup memory nodes in fdt");
         exit(1);
     }
 
     ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
     if (ret < 0) {
-        fprintf(stderr, "couldn't setup vio devices in fdt\n");
+        error_report("couldn't setup vio devices in fdt");
         exit(1);
     }
 
     if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
         ret = spapr_rng_populate_dt(fdt);
         if (ret < 0) {
-            fprintf(stderr, "could not set up rng device in the fdt\n");
+            error_report("could not set up rng device in the fdt");
             exit(1);
         }
     }
@@ -973,7 +973,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
     /* RTAS */
     ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
     if (ret < 0) {
-        fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
+        error_report("Couldn't set up RTAS device tree properties");
     }
 
     /* cpus */
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 26a067951c54..4b1a943b8e4d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -816,7 +816,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
                       drc_indexes->data,
                       drc_indexes->len * sizeof(uint32_t));
     if (ret) {
-        fprintf(stderr, "Couldn't create ibm,drc-indexes property\n");
+        error_report("Couldn't create ibm,drc-indexes property");
         goto out;
     }
 
@@ -824,21 +824,21 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
                       drc_power_domains->data,
                       drc_power_domains->len * sizeof(uint32_t));
     if (ret) {
-        fprintf(stderr, "Couldn't finalize ibm,drc-power-domains property\n");
+        error_report("Couldn't finalize ibm,drc-power-domains property");
         goto out;
     }
 
     ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names",
                       drc_names->str, drc_names->len);
     if (ret) {
-        fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
+        error_report("Couldn't finalize ibm,drc-names property");
         goto out;
     }
 
     ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types",
                       drc_types->str, drc_types->len);
     if (ret) {
-        fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
+        error_report("Couldn't finalize ibm,drc-types property");
         goto out;
     }
 
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index d57b05d5c0f1..53c2cd4fb657 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -310,8 +310,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
     char tmp[32];
 
     if (spapr_tce_find_by_liobn(liobn)) {
-        fprintf(stderr, "Attempted to create TCE table with duplicate"
-                " LIOBN 0x%x\n", liobn);
+        error_report("Attempted to create TCE table with duplicate"
+                " LIOBN 0x%x", liobn);
         return NULL;
     }
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index dc058e512b86..27b5ad4bc437 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -27,6 +27,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "qemu/log.h"
+#include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/char.h"
 #include "hw/qdev.h"
@@ -716,7 +717,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
 
     ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
     if (ret < 0) {
-        fprintf(stderr, "Couldn't add RTAS reserve entry: %s\n",
+        error_report("Couldn't add RTAS reserve entry: %s",
                 fdt_strerror(ret));
         return ret;
     }
@@ -724,7 +725,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
                                 rtas_addr);
     if (ret < 0) {
-        fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
+        error_report("Couldn't add linux,rtas-base property: %s",
                 fdt_strerror(ret));
         return ret;
     }
@@ -732,7 +733,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
                                 rtas_addr);
     if (ret < 0) {
-        fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
+        error_report("Couldn't add linux,rtas-entry property: %s",
                 fdt_strerror(ret));
         return ret;
     }
@@ -740,7 +741,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
                                 rtas_size);
     if (ret < 0) {
-        fprintf(stderr, "Couldn't add rtas-size property: %s\n",
+        error_report("Couldn't add rtas-size property: %s",
                 fdt_strerror(ret));
         return ret;
     }
@@ -755,7 +756,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
         ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
                                     i + RTAS_TOKEN_BASE);
         if (ret < 0) {
-            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
+            error_report("Couldn't add rtas token for %s: %s",
                     call->name, fdt_strerror(ret));
             return ret;
         }
@@ -770,7 +771,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
                      sizeof(lrdr_capacity));
     if (ret < 0) {
-        fprintf(stderr, "Couldn't add ibm,lrdr-capacity rtas property\n");
+        error_report("Couldn't add ibm,lrdr-capacity rtas property");
         return ret;
     }
 
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index f93244d7c182..497028f075e3 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -20,6 +20,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "qemu/log.h"
@@ -276,7 +277,7 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq)
     uint8_t byte;
 
     if (!dev->crq.qsize) {
-        fprintf(stderr, "spapr_vio_send_creq on uninitialized queue\n");
+        error_report("spapr_vio_send_creq on uninitialized queue");
         return -1;
     }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform
  2016-07-25 14:24 [Qemu-devel] [PATCH 0/3] Add PowerNV skeleton Cédric Le Goater
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file Cédric Le Goater
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf Cédric Le Goater
@ 2016-07-25 14:24 ` Cédric Le Goater
  2016-07-26  6:23   ` David Gibson
  2 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-25 14:24 UTC (permalink / raw)
  To: David Gibson
  Cc: Alexander Graf, qemu-devel, qemu-ppc, Benjamin Herrenschmidt,
	Cédric Le Goater

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

No devices yet, not even an interrupt controller, just to get
started.

(Folded in Stewart Smith patch to add command lien support)

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: updated for qemu-2.7
      replaced fprintf by error_report
      used a common definition of _FDT macro
      removed VMStateDescription as migration is not yet supported
      added IBM Copyright statements
]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 default-configs/ppc64-softmmu.mak |   1 +
 hw/ppc/Makefile.objs              |   2 +
 hw/ppc/pnv.c                      | 593 ++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h              |  35 +++
 4 files changed, 631 insertions(+)
 create mode 100644 hw/ppc/pnv.c
 create mode 100644 include/hw/ppc/pnv.h

diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index c4be59f638ed..516a6e25aba3 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -40,6 +40,7 @@ CONFIG_I8259=y
 CONFIG_XILINX=y
 CONFIG_XILINX_ETHLITE=y
 CONFIG_PSERIES=y
+CONFIG_POWERNV=y
 CONFIG_PREP=y
 CONFIG_MAC=y
 CONFIG_E500=y
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 91a3420f473a..cbde482dd1b4 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -5,6 +5,8 @@ obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
 obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
 obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
 obj-$(CONFIG_PSERIES) += spapr_cpu_core.o
+# IBM PowerNV
+obj-$(CONFIG_POWERNV) += pnv.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
 obj-y += spapr_pci_vfio.o
 endif
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
new file mode 100644
index 000000000000..5096a073e829
--- /dev/null
+++ b/hw/ppc/pnv.c
@@ -0,0 +1,593 @@
+/*
+ * QEMU PowerPC PowerNV model
+ *
+ * Copyright (c) 2004-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2010 David Gibson, IBM Corporation.
+ * Copyright (c) 2014-2016 BenH, 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 "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "hw/hw.h"
+#include "hw/fw-path-provider.h"
+#include "elf.h"
+#include "net/net.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/cpus.h"
+#include "sysemu/kvm.h"
+#include "sysemu/numa.h"
+#include "kvm_ppc.h"
+#include "mmu-hash64.h"
+#include "qom/cpu.h"
+
+#include "hw/boards.h"
+#include "hw/ppc/fdt.h"
+#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv.h"
+#include "hw/loader.h"
+
+#include "exec/address-spaces.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "trace.h"
+#include "hw/nmi.h"
+
+#include "hw/compat.h"
+
+#include <libfdt.h>
+
+#define FDT_ADDR                0x01000000
+#define FDT_MAX_SIZE            0x00100000
+#define FW_MAX_SIZE             0x00400000
+#define FW_FILE_NAME            "skiboot.lid"
+#define KERNEL_FILE_NAME        "skiroot.lid"
+#define KERNEL_LOAD_ADDR        0x20000000
+
+#define TIMEBASE_FREQ           512000000ULL
+
+#define MAX_CPUS                255
+
+#define PHANDLE_XICP            0x00001111
+
+typedef struct sPowerNVMachineState sPowerNVMachineState;
+
+#define TYPE_POWERNV_MACHINE      "powernv-machine"
+#define POWERNV_MACHINE(obj) \
+    OBJECT_CHECK(sPowerNVMachineState, (obj), TYPE_POWERNV_MACHINE)
+
+/**
+ * sPowerNVMachineState:
+ */
+struct sPowerNVMachineState {
+    /*< private >*/
+    MachineState parent_obj;
+    PnvSystem sys;
+};
+
+static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
+                                     size_t maxsize)
+{
+    size_t maxcells = maxsize / sizeof(uint32_t);
+    int i, j, count;
+    uint32_t *p = prop;
+
+    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
+        struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
+
+        if (!sps->page_shift) {
+            break;
+        }
+        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
+            if (sps->enc[count].page_shift == 0) {
+                break;
+            }
+        }
+        if ((p - prop) >= (maxcells - 3 - count * 2)) {
+            break;
+        }
+        *(p++) = cpu_to_be32(sps->page_shift);
+        *(p++) = cpu_to_be32(sps->slb_enc);
+        *(p++) = cpu_to_be32(count);
+        for (j = 0; j < count; j++) {
+            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
+            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
+        }
+    }
+
+    return (p - prop) * sizeof(uint32_t);
+}
+
+static void powernv_populate_memory_node(void *fdt, int nodeid, hwaddr start,
+                                         hwaddr size)
+{
+    /* Probablly bogus, need to match with what's going on in CPU nodes */
+    uint32_t chip_id[] = {
+        cpu_to_be32(0x0), cpu_to_be32(nodeid)
+    };
+    char *mem_name;
+    uint64_t mem_reg_property[2];
+
+    mem_reg_property[0] = cpu_to_be64(start);
+    mem_reg_property[1] = cpu_to_be64(size);
+
+    mem_name = g_strdup_printf("memory@"TARGET_FMT_lx, start);
+    _FDT((fdt_begin_node(fdt, mem_name)));
+    g_free(mem_name);
+    _FDT((fdt_property_string(fdt, "device_type", "memory")));
+    _FDT((fdt_property(fdt, "reg", mem_reg_property,
+                       sizeof(mem_reg_property))));
+    _FDT((fdt_property(fdt, "ibm,chip-id", chip_id, sizeof(chip_id))));
+    _FDT((fdt_end_node(fdt)));
+}
+
+static int powernv_populate_memory(void *fdt)
+{
+    hwaddr mem_start, node_size;
+    int i, nb_nodes = nb_numa_nodes;
+    NodeInfo *nodes = numa_info;
+    NodeInfo ramnode;
+
+    /* No NUMA nodes, assume there is just one node with whole RAM */
+    if (!nb_numa_nodes) {
+        nb_nodes = 1;
+        ramnode.node_mem = ram_size;
+        nodes = &ramnode;
+    }
+
+    for (i = 0, mem_start = 0; i < nb_nodes; ++i) {
+        if (!nodes[i].node_mem) {
+            continue;
+        }
+        if (mem_start >= ram_size) {
+            node_size = 0;
+        } else {
+            node_size = nodes[i].node_mem;
+            if (node_size > ram_size - mem_start) {
+                node_size = ram_size - mem_start;
+            }
+        }
+        for ( ; node_size; ) {
+            hwaddr sizetmp = pow2floor(node_size);
+
+            /* mem_start != 0 here */
+            if (ctzl(mem_start) < ctzl(sizetmp)) {
+                sizetmp = 1ULL << ctzl(mem_start);
+            }
+
+            powernv_populate_memory_node(fdt, i, mem_start, sizetmp);
+            node_size -= sizetmp;
+            mem_start += sizetmp;
+        }
+    }
+
+    return 0;
+}
+
+static void powernv_create_cpu_node(void *fdt, CPUState *cs, int smt_threads)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    DeviceClass *dc = DEVICE_GET_CLASS(cs);
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
+    uint32_t servers_prop[smt_threads];
+    uint32_t gservers_prop[smt_threads * 2];
+    int i, index = ppc_get_vcpu_dt_id(cpu);
+    uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
+                       0xffffffff, 0xffffffff};
+    uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
+    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
+    uint32_t page_sizes_prop[64];
+    size_t page_sizes_prop_size;
+    char *nodename;
+
+    if ((index % smt_threads) != 0) {
+        return;
+    }
+
+    nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
+
+    _FDT((fdt_begin_node(fdt, nodename)));
+
+    g_free(nodename);
+
+    _FDT((fdt_property_cell(fdt, "reg", index)));
+    _FDT((fdt_property_string(fdt, "device_type", "cpu")));
+
+    _FDT((fdt_property_cell(fdt, "cpu-version", env->spr[SPR_PVR])));
+    _FDT((fdt_property_cell(fdt, "d-cache-block-size",
+                            env->dcache_line_size)));
+    _FDT((fdt_property_cell(fdt, "d-cache-line-size",
+                            env->dcache_line_size)));
+    _FDT((fdt_property_cell(fdt, "i-cache-block-size",
+                            env->icache_line_size)));
+    _FDT((fdt_property_cell(fdt, "i-cache-line-size",
+                            env->icache_line_size)));
+
+    if (pcc->l1_dcache_size) {
+        _FDT((fdt_property_cell(fdt, "d-cache-size", pcc->l1_dcache_size)));
+    } else {
+        error_report("Warning: Unknown L1 dcache size for cpu");
+    }
+    if (pcc->l1_icache_size) {
+        _FDT((fdt_property_cell(fdt, "i-cache-size", pcc->l1_icache_size)));
+    } else {
+        error_report("Warning: Unknown L1 icache size for cpu");
+    }
+
+    _FDT((fdt_property_cell(fdt, "timebase-frequency", tbfreq)));
+    _FDT((fdt_property_cell(fdt, "clock-frequency", cpufreq)));
+    _FDT((fdt_property_cell(fdt, "ibm,slb-size", env->slb_nr)));
+    _FDT((fdt_property_string(fdt, "status", "okay")));
+    _FDT((fdt_property(fdt, "64-bit", NULL, 0)));
+
+    if (env->spr_cb[SPR_PURR].oea_read) {
+        _FDT((fdt_property(fdt, "ibm,purr", NULL, 0)));
+    }
+
+    if (env->mmu_model & POWERPC_MMU_1TSEG) {
+        _FDT((fdt_property(fdt, "ibm,processor-segment-sizes",
+                           segs, sizeof(segs))));
+    }
+
+    /* Advertise VMX/VSX (vector extensions) if available
+     *   0 / no property == no vector extensions
+     *   1               == VMX / Altivec available
+     *   2               == VSX available */
+    if (env->insns_flags & PPC_ALTIVEC) {
+        uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
+
+        _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
+    }
+
+    /* Advertise DFP (Decimal Floating Point) if available
+     *   0 / no property == no DFP
+     *   1               == DFP available */
+    if (env->insns_flags2 & PPC2_DFP) {
+        _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
+    }
+
+    page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
+                                                  sizeof(page_sizes_prop));
+    if (page_sizes_prop_size) {
+        _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
+                           page_sizes_prop, page_sizes_prop_size)));
+    }
+
+    /* XXX Just a hack for now */
+    _FDT((fdt_property_cell(fdt, "ibm,chip-id", 0)));
+
+    if (cpu->cpu_version) {
+        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
+    }
+
+    /* Build interrupt servers and gservers properties */
+    for (i = 0; i < smt_threads; i++) {
+        servers_prop[i] = cpu_to_be32(index + i);
+        /* Hack, direct the group queues back to cpu 0 */
+        gservers_prop[i * 2] = cpu_to_be32(index + i);
+        gservers_prop[i * 2 + 1] = 0;
+    }
+    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
+                       servers_prop, sizeof(servers_prop))));
+    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
+                       gservers_prop, sizeof(gservers_prop))));
+
+    _FDT((fdt_end_node(fdt)));
+}
+
+static void *powernv_create_fdt(PnvSystem *sys, const char *kernel_cmdline,
+                                uint32_t initrd_base, uint32_t initrd_size)
+{
+    void *fdt;
+    CPUState *cs;
+    int smt = kvmppc_smt_threads();
+    uint32_t start_prop = cpu_to_be32(initrd_base);
+    uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
+    char *buf;
+    const char plat_compat[] = "qemu,powernv\0ibm,powernv";
+
+    fdt = g_malloc0(FDT_MAX_SIZE);
+    _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
+    _FDT((fdt_finish_reservemap(fdt)));
+
+    /* Root node */
+    _FDT((fdt_begin_node(fdt, "")));
+    _FDT((fdt_property_string(fdt, "model", "IBM PowerNV (emulated by qemu)")));
+    _FDT((fdt_property(fdt, "compatible", plat_compat, sizeof(plat_compat))));
+
+    /*
+     * Add info to guest to indentify which host is it being run on
+     * and what is the uuid of the guest
+     */
+    if (kvmppc_get_host_model(&buf)) {
+        _FDT((fdt_property_string(fdt, "host-model", buf)));
+        g_free(buf);
+    }
+    if (kvmppc_get_host_serial(&buf)) {
+        _FDT((fdt_property_string(fdt, "host-serial", buf)));
+        g_free(buf);
+    }
+
+    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
+                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
+                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
+                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
+                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
+                          qemu_uuid[14], qemu_uuid[15]);
+
+    _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
+    g_free(buf);
+
+    _FDT((fdt_begin_node(fdt, "chosen")));
+    if (kernel_cmdline) {
+        _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
+    }
+    _FDT((fdt_property(fdt, "linux,initrd-start",
+                       &start_prop, sizeof(start_prop))));
+    _FDT((fdt_property(fdt, "linux,initrd-end",
+                       &end_prop, sizeof(end_prop))));
+    _FDT((fdt_end_node(fdt)));
+
+    _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
+    _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
+
+    /* cpus */
+    _FDT((fdt_begin_node(fdt, "cpus")));
+    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
+    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
+
+    CPU_FOREACH(cs) {
+        powernv_create_cpu_node(fdt, cs, smt);
+    }
+
+    _FDT((fdt_end_node(fdt)));
+
+    /* Memory */
+    _FDT((powernv_populate_memory(fdt)));
+
+    /* /hypervisor node */
+    if (kvm_enabled()) {
+        uint8_t hypercall[16];
+
+        /* indicate KVM hypercall interface */
+        _FDT((fdt_begin_node(fdt, "hypervisor")));
+        _FDT((fdt_property_string(fdt, "compatible", "linux,kvm")));
+        if (kvmppc_has_cap_fixup_hcalls()) {
+            /*
+             * Older KVM versions with older guest kernels were broken with the
+             * magic page, don't allow the guest to map it.
+             */
+            kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
+                                 sizeof(hypercall));
+            _FDT((fdt_property(fdt, "hcall-instructions", hypercall,
+                              sizeof(hypercall))));
+        }
+        _FDT((fdt_end_node(fdt)));
+    }
+
+    _FDT((fdt_end_node(fdt))); /* close root node */
+    _FDT((fdt_finish(fdt)));
+
+    return fdt;
+}
+
+static void powernv_cpu_reset(void *opaque)
+{
+    PowerPCCPU *cpu = opaque;
+    CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
+
+    cpu_reset(cs);
+
+    env->spr[SPR_PIR] = ppc_get_vcpu_dt_id(cpu);
+    env->spr[SPR_HIOR] = 0;
+    env->gpr[3] = FDT_ADDR;
+    env->nip = 0x10;
+    env->msr |= MSR_HVB;
+}
+
+static void pnv_create_chip(PnvSystem *sys, unsigned int chip_no)
+{
+    PnvChip *chip = &sys->chips[chip_no];
+
+    if (chip_no >= PNV_MAX_CHIPS) {
+            return;
+    }
+
+    /* XXX Improve chip numbering to better match HW */
+    chip->chip_id = chip_no;
+}
+
+static void ppc_powernv_init(MachineState *machine)
+{
+    ram_addr_t ram_size = machine->ram_size;
+    const char *cpu_model = machine->cpu_model;
+    const char *kernel_filename = machine->kernel_filename;
+    const char *initrd_filename = machine->initrd_filename;
+    uint32_t initrd_base = 0;
+    long initrd_size = 0;
+    PowerPCCPU *cpu;
+    CPUPPCState *env;
+    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    sPowerNVMachineState *pnv_machine = POWERNV_MACHINE(machine);
+    PnvSystem *sys = &pnv_machine->sys;
+    long fw_size;
+    char *filename;
+    void *fdt;
+    int i;
+
+    /* init CPUs */
+    if (cpu_model == NULL) {
+        cpu_model = kvm_enabled() ? "host" : "POWER8";
+    }
+
+    for (i = 0; i < smp_cpus; i++) {
+        cpu = cpu_ppc_init(cpu_model);
+        if (cpu == NULL) {
+            error_report("Unable to find PowerPC CPU definition");
+            exit(1);
+        }
+        env = &cpu->env;
+
+        /* Set time-base frequency to 512 MHz */
+        cpu_ppc_tb_init(env, TIMEBASE_FREQ);
+
+        /* MSR[IP] doesn't exist nowadays */
+        env->msr_mask &= ~(1 << 6);
+
+        qemu_register_reset(powernv_cpu_reset, cpu);
+    }
+
+    /* allocate RAM */
+    memory_region_allocate_system_memory(ram, NULL, "ppc_powernv.ram",
+                                         ram_size);
+    memory_region_add_subregion(sysmem, 0, ram);
+
+    /* XXX We should decide how many chips to create based on #cores and
+     * Venice vs. Murano vs. Naples chip type etc..., for now, just create
+     * one chip. Also creation of the CPUs should be done per-chip
+     */
+    sys->num_chips = 1;
+
+    /* Create only one PHB for now until I figure out what's wrong
+     * when I create more (resource assignment failures in Linux)
+     */
+    pnv_create_chip(sys, 0);
+
+    if (bios_name == NULL) {
+        bios_name = FW_FILE_NAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
+    if (fw_size < 0) {
+        hw_error("qemu: could not load OPAL '%s'\n", filename);
+        exit(1);
+    }
+    g_free(filename);
+
+
+    if (kernel_filename == NULL) {
+        kernel_filename = KERNEL_FILE_NAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, kernel_filename);
+    fw_size = load_image_targphys(filename, 0x20000000, 0x2000000);
+    if (fw_size < 0) {
+        hw_error("qemu: could not load kernel'%s'\n", filename);
+        exit(1);
+    }
+    g_free(filename);
+
+    /* load initrd */
+    if (initrd_filename) {
+            /* Try to locate the initrd in the gap between the kernel
+             * and the firmware. Add a bit of space just in case
+             */
+            initrd_base = 0x40000000;
+            initrd_size = load_image_targphys(initrd_filename, initrd_base,
+                                              0x10000000); /* 128MB max */
+            if (initrd_size < 0) {
+                    error_report("qemu: could not load initial ram disk '%s'",
+                            initrd_filename);
+                    exit(1);
+            }
+    } else {
+            initrd_base = 0;
+            initrd_size = 0;
+    }
+    fdt = powernv_create_fdt(sys, machine->kernel_cmdline,
+                             initrd_base, initrd_size);
+    cpu_physical_memory_write(FDT_ADDR, fdt, fdt_totalsize(fdt));
+}
+
+static int powernv_kvm_type(const char *vm_type)
+{
+    /* Always force PR KVM */
+    return 2;
+}
+
+static void ppc_cpu_do_nmi_on_cpu(void *arg)
+{
+    CPUState *cs = arg;
+
+    cpu_synchronize_state(cs);
+    ppc_cpu_do_system_reset(cs);
+}
+
+static void powernv_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
+    }
+}
+
+static void powernv_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    NMIClass *nc = NMI_CLASS(oc);
+
+    mc->init = ppc_powernv_init;
+    mc->block_default_type = IF_SCSI;
+    mc->max_cpus = MAX_CPUS;
+    mc->no_parallel = 1;
+    mc->default_boot_order = NULL;
+    mc->kvm_type = powernv_kvm_type;
+
+    nc->nmi_monitor_handler = powernv_nmi;
+}
+
+static const TypeInfo powernv_machine_info = {
+    .name          = TYPE_POWERNV_MACHINE,
+    .parent        = TYPE_MACHINE,
+    .abstract      = true,
+    .instance_size = sizeof(sPowerNVMachineState),
+    .class_init    = powernv_machine_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_NMI },
+        { }
+    },
+};
+
+static void powernv_machine_2_5_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = "powernv-2.5";
+    mc->desc = "PowerNV v2.5";
+    mc->alias = "powernv";
+}
+
+static const TypeInfo powernv_machine_2_5_info = {
+    .name          = MACHINE_TYPE_NAME("powernv-2.5"),
+    .parent        = TYPE_POWERNV_MACHINE,
+    .class_init    = powernv_machine_2_5_class_init,
+};
+
+static void powernv_machine_register_types(void)
+{
+    type_register_static(&powernv_machine_info);
+    type_register_static(&powernv_machine_2_5_info);
+}
+
+type_init(powernv_machine_register_types)
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
new file mode 100644
index 000000000000..383a336e7cd3
--- /dev/null
+++ b/include/hw/ppc/pnv.h
@@ -0,0 +1,35 @@
+/*
+ * QEMU PowerNV various definitions
+ *
+ * Copyright (c) 2014-2016 BenH, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _PPC_PNV_H
+#define _PPC_PNV_H
+
+#include "hw/hw.h"
+
+/* Should we turn that into a QOjb of some sort ? */
+typedef struct PnvChip {
+    uint32_t         chip_id;
+} PnvChip;
+
+typedef struct PnvSystem {
+    uint32_t  num_chips;
+#define PNV_MAX_CHIPS      1
+    PnvChip   chips[PNV_MAX_CHIPS];
+} PnvSystem;
+
+#endif /* _PPC_PNV_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file Cédric Le Goater
@ 2016-07-26  5:57   ` David Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2016-07-26  5:57 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Alexander Graf, qemu-devel, qemu-ppc

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

On Mon, Jul 25, 2016 at 04:24:41PM +0200, Cédric Le Goater wrote:
> spapr_pci would also be a good candidate but the macro _FDT is
> slightly different. It returns and does not exit.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Ugh, I so want to get rid of that ugly macro.  It may be a while
before I have time to revisit my qdt cleanups though, so in the
mentime we might as well have only one definition of the thing.

Applied to ppc-for-2.8.

> ---
>  hw/ppc/spapr.c        | 11 +----------
>  hw/ppc/spapr_events.c | 11 +----------
>  include/hw/ppc/fdt.h  | 23 +++++++++++++++++++++++
>  3 files changed, 25 insertions(+), 20 deletions(-)
>  create mode 100644 include/hw/ppc/fdt.h
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9193ac2c122b..538ff5a46768 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -47,6 +47,7 @@
>  #include "hw/ppc/ppc.h"
>  #include "hw/loader.h"
>  
> +#include "hw/ppc/fdt.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
>  #include "hw/pci-host/spapr.h"
> @@ -299,16 +300,6 @@ static hwaddr spapr_node0_size(void)
>      return machine->ram_size;
>  }
>  
> -#define _FDT(exp) \
> -    do { \
> -        int ret = (exp);                                           \
> -        if (ret < 0) {                                             \
> -            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
> -                    #exp, fdt_strerror(ret));                      \
> -            exit(1);                                               \
> -        }                                                          \
> -    } while (0)
> -
>  static void add_str(GString *s, const gchar *s1)
>  {
>      g_string_append_len(s, s1, strlen(s1) + 1);
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index b0668b34a927..4c7b6aeab630 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -32,6 +32,7 @@
>  #include "hw/qdev.h"
>  #include "sysemu/device_tree.h"
>  
> +#include "hw/ppc/fdt.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
>  #include "hw/pci/pci.h"
> @@ -210,16 +211,6 @@ struct hp_log_full {
>  #define EVENT_MASK_HOTPLUG                   0x10000000
>  #define EVENT_MASK_IO                        0x08000000
>  
> -#define _FDT(exp) \
> -    do { \
> -        int ret = (exp);                                           \
> -        if (ret < 0) {                                             \
> -            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
> -                    #exp, fdt_strerror(ret));                      \
> -            exit(1);                                               \
> -        }                                                          \
> -    } while (0)
> -
>  void spapr_events_fdt_skel(void *fdt, uint32_t check_exception_irq)
>  {
>      uint32_t irq_ranges[] = {cpu_to_be32(check_exception_irq), cpu_to_be32(1)};
> diff --git a/include/hw/ppc/fdt.h b/include/hw/ppc/fdt.h
> new file mode 100644
> index 000000000000..fff3e1b57763
> --- /dev/null
> +++ b/include/hw/ppc/fdt.h
> @@ -0,0 +1,23 @@
> +/*
> + * QEMU PowerPC helper routines for the device tree.
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef PPC_FDT_H
> +#define PPC_FDT_H
> +
> +#define _FDT(exp)                               \
> +    do { \
> +        int ret = (exp);                                           \
> +        if (ret < 0) {                                             \
> +            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
> +                    #exp, fdt_strerror(ret));                      \
> +            exit(1);                                               \
> +        }                                                          \
> +    } while (0)
> +
> +#endif /* PPC_FDT_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: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf Cédric Le Goater
@ 2016-07-26  5:58   ` David Gibson
  2016-07-26  6:29     ` Cédric Le Goater
  0 siblings, 1 reply; 10+ messages in thread
From: David Gibson @ 2016-07-26  5:58 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Alexander Graf, qemu-devel, qemu-ppc

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

On Mon, Jul 25, 2016 at 04:24:42PM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

So, when I did some previous conversions to error_report(), I left out
the DT code, in the hopes I'd obsolete it with a more thorough
cleanup.  Since that won't happen for a while yet, I'm happy enough to
queue this for 2.8.

However, if you're going to change all these instances, you should
probably change the one in _FDT as well.

> ---
>  hw/ppc/spapr.c       | 12 ++++++------
>  hw/ppc/spapr_drc.c   |  8 ++++----
>  hw/ppc/spapr_iommu.c |  4 ++--
>  hw/ppc/spapr_rtas.c  | 13 +++++++------
>  hw/ppc/spapr_vio.c   |  3 ++-
>  5 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 538ff5a46768..4d073ce39e87 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -647,13 +647,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>          _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
>                                 pcc->l1_dcache_size)));
>      } else {
> -        fprintf(stderr, "Warning: Unknown L1 dcache size for cpu\n");
> +        error_report("Warning: Unknown L1 dcache size for cpu");
>      }
>      if (pcc->l1_icache_size) {
>          _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
>                                 pcc->l1_icache_size)));
>      } else {
> -        fprintf(stderr, "Warning: Unknown L1 icache size for cpu\n");
> +        error_report("Warning: Unknown L1 icache size for cpu");
>      }
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> @@ -944,20 +944,20 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>  
>      ret = spapr_populate_memory(spapr, fdt);
>      if (ret < 0) {
> -        fprintf(stderr, "couldn't setup memory nodes in fdt\n");
> +        error_report("couldn't setup memory nodes in fdt");
>          exit(1);
>      }
>  
>      ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
>      if (ret < 0) {
> -        fprintf(stderr, "couldn't setup vio devices in fdt\n");
> +        error_report("couldn't setup vio devices in fdt");
>          exit(1);
>      }
>  
>      if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
>          ret = spapr_rng_populate_dt(fdt);
>          if (ret < 0) {
> -            fprintf(stderr, "could not set up rng device in the fdt\n");
> +            error_report("could not set up rng device in the fdt");
>              exit(1);
>          }
>      }
> @@ -973,7 +973,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>      /* RTAS */
>      ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
> +        error_report("Couldn't set up RTAS device tree properties");
>      }
>  
>      /* cpus */
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 26a067951c54..4b1a943b8e4d 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -816,7 +816,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>                        drc_indexes->data,
>                        drc_indexes->len * sizeof(uint32_t));
>      if (ret) {
> -        fprintf(stderr, "Couldn't create ibm,drc-indexes property\n");
> +        error_report("Couldn't create ibm,drc-indexes property");
>          goto out;
>      }
>  
> @@ -824,21 +824,21 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>                        drc_power_domains->data,
>                        drc_power_domains->len * sizeof(uint32_t));
>      if (ret) {
> -        fprintf(stderr, "Couldn't finalize ibm,drc-power-domains property\n");
> +        error_report("Couldn't finalize ibm,drc-power-domains property");
>          goto out;
>      }
>  
>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names",
>                        drc_names->str, drc_names->len);
>      if (ret) {
> -        fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
> +        error_report("Couldn't finalize ibm,drc-names property");
>          goto out;
>      }
>  
>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types",
>                        drc_types->str, drc_types->len);
>      if (ret) {
> -        fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
> +        error_report("Couldn't finalize ibm,drc-types property");
>          goto out;
>      }
>  
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index d57b05d5c0f1..53c2cd4fb657 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -310,8 +310,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
>      char tmp[32];
>  
>      if (spapr_tce_find_by_liobn(liobn)) {
> -        fprintf(stderr, "Attempted to create TCE table with duplicate"
> -                " LIOBN 0x%x\n", liobn);
> +        error_report("Attempted to create TCE table with duplicate"
> +                " LIOBN 0x%x", liobn);
>          return NULL;
>      }
>  
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index dc058e512b86..27b5ad4bc437 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -27,6 +27,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "qemu/log.h"
> +#include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/char.h"
>  #include "hw/qdev.h"
> @@ -716,7 +717,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>  
>      ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't add RTAS reserve entry: %s\n",
> +        error_report("Couldn't add RTAS reserve entry: %s",
>                  fdt_strerror(ret));
>          return ret;
>      }
> @@ -724,7 +725,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
>                                  rtas_addr);
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
> +        error_report("Couldn't add linux,rtas-base property: %s",
>                  fdt_strerror(ret));
>          return ret;
>      }
> @@ -732,7 +733,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
>                                  rtas_addr);
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
> +        error_report("Couldn't add linux,rtas-entry property: %s",
>                  fdt_strerror(ret));
>          return ret;
>      }
> @@ -740,7 +741,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
>                                  rtas_size);
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't add rtas-size property: %s\n",
> +        error_report("Couldn't add rtas-size property: %s",
>                  fdt_strerror(ret));
>          return ret;
>      }
> @@ -755,7 +756,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>          ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
>                                      i + RTAS_TOKEN_BASE);
>          if (ret < 0) {
> -            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
> +            error_report("Couldn't add rtas token for %s: %s",
>                      call->name, fdt_strerror(ret));
>              return ret;
>          }
> @@ -770,7 +771,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>      ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
>                       sizeof(lrdr_capacity));
>      if (ret < 0) {
> -        fprintf(stderr, "Couldn't add ibm,lrdr-capacity rtas property\n");
> +        error_report("Couldn't add ibm,lrdr-capacity rtas property");
>          return ret;
>      }
>  
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index f93244d7c182..497028f075e3 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "hw/hw.h"
>  #include "qemu/log.h"
> @@ -276,7 +277,7 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq)
>      uint8_t byte;
>  
>      if (!dev->crq.qsize) {
> -        fprintf(stderr, "spapr_vio_send_creq on uninitialized queue\n");
> +        error_report("spapr_vio_send_creq on uninitialized queue");
>          return -1;
>      }
>  

-- 
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: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform
  2016-07-25 14:24 ` [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform Cédric Le Goater
@ 2016-07-26  6:23   ` David Gibson
  2016-07-28 17:27     ` Cédric Le Goater
  0 siblings, 1 reply; 10+ messages in thread
From: David Gibson @ 2016-07-26  6:23 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Alexander Graf, qemu-devel, qemu-ppc, Benjamin Herrenschmidt

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

On Mon, Jul 25, 2016 at 04:24:43PM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> No devices yet, not even an interrupt controller, just to get
> started.
> 
> (Folded in Stewart Smith patch to add command lien support)
                                                ^^^^ typo


> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [clg: updated for qemu-2.7
>       replaced fprintf by error_report
>       used a common definition of _FDT macro
>       removed VMStateDescription as migration is not yet supported
>       added IBM Copyright statements
> ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  default-configs/ppc64-softmmu.mak |   1 +
>  hw/ppc/Makefile.objs              |   2 +
>  hw/ppc/pnv.c                      | 593 ++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h              |  35 +++
>  4 files changed, 631 insertions(+)
>  create mode 100644 hw/ppc/pnv.c
>  create mode 100644 include/hw/ppc/pnv.h
> 
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index c4be59f638ed..516a6e25aba3 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -40,6 +40,7 @@ CONFIG_I8259=y
>  CONFIG_XILINX=y
>  CONFIG_XILINX_ETHLITE=y
>  CONFIG_PSERIES=y
> +CONFIG_POWERNV=y
>  CONFIG_PREP=y
>  CONFIG_MAC=y
>  CONFIG_E500=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 91a3420f473a..cbde482dd1b4 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -5,6 +5,8 @@ obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o
> +# IBM PowerNV
> +obj-$(CONFIG_POWERNV) += pnv.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o
>  endif
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> new file mode 100644
> index 000000000000..5096a073e829
> --- /dev/null
> +++ b/hw/ppc/pnv.c
> @@ -0,0 +1,593 @@
> +/*
> + * QEMU PowerPC PowerNV model
> + *
> + * Copyright (c) 2004-2007 Fabrice Bellard
> + * Copyright (c) 2007 Jocelyn Mayer
> + * Copyright (c) 2010 David Gibson, IBM Corporation.
> + * Copyright (c) 2014-2016 BenH, 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 "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/hw.h"
> +#include "hw/fw-path-provider.h"
> +#include "elf.h"
> +#include "net/net.h"
> +#include "sysemu/block-backend.h"
> +#include "sysemu/cpus.h"
> +#include "sysemu/kvm.h"
> +#include "sysemu/numa.h"
> +#include "kvm_ppc.h"
> +#include "mmu-hash64.h"
> +#include "qom/cpu.h"
> +
> +#include "hw/boards.h"
> +#include "hw/ppc/fdt.h"
> +#include "hw/ppc/ppc.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/loader.h"
> +
> +#include "exec/address-spaces.h"
> +#include "qemu/config-file.h"
> +#include "qemu/error-report.h"
> +#include "trace.h"
> +#include "hw/nmi.h"
> +
> +#include "hw/compat.h"
> +
> +#include <libfdt.h>
> +
> +#define FDT_ADDR                0x01000000
> +#define FDT_MAX_SIZE            0x00100000
> +#define FW_MAX_SIZE             0x00400000
> +#define FW_FILE_NAME            "skiboot.lid"
> +#define KERNEL_FILE_NAME        "skiroot.lid"
> +#define KERNEL_LOAD_ADDR        0x20000000

The KERNEL_* names don't really make sense here.

> +#define TIMEBASE_FREQ           512000000ULL
> +
> +#define MAX_CPUS                255
> +
> +#define PHANDLE_XICP            0x00001111

Since you're not implementing an interrupt controller yet, I don't
think you need this define.


> +
> +typedef struct sPowerNVMachineState sPowerNVMachineState;
> +
> +#define TYPE_POWERNV_MACHINE      "powernv-machine"
> +#define POWERNV_MACHINE(obj) \
> +    OBJECT_CHECK(sPowerNVMachineState, (obj), TYPE_POWERNV_MACHINE)
> +
> +/**
> + * sPowerNVMachineState:
> + */
> +struct sPowerNVMachineState {
> +    /*< private >*/
> +    MachineState parent_obj;
> +    PnvSystem sys;

Having all the system specific information in this substructure is a
bit awkward.  I think it would be preferable to just open code the
fields you need directly into sPowerNVMachineState.

> +};
> +
> +static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
> +                                     size_t maxsize)
> +{
> +    size_t maxcells = maxsize / sizeof(uint32_t);
> +    int i, j, count;
> +    uint32_t *p = prop;
> +
> +    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
> +        struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
> +
> +        if (!sps->page_shift) {
> +            break;
> +        }
> +        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
> +            if (sps->enc[count].page_shift == 0) {
> +                break;
> +            }
> +        }
> +        if ((p - prop) >= (maxcells - 3 - count * 2)) {
> +            break;
> +        }
> +        *(p++) = cpu_to_be32(sps->page_shift);
> +        *(p++) = cpu_to_be32(sps->slb_enc);
> +        *(p++) = cpu_to_be32(count);
> +        for (j = 0; j < count; j++) {
> +            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
> +            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
> +        }
> +    }
> +
> +    return (p - prop) * sizeof(uint32_t);
> +}

Hm, I do wonder if we should make a place for helper functions that
are common between spapr and powernv.

> +static void powernv_populate_memory_node(void *fdt, int nodeid, hwaddr start,
> +                                         hwaddr size)
> +{
> +    /* Probablly bogus, need to match with what's going on in CPU nodes */
> +    uint32_t chip_id[] = {
> +        cpu_to_be32(0x0), cpu_to_be32(nodeid)
> +    };
> +    char *mem_name;
> +    uint64_t mem_reg_property[2];
> +
> +    mem_reg_property[0] = cpu_to_be64(start);
> +    mem_reg_property[1] = cpu_to_be64(size);
> +
> +    mem_name = g_strdup_printf("memory@"TARGET_FMT_lx, start);
> +    _FDT((fdt_begin_node(fdt, mem_name)));
> +    g_free(mem_name);
> +    _FDT((fdt_property_string(fdt, "device_type", "memory")));
> +    _FDT((fdt_property(fdt, "reg", mem_reg_property,
> +                       sizeof(mem_reg_property))));
> +    _FDT((fdt_property(fdt, "ibm,chip-id", chip_id, sizeof(chip_id))));
> +    _FDT((fdt_end_node(fdt)));
> +}
> +
> +static int powernv_populate_memory(void *fdt)
> +{
> +    hwaddr mem_start, node_size;
> +    int i, nb_nodes = nb_numa_nodes;
> +    NodeInfo *nodes = numa_info;
> +    NodeInfo ramnode;
> +
> +    /* No NUMA nodes, assume there is just one node with whole RAM */
> +    if (!nb_numa_nodes) {
> +        nb_nodes = 1;
> +        ramnode.node_mem = ram_size;
> +        nodes = &ramnode;
> +    }
> +
> +    for (i = 0, mem_start = 0; i < nb_nodes; ++i) {
> +        if (!nodes[i].node_mem) {
> +            continue;
> +        }
> +        if (mem_start >= ram_size) {
> +            node_size = 0;
> +        } else {
> +            node_size = nodes[i].node_mem;
> +            if (node_size > ram_size - mem_start) {
> +                node_size = ram_size - mem_start;
> +            }
> +        }
> +        for ( ; node_size; ) {
> +            hwaddr sizetmp = pow2floor(node_size);
> +
> +            /* mem_start != 0 here */
> +            if (ctzl(mem_start) < ctzl(sizetmp)) {
> +                sizetmp = 1ULL << ctzl(mem_start);
> +            }
> +
> +            powernv_populate_memory_node(fdt, i, mem_start, sizetmp);
> +            node_size -= sizetmp;
> +            mem_start += sizetmp;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static void powernv_create_cpu_node(void *fdt, CPUState *cs, int smt_threads)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    DeviceClass *dc = DEVICE_GET_CLASS(cs);
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> +    uint32_t servers_prop[smt_threads];
> +    uint32_t gservers_prop[smt_threads * 2];
> +    int i, index = ppc_get_vcpu_dt_id(cpu);
> +    uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> +                       0xffffffff, 0xffffffff};
> +    uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
> +    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
> +    uint32_t page_sizes_prop[64];
> +    size_t page_sizes_prop_size;
> +    char *nodename;
> +
> +    if ((index % smt_threads) != 0) {
> +        return;
> +    }

This hack exists in spapr for historical reasons.  I'd prefer if on
powernv you treat cpu cores as first class objects from the beginning,
and just call this once per core.

> +    nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
> +
> +    _FDT((fdt_begin_node(fdt, nodename)));
> +
> +    g_free(nodename);
> +
> +    _FDT((fdt_property_cell(fdt, "reg", index)));
> +    _FDT((fdt_property_string(fdt, "device_type", "cpu")));
> +
> +    _FDT((fdt_property_cell(fdt, "cpu-version", env->spr[SPR_PVR])));
> +    _FDT((fdt_property_cell(fdt, "d-cache-block-size",
> +                            env->dcache_line_size)));
> +    _FDT((fdt_property_cell(fdt, "d-cache-line-size",
> +                            env->dcache_line_size)));
> +    _FDT((fdt_property_cell(fdt, "i-cache-block-size",
> +                            env->icache_line_size)));
> +    _FDT((fdt_property_cell(fdt, "i-cache-line-size",
> +                            env->icache_line_size)));
> +
> +    if (pcc->l1_dcache_size) {
> +        _FDT((fdt_property_cell(fdt, "d-cache-size", pcc->l1_dcache_size)));
> +    } else {
> +        error_report("Warning: Unknown L1 dcache size for cpu");
> +    }
> +    if (pcc->l1_icache_size) {
> +        _FDT((fdt_property_cell(fdt, "i-cache-size", pcc->l1_icache_size)));
> +    } else {
> +        error_report("Warning: Unknown L1 icache size for cpu");
> +    }
> +
> +    _FDT((fdt_property_cell(fdt, "timebase-frequency", tbfreq)));
> +    _FDT((fdt_property_cell(fdt, "clock-frequency", cpufreq)));
> +    _FDT((fdt_property_cell(fdt, "ibm,slb-size", env->slb_nr)));
> +    _FDT((fdt_property_string(fdt, "status", "okay")));
> +    _FDT((fdt_property(fdt, "64-bit", NULL, 0)));
> +
> +    if (env->spr_cb[SPR_PURR].oea_read) {
> +        _FDT((fdt_property(fdt, "ibm,purr", NULL, 0)));
> +    }
> +
> +    if (env->mmu_model & POWERPC_MMU_1TSEG) {
> +        _FDT((fdt_property(fdt, "ibm,processor-segment-sizes",
> +                           segs, sizeof(segs))));
> +    }
> +
> +    /* Advertise VMX/VSX (vector extensions) if available
> +     *   0 / no property == no vector extensions
> +     *   1               == VMX / Altivec available
> +     *   2               == VSX available */
> +    if (env->insns_flags & PPC_ALTIVEC) {
> +        uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
> +
> +        _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
> +    }
> +
> +    /* Advertise DFP (Decimal Floating Point) if available
> +     *   0 / no property == no DFP
> +     *   1               == DFP available */
> +    if (env->insns_flags2 & PPC2_DFP) {
> +        _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
> +    }
> +
> +    page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
> +                                                  sizeof(page_sizes_prop));
> +    if (page_sizes_prop_size) {
> +        _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
> +                           page_sizes_prop, page_sizes_prop_size)));
> +    }
> +
> +    /* XXX Just a hack for now */
> +    _FDT((fdt_property_cell(fdt, "ibm,chip-id", 0)));
> +
> +    if (cpu->cpu_version) {
> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
> +    }
> +
> +    /* Build interrupt servers and gservers properties */
> +    for (i = 0; i < smt_threads; i++) {
> +        servers_prop[i] = cpu_to_be32(index + i);
> +        /* Hack, direct the group queues back to cpu 0 */
> +        gservers_prop[i * 2] = cpu_to_be32(index + i);
> +        gservers_prop[i * 2 + 1] = 0;
> +    }
> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
> +                       servers_prop, sizeof(servers_prop))));
> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
> +                       gservers_prop, sizeof(gservers_prop))));
> +
> +    _FDT((fdt_end_node(fdt)));
> +}
> +
> +static void *powernv_create_fdt(PnvSystem *sys, const char *kernel_cmdline,
> +                                uint32_t initrd_base, uint32_t initrd_size)
> +{
> +    void *fdt;
> +    CPUState *cs;
> +    int smt = kvmppc_smt_threads();
> +    uint32_t start_prop = cpu_to_be32(initrd_base);
> +    uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
> +    char *buf;
> +    const char plat_compat[] = "qemu,powernv\0ibm,powernv";
> +
> +    fdt = g_malloc0(FDT_MAX_SIZE);
> +    _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
> +    _FDT((fdt_finish_reservemap(fdt)));
> +
> +    /* Root node */
> +    _FDT((fdt_begin_node(fdt, "")));
> +    _FDT((fdt_property_string(fdt, "model", "IBM PowerNV (emulated by qemu)")));
> +    _FDT((fdt_property(fdt, "compatible", plat_compat, sizeof(plat_compat))));
> +
> +    /*
> +     * Add info to guest to indentify which host is it being run on
> +     * and what is the uuid of the guest
> +     */
> +    if (kvmppc_get_host_model(&buf)) {
> +        _FDT((fdt_property_string(fdt, "host-model", buf)));
> +        g_free(buf);
> +    }
> +    if (kvmppc_get_host_serial(&buf)) {
> +        _FDT((fdt_property_string(fdt, "host-serial", buf)));
> +        g_free(buf);
> +    }

Those seem dubious for a non-paravirt platform.

> +
> +    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> +                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
> +                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> +                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
> +                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> +                          qemu_uuid[14], qemu_uuid[15]);
> +
> +    _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
> +    g_free(buf);
> +
> +    _FDT((fdt_begin_node(fdt, "chosen")));
> +    if (kernel_cmdline) {
> +        _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
> +    }
> +    _FDT((fdt_property(fdt, "linux,initrd-start",
> +                       &start_prop, sizeof(start_prop))));
> +    _FDT((fdt_property(fdt, "linux,initrd-end",
> +                       &end_prop, sizeof(end_prop))));
> +    _FDT((fdt_end_node(fdt)));
> +
> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
> +
> +    /* cpus */
> +    _FDT((fdt_begin_node(fdt, "cpus")));
> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
> +
> +    CPU_FOREACH(cs) {
> +        powernv_create_cpu_node(fdt, cs, smt);
> +    }
> +
> +    _FDT((fdt_end_node(fdt)));
> +
> +    /* Memory */
> +    _FDT((powernv_populate_memory(fdt)));
> +
> +    /* /hypervisor node */

This really doesn't seem like it makes sense on a non-paravirt platform.

> +    if (kvm_enabled()) {
> +        uint8_t hypercall[16];
> +
> +        /* indicate KVM hypercall interface */
> +        _FDT((fdt_begin_node(fdt, "hypervisor")));
> +        _FDT((fdt_property_string(fdt, "compatible", "linux,kvm")));
> +        if (kvmppc_has_cap_fixup_hcalls()) {
> +            /*
> +             * Older KVM versions with older guest kernels were broken with the
> +             * magic page, don't allow the guest to map it.
> +             */
> +            kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
> +                                 sizeof(hypercall));
> +            _FDT((fdt_property(fdt, "hcall-instructions", hypercall,
> +                              sizeof(hypercall))));
> +        }
> +        _FDT((fdt_end_node(fdt)));
> +    }
> +
> +    _FDT((fdt_end_node(fdt))); /* close root node */
> +    _FDT((fdt_finish(fdt)));
> +
> +    return fdt;
> +}
> +
> +static void powernv_cpu_reset(void *opaque)
> +{
> +    PowerPCCPU *cpu = opaque;
> +    CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
> +
> +    cpu_reset(cs);
> +
> +    env->spr[SPR_PIR] = ppc_get_vcpu_dt_id(cpu);
> +    env->spr[SPR_HIOR] = 0;
> +    env->gpr[3] = FDT_ADDR;
> +    env->nip = 0x10;
> +    env->msr |= MSR_HVB;
> +}
> +
> +static void pnv_create_chip(PnvSystem *sys, unsigned int chip_no)
> +{
> +    PnvChip *chip = &sys->chips[chip_no];

Hm, my inclination would be to make the pnv chips a full QOM type,
unless there's a reason not to.

> +
> +    if (chip_no >= PNV_MAX_CHIPS) {
> +            return;
> +    }
> +
> +    /* XXX Improve chip numbering to better match HW */
> +    chip->chip_id = chip_no;
> +}
> +
> +static void ppc_powernv_init(MachineState *machine)
> +{
> +    ram_addr_t ram_size = machine->ram_size;
> +    const char *cpu_model = machine->cpu_model;
> +    const char *kernel_filename = machine->kernel_filename;
> +    const char *initrd_filename = machine->initrd_filename;
> +    uint32_t initrd_base = 0;
> +    long initrd_size = 0;
> +    PowerPCCPU *cpu;
> +    CPUPPCState *env;
> +    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +    sPowerNVMachineState *pnv_machine = POWERNV_MACHINE(machine);
> +    PnvSystem *sys = &pnv_machine->sys;
> +    long fw_size;
> +    char *filename;
> +    void *fdt;
> +    int i;
> +
> +    /* init CPUs */
> +    if (cpu_model == NULL) {
> +        cpu_model = kvm_enabled() ? "host" : "POWER8";
> +    }
> +
> +    for (i = 0; i < smp_cpus; i++) {
> +        cpu = cpu_ppc_init(cpu_model);
> +        if (cpu == NULL) {
> +            error_report("Unable to find PowerPC CPU definition");
> +            exit(1);
> +        }
> +        env = &cpu->env;
> +
> +        /* Set time-base frequency to 512 MHz */
> +        cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> +
> +        /* MSR[IP] doesn't exist nowadays */
> +        env->msr_mask &= ~(1 << 6);
> +
> +        qemu_register_reset(powernv_cpu_reset, cpu);

As noted above, I think we want to start powernv off with "new style"
cpu instantiation.  So for powernv, I think you want the machine to
create chip objects, which will create core objects which will create
the vcpu/thread objects.

> +    }
> +
> +    /* allocate RAM */
> +    memory_region_allocate_system_memory(ram, NULL, "ppc_powernv.ram",
> +                                         ram_size);
> +    memory_region_add_subregion(sysmem, 0, ram);
> +
> +    /* XXX We should decide how many chips to create based on #cores and
> +     * Venice vs. Murano vs. Naples chip type etc..., for now, just create
> +     * one chip. Also creation of the CPUs should be done per-chip
> +     */
> +    sys->num_chips = 1;
> +
> +    /* Create only one PHB for now until I figure out what's wrong
> +     * when I create more (resource assignment failures in Linux)
> +     */
> +    pnv_create_chip(sys, 0);
> +
> +    if (bios_name == NULL) {
> +        bios_name = FW_FILE_NAME;
> +    }
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +    fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
> +    if (fw_size < 0) {
> +        hw_error("qemu: could not load OPAL '%s'\n", filename);
> +        exit(1);
> +    }
> +    g_free(filename);
> +
> +
> +    if (kernel_filename == NULL) {
> +        kernel_filename = KERNEL_FILE_NAME;
> +    }
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, kernel_filename);
> +    fw_size = load_image_targphys(filename, 0x20000000, 0x2000000);
> +    if (fw_size < 0) {
> +        hw_error("qemu: could not load kernel'%s'\n", filename);
> +        exit(1);
> +    }
> +    g_free(filename);
> +
> +    /* load initrd */
> +    if (initrd_filename) {
> +            /* Try to locate the initrd in the gap between the kernel
> +             * and the firmware. Add a bit of space just in case
> +             */
> +            initrd_base = 0x40000000;
> +            initrd_size = load_image_targphys(initrd_filename, initrd_base,
> +                                              0x10000000); /* 128MB max */
> +            if (initrd_size < 0) {
> +                    error_report("qemu: could not load initial ram disk '%s'",
> +                            initrd_filename);
> +                    exit(1);
> +            }
> +    } else {
> +            initrd_base = 0;
> +            initrd_size = 0;
> +    }
> +    fdt = powernv_create_fdt(sys, machine->kernel_cmdline,
> +                             initrd_base, initrd_size);

So the fact that spapr fdt creation is split between machine init and
reset time causes us a fair bit of pain.

> +    cpu_physical_memory_write(FDT_ADDR, fdt, fdt_totalsize(fdt));
> +}
> +
> +static int powernv_kvm_type(const char *vm_type)
> +{
> +    /* Always force PR KVM */
> +    return 2;

This doesn't really seem right to me.  I think you should be giving an
error if the user tries to specify HV, rather than silently changing
to PR.

> +}
> +
> +static void ppc_cpu_do_nmi_on_cpu(void *arg)
> +{
> +    CPUState *cs = arg;
> +
> +    cpu_synchronize_state(cs);
> +    ppc_cpu_do_system_reset(cs);
> +}
> +
> +static void powernv_nmi(NMIState *n, int cpu_index, Error **errp)
> +{
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
> +    }
> +}

It may be simpler to just leave the nmi stuff out for now, until you
have a real implementation.

> +
> +static void powernv_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    NMIClass *nc = NMI_CLASS(oc);
> +
> +    mc->init = ppc_powernv_init;
> +    mc->block_default_type = IF_SCSI;
> +    mc->max_cpus = MAX_CPUS;
> +    mc->no_parallel = 1;
> +    mc->default_boot_order = NULL;
> +    mc->kvm_type = powernv_kvm_type;
> +
> +    nc->nmi_monitor_handler = powernv_nmi;
> +}
> +
> +static const TypeInfo powernv_machine_info = {
> +    .name          = TYPE_POWERNV_MACHINE,
> +    .parent        = TYPE_MACHINE,
> +    .abstract      = true,
> +    .instance_size = sizeof(sPowerNVMachineState),
> +    .class_init    = powernv_machine_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_NMI },
> +        { }
> +    },
> +};
> +
> +static void powernv_machine_2_5_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->name = "powernv-2.5";

Looks like the version numbthis needs an update for current qemu.

> +    mc->desc = "PowerNV v2.5";
> +    mc->alias = "powernv";
> +}
> +
> +static const TypeInfo powernv_machine_2_5_info = {
> +    .name          = MACHINE_TYPE_NAME("powernv-2.5"),
> +    .parent        = TYPE_POWERNV_MACHINE,
> +    .class_init    = powernv_machine_2_5_class_init,
> +};
> +
> +static void powernv_machine_register_types(void)
> +{
> +    type_register_static(&powernv_machine_info);
> +    type_register_static(&powernv_machine_2_5_info);
> +}
> +
> +type_init(powernv_machine_register_types)
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> new file mode 100644
> index 000000000000..383a336e7cd3
> --- /dev/null
> +++ b/include/hw/ppc/pnv.h
> @@ -0,0 +1,35 @@
> +/*
> + * QEMU PowerNV various definitions
> + *
> + * Copyright (c) 2014-2016 BenH, IBM Corporation.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _PPC_PNV_H
> +#define _PPC_PNV_H
> +
> +#include "hw/hw.h"
> +
> +/* Should we turn that into a QOjb of some sort ? */
> +typedef struct PnvChip {
> +    uint32_t         chip_id;
> +} PnvChip;
> +
> +typedef struct PnvSystem {
> +    uint32_t  num_chips;
> +#define PNV_MAX_CHIPS      1
> +    PnvChip   chips[PNV_MAX_CHIPS];
> +} PnvSystem;
> +
> +#endif /* _PPC_PNV_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: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf
  2016-07-26  5:58   ` David Gibson
@ 2016-07-26  6:29     ` Cédric Le Goater
  0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-26  6:29 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-devel, qemu-ppc

On 07/26/2016 07:58 AM, David Gibson wrote:
> On Mon, Jul 25, 2016 at 04:24:42PM +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> So, when I did some previous conversions to error_report(), I left out
> the DT code, in the hopes I'd obsolete it with a more thorough
> cleanup.  Since that won't happen for a while yet, I'm happy enough to
> queue this for 2.8.
> 
> However, if you're going to change all these instances, you should
> probably change the one in _FDT as well.

Indeed :) 

I will resend this one then.

Thanks,

C. 


>> ---
>>  hw/ppc/spapr.c       | 12 ++++++------
>>  hw/ppc/spapr_drc.c   |  8 ++++----
>>  hw/ppc/spapr_iommu.c |  4 ++--
>>  hw/ppc/spapr_rtas.c  | 13 +++++++------
>>  hw/ppc/spapr_vio.c   |  3 ++-
>>  5 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 538ff5a46768..4d073ce39e87 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -647,13 +647,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>>          _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
>>                                 pcc->l1_dcache_size)));
>>      } else {
>> -        fprintf(stderr, "Warning: Unknown L1 dcache size for cpu\n");
>> +        error_report("Warning: Unknown L1 dcache size for cpu");
>>      }
>>      if (pcc->l1_icache_size) {
>>          _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
>>                                 pcc->l1_icache_size)));
>>      } else {
>> -        fprintf(stderr, "Warning: Unknown L1 icache size for cpu\n");
>> +        error_report("Warning: Unknown L1 icache size for cpu");
>>      }
>>  
>>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>> @@ -944,20 +944,20 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>>  
>>      ret = spapr_populate_memory(spapr, fdt);
>>      if (ret < 0) {
>> -        fprintf(stderr, "couldn't setup memory nodes in fdt\n");
>> +        error_report("couldn't setup memory nodes in fdt");
>>          exit(1);
>>      }
>>  
>>      ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
>>      if (ret < 0) {
>> -        fprintf(stderr, "couldn't setup vio devices in fdt\n");
>> +        error_report("couldn't setup vio devices in fdt");
>>          exit(1);
>>      }
>>  
>>      if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
>>          ret = spapr_rng_populate_dt(fdt);
>>          if (ret < 0) {
>> -            fprintf(stderr, "could not set up rng device in the fdt\n");
>> +            error_report("could not set up rng device in the fdt");
>>              exit(1);
>>          }
>>      }
>> @@ -973,7 +973,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>>      /* RTAS */
>>      ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
>> +        error_report("Couldn't set up RTAS device tree properties");
>>      }
>>  
>>      /* cpus */
>> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
>> index 26a067951c54..4b1a943b8e4d 100644
>> --- a/hw/ppc/spapr_drc.c
>> +++ b/hw/ppc/spapr_drc.c
>> @@ -816,7 +816,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>>                        drc_indexes->data,
>>                        drc_indexes->len * sizeof(uint32_t));
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't create ibm,drc-indexes property\n");
>> +        error_report("Couldn't create ibm,drc-indexes property");
>>          goto out;
>>      }
>>  
>> @@ -824,21 +824,21 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>>                        drc_power_domains->data,
>>                        drc_power_domains->len * sizeof(uint32_t));
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-power-domains property\n");
>> +        error_report("Couldn't finalize ibm,drc-power-domains property");
>>          goto out;
>>      }
>>  
>>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names",
>>                        drc_names->str, drc_names->len);
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
>> +        error_report("Couldn't finalize ibm,drc-names property");
>>          goto out;
>>      }
>>  
>>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types",
>>                        drc_types->str, drc_types->len);
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
>> +        error_report("Couldn't finalize ibm,drc-types property");
>>          goto out;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index d57b05d5c0f1..53c2cd4fb657 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -310,8 +310,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
>>      char tmp[32];
>>  
>>      if (spapr_tce_find_by_liobn(liobn)) {
>> -        fprintf(stderr, "Attempted to create TCE table with duplicate"
>> -                " LIOBN 0x%x\n", liobn);
>> +        error_report("Attempted to create TCE table with duplicate"
>> +                " LIOBN 0x%x", liobn);
>>          return NULL;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index dc058e512b86..27b5ad4bc437 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -27,6 +27,7 @@
>>  #include "qemu/osdep.h"
>>  #include "cpu.h"
>>  #include "qemu/log.h"
>> +#include "qemu/error-report.h"
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/char.h"
>>  #include "hw/qdev.h"
>> @@ -716,7 +717,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>  
>>      ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add RTAS reserve entry: %s\n",
>> +        error_report("Couldn't add RTAS reserve entry: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -724,7 +725,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
>>                                  rtas_addr);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
>> +        error_report("Couldn't add linux,rtas-base property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -732,7 +733,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
>>                                  rtas_addr);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
>> +        error_report("Couldn't add linux,rtas-entry property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -740,7 +741,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
>>                                  rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add rtas-size property: %s\n",
>> +        error_report("Couldn't add rtas-size property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -755,7 +756,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>          ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
>>                                      i + RTAS_TOKEN_BASE);
>>          if (ret < 0) {
>> -            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
>> +            error_report("Couldn't add rtas token for %s: %s",
>>                      call->name, fdt_strerror(ret));
>>              return ret;
>>          }
>> @@ -770,7 +771,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>      ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
>>                       sizeof(lrdr_capacity));
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add ibm,lrdr-capacity rtas property\n");
>> +        error_report("Couldn't add ibm,lrdr-capacity rtas property");
>>          return ret;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
>> index f93244d7c182..497028f075e3 100644
>> --- a/hw/ppc/spapr_vio.c
>> +++ b/hw/ppc/spapr_vio.c
>> @@ -20,6 +20,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>>  #include "qapi/error.h"
>>  #include "hw/hw.h"
>>  #include "qemu/log.h"
>> @@ -276,7 +277,7 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq)
>>      uint8_t byte;
>>  
>>      if (!dev->crq.qsize) {
>> -        fprintf(stderr, "spapr_vio_send_creq on uninitialized queue\n");
>> +        error_report("spapr_vio_send_creq on uninitialized queue");
>>          return -1;
>>      }
>>  
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform
  2016-07-26  6:23   ` David Gibson
@ 2016-07-28 17:27     ` Cédric Le Goater
  2016-07-29  3:32       ` David Gibson
  0 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-07-28 17:27 UTC (permalink / raw)
  To: David Gibson; +Cc: Alexander Graf, qemu-devel, qemu-ppc, Benjamin Herrenschmidt

Hello,

On 07/26/2016 08:23 AM, David Gibson wrote:
> On Mon, Jul 25, 2016 at 04:24:43PM +0200, Cédric Le Goater wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> No devices yet, not even an interrupt controller, just to get
>> started.
>>
>> (Folded in Stewart Smith patch to add command lien support)
>                                                 ^^^^ typo

and it looks like french to me. The changelog needs an update with
a little paragraph on what we are not emulating. The lowlevel firmware 
Hostboot should be mentionned.

>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> [clg: updated for qemu-2.7
>>       replaced fprintf by error_report
>>       used a common definition of _FDT macro
>>       removed VMStateDescription as migration is not yet supported
>>       added IBM Copyright statements
>> ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  default-configs/ppc64-softmmu.mak |   1 +
>>  hw/ppc/Makefile.objs              |   2 +
>>  hw/ppc/pnv.c                      | 593 ++++++++++++++++++++++++++++++++++++++
>>  include/hw/ppc/pnv.h              |  35 +++
>>  4 files changed, 631 insertions(+)
>>  create mode 100644 hw/ppc/pnv.c
>>  create mode 100644 include/hw/ppc/pnv.h
>>
>> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
>> index c4be59f638ed..516a6e25aba3 100644
>> --- a/default-configs/ppc64-softmmu.mak
>> +++ b/default-configs/ppc64-softmmu.mak
>> @@ -40,6 +40,7 @@ CONFIG_I8259=y
>>  CONFIG_XILINX=y
>>  CONFIG_XILINX_ETHLITE=y
>>  CONFIG_PSERIES=y
>> +CONFIG_POWERNV=y
>>  CONFIG_PREP=y
>>  CONFIG_MAC=y
>>  CONFIG_E500=y
>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>> index 91a3420f473a..cbde482dd1b4 100644
>> --- a/hw/ppc/Makefile.objs
>> +++ b/hw/ppc/Makefile.objs
>> @@ -5,6 +5,8 @@ obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>>  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o
>> +# IBM PowerNV
>> +obj-$(CONFIG_POWERNV) += pnv.o
>>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>>  obj-y += spapr_pci_vfio.o
>>  endif
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> new file mode 100644
>> index 000000000000..5096a073e829
>> --- /dev/null
>> +++ b/hw/ppc/pnv.c
>> @@ -0,0 +1,593 @@
>> +/*
>> + * QEMU PowerPC PowerNV model
>> + *
>> + * Copyright (c) 2004-2007 Fabrice Bellard
>> + * Copyright (c) 2007 Jocelyn Mayer
>> + * Copyright (c) 2010 David Gibson, IBM Corporation.
>> + * Copyright (c) 2014-2016 BenH, 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 "qemu/osdep.h"
>> +#include "sysemu/sysemu.h"
>> +#include "hw/hw.h"
>> +#include "hw/fw-path-provider.h"
>> +#include "elf.h"
>> +#include "net/net.h"
>> +#include "sysemu/block-backend.h"
>> +#include "sysemu/cpus.h"
>> +#include "sysemu/kvm.h"
>> +#include "sysemu/numa.h"
>> +#include "kvm_ppc.h"
>> +#include "mmu-hash64.h"
>> +#include "qom/cpu.h"
>> +
>> +#include "hw/boards.h"
>> +#include "hw/ppc/fdt.h"
>> +#include "hw/ppc/ppc.h"
>> +#include "hw/ppc/pnv.h"
>> +#include "hw/loader.h"
>> +
>> +#include "exec/address-spaces.h"
>> +#include "qemu/config-file.h"
>> +#include "qemu/error-report.h"
>> +#include "trace.h"
>> +#include "hw/nmi.h"
>> +
>> +#include "hw/compat.h"
>> +
>> +#include <libfdt.h>
>> +
>> +#define FDT_ADDR                0x01000000
>> +#define FDT_MAX_SIZE            0x00100000
>> +#define FW_MAX_SIZE             0x00400000
>> +#define FW_FILE_NAME            "skiboot.lid"
>> +#define KERNEL_FILE_NAME        "skiroot.lid"
>> +#define KERNEL_LOAD_ADDR        0x20000000
> 
> The KERNEL_* names don't really make sense here.

yes.

>> +#define TIMEBASE_FREQ           512000000ULL
>> +
>> +#define MAX_CPUS                255
>> +
>> +#define PHANDLE_XICP            0x00001111
> 
> Since you're not implementing an interrupt controller yet, I don't
> think you need this define.

indeed. This is too early.
 
>> +
>> +typedef struct sPowerNVMachineState sPowerNVMachineState;
>> +
>> +#define TYPE_POWERNV_MACHINE      "powernv-machine"
>> +#define POWERNV_MACHINE(obj) \
>> +    OBJECT_CHECK(sPowerNVMachineState, (obj), TYPE_POWERNV_MACHINE)
>> +
>> +/**
>> + * sPowerNVMachineState:
>> + */
>> +struct sPowerNVMachineState {
>> +    /*< private >*/
>> +    MachineState parent_obj;
>> +    PnvSystem sys;
> 
> Having all the system specific information in this substructure is a
> bit awkward.  I think it would be preferable to just open code the
> fields you need directly into sPowerNVMachineState.

I agree.

>> +};
>> +
>> +static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
>> +                                     size_t maxsize)
>> +{
>> +    size_t maxcells = maxsize / sizeof(uint32_t);
>> +    int i, j, count;
>> +    uint32_t *p = prop;
>> +
>> +    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
>> +        struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
>> +
>> +        if (!sps->page_shift) {
>> +            break;
>> +        }
>> +        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
>> +            if (sps->enc[count].page_shift == 0) {
>> +                break;
>> +            }
>> +        }
>> +        if ((p - prop) >= (maxcells - 3 - count * 2)) {
>> +            break;
>> +        }
>> +        *(p++) = cpu_to_be32(sps->page_shift);
>> +        *(p++) = cpu_to_be32(sps->slb_enc);
>> +        *(p++) = cpu_to_be32(count);
>> +        for (j = 0; j < count; j++) {
>> +            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
>> +            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
>> +        }
>> +    }
>> +
>> +    return (p - prop) * sizeof(uint32_t);
>> +}
> 
> Hm, I do wonder if we should make a place for helper functions that
> are common between spapr and powernv.

done.

powernv_populate_memory() and powernv_populate_memory_node() are also 
very similar in spapr. I wonder if we can merge them ? 

>> +static void powernv_populate_memory_node(void *fdt, int nodeid, hwaddr start,
>> +                                         hwaddr size)
>> +{
>> +    /* Probablly bogus, need to match with what's going on in CPU nodes */
>> +    uint32_t chip_id[] = {
>> +        cpu_to_be32(0x0), cpu_to_be32(nodeid)
>> +    };
>> +    char *mem_name;
>> +    uint64_t mem_reg_property[2];
>> +
>> +    mem_reg_property[0] = cpu_to_be64(start);
>> +    mem_reg_property[1] = cpu_to_be64(size);
>> +
>> +    mem_name = g_strdup_printf("memory@"TARGET_FMT_lx, start);
>> +    _FDT((fdt_begin_node(fdt, mem_name)));
>> +    g_free(mem_name);
>> +    _FDT((fdt_property_string(fdt, "device_type", "memory")));
>> +    _FDT((fdt_property(fdt, "reg", mem_reg_property,
>> +                       sizeof(mem_reg_property))));
>> +    _FDT((fdt_property(fdt, "ibm,chip-id", chip_id, sizeof(chip_id))));
>> +    _FDT((fdt_end_node(fdt)));
>> +}
>> +
>> +static int powernv_populate_memory(void *fdt)
>> +{
>> +    hwaddr mem_start, node_size;
>> +    int i, nb_nodes = nb_numa_nodes;
>> +    NodeInfo *nodes = numa_info;
>> +    NodeInfo ramnode;
>> +
>> +    /* No NUMA nodes, assume there is just one node with whole RAM */
>> +    if (!nb_numa_nodes) {
>> +        nb_nodes = 1;
>> +        ramnode.node_mem = ram_size;
>> +        nodes = &ramnode;
>> +    }
>> +
>> +    for (i = 0, mem_start = 0; i < nb_nodes; ++i) {
>> +        if (!nodes[i].node_mem) {
>> +            continue;
>> +        }
>> +        if (mem_start >= ram_size) {
>> +            node_size = 0;
>> +        } else {
>> +            node_size = nodes[i].node_mem;
>> +            if (node_size > ram_size - mem_start) {
>> +                node_size = ram_size - mem_start;
>> +            }
>> +        }
>> +        for ( ; node_size; ) {
>> +            hwaddr sizetmp = pow2floor(node_size);
>> +
>> +            /* mem_start != 0 here */
>> +            if (ctzl(mem_start) < ctzl(sizetmp)) {
>> +                sizetmp = 1ULL << ctzl(mem_start);
>> +            }
>> +
>> +            powernv_populate_memory_node(fdt, i, mem_start, sizetmp);
>> +            node_size -= sizetmp;
>> +            mem_start += sizetmp;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void powernv_create_cpu_node(void *fdt, CPUState *cs, int smt_threads)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +    CPUPPCState *env = &cpu->env;
>> +    DeviceClass *dc = DEVICE_GET_CLASS(cs);
>> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
>> +    uint32_t servers_prop[smt_threads];
>> +    uint32_t gservers_prop[smt_threads * 2];
>> +    int i, index = ppc_get_vcpu_dt_id(cpu);
>> +    uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
>> +                       0xffffffff, 0xffffffff};
>> +    uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
>> +    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
>> +    uint32_t page_sizes_prop[64];
>> +    size_t page_sizes_prop_size;
>> +    char *nodename;
>> +
>> +    if ((index % smt_threads) != 0) {
>> +        return;
>> +    }
> 
> This hack exists in spapr for historical reasons.  I'd prefer if on
> powernv you treat cpu cores as first class objects from the beginning,
> and just call this once per core.

like this is done with sPAPRCPUCore I suppose. 

I will get inspiration from the spapr cores and see if I can find some 
common ground with the powernv cores. we should.

>> +    nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
>> +
>> +    _FDT((fdt_begin_node(fdt, nodename)));
>> +
>> +    g_free(nodename);
>> +
>> +    _FDT((fdt_property_cell(fdt, "reg", index)));
>> +    _FDT((fdt_property_string(fdt, "device_type", "cpu")));
>> +
>> +    _FDT((fdt_property_cell(fdt, "cpu-version", env->spr[SPR_PVR])));
>> +    _FDT((fdt_property_cell(fdt, "d-cache-block-size",
>> +                            env->dcache_line_size)));
>> +    _FDT((fdt_property_cell(fdt, "d-cache-line-size",
>> +                            env->dcache_line_size)));
>> +    _FDT((fdt_property_cell(fdt, "i-cache-block-size",
>> +                            env->icache_line_size)));
>> +    _FDT((fdt_property_cell(fdt, "i-cache-line-size",
>> +                            env->icache_line_size)));
>> +
>> +    if (pcc->l1_dcache_size) {
>> +        _FDT((fdt_property_cell(fdt, "d-cache-size", pcc->l1_dcache_size)));
>> +    } else {
>> +        error_report("Warning: Unknown L1 dcache size for cpu");
>> +    }
>> +    if (pcc->l1_icache_size) {
>> +        _FDT((fdt_property_cell(fdt, "i-cache-size", pcc->l1_icache_size)));
>> +    } else {
>> +        error_report("Warning: Unknown L1 icache size for cpu");
>> +    }
>> +
>> +    _FDT((fdt_property_cell(fdt, "timebase-frequency", tbfreq)));
>> +    _FDT((fdt_property_cell(fdt, "clock-frequency", cpufreq)));
>> +    _FDT((fdt_property_cell(fdt, "ibm,slb-size", env->slb_nr)));
>> +    _FDT((fdt_property_string(fdt, "status", "okay")));
>> +    _FDT((fdt_property(fdt, "64-bit", NULL, 0)));
>> +
>> +    if (env->spr_cb[SPR_PURR].oea_read) {
>> +        _FDT((fdt_property(fdt, "ibm,purr", NULL, 0)));
>> +    }
>> +
>> +    if (env->mmu_model & POWERPC_MMU_1TSEG) {
>> +        _FDT((fdt_property(fdt, "ibm,processor-segment-sizes",
>> +                           segs, sizeof(segs))));
>> +    }
>> +
>> +    /* Advertise VMX/VSX (vector extensions) if available
>> +     *   0 / no property == no vector extensions
>> +     *   1               == VMX / Altivec available
>> +     *   2               == VSX available */
>> +    if (env->insns_flags & PPC_ALTIVEC) {
>> +        uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
>> +
>> +        _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
>> +    }
>> +
>> +    /* Advertise DFP (Decimal Floating Point) if available
>> +     *   0 / no property == no DFP
>> +     *   1               == DFP available */
>> +    if (env->insns_flags2 & PPC2_DFP) {
>> +        _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
>> +    }
>> +
>> +    page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
>> +                                                  sizeof(page_sizes_prop));
>> +    if (page_sizes_prop_size) {
>> +        _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
>> +                           page_sizes_prop, page_sizes_prop_size)));
>> +    }
>> +
>> +    /* XXX Just a hack for now */
>> +    _FDT((fdt_property_cell(fdt, "ibm,chip-id", 0)));
>> +
>> +    if (cpu->cpu_version) {
>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>> +    }
>> +
>> +    /* Build interrupt servers and gservers properties */
>> +    for (i = 0; i < smt_threads; i++) {
>> +        servers_prop[i] = cpu_to_be32(index + i);
>> +        /* Hack, direct the group queues back to cpu 0 */
>> +        gservers_prop[i * 2] = cpu_to_be32(index + i);
>> +        gservers_prop[i * 2 + 1] = 0;
>> +    }
>> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>> +                       servers_prop, sizeof(servers_prop))));
>> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>> +                       gservers_prop, sizeof(gservers_prop))));
>> +
>> +    _FDT((fdt_end_node(fdt)));
>> +}
>> +
>> +static void *powernv_create_fdt(PnvSystem *sys, const char *kernel_cmdline,
>> +                                uint32_t initrd_base, uint32_t initrd_size)
>> +{
>> +    void *fdt;
>> +    CPUState *cs;
>> +    int smt = kvmppc_smt_threads();
>> +    uint32_t start_prop = cpu_to_be32(initrd_base);
>> +    uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
>> +    char *buf;
>> +    const char plat_compat[] = "qemu,powernv\0ibm,powernv";
>> +
>> +    fdt = g_malloc0(FDT_MAX_SIZE);
>> +    _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
>> +    _FDT((fdt_finish_reservemap(fdt)));
>> +
>> +    /* Root node */
>> +    _FDT((fdt_begin_node(fdt, "")));
>> +    _FDT((fdt_property_string(fdt, "model", "IBM PowerNV (emulated by qemu)")));
>> +    _FDT((fdt_property(fdt, "compatible", plat_compat, sizeof(plat_compat))));
>> +
>> +    /*
>> +     * Add info to guest to indentify which host is it being run on
>> +     * and what is the uuid of the guest
>> +     */
>> +    if (kvmppc_get_host_model(&buf)) {
>> +        _FDT((fdt_property_string(fdt, "host-model", buf)));
>> +        g_free(buf);
>> +    }
>> +    if (kvmppc_get_host_serial(&buf)) {
>> +        _FDT((fdt_property_string(fdt, "host-serial", buf)));
>> +        g_free(buf);
>> +    }
> 
> Those seem dubious for a non-paravirt platform.

OK. I will remove all the KVM related parts. We can add them later on when
we work on kvm-pr support.

>> +
>> +    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
>> +                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
>> +                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
>> +                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
>> +                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
>> +                          qemu_uuid[14], qemu_uuid[15]);
>> +
>> +    _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
>> +    g_free(buf);
>> +
>> +    _FDT((fdt_begin_node(fdt, "chosen")));
>> +    if (kernel_cmdline) {
>> +        _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
>> +    }
>> +    _FDT((fdt_property(fdt, "linux,initrd-start",
>> +                       &start_prop, sizeof(start_prop))));
>> +    _FDT((fdt_property(fdt, "linux,initrd-end",
>> +                       &end_prop, sizeof(end_prop))));
>> +    _FDT((fdt_end_node(fdt)));
>> +
>> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>> +
>> +    /* cpus */
>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
>> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
>> +
>> +    CPU_FOREACH(cs) {
>> +        powernv_create_cpu_node(fdt, cs, smt);
>> +    }
>> +
>> +    _FDT((fdt_end_node(fdt)));
>> +
>> +    /* Memory */
>> +    _FDT((powernv_populate_memory(fdt)));
>> +
>> +    /* /hypervisor node */
> 
> This really doesn't seem like it makes sense on a non-paravirt platform.
> 
>> +    if (kvm_enabled()) {
>> +        uint8_t hypercall[16];
>> +
>> +        /* indicate KVM hypercall interface */
>> +        _FDT((fdt_begin_node(fdt, "hypervisor")));
>> +        _FDT((fdt_property_string(fdt, "compatible", "linux,kvm")));
>> +        if (kvmppc_has_cap_fixup_hcalls()) {
>> +            /*
>> +             * Older KVM versions with older guest kernels were broken with the
>> +             * magic page, don't allow the guest to map it.
>> +             */
>> +            kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
>> +                                 sizeof(hypercall));
>> +            _FDT((fdt_property(fdt, "hcall-instructions", hypercall,
>> +                              sizeof(hypercall))));
>> +        }
>> +        _FDT((fdt_end_node(fdt)));
>> +    }
>> +
>> +    _FDT((fdt_end_node(fdt))); /* close root node */
>> +    _FDT((fdt_finish(fdt)));
>> +
>> +    return fdt;
>> +}
>> +
>> +static void powernv_cpu_reset(void *opaque)
>> +{
>> +    PowerPCCPU *cpu = opaque;
>> +    CPUState *cs = CPU(cpu);
>> +    CPUPPCState *env = &cpu->env;
>> +
>> +    cpu_reset(cs);
>> +
>> +    env->spr[SPR_PIR] = ppc_get_vcpu_dt_id(cpu);
>> +    env->spr[SPR_HIOR] = 0;
>> +    env->gpr[3] = FDT_ADDR;
>> +    env->nip = 0x10;
>> +    env->msr |= MSR_HVB;
>> +}
>> +
>> +static void pnv_create_chip(PnvSystem *sys, unsigned int chip_no)
>> +{
>> +    PnvChip *chip = &sys->chips[chip_no];
> 
> Hm, my inclination would be to make the pnv chips a full QOM type,
> unless there's a reason not to.

Yes. let's go that way and I will try to port the rest of the patchset
before sending to catch any issues.

>> +
>> +    if (chip_no >= PNV_MAX_CHIPS) {
>> +            return;
>> +    }
>> +
>> +    /* XXX Improve chip numbering to better match HW */
>> +    chip->chip_id = chip_no;
>> +}
>> +
>> +static void ppc_powernv_init(MachineState *machine)
>> +{
>> +    ram_addr_t ram_size = machine->ram_size;
>> +    const char *cpu_model = machine->cpu_model;
>> +    const char *kernel_filename = machine->kernel_filename;
>> +    const char *initrd_filename = machine->initrd_filename;
>> +    uint32_t initrd_base = 0;
>> +    long initrd_size = 0;
>> +    PowerPCCPU *cpu;
>> +    CPUPPCState *env;
>> +    MemoryRegion *sysmem = get_system_memory();
>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>> +    sPowerNVMachineState *pnv_machine = POWERNV_MACHINE(machine);
>> +    PnvSystem *sys = &pnv_machine->sys;
>> +    long fw_size;
>> +    char *filename;
>> +    void *fdt;
>> +    int i;
>> +
>> +    /* init CPUs */
>> +    if (cpu_model == NULL) {
>> +        cpu_model = kvm_enabled() ? "host" : "POWER8";
>> +    }
>> +
>> +    for (i = 0; i < smp_cpus; i++) {
>> +        cpu = cpu_ppc_init(cpu_model);
>> +        if (cpu == NULL) {
>> +            error_report("Unable to find PowerPC CPU definition");
>> +            exit(1);
>> +        }
>> +        env = &cpu->env;
>> +
>> +        /* Set time-base frequency to 512 MHz */
>> +        cpu_ppc_tb_init(env, TIMEBASE_FREQ);
>> +
>> +        /* MSR[IP] doesn't exist nowadays */
>> +        env->msr_mask &= ~(1 << 6);
>> +
>> +        qemu_register_reset(powernv_cpu_reset, cpu);
> 
> As noted above, I think we want to start powernv off with "new style"
> cpu instantiation.  So for powernv, I think you want the machine to
> create chip objects, which will create core objects which will create
> the vcpu/thread objects.

ok.

>> +    }
>> +
>> +    /* allocate RAM */
>> +    memory_region_allocate_system_memory(ram, NULL, "ppc_powernv.ram",
>> +                                         ram_size);
>> +    memory_region_add_subregion(sysmem, 0, ram);
>> +
>> +    /* XXX We should decide how many chips to create based on #cores and
>> +     * Venice vs. Murano vs. Naples chip type etc..., for now, just create
>> +     * one chip. Also creation of the CPUs should be done per-chip
>> +     */
>> +    sys->num_chips = 1;
>> +
>> +    /* Create only one PHB for now until I figure out what's wrong
>> +     * when I create more (resource assignment failures in Linux)
>> +     */
>> +    pnv_create_chip(sys, 0);
>> +
>> +    if (bios_name == NULL) {
>> +        bios_name = FW_FILE_NAME;
>> +    }
>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> +    fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
>> +    if (fw_size < 0) {
>> +        hw_error("qemu: could not load OPAL '%s'\n", filename);
>> +        exit(1);
>> +    }
>> +    g_free(filename);
>> +
>> +
>> +    if (kernel_filename == NULL) {
>> +        kernel_filename = KERNEL_FILE_NAME;
>> +    }
>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, kernel_filename);
>> +    fw_size = load_image_targphys(filename, 0x20000000, 0x2000000);
>> +    if (fw_size < 0) {
>> +        hw_error("qemu: could not load kernel'%s'\n", filename);
>> +        exit(1);
>> +    }
>> +    g_free(filename);
>> +
>> +    /* load initrd */
>> +    if (initrd_filename) {
>> +            /* Try to locate the initrd in the gap between the kernel
>> +             * and the firmware. Add a bit of space just in case
>> +             */
>> +            initrd_base = 0x40000000;
>> +            initrd_size = load_image_targphys(initrd_filename, initrd_base,
>> +                                              0x10000000); /* 128MB max */
>> +            if (initrd_size < 0) {
>> +                    error_report("qemu: could not load initial ram disk '%s'",
>> +                            initrd_filename);
>> +                    exit(1);
>> +            }
>> +    } else {
>> +            initrd_base = 0;
>> +            initrd_size = 0;
>> +    }
>> +    fdt = powernv_create_fdt(sys, machine->kernel_cmdline,
>> +                             initrd_base, initrd_size);
> 
> So the fact that spapr fdt creation is split between machine init and
> reset time causes us a fair bit of pain.

what do you mean ? we should not be following the same spapr pattern,
that is to create a fdt skeleton in init and finalize the fdt creation 
in reset. I have a patch just doing that to populate the tree with 
some more devices for IPMI. So I guess I need to rework that part.
 
So we should be using a realize ops to handle the last bits of the fdt 
creation ? 

>> +    cpu_physical_memory_write(FDT_ADDR, fdt, fdt_totalsize(fdt));
>> +}
>> +
>> +static int powernv_kvm_type(const char *vm_type)
>> +{
>> +    /* Always force PR KVM */
>> +    return 2;
> 
> This doesn't really seem right to me.  I think you should be giving an
> error if the user tries to specify HV, rather than silently changing
> to PR.

OK. I will drop the whole KVM feature for the moment.

>> +}
>> +
>> +static void ppc_cpu_do_nmi_on_cpu(void *arg)
>> +{
>> +    CPUState *cs = arg;
>> +
>> +    cpu_synchronize_state(cs);
>> +    ppc_cpu_do_system_reset(cs);
>> +}
>> +
>> +static void powernv_nmi(NMIState *n, int cpu_index, Error **errp)
>> +{
>> +    CPUState *cs;
>> +
>> +    CPU_FOREACH(cs) {
>> +        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
>> +    }
>> +}
> 
> It may be simpler to just leave the nmi stuff out for now, until you
> have a real implementation.

OK.

> 
>> +
>> +static void powernv_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    NMIClass *nc = NMI_CLASS(oc);
>> +
>> +    mc->init = ppc_powernv_init;
>> +    mc->block_default_type = IF_SCSI;
>> +    mc->max_cpus = MAX_CPUS;
>> +    mc->no_parallel = 1;
>> +    mc->default_boot_order = NULL;
>> +    mc->kvm_type = powernv_kvm_type;
>> +
>> +    nc->nmi_monitor_handler = powernv_nmi;
>> +}
>> +
>> +static const TypeInfo powernv_machine_info = {
>> +    .name          = TYPE_POWERNV_MACHINE,
>> +    .parent        = TYPE_MACHINE,
>> +    .abstract      = true,
>> +    .instance_size = sizeof(sPowerNVMachineState),
>> +    .class_init    = powernv_machine_class_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_NMI },
>> +        { }
>> +    },
>> +};
>> +
>> +static void powernv_machine_2_5_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    mc->name = "powernv-2.5";
> 
> Looks like the version numbthis needs an update for current qemu.

I guess we should be using v1 as this is the first version of PowerNV, 
unless we want to tie that to Opal (v3). would that make sense ? 

Thanks,

C. 

>> +    mc->desc = "PowerNV v2.5";
>> +    mc->alias = "powernv";
>> +}
>> +
>> +static const TypeInfo powernv_machine_2_5_info = {
>> +    .name          = MACHINE_TYPE_NAME("powernv-2.5"),
>> +    .parent        = TYPE_POWERNV_MACHINE,
>> +    .class_init    = powernv_machine_2_5_class_init,
>> +};
>> +
>> +static void powernv_machine_register_types(void)
>> +{
>> +    type_register_static(&powernv_machine_info);
>> +    type_register_static(&powernv_machine_2_5_info);
>> +}
>> +
>> +type_init(powernv_machine_register_types)
>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>> new file mode 100644
>> index 000000000000..383a336e7cd3
>> --- /dev/null
>> +++ b/include/hw/ppc/pnv.h
>> @@ -0,0 +1,35 @@
>> +/*
>> + * QEMU PowerNV various definitions
>> + *
>> + * Copyright (c) 2014-2016 BenH, IBM Corporation.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef _PPC_PNV_H
>> +#define _PPC_PNV_H
>> +
>> +#include "hw/hw.h"
>> +
>> +/* Should we turn that into a QOjb of some sort ? */
>> +typedef struct PnvChip {
>> +    uint32_t         chip_id;
>> +} PnvChip;
>> +
>> +typedef struct PnvSystem {
>> +    uint32_t  num_chips;
>> +#define PNV_MAX_CHIPS      1
>> +    PnvChip   chips[PNV_MAX_CHIPS];
>> +} PnvSystem;
>> +
>> +#endif /* _PPC_PNV_H */
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform
  2016-07-28 17:27     ` Cédric Le Goater
@ 2016-07-29  3:32       ` David Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2016-07-29  3:32 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Alexander Graf, qemu-devel, qemu-ppc, Benjamin Herrenschmidt

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

On Thu, Jul 28, 2016 at 07:27:02PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> On 07/26/2016 08:23 AM, David Gibson wrote:
> > On Mon, Jul 25, 2016 at 04:24:43PM +0200, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>
> >> No devices yet, not even an interrupt controller, just to get
> >> started.
> >>
> >> (Folded in Stewart Smith patch to add command lien support)
> >                                                 ^^^^ typo
> 
> and it looks like french to me.

Heh, fair enough.  Oh, speaking of mispelling nits, you want
s/skeletton/skeleton/ in the subject.

> The changelog needs an update with
> a little paragraph on what we are not emulating. The lowlevel firmware 
> Hostboot should be mentionned.

Ok.

> 
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> [clg: updated for qemu-2.7
> >>       replaced fprintf by error_report
> >>       used a common definition of _FDT macro
> >>       removed VMStateDescription as migration is not yet supported
> >>       added IBM Copyright statements
> >> ]
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>  default-configs/ppc64-softmmu.mak |   1 +
> >>  hw/ppc/Makefile.objs              |   2 +
> >>  hw/ppc/pnv.c                      | 593 ++++++++++++++++++++++++++++++++++++++
> >>  include/hw/ppc/pnv.h              |  35 +++
> >>  4 files changed, 631 insertions(+)
> >>  create mode 100644 hw/ppc/pnv.c
> >>  create mode 100644 include/hw/ppc/pnv.h
> >>
> >> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> >> index c4be59f638ed..516a6e25aba3 100644
> >> --- a/default-configs/ppc64-softmmu.mak
> >> +++ b/default-configs/ppc64-softmmu.mak
> >> @@ -40,6 +40,7 @@ CONFIG_I8259=y
> >>  CONFIG_XILINX=y
> >>  CONFIG_XILINX_ETHLITE=y
> >>  CONFIG_PSERIES=y
> >> +CONFIG_POWERNV=y
> >>  CONFIG_PREP=y
> >>  CONFIG_MAC=y
> >>  CONFIG_E500=y
> >> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> >> index 91a3420f473a..cbde482dd1b4 100644
> >> --- a/hw/ppc/Makefile.objs
> >> +++ b/hw/ppc/Makefile.objs
> >> @@ -5,6 +5,8 @@ obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> >>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> >>  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> >>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o
> >> +# IBM PowerNV
> >> +obj-$(CONFIG_POWERNV) += pnv.o
> >>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >>  obj-y += spapr_pci_vfio.o
> >>  endif
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> new file mode 100644
> >> index 000000000000..5096a073e829
> >> --- /dev/null
> >> +++ b/hw/ppc/pnv.c
> >> @@ -0,0 +1,593 @@
> >> +/*
> >> + * QEMU PowerPC PowerNV model
> >> + *
> >> + * Copyright (c) 2004-2007 Fabrice Bellard
> >> + * Copyright (c) 2007 Jocelyn Mayer
> >> + * Copyright (c) 2010 David Gibson, IBM Corporation.
> >> + * Copyright (c) 2014-2016 BenH, 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 "qemu/osdep.h"
> >> +#include "sysemu/sysemu.h"
> >> +#include "hw/hw.h"
> >> +#include "hw/fw-path-provider.h"
> >> +#include "elf.h"
> >> +#include "net/net.h"
> >> +#include "sysemu/block-backend.h"
> >> +#include "sysemu/cpus.h"
> >> +#include "sysemu/kvm.h"
> >> +#include "sysemu/numa.h"
> >> +#include "kvm_ppc.h"
> >> +#include "mmu-hash64.h"
> >> +#include "qom/cpu.h"
> >> +
> >> +#include "hw/boards.h"
> >> +#include "hw/ppc/fdt.h"
> >> +#include "hw/ppc/ppc.h"
> >> +#include "hw/ppc/pnv.h"
> >> +#include "hw/loader.h"
> >> +
> >> +#include "exec/address-spaces.h"
> >> +#include "qemu/config-file.h"
> >> +#include "qemu/error-report.h"
> >> +#include "trace.h"
> >> +#include "hw/nmi.h"
> >> +
> >> +#include "hw/compat.h"
> >> +
> >> +#include <libfdt.h>
> >> +
> >> +#define FDT_ADDR                0x01000000
> >> +#define FDT_MAX_SIZE            0x00100000
> >> +#define FW_MAX_SIZE             0x00400000
> >> +#define FW_FILE_NAME            "skiboot.lid"
> >> +#define KERNEL_FILE_NAME        "skiroot.lid"
> >> +#define KERNEL_LOAD_ADDR        0x20000000
> > 
> > The KERNEL_* names don't really make sense here.
> 
> yes.
> 
> >> +#define TIMEBASE_FREQ           512000000ULL
> >> +
> >> +#define MAX_CPUS                255
> >> +
> >> +#define PHANDLE_XICP            0x00001111
> > 
> > Since you're not implementing an interrupt controller yet, I don't
> > think you need this define.
> 
> indeed. This is too early.
>  
> >> +
> >> +typedef struct sPowerNVMachineState sPowerNVMachineState;
> >> +
> >> +#define TYPE_POWERNV_MACHINE      "powernv-machine"
> >> +#define POWERNV_MACHINE(obj) \
> >> +    OBJECT_CHECK(sPowerNVMachineState, (obj), TYPE_POWERNV_MACHINE)
> >> +
> >> +/**
> >> + * sPowerNVMachineState:
> >> + */
> >> +struct sPowerNVMachineState {
> >> +    /*< private >*/
> >> +    MachineState parent_obj;
> >> +    PnvSystem sys;
> > 
> > Having all the system specific information in this substructure is a
> > bit awkward.  I think it would be preferable to just open code the
> > fields you need directly into sPowerNVMachineState.
> 
> I agree.
> 
> >> +};
> >> +
> >> +static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
> >> +                                     size_t maxsize)
> >> +{
> >> +    size_t maxcells = maxsize / sizeof(uint32_t);
> >> +    int i, j, count;
> >> +    uint32_t *p = prop;
> >> +
> >> +    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
> >> +        struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
> >> +
> >> +        if (!sps->page_shift) {
> >> +            break;
> >> +        }
> >> +        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
> >> +            if (sps->enc[count].page_shift == 0) {
> >> +                break;
> >> +            }
> >> +        }
> >> +        if ((p - prop) >= (maxcells - 3 - count * 2)) {
> >> +            break;
> >> +        }
> >> +        *(p++) = cpu_to_be32(sps->page_shift);
> >> +        *(p++) = cpu_to_be32(sps->slb_enc);
> >> +        *(p++) = cpu_to_be32(count);
> >> +        for (j = 0; j < count; j++) {
> >> +            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
> >> +            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
> >> +        }
> >> +    }
> >> +
> >> +    return (p - prop) * sizeof(uint32_t);
> >> +}
> > 
> > Hm, I do wonder if we should make a place for helper functions that
> > are common between spapr and powernv.
> 
> done.
> 
> powernv_populate_memory() and powernv_populate_memory_node() are also 
> very similar in spapr. I wonder if we can merge them ? 

If they're not identical, leave them separate for now.  I'm hoping to
do a big rework of the device tree construction, which should make
life easier for both platforms, so we can revisit this once that's
done.

> >> +static void powernv_populate_memory_node(void *fdt, int nodeid, hwaddr start,
> >> +                                         hwaddr size)
> >> +{
> >> +    /* Probablly bogus, need to match with what's going on in CPU nodes */
> >> +    uint32_t chip_id[] = {
> >> +        cpu_to_be32(0x0), cpu_to_be32(nodeid)
> >> +    };
> >> +    char *mem_name;
> >> +    uint64_t mem_reg_property[2];
> >> +
> >> +    mem_reg_property[0] = cpu_to_be64(start);
> >> +    mem_reg_property[1] = cpu_to_be64(size);
> >> +
> >> +    mem_name = g_strdup_printf("memory@"TARGET_FMT_lx, start);
> >> +    _FDT((fdt_begin_node(fdt, mem_name)));
> >> +    g_free(mem_name);
> >> +    _FDT((fdt_property_string(fdt, "device_type", "memory")));
> >> +    _FDT((fdt_property(fdt, "reg", mem_reg_property,
> >> +                       sizeof(mem_reg_property))));
> >> +    _FDT((fdt_property(fdt, "ibm,chip-id", chip_id, sizeof(chip_id))));
> >> +    _FDT((fdt_end_node(fdt)));
> >> +}
> >> +
> >> +static int powernv_populate_memory(void *fdt)
> >> +{
> >> +    hwaddr mem_start, node_size;
> >> +    int i, nb_nodes = nb_numa_nodes;
> >> +    NodeInfo *nodes = numa_info;
> >> +    NodeInfo ramnode;
> >> +
> >> +    /* No NUMA nodes, assume there is just one node with whole RAM */
> >> +    if (!nb_numa_nodes) {
> >> +        nb_nodes = 1;
> >> +        ramnode.node_mem = ram_size;
> >> +        nodes = &ramnode;
> >> +    }
> >> +
> >> +    for (i = 0, mem_start = 0; i < nb_nodes; ++i) {
> >> +        if (!nodes[i].node_mem) {
> >> +            continue;
> >> +        }
> >> +        if (mem_start >= ram_size) {
> >> +            node_size = 0;
> >> +        } else {
> >> +            node_size = nodes[i].node_mem;
> >> +            if (node_size > ram_size - mem_start) {
> >> +                node_size = ram_size - mem_start;
> >> +            }
> >> +        }
> >> +        for ( ; node_size; ) {
> >> +            hwaddr sizetmp = pow2floor(node_size);
> >> +
> >> +            /* mem_start != 0 here */
> >> +            if (ctzl(mem_start) < ctzl(sizetmp)) {
> >> +                sizetmp = 1ULL << ctzl(mem_start);
> >> +            }
> >> +
> >> +            powernv_populate_memory_node(fdt, i, mem_start, sizetmp);
> >> +            node_size -= sizetmp;
> >> +            mem_start += sizetmp;
> >> +        }
> >> +    }
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +static void powernv_create_cpu_node(void *fdt, CPUState *cs, int smt_threads)
> >> +{
> >> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> >> +    CPUPPCState *env = &cpu->env;
> >> +    DeviceClass *dc = DEVICE_GET_CLASS(cs);
> >> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> >> +    uint32_t servers_prop[smt_threads];
> >> +    uint32_t gservers_prop[smt_threads * 2];
> >> +    int i, index = ppc_get_vcpu_dt_id(cpu);
> >> +    uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> >> +                       0xffffffff, 0xffffffff};
> >> +    uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
> >> +    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
> >> +    uint32_t page_sizes_prop[64];
> >> +    size_t page_sizes_prop_size;
> >> +    char *nodename;
> >> +
> >> +    if ((index % smt_threads) != 0) {
> >> +        return;
> >> +    }
> > 
> > This hack exists in spapr for historical reasons.  I'd prefer if on
> > powernv you treat cpu cores as first class objects from the beginning,
> > and just call this once per core.
> 
> like this is done with sPAPRCPUCore I suppose. 

It isn't at the moment, but I hope to clean it up so that's the case
once we get a chance.

> I will get inspiration from the spapr cores and see if I can find some 
> common ground with the powernv cores. we should.
> 
> >> +    nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
> >> +
> >> +    _FDT((fdt_begin_node(fdt, nodename)));
> >> +
> >> +    g_free(nodename);
> >> +
> >> +    _FDT((fdt_property_cell(fdt, "reg", index)));
> >> +    _FDT((fdt_property_string(fdt, "device_type", "cpu")));
> >> +
> >> +    _FDT((fdt_property_cell(fdt, "cpu-version", env->spr[SPR_PVR])));
> >> +    _FDT((fdt_property_cell(fdt, "d-cache-block-size",
> >> +                            env->dcache_line_size)));
> >> +    _FDT((fdt_property_cell(fdt, "d-cache-line-size",
> >> +                            env->dcache_line_size)));
> >> +    _FDT((fdt_property_cell(fdt, "i-cache-block-size",
> >> +                            env->icache_line_size)));
> >> +    _FDT((fdt_property_cell(fdt, "i-cache-line-size",
> >> +                            env->icache_line_size)));
> >> +
> >> +    if (pcc->l1_dcache_size) {
> >> +        _FDT((fdt_property_cell(fdt, "d-cache-size", pcc->l1_dcache_size)));
> >> +    } else {
> >> +        error_report("Warning: Unknown L1 dcache size for cpu");
> >> +    }
> >> +    if (pcc->l1_icache_size) {
> >> +        _FDT((fdt_property_cell(fdt, "i-cache-size", pcc->l1_icache_size)));
> >> +    } else {
> >> +        error_report("Warning: Unknown L1 icache size for cpu");
> >> +    }
> >> +
> >> +    _FDT((fdt_property_cell(fdt, "timebase-frequency", tbfreq)));
> >> +    _FDT((fdt_property_cell(fdt, "clock-frequency", cpufreq)));
> >> +    _FDT((fdt_property_cell(fdt, "ibm,slb-size", env->slb_nr)));
> >> +    _FDT((fdt_property_string(fdt, "status", "okay")));
> >> +    _FDT((fdt_property(fdt, "64-bit", NULL, 0)));
> >> +
> >> +    if (env->spr_cb[SPR_PURR].oea_read) {
> >> +        _FDT((fdt_property(fdt, "ibm,purr", NULL, 0)));
> >> +    }
> >> +
> >> +    if (env->mmu_model & POWERPC_MMU_1TSEG) {
> >> +        _FDT((fdt_property(fdt, "ibm,processor-segment-sizes",
> >> +                           segs, sizeof(segs))));
> >> +    }
> >> +
> >> +    /* Advertise VMX/VSX (vector extensions) if available
> >> +     *   0 / no property == no vector extensions
> >> +     *   1               == VMX / Altivec available
> >> +     *   2               == VSX available */
> >> +    if (env->insns_flags & PPC_ALTIVEC) {
> >> +        uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
> >> +
> >> +        _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
> >> +    }
> >> +
> >> +    /* Advertise DFP (Decimal Floating Point) if available
> >> +     *   0 / no property == no DFP
> >> +     *   1               == DFP available */
> >> +    if (env->insns_flags2 & PPC2_DFP) {
> >> +        _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
> >> +    }
> >> +
> >> +    page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
> >> +                                                  sizeof(page_sizes_prop));
> >> +    if (page_sizes_prop_size) {
> >> +        _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
> >> +                           page_sizes_prop, page_sizes_prop_size)));
> >> +    }
> >> +
> >> +    /* XXX Just a hack for now */
> >> +    _FDT((fdt_property_cell(fdt, "ibm,chip-id", 0)));
> >> +
> >> +    if (cpu->cpu_version) {
> >> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
> >> +    }
> >> +
> >> +    /* Build interrupt servers and gservers properties */
> >> +    for (i = 0; i < smt_threads; i++) {
> >> +        servers_prop[i] = cpu_to_be32(index + i);
> >> +        /* Hack, direct the group queues back to cpu 0 */
> >> +        gservers_prop[i * 2] = cpu_to_be32(index + i);
> >> +        gservers_prop[i * 2 + 1] = 0;
> >> +    }
> >> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
> >> +                       servers_prop, sizeof(servers_prop))));
> >> +    _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
> >> +                       gservers_prop, sizeof(gservers_prop))));
> >> +
> >> +    _FDT((fdt_end_node(fdt)));
> >> +}
> >> +
> >> +static void *powernv_create_fdt(PnvSystem *sys, const char *kernel_cmdline,
> >> +                                uint32_t initrd_base, uint32_t initrd_size)
> >> +{
> >> +    void *fdt;
> >> +    CPUState *cs;
> >> +    int smt = kvmppc_smt_threads();
> >> +    uint32_t start_prop = cpu_to_be32(initrd_base);
> >> +    uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
> >> +    char *buf;
> >> +    const char plat_compat[] = "qemu,powernv\0ibm,powernv";
> >> +
> >> +    fdt = g_malloc0(FDT_MAX_SIZE);
> >> +    _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
> >> +    _FDT((fdt_finish_reservemap(fdt)));
> >> +
> >> +    /* Root node */
> >> +    _FDT((fdt_begin_node(fdt, "")));
> >> +    _FDT((fdt_property_string(fdt, "model", "IBM PowerNV (emulated by qemu)")));
> >> +    _FDT((fdt_property(fdt, "compatible", plat_compat, sizeof(plat_compat))));
> >> +
> >> +    /*
> >> +     * Add info to guest to indentify which host is it being run on
> >> +     * and what is the uuid of the guest
> >> +     */
> >> +    if (kvmppc_get_host_model(&buf)) {
> >> +        _FDT((fdt_property_string(fdt, "host-model", buf)));
> >> +        g_free(buf);
> >> +    }
> >> +    if (kvmppc_get_host_serial(&buf)) {
> >> +        _FDT((fdt_property_string(fdt, "host-serial", buf)));
> >> +        g_free(buf);
> >> +    }
> > 
> > Those seem dubious for a non-paravirt platform.
> 
> OK. I will remove all the KVM related parts. We can add them later on when
> we work on kvm-pr support.
> 
> >> +
> >> +    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> >> +                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
> >> +                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> >> +                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
> >> +                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> >> +                          qemu_uuid[14], qemu_uuid[15]);
> >> +
> >> +    _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
> >> +    g_free(buf);
> >> +
> >> +    _FDT((fdt_begin_node(fdt, "chosen")));
> >> +    if (kernel_cmdline) {
> >> +        _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
> >> +    }
> >> +    _FDT((fdt_property(fdt, "linux,initrd-start",
> >> +                       &start_prop, sizeof(start_prop))));
> >> +    _FDT((fdt_property(fdt, "linux,initrd-end",
> >> +                       &end_prop, sizeof(end_prop))));
> >> +    _FDT((fdt_end_node(fdt)));
> >> +
> >> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
> >> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
> >> +
> >> +    /* cpus */
> >> +    _FDT((fdt_begin_node(fdt, "cpus")));
> >> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
> >> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
> >> +
> >> +    CPU_FOREACH(cs) {
> >> +        powernv_create_cpu_node(fdt, cs, smt);
> >> +    }
> >> +
> >> +    _FDT((fdt_end_node(fdt)));
> >> +
> >> +    /* Memory */
> >> +    _FDT((powernv_populate_memory(fdt)));
> >> +
> >> +    /* /hypervisor node */
> > 
> > This really doesn't seem like it makes sense on a non-paravirt platform.
> > 
> >> +    if (kvm_enabled()) {
> >> +        uint8_t hypercall[16];
> >> +
> >> +        /* indicate KVM hypercall interface */
> >> +        _FDT((fdt_begin_node(fdt, "hypervisor")));
> >> +        _FDT((fdt_property_string(fdt, "compatible", "linux,kvm")));
> >> +        if (kvmppc_has_cap_fixup_hcalls()) {
> >> +            /*
> >> +             * Older KVM versions with older guest kernels were broken with the
> >> +             * magic page, don't allow the guest to map it.
> >> +             */
> >> +            kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
> >> +                                 sizeof(hypercall));
> >> +            _FDT((fdt_property(fdt, "hcall-instructions", hypercall,
> >> +                              sizeof(hypercall))));
> >> +        }
> >> +        _FDT((fdt_end_node(fdt)));
> >> +    }
> >> +
> >> +    _FDT((fdt_end_node(fdt))); /* close root node */
> >> +    _FDT((fdt_finish(fdt)));
> >> +
> >> +    return fdt;
> >> +}
> >> +
> >> +static void powernv_cpu_reset(void *opaque)
> >> +{
> >> +    PowerPCCPU *cpu = opaque;
> >> +    CPUState *cs = CPU(cpu);
> >> +    CPUPPCState *env = &cpu->env;
> >> +
> >> +    cpu_reset(cs);
> >> +
> >> +    env->spr[SPR_PIR] = ppc_get_vcpu_dt_id(cpu);
> >> +    env->spr[SPR_HIOR] = 0;
> >> +    env->gpr[3] = FDT_ADDR;
> >> +    env->nip = 0x10;
> >> +    env->msr |= MSR_HVB;
> >> +}
> >> +
> >> +static void pnv_create_chip(PnvSystem *sys, unsigned int chip_no)
> >> +{
> >> +    PnvChip *chip = &sys->chips[chip_no];
> > 
> > Hm, my inclination would be to make the pnv chips a full QOM type,
> > unless there's a reason not to.
> 
> Yes. let's go that way and I will try to port the rest of the patchset
> before sending to catch any issues.
> 
> >> +
> >> +    if (chip_no >= PNV_MAX_CHIPS) {
> >> +            return;
> >> +    }
> >> +
> >> +    /* XXX Improve chip numbering to better match HW */
> >> +    chip->chip_id = chip_no;
> >> +}
> >> +
> >> +static void ppc_powernv_init(MachineState *machine)
> >> +{
> >> +    ram_addr_t ram_size = machine->ram_size;
> >> +    const char *cpu_model = machine->cpu_model;
> >> +    const char *kernel_filename = machine->kernel_filename;
> >> +    const char *initrd_filename = machine->initrd_filename;
> >> +    uint32_t initrd_base = 0;
> >> +    long initrd_size = 0;
> >> +    PowerPCCPU *cpu;
> >> +    CPUPPCState *env;
> >> +    MemoryRegion *sysmem = get_system_memory();
> >> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> >> +    sPowerNVMachineState *pnv_machine = POWERNV_MACHINE(machine);
> >> +    PnvSystem *sys = &pnv_machine->sys;
> >> +    long fw_size;
> >> +    char *filename;
> >> +    void *fdt;
> >> +    int i;
> >> +
> >> +    /* init CPUs */
> >> +    if (cpu_model == NULL) {
> >> +        cpu_model = kvm_enabled() ? "host" : "POWER8";
> >> +    }
> >> +
> >> +    for (i = 0; i < smp_cpus; i++) {
> >> +        cpu = cpu_ppc_init(cpu_model);
> >> +        if (cpu == NULL) {
> >> +            error_report("Unable to find PowerPC CPU definition");
> >> +            exit(1);
> >> +        }
> >> +        env = &cpu->env;
> >> +
> >> +        /* Set time-base frequency to 512 MHz */
> >> +        cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> >> +
> >> +        /* MSR[IP] doesn't exist nowadays */
> >> +        env->msr_mask &= ~(1 << 6);
> >> +
> >> +        qemu_register_reset(powernv_cpu_reset, cpu);
> > 
> > As noted above, I think we want to start powernv off with "new style"
> > cpu instantiation.  So for powernv, I think you want the machine to
> > create chip objects, which will create core objects which will create
> > the vcpu/thread objects.
> 
> ok.
> 
> >> +    }
> >> +
> >> +    /* allocate RAM */
> >> +    memory_region_allocate_system_memory(ram, NULL, "ppc_powernv.ram",
> >> +                                         ram_size);
> >> +    memory_region_add_subregion(sysmem, 0, ram);
> >> +
> >> +    /* XXX We should decide how many chips to create based on #cores and
> >> +     * Venice vs. Murano vs. Naples chip type etc..., for now, just create
> >> +     * one chip. Also creation of the CPUs should be done per-chip
> >> +     */
> >> +    sys->num_chips = 1;
> >> +
> >> +    /* Create only one PHB for now until I figure out what's wrong
> >> +     * when I create more (resource assignment failures in Linux)
> >> +     */
> >> +    pnv_create_chip(sys, 0);
> >> +
> >> +    if (bios_name == NULL) {
> >> +        bios_name = FW_FILE_NAME;
> >> +    }
> >> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> >> +    fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
> >> +    if (fw_size < 0) {
> >> +        hw_error("qemu: could not load OPAL '%s'\n", filename);
> >> +        exit(1);
> >> +    }
> >> +    g_free(filename);
> >> +
> >> +
> >> +    if (kernel_filename == NULL) {
> >> +        kernel_filename = KERNEL_FILE_NAME;
> >> +    }
> >> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, kernel_filename);
> >> +    fw_size = load_image_targphys(filename, 0x20000000, 0x2000000);
> >> +    if (fw_size < 0) {
> >> +        hw_error("qemu: could not load kernel'%s'\n", filename);
> >> +        exit(1);
> >> +    }
> >> +    g_free(filename);
> >> +
> >> +    /* load initrd */
> >> +    if (initrd_filename) {
> >> +            /* Try to locate the initrd in the gap between the kernel
> >> +             * and the firmware. Add a bit of space just in case
> >> +             */
> >> +            initrd_base = 0x40000000;
> >> +            initrd_size = load_image_targphys(initrd_filename, initrd_base,
> >> +                                              0x10000000); /* 128MB max */
> >> +            if (initrd_size < 0) {
> >> +                    error_report("qemu: could not load initial ram disk '%s'",
> >> +                            initrd_filename);
> >> +                    exit(1);
> >> +            }
> >> +    } else {
> >> +            initrd_base = 0;
> >> +            initrd_size = 0;
> >> +    }
> >> +    fdt = powernv_create_fdt(sys, machine->kernel_cmdline,
> >> +                             initrd_base, initrd_size);
> > 
> > So the fact that spapr fdt creation is split between machine init and
> > reset time causes us a fair bit of pain.
> 
> what do you mean ? we should not be following the same spapr pattern,
> that is to create a fdt skeleton in init and finalize the fdt creation 
> in reset. I have a patch just doing that to populate the tree with 
> some more devices for IPMI. So I guess I need to rework that part.

Basically, yes.  I think you'll have an easier time if you do all of
the dt construction at reset time.  Again, spapr is split the way it
is because of history - I hope to move all its dt construction to
reset time as part of the previously mentioned dt construction rework.

> So we should be using a realize ops to handle the last bits of the fdt 
> creation ? 

You shouldn't need that.  Just do the full dt construction at the
point where you currently do the dt finalization.

> >> +    cpu_physical_memory_write(FDT_ADDR, fdt, fdt_totalsize(fdt));
> >> +}
> >> +
> >> +static int powernv_kvm_type(const char *vm_type)
> >> +{
> >> +    /* Always force PR KVM */
> >> +    return 2;
> > 
> > This doesn't really seem right to me.  I think you should be giving an
> > error if the user tries to specify HV, rather than silently changing
> > to PR.
> 
> OK. I will drop the whole KVM feature for the moment.
> 
> >> +}
> >> +
> >> +static void ppc_cpu_do_nmi_on_cpu(void *arg)
> >> +{
> >> +    CPUState *cs = arg;
> >> +
> >> +    cpu_synchronize_state(cs);
> >> +    ppc_cpu_do_system_reset(cs);
> >> +}
> >> +
> >> +static void powernv_nmi(NMIState *n, int cpu_index, Error **errp)
> >> +{
> >> +    CPUState *cs;
> >> +
> >> +    CPU_FOREACH(cs) {
> >> +        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
> >> +    }
> >> +}
> > 
> > It may be simpler to just leave the nmi stuff out for now, until you
> > have a real implementation.
> 
> OK.
> 
> > 
> >> +
> >> +static void powernv_machine_class_init(ObjectClass *oc, void *data)
> >> +{
> >> +    MachineClass *mc = MACHINE_CLASS(oc);
> >> +    NMIClass *nc = NMI_CLASS(oc);
> >> +
> >> +    mc->init = ppc_powernv_init;
> >> +    mc->block_default_type = IF_SCSI;
> >> +    mc->max_cpus = MAX_CPUS;
> >> +    mc->no_parallel = 1;
> >> +    mc->default_boot_order = NULL;
> >> +    mc->kvm_type = powernv_kvm_type;
> >> +
> >> +    nc->nmi_monitor_handler = powernv_nmi;
> >> +}
> >> +
> >> +static const TypeInfo powernv_machine_info = {
> >> +    .name          = TYPE_POWERNV_MACHINE,
> >> +    .parent        = TYPE_MACHINE,
> >> +    .abstract      = true,
> >> +    .instance_size = sizeof(sPowerNVMachineState),
> >> +    .class_init    = powernv_machine_class_init,
> >> +    .interfaces = (InterfaceInfo[]) {
> >> +        { TYPE_NMI },
> >> +        { }
> >> +    },
> >> +};
> >> +
> >> +static void powernv_machine_2_5_class_init(ObjectClass *oc, void *data)
> >> +{
> >> +    MachineClass *mc = MACHINE_CLASS(oc);
> >> +
> >> +    mc->name = "powernv-2.5";
> > 
> > Looks like the version numbthis needs an update for current qemu.
> 
> I guess we should be using v1 as this is the first version of PowerNV, 
> unless we want to tie that to Opal (v3). would that make sense ?

No, tying it to the qemu version makes most sense.  Or even not
bothering with versions at all, until the machine type has achieved
some more stability.

Machine type versions are primarily about migration safety.  For
example, suppose between qemu-2.8 and qemu-2.9 you added an extra
optional device.  The guest wouldn't care about the change (as long as
it was correctly advertise in the device tree) - however the extra
device would break migration between qemu-2.8 and qemu-2.9 (because
the device would exist in qemu-2.9, but have no data in the migration
stream to initialize it).  In that case you'd want a "powernv-2.8"
machine type version which specifically excludes the device to allow
migration.

If you don't care about migration, it's probably best to leave the
machine type unversioned, then start adding versions when / if you
do.

> 
> Thanks,
> 
> C. 
> 
> >> +    mc->desc = "PowerNV v2.5";
> >> +    mc->alias = "powernv";
> >> +}
> >> +
> >> +static const TypeInfo powernv_machine_2_5_info = {
> >> +    .name          = MACHINE_TYPE_NAME("powernv-2.5"),
> >> +    .parent        = TYPE_POWERNV_MACHINE,
> >> +    .class_init    = powernv_machine_2_5_class_init,
> >> +};
> >> +
> >> +static void powernv_machine_register_types(void)
> >> +{
> >> +    type_register_static(&powernv_machine_info);
> >> +    type_register_static(&powernv_machine_2_5_info);
> >> +}
> >> +
> >> +type_init(powernv_machine_register_types)
> >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> >> new file mode 100644
> >> index 000000000000..383a336e7cd3
> >> --- /dev/null
> >> +++ b/include/hw/ppc/pnv.h
> >> @@ -0,0 +1,35 @@
> >> +/*
> >> + * QEMU PowerNV various definitions
> >> + *
> >> + * Copyright (c) 2014-2016 BenH, IBM Corporation.
> >> + *
> >> + * This library is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU Lesser General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2 of the License, or (at your option) any later version.
> >> + *
> >> + * This library is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * Lesser General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU Lesser General Public
> >> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +#ifndef _PPC_PNV_H
> >> +#define _PPC_PNV_H
> >> +
> >> +#include "hw/hw.h"
> >> +
> >> +/* Should we turn that into a QOjb of some sort ? */
> >> +typedef struct PnvChip {
> >> +    uint32_t         chip_id;
> >> +} PnvChip;
> >> +
> >> +typedef struct PnvSystem {
> >> +    uint32_t  num_chips;
> >> +#define PNV_MAX_CHIPS      1
> >> +    PnvChip   chips[PNV_MAX_CHIPS];
> >> +} PnvSystem;
> >> +
> >> +#endif /* _PPC_PNV_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: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-29  3:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-25 14:24 [Qemu-devel] [PATCH 0/3] Add PowerNV skeleton Cédric Le Goater
2016-07-25 14:24 ` [Qemu-devel] [PATCH 1/3] hw/ppc: include fdt helper routine in a common file Cédric Le Goater
2016-07-26  5:57   ` David Gibson
2016-07-25 14:24 ` [Qemu-devel] [PATCH 2/3] hw/ppc: use error_report instead of fprintf Cédric Le Goater
2016-07-26  5:58   ` David Gibson
2016-07-26  6:29     ` Cédric Le Goater
2016-07-25 14:24 ` [Qemu-devel] [PATCH 3/3] ppc/pnv: Add skeletton PowerNV platform Cédric Le Goater
2016-07-26  6:23   ` David Gibson
2016-07-28 17:27     ` Cédric Le Goater
2016-07-29  3:32       ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.