[Qemu-devel] [0/3] Further ppc & pseries updates

[Qemu-devel] [0/3] Further ppc & pseries updates

[David Gibson]

Here are several more updates for the ppc target and the pseries
machine in particular.  This obsoletes the earlier copies of the first
two patches I sent today.

The first patch more correctly handles CPU features which are present
on some CPUs we know about, but which are not yet emulated in TCG.
The second adds limited PCI support to the pseries machine (this copy
fixes a compile bug in the previous version that hits with some
compilers).  The third adds support for the "info tlb" monitor command
on Book3S ppc cpus.

[Qemu-devel] [PATCH 1/3] ppc: Alter CPU state to mask out TCG unimplemented instructions as appropriate

[David Gibson]

The CPU state contains two bitmaps, initialized from the CPU spec
which describes which instructions are implemented on the CPU.  A
couple of bits are defined which cover instructions (VSX and DFP)
which are not currently implemented in TCG.  So far, these are only
used to handle the case of -cpu host because a KVM guest can use
the instructions when the host CPU supports them.

However, it's a mild layering violation to simply not include those
bits in the CPU descriptions for those CPUs that do support them,
just because we can't handle them in TCG.  This patch corrects the
situation, so that the instruction bits _are_ shown correctly in the
cpu spec table, but are masked out from the cpu state in the non-KVM
case.

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

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 3ef4eba..e84108c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1856,6 +1856,30 @@ enum {
     /* popcntw and popcntd instructions                                      */
     PPC_POPCNTWD       = 0x8000000000000000ULL,
 
+#define PPC_TCG_INSNS  (PPC_INSNS_BASE | PPC_POWER | PPC_POWER2 \
+                        | PPC_POWER_RTC | PPC_POWER_BR | PPC_64B \
+                        | PPC_64BX | PPC_64H | PPC_WAIT | PPC_MFTB \
+                        | PPC_602_SPEC | PPC_ISEL | PPC_POPCNTB \
+                        | PPC_STRING | PPC_FLOAT | PPC_FLOAT_EXT \
+                        | PPC_FLOAT_FSQRT | PPC_FLOAT_FRES \
+                        | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES \
+                        | PPC_FLOAT_FSEL | PPC_FLOAT_STFIWX \
+                        | PPC_ALTIVEC | PPC_SPE | PPC_SPE_SINGLE \
+                        | PPC_SPE_DOUBLE | PPC_MEM_TLBIA \
+                        | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC \
+                        | PPC_MEM_SYNC | PPC_MEM_EIEIO \
+                        | PPC_CACHE | PPC_CACHE_ICBI \
+                        | PPC_CACHE_DCBZ | PPC_CACHE_DCBZT \
+                        | PPC_CACHE_DCBA | PPC_CACHE_LOCK \
+                        | PPC_EXTERN | PPC_SEGMENT | PPC_6xx_TLB \
+                        | PPC_74xx_TLB | PPC_40x_TLB | PPC_SEGMENT_64B \
+                        | PPC_SLBI | PPC_WRTEE | PPC_40x_EXCP \
+                        | PPC_405_MAC | PPC_440_SPEC | PPC_BOOKE \
+                        | PPC_MFAPIDI | PPC_TLBIVA | PPC_TLBIVAX \
+                        | PPC_4xx_COMMON | PPC_40x_ICBT | PPC_RFMCI \
+                        | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_DCRUX \
+                        | PPC_POPCNTWD)
+
     /* extended type values */
 
     /* BookE 2.06 PowerPC specification                                      */
@@ -1864,6 +1888,8 @@ enum {
     PPC2_VSX           = 0x0000000000000002ULL,
     /* Decimal Floating Point (DFP)                                          */
     PPC2_DFP           = 0x0000000000000004ULL,
+
+#define PPC_TCG_INSNS2 (PPC2_BOOKE206)
 };
 
 /*****************************************************************************/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4dfd7f3..854bc65 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -6519,9 +6519,7 @@ static void init_proc_970MP (CPUPPCState *env)
                               PPC_64B | PPC_ALTIVEC |                         \
                               PPC_SEGMENT_64B | PPC_SLBI |                    \
                               PPC_POPCNTB | PPC_POPCNTWD)
-/* FIXME: Should also have PPC2_VSX and PPC2_DFP, but we don't
- * implement those in TCG yet */
-#define POWERPC_INSNS2_POWER7 (PPC_NONE)
+#define POWERPC_INSNS2_POWER7 (PPC2_VSX | PPC2_DFP)
 #define POWERPC_MSRM_POWER7   (0x800000000204FF36ULL)
 #define POWERPC_MMU_POWER7    (POWERPC_MMU_2_06)
 #define POWERPC_EXCP_POWER7   (POWERPC_EXCP_POWER7)
@@ -9848,6 +9846,22 @@ int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
     env->bus_model = def->bus_model;
     env->insns_flags = def->insns_flags;
     env->insns_flags2 = def->insns_flags2;
+    if (!kvm_enabled()) {
+        /* TCG doesn't (yet) emulate some groups of instructions that
+         * are implemented on some otherwise supported CPUs (e.g. VSX
+         * and decimal floating point instructions on POWER7).  We
+         * remove unsupported instruction groups from the cpu state's
+         * instruction masks and hope the guest can cope.  For at
+         * least the pseries machine, the unavailability of these
+         * instructions can be advertise to the guest via the device
+         * tree.
+         *
+         * FIXME: we should have a similar masking for CPU features
+         * not accessible under KVM, but so far, there aren't any of
+         * those. */
+        env->insns_flags &= PPC_TCG_INSNS;
+        env->insns_flags2 &= PPC_TCG_INSNS2;
+    } 
     env->flags = def->flags;
     env->bfd_mach = def->bfd_mach;
     env->check_pow = def->check_pow;
-- 
1.7.7

[Qemu-devel] [PATCH 2/3] pseries: Add partial support for PCI

[David Gibson]

This patch adds a PCI bus to the pseries machine.  This instantiates
the qemu generic PCI bus code, advertises a PCI host bridge in the
guest's device tree and implements the RTAS methods specified by PAPR
to access PCI config space.  It also sets up the memory regions we
need to provide windows into the PCI memory and IO space, and
advertises those to the guest.

However, because qemu can't yet emulate an IOMMU, which is mandatory on
pseries, PCI devices which use DMA (i.e. most of them) will not work with
this code alone.  Still, this is enough to support the virtio_pci device
(which probably _should_ use emulated PCI DMA, but is specced to use
direct hypervisor access to guest physical memory instead).

Signed-off-by: Alexey Kardashevskiy <aik@au1.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile.target |    1 +
 hw/spapr.c      |   36 ++++-
 hw/spapr.h      |    2 +
 hw/spapr_pci.c  |  508 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/spapr_pci.h  |   59 +++++++
 5 files changed, 602 insertions(+), 4 deletions(-)
 create mode 100644 hw/spapr_pci.c
 create mode 100644 hw/spapr_pci.h

diff --git a/Makefile.target b/Makefile.target
index fe5f6f7..2329df5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -248,6 +248,7 @@ obj-ppc-y += ppc_newworld.o
 # IBM pSeries (sPAPR)
 obj-ppc-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
 obj-ppc-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
+obj-ppc-$(CONFIG_PSERIES) += spapr_pci.o device-hotplug.o pci-hotplug.o
 # PowerPC 4xx boards
 obj-ppc-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
 obj-ppc-y += ppc440.o ppc440_bamboo.o
diff --git a/hw/spapr.c b/hw/spapr.c
index 933af32..bdaa938 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -39,10 +39,12 @@
 
 #include "hw/spapr.h"
 #include "hw/spapr_vio.h"
+#include "hw/spapr_pci.h"
 #include "hw/xics.h"
 
 #include "kvm.h"
 #include "kvm_ppc.h"
+#include "pci.h"
 
 #include "exec-memory.h"
 
@@ -62,6 +64,11 @@
 #define MAX_CPUS                256
 #define XICS_IRQS		1024
 
+#define SPAPR_PCI_BUID          0x800000020000001ULL
+#define SPAPR_PCI_MEM_WIN_ADDR  (0x10000000000ULL + 0xA0000000)
+#define SPAPR_PCI_MEM_WIN_SIZE  0x20000000
+#define SPAPR_PCI_IO_WIN_ADDR   (0x10000000000ULL + 0x80000000)
+
 #define PHANDLE_XICP            0x00001111
 
 sPAPREnvironment *spapr;
@@ -146,6 +153,14 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                        &end_prop, sizeof(end_prop))));
     _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
 
+    /*
+     * Because we don't always invoke any firmware, we can't rely on
+     * that to do BAR allocation.  Long term, we should probably do
+     * that ourselves, but for now, this setting (plus advertising the
+     * current BARs as 0) causes sufficiently recent kernels to to the
+     * BAR assignment themselves */
+    _FDT((fdt_property_cell(fdt, "linux,pci-probe-only", 0)));
+
     _FDT((fdt_end_node(fdt)));
 
     /* memory node(s) */
@@ -308,6 +323,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
 {
     int ret;
     void *fdt;
+    sPAPRPHBState *phb;
 
     fdt = g_malloc(FDT_MAX_SIZE);
 
@@ -320,6 +336,15 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
         exit(1);
     }
 
+    QLIST_FOREACH(phb, &spapr->phbs, list) {
+        ret = spapr_populate_pci_devices(phb, PHANDLE_XICP, fdt);
+    }
+
+    if (ret < 0) {
+        fprintf(stderr, "couldn't setup PCI devices in fdt\n");
+        exit(1);
+    }
+
     /* RTAS */
     ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
     if (ret < 0) {
@@ -478,6 +503,12 @@ static void ppc_spapr_init(ram_addr_t ram_size,
         }
     }
 
+    /* Set up PCI */
+    spapr_create_phb(spapr, "pci", SPAPR_PCI_BUID,
+                     SPAPR_PCI_MEM_WIN_ADDR,
+                     SPAPR_PCI_MEM_WIN_SIZE,
+                     SPAPR_PCI_IO_WIN_ADDR);
+
     for (i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
 
@@ -488,10 +519,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
         if (strcmp(nd->model, "ibmveth") == 0) {
             spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd);
         } else {
-            fprintf(stderr, "pSeries (sPAPR) platform does not support "
-                    "NIC model '%s' (only ibmveth is supported)\n",
-                    nd->model);
-            exit(1);
+            pci_nic_init_nofail(&nd_table[i], nd->model, NULL);
         }
     }
 
diff --git a/hw/spapr.h b/hw/spapr.h
index 6657c33..df88f6a 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -4,10 +4,12 @@
 #include "hw/xics.h"
 
 struct VIOsPAPRBus;
+struct sPAPRPHBState;
 struct icp_state;
 
 typedef struct sPAPREnvironment {
     struct VIOsPAPRBus *vio_bus;
+    QLIST_HEAD(, sPAPRPHBState) phbs;
     struct icp_state *icp;
 
     target_phys_addr_t ram_limit;
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
new file mode 100644
index 0000000..2a5e637
--- /dev/null
+++ b/hw/spapr_pci.c
@@ -0,0 +1,508 @@
+/*
+ * QEMU sPAPR PCI host originated from Uninorth PCI host
+ *
+ * Copyright (c) 2011 Alexey Kardashevskiy, IBM Corporation.
+ * Copyright (C) 2011 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "hw.h"
+#include "pci.h"
+#include "pci_host.h"
+#include "hw/spapr.h"
+#include "hw/spapr_pci.h"
+#include "exec-memory.h"
+#include <libfdt.h>
+
+#include "hw/pci_internals.h"
+
+static const uint32_t bars[] = {
+    PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1,
+    PCI_BASE_ADDRESS_2, PCI_BASE_ADDRESS_3,
+    PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5
+    /*, PCI_ROM_ADDRESS*/
+};
+
+static PCIDevice *find_dev(sPAPREnvironment *spapr,
+                           uint64_t buid, uint32_t config_addr)
+{
+    DeviceState *qdev;
+    int devfn = (config_addr >> 8) & 0xFF;
+    sPAPRPHBState *phb;
+
+    QLIST_FOREACH(phb, &spapr->phbs, list) {
+        if (phb->buid != buid) {
+            continue;
+        }
+
+        QLIST_FOREACH(qdev, &phb->host_state.bus->qbus.children, sibling) {
+            PCIDevice *dev = (PCIDevice *)qdev;
+            if (dev->devfn == devfn) {
+                return dev;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+static void rtas_ibm_read_pci_config(sPAPREnvironment *spapr,
+                                     uint32_t token, uint32_t nargs,
+                                     target_ulong args,
+                                     uint32_t nret, target_ulong rets)
+{
+    uint32_t val, size, addr;
+    uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+    PCIDevice *dev = find_dev(spapr, buid, rtas_ld(args, 0));
+
+    if (!dev) {
+        rtas_st(rets, 0, -1);
+        return;
+    }
+    size = rtas_ld(args, 3);
+    addr = rtas_ld(args, 0) & 0xFF;
+    val = pci_default_read_config(dev, addr, size);
+    rtas_st(rets, 0, 0);
+    rtas_st(rets, 1, val);
+}
+
+static void rtas_read_pci_config(sPAPREnvironment *spapr,
+                                 uint32_t token, uint32_t nargs,
+                                 target_ulong args,
+                                 uint32_t nret, target_ulong rets)
+{
+    uint32_t val, size, addr;
+    PCIDevice *dev = find_dev(spapr, 0, rtas_ld(args, 0));
+
+    if (!dev) {
+        rtas_st(rets, 0, -1);
+        return;
+    }
+    size = rtas_ld(args, 1);
+    addr = rtas_ld(args, 0) & 0xFF;
+    val = pci_default_read_config(dev, addr, size);
+    rtas_st(rets, 0, 0);
+    rtas_st(rets, 1, val);
+}
+
+static void rtas_ibm_write_pci_config(sPAPREnvironment *spapr,
+                                      uint32_t token, uint32_t nargs,
+                                      target_ulong args,
+                                      uint32_t nret, target_ulong rets)
+{
+    uint32_t val, size, addr;
+    uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+    PCIDevice *dev = find_dev(spapr, buid, rtas_ld(args, 0));
+
+    if (!dev) {
+        rtas_st(rets, 0, -1);
+        return;
+    }
+    val = rtas_ld(args, 4);
+    size = rtas_ld(args, 3);
+    addr = rtas_ld(args, 0) & 0xFF;
+    pci_default_write_config(dev, addr, val, size);
+    rtas_st(rets, 0, 0);
+}
+
+static void rtas_write_pci_config(sPAPREnvironment *spapr,
+                                  uint32_t token, uint32_t nargs,
+                                  target_ulong args,
+                                  uint32_t nret, target_ulong rets)
+{
+    uint32_t val, size, addr;
+    PCIDevice *dev = find_dev(spapr, 0, rtas_ld(args, 0));
+
+    if (!dev) {
+        rtas_st(rets, 0, -1);
+        return;
+    }
+    val = rtas_ld(args, 2);
+    size = rtas_ld(args, 1);
+    addr = rtas_ld(args, 0) & 0xFF;
+    pci_default_write_config(dev, addr, val, size);
+    rtas_st(rets, 0, 0);
+}
+
+static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+    /*
+     * Here we need to convert pci_dev + irq_num to some unique value
+     * which is less than number of IRQs on the specific bus (now it
+     * is 16).  At the moment irq_num == device_id (number of the
+     * slot?)
+     * FIXME: we should swizzle in fn and irq_num
+     */
+    return (pci_dev->devfn >> 3) % SPAPR_PCI_NUM_LSI;
+}
+
+static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
+{
+    /*
+     * Here we use the number returned by pci_spapr_map_irq to find a
+     * corresponding qemu_irq.
+     */
+    sPAPRPHBState *phb = opaque;
+
+    qemu_set_irq(phb->lsi_table[irq_num].qirq, level);
+}
+
+static int spapr_phb_init(SysBusDevice *s)
+{
+    sPAPRPHBState *phb = FROM_SYSBUS(sPAPRPHBState, s);
+    int i;
+
+    /* Initialize the LSI table */
+    for (i = 0; i < SPAPR_PCI_NUM_LSI; i++) {
+        qemu_irq qirq;
+        uint32_t num;
+
+        qirq = spapr_allocate_irq(0, &num);
+        if (!qirq) {
+            return -1;
+        }
+
+        phb->lsi_table[i].dt_irq = num;
+        phb->lsi_table[i].qirq = qirq;
+    }
+
+    return 0;
+}
+
+static int spapr_main_pci_host_init(PCIDevice *d)
+{
+    return 0;
+}
+
+static PCIDeviceInfo spapr_main_pci_host_info = {
+    .qdev.name = "spapr-pci-host-bridge",
+    .qdev.size = sizeof(PCIDevice),
+    .init      = spapr_main_pci_host_init,
+};
+
+static void spapr_register_devices(void)
+{
+    sysbus_register_dev("spapr-pci-host-bridge", sizeof(sPAPRPHBState),
+                        spapr_phb_init);
+    pci_qdev_register(&spapr_main_pci_host_info);
+}
+
+device_init(spapr_register_devices)
+
+static uint64_t spapr_io_read(void *opaque, target_phys_addr_t addr,
+                              unsigned size)
+{
+    switch (size) {
+    case 1:
+        return cpu_inb(addr);
+    case 2:
+        return cpu_inw(addr);
+    case 4:
+        return cpu_inl(addr);
+    }
+    assert(0);
+}
+
+static void spapr_io_write(void *opaque, target_phys_addr_t addr,
+                           uint64_t data, unsigned size)
+{
+    switch (size) {
+    case 1:
+        cpu_outb(addr, data);
+        return;
+    case 2:
+        cpu_outw(addr, data);
+        return;
+    case 4:
+        cpu_outl(addr, data);
+        return;
+    }
+    assert(0);
+}
+
+static MemoryRegionOps spapr_io_ops = {
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .read = spapr_io_read,
+    .write = spapr_io_write
+};
+
+void spapr_create_phb(sPAPREnvironment *spapr,
+                      const char *busname, uint64_t buid,
+                      uint64_t mem_win_addr, uint64_t mem_win_size,
+                      uint64_t io_win_addr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+    sPAPRPHBState *phb;
+    PCIBus *bus;
+    char namebuf[strlen(busname)+11];
+
+    dev = qdev_create(NULL, "spapr-pci-host-bridge");
+    qdev_init_nofail(dev);
+    s = sysbus_from_qdev(dev);
+    phb = FROM_SYSBUS(sPAPRPHBState, s);
+
+    phb->mem_win_addr = mem_win_addr;
+
+    sprintf(namebuf, "%s-mem", busname);
+    memory_region_init(&phb->memspace, namebuf, INT64_MAX);
+
+    sprintf(namebuf, "%s-memwindow", busname);
+    memory_region_init_alias(&phb->memwindow, namebuf, &phb->memspace,
+                             SPAPR_PCI_MEM_WIN_BUS_OFFSET, mem_win_size);
+    memory_region_add_subregion(get_system_memory(), mem_win_addr,
+                                &phb->memwindow);
+
+    phb->io_win_addr = io_win_addr;
+
+    /* On ppc, we only have MMIO no specific IO space from the CPU
+     * perspective.  In theory we ought to be able to embed the PCI IO
+     * memory region direction in the system memory space.  However,
+     * if any of the IO BAR subregions use the old_portio mechanism,
+     * that won't be processed properly unless accessed from the
+     * system io address space.  This hack to bounce things via
+     * system_io works around the problem until all the users of
+     * old_portion are updated */
+    sprintf(namebuf, "%s-io", busname);
+    memory_region_init(&phb->iospace, namebuf, SPAPR_PCI_IO_WIN_SIZE);
+    /* FIXME: fix to support multiple PHBs */
+    memory_region_add_subregion(get_system_io(), 0, &phb->iospace);
+
+    sprintf(namebuf, "%s-iowindow", busname);
+    memory_region_init_io(&phb->iowindow, &spapr_io_ops, phb,
+                          namebuf, SPAPR_PCI_IO_WIN_SIZE);
+    memory_region_add_subregion(get_system_memory(), io_win_addr,
+                                &phb->iowindow);
+
+    phb->host_state.bus = bus = pci_register_bus(&phb->busdev.qdev, busname,
+                                                 pci_spapr_set_irq,
+                                                 pci_spapr_map_irq,
+                                                 phb,
+                                                 &phb->memspace, &phb->iospace,
+                                                 PCI_DEVFN(0, 0),
+                                                 SPAPR_PCI_NUM_LSI);
+
+    spapr_rtas_register("read-pci-config", rtas_read_pci_config);
+    spapr_rtas_register("write-pci-config", rtas_write_pci_config);
+    spapr_rtas_register("ibm,read-pci-config", rtas_ibm_read_pci_config);
+    spapr_rtas_register("ibm,write-pci-config", rtas_ibm_write_pci_config);
+
+    QLIST_INSERT_HEAD(&spapr->phbs, phb, list);
+
+    /* pci_bus_set_mem_base(bus, mem_va_start - SPAPR_PCI_MEM_BAR_START); */
+}
+
+/* Macros to operate with address in OF binding to PCI */
+#define b_x(x, p, l)    (((x) & ((1<<(l))-1)) << (p))
+#define b_n(x)          b_x((x), 31, 1) /* 0 if relocatable */
+#define b_p(x)          b_x((x), 30, 1) /* 1 if prefetchable */
+#define b_t(x)          b_x((x), 29, 1) /* 1 if the address is aliased */
+#define b_ss(x)         b_x((x), 24, 2) /* the space code */
+#define b_bbbbbbbb(x)   b_x((x), 16, 8) /* bus number */
+#define b_ddddd(x)      b_x((x), 11, 5) /* device number */
+#define b_fff(x)        b_x((x), 8, 3)  /* function number */
+#define b_rrrrrrrr(x)   b_x((x), 0, 8)  /* register number */
+
+static uint32_t regtype_to_ss(uint8_t type)
+{
+    if (type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+        return 3;
+    }
+    if (type == PCI_BASE_ADDRESS_SPACE_IO) {
+        return 1;
+    }
+    return 2;
+}
+
+int spapr_populate_pci_devices(sPAPRPHBState *phb,
+                               uint32_t xics_phandle,
+                               void *fdt)
+{
+    PCIBus *bus = phb->host_state.bus;
+    int bus_off, node_off = 0, devid, fn, i, n, devices;
+    DeviceState *qdev;
+    char nodename[256];
+    struct {
+        uint32_t hi;
+        uint64_t addr;
+        uint64_t size;
+    } __attribute__((packed)) reg[PCI_NUM_REGIONS + 1],
+          assigned_addresses[PCI_NUM_REGIONS];
+    uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
+    struct {
+        uint32_t hi;
+        uint64_t child;
+        uint64_t parent;
+        uint64_t size;
+    } __attribute__((packed)) ranges[] = {
+        {
+            cpu_to_be32(b_ss(1)), cpu_to_be64(0),
+            cpu_to_be64(phb->io_win_addr),
+            cpu_to_be64(memory_region_size(&phb->iospace)),
+        },
+        {
+            cpu_to_be32(b_ss(2)), cpu_to_be64(SPAPR_PCI_MEM_WIN_BUS_OFFSET),
+            cpu_to_be64(phb->mem_win_addr),
+            cpu_to_be64(memory_region_size(&phb->memwindow)),
+        },
+    };
+    uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 };
+    uint32_t interrupt_map_mask[] = {
+        cpu_to_be32(b_ddddd(-1)|b_fff(-1)), 0x0, 0x0, 0x0};
+    uint32_t interrupt_map[bus->nirq][7];
+
+    /* Start populating the FDT */
+    sprintf(nodename, "pci@%" PRIx64, phb->buid);
+    bus_off = fdt_add_subnode(fdt, 0, nodename);
+    if (bus_off < 0) {
+        return bus_off;
+    }
+
+#define _FDT(exp) \
+    do { \
+        int ret = (exp);                                           \
+        if (ret < 0) {                                             \
+            return ret;                                            \
+        }                                                          \
+    } while (0)
+
+    /* Write PHB properties */
+    _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
+    _FDT(fdt_setprop_string(fdt, bus_off, "compatible", "IBM,Logical_PHB"));
+    _FDT(fdt_setprop_cell(fdt, bus_off, "#address-cells", 0x3));
+    _FDT(fdt_setprop_cell(fdt, bus_off, "#size-cells", 0x2));
+    _FDT(fdt_setprop_cell(fdt, bus_off, "#interrupt-cells", 0x1));
+    _FDT(fdt_setprop(fdt, bus_off, "used-by-rtas", NULL, 0));
+    _FDT(fdt_setprop(fdt, bus_off, "bus-range", &bus_range, sizeof(bus_range)));
+    _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof(ranges)));
+    _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg)));
+    _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
+                     &interrupt_map_mask, sizeof(interrupt_map_mask)));
+
+    /* Populate PCI devices and allocate IRQs */
+    devices = 0;
+    QLIST_FOREACH(qdev, &bus->qbus.children, sibling) {
+        PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
+        int irq_index = pci_spapr_map_irq(dev, 0);
+        uint32_t *irqmap = interrupt_map[devices];
+        uint8_t *config = dev->config;
+
+        devid = dev->devfn >> 3;
+        fn = dev->devfn & 7;
+
+        sprintf(nodename, "pci@%u,%u", devid, fn);
+
+        /* Allocate interrupt from the map */
+        if (devid > bus->nirq)  {
+            printf("Unexpected behaviour in spapr_populate_pci_devices,"
+                    "wrong devid %u\n", devid);
+            exit(-1);
+        }
+        irqmap[0] = cpu_to_be32(b_ddddd(devid)|b_fff(fn));
+        irqmap[1] = 0;
+        irqmap[2] = 0;
+        irqmap[3] = 0;
+        irqmap[4] = cpu_to_be32(xics_phandle);
+        irqmap[5] = cpu_to_be32(phb->lsi_table[irq_index].dt_irq);
+        irqmap[6] = cpu_to_be32(0x8);
+
+        /* Add node to FDT */
+        node_off = fdt_add_subnode(fdt, bus_off, nodename);
+        if (node_off < 0) {
+            return node_off;
+        }
+
+        _FDT(fdt_setprop_cell(fdt, node_off, "vendor-id",
+                              pci_get_word(&config[PCI_VENDOR_ID])));
+        _FDT(fdt_setprop_cell(fdt, node_off, "device-id",
+                              pci_get_word(&config[PCI_DEVICE_ID])));
+        _FDT(fdt_setprop_cell(fdt, node_off, "revision-id",
+                              pci_get_byte(&config[PCI_REVISION_ID])));
+        _FDT(fdt_setprop_cell(fdt, node_off, "class-code",
+                              pci_get_long(&config[PCI_CLASS_REVISION]) >> 8));
+        _FDT(fdt_setprop_cell(fdt, node_off, "subsystem-id",
+                              pci_get_word(&config[PCI_SUBSYSTEM_ID])));
+        _FDT(fdt_setprop_cell(fdt, node_off, "subsystem-vendor-id",
+                              pci_get_word(&config[PCI_SUBSYSTEM_VENDOR_ID])));
+
+        /* Config space region comes first */
+        reg[0].hi = cpu_to_be32(
+            b_n(0) |
+            b_p(0) |
+            b_t(0) |
+            b_ss(0/*config*/) |
+            b_bbbbbbbb(0) |
+            b_ddddd(devid) |
+            b_fff(fn));
+        reg[0].addr = 0;
+        reg[0].size = 0;
+
+        n = 0;
+        for (i = 0; i < PCI_NUM_REGIONS; ++i) {
+            if (0 == dev->io_regions[i].size) {
+                continue;
+            }
+
+            reg[n+1].hi = cpu_to_be32(
+                b_n(0) |
+                b_p(0) |
+                b_t(0) |
+                b_ss(regtype_to_ss(dev->io_regions[i].type)) |
+                b_bbbbbbbb(0) |
+                b_ddddd(devid) |
+                b_fff(fn) |
+                b_rrrrrrrr(bars[i]));
+            reg[n+1].addr = 0;
+            reg[n+1].size = cpu_to_be64(dev->io_regions[i].size);
+
+            assigned_addresses[n].hi = cpu_to_be32(
+                b_n(1) |
+                b_p(0) |
+                b_t(0) |
+                b_ss(regtype_to_ss(dev->io_regions[i].type)) |
+                b_bbbbbbbb(0) |
+                b_ddddd(devid) |
+                b_fff(fn) |
+                b_rrrrrrrr(bars[i]));
+
+            /*
+             * Writing zeroes to assigned_addresses causes the guest kernel to
+             * reassign BARs
+             */
+            assigned_addresses[n].addr = cpu_to_be64(dev->io_regions[i].addr);
+            assigned_addresses[n].size = reg[n+1].size;
+
+            ++n;
+        }
+        _FDT(fdt_setprop(fdt, node_off, "reg", reg, sizeof(reg[0])*(n+1)));
+        _FDT(fdt_setprop(fdt, node_off, "assigned-addresses",
+                         assigned_addresses,
+                         sizeof(assigned_addresses[0])*(n)));
+        _FDT(fdt_setprop_cell(fdt, node_off, "interrupts",
+                              pci_get_byte(&config[PCI_INTERRUPT_PIN])));
+
+        ++devices;
+    }
+
+    /* Write interrupt map */
+    _FDT(fdt_setprop(fdt, bus_off, "interrupt-map", &interrupt_map,
+                     devices * sizeof(interrupt_map[0])));
+
+    return 0;
+}
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
new file mode 100644
index 0000000..4bb8dfb
--- /dev/null
+++ b/hw/spapr_pci.h
@@ -0,0 +1,59 @@
+/*
+ * QEMU SPAPR PCI BUS definitions
+ *
+ * Copyright (c) 2011 Alexey Kardashevskiy <aik@au1.ibm.com>
+ *
+ * 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/>.
+ */
+#if !defined(__HW_SPAPR_PCI_H__)
+#define __HW_SPAPR_PCI_H__
+
+#include "hw/pci_host.h"
+#include "hw/xics.h"
+
+typedef struct sPAPREnvironment sPAPREnvironment;
+
+#define SPAPR_PCI_NUM_LSI   16
+
+typedef struct sPAPRPHBState {
+    SysBusDevice busdev;
+    PCIHostState host_state;
+
+    uint64_t buid;
+
+    MemoryRegion memspace, iospace;
+    target_phys_addr_t mem_win_addr, io_win_addr;
+    MemoryRegion memwindow, iowindow;
+
+    struct {
+        uint32_t dt_irq;
+        qemu_irq qirq;
+    } lsi_table[SPAPR_PCI_NUM_LSI];
+
+    QLIST_ENTRY(sPAPRPHBState) list;
+} sPAPRPHBState;
+
+#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
+#define SPAPR_PCI_IO_WIN_SIZE        0x10000
+
+void spapr_create_phb(sPAPREnvironment *spapr,
+                      const char *busname, uint64_t buid,
+                      uint64_t mem_win_addr, uint64_t mem_win_size,
+                      uint64_t io_win_addr);
+
+int spapr_populate_pci_devices(sPAPRPHBState *phb,
+                               uint32_t xics_phandle,
+                               void *fdt);
+
+#endif /* __HW_SPAPR_PCI_H__ */
-- 
1.7.7

[Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[David Gibson]

From: Nishanth Aravamudan <nacc@us.ibm.com>

When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
qemu monitor reports "dump_mmu: unimplemented".  However, during
bringup work, it can be quite handy to have the SLB entries, which are
available in the CPUPPCState.  This patch adds an implementation of
info tlb for book3s, which dumps the SLB.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
 1 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 137a494..29c7050 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
     }
 }
 
+static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
+                              CPUState *env)
+{
+    int i;
+    uint64_t slbe, slbv;
+
+    cpu_synchronize_state(env);
+
+    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
+    for (i = 0; i < env->slb_nr; i++) {
+        slbe = env->slb[i].esid;
+        slbv = env->slb[i].vsid;
+        if (slbe == 0 && slbv == 0) {
+            continue;
+        }
+        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
+                    i, slbe, slbv);
+    }
+}
+
 void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
 {
-    switch (env->mmu_model) {
-    case POWERPC_MMU_BOOKE206:
+    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
         mmubooke206_dump_mmu(f, cpu_fprintf, env);
-        break;
-    default:
-        cpu_fprintf(f, "%s: unimplemented\n", __func__);
+    } else {
+        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {
+            mmubooks_dump_mmu(f, cpu_fprintf, env);
+        } else {
+            cpu_fprintf(f, "%s: unimplemented\n", __func__);
+        }
     }
 }
 
-- 
1.7.7

Re: [Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[Alexander Graf]

On 31.10.2011, at 04:16, David Gibson wrote:

> From: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> qemu monitor reports "dump_mmu: unimplemented".  However, during
> bringup work, it can be quite handy to have the SLB entries, which are
> available in the CPUPPCState.  This patch adds an implementation of
> info tlb for book3s, which dumps the SLB.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
> 1 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index 137a494..29c7050 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
>     }
> }
> 
> +static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> +                              CPUState *env)
> +{
> +    int i;
> +    uint64_t slbe, slbv;
> +
> +    cpu_synchronize_state(env);
> +
> +    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> +    for (i = 0; i < env->slb_nr; i++) {
> +        slbe = env->slb[i].esid;
> +        slbv = env->slb[i].vsid;

From cpu.h:

#if defined(TARGET_PPC64)
    /* Address space register */
    target_ulong asr;
    /* PowerPC 64 SLB area */
    ppc_slb_t slb[64];
    int slb_nr;
#endif


> +        if (slbe == 0 && slbv == 0) {
> +            continue;
> +        }
> +        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
> +                    i, slbe, slbv);
> +    }
> +}
> +
> void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
> {
> -    switch (env->mmu_model) {
> -    case POWERPC_MMU_BOOKE206:
> +    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
>         mmubooke206_dump_mmu(f, cpu_fprintf, env);
> -        break;
> -    default:
> -        cpu_fprintf(f, "%s: unimplemented\n", __func__);
> +    } else {
> +        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {

I would actually prefer to explicitly keep the switch and match on all implementations explicitly. Also, have you verified this works without CONFIG_PPC64 set? In cpu.h I see the following:

#if defined(TARGET_PPC64)
#define POWERPC_MMU_64       0x00010000
#define POWERPC_MMU_1TSEG    0x00020000
    /* 64 bits PowerPC MMU                                     */
    POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
    /* 620 variant (no segment exceptions)                     */
    POWERPC_MMU_620        = POWERPC_MMU_64 | 0x00000002,
    /* Architecture 2.06 variant                               */
    POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG | 0x00000003,
#endif /* defined(TARGET_PPC64) */

So POWERPC_MMU_64B shouldn't be defined for qemu-system-ppc.


Alex

Re: [Qemu-devel] [PATCH 1/3] ppc: Alter CPU state to mask out TCG unimplemented instructions as appropriate

[Alexander Graf]

On 31.10.2011, at 04:16, David Gibson wrote:

> The CPU state contains two bitmaps, initialized from the CPU spec
> which describes which instructions are implemented on the CPU.  A
> couple of bits are defined which cover instructions (VSX and DFP)
> which are not currently implemented in TCG.  So far, these are only
> used to handle the case of -cpu host because a KVM guest can use
> the instructions when the host CPU supports them.
> 
> However, it's a mild layering violation to simply not include those
> bits in the CPU descriptions for those CPUs that do support them,
> just because we can't handle them in TCG.  This patch corrects the
> situation, so that the instruction bits _are_ shown correctly in the
> cpu spec table, but are masked out from the cpu state in the non-KVM
> case.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Thanks, applied.


Alex

Re: [Qemu-devel] [PATCH 2/3] pseries: Add partial support for PCI

[Alexander Graf]

On 31.10.2011, at 04:16, David Gibson wrote:

> This patch adds a PCI bus to the pseries machine.  This instantiates
> the qemu generic PCI bus code, advertises a PCI host bridge in the
> guest's device tree and implements the RTAS methods specified by PAPR
> to access PCI config space.  It also sets up the memory regions we
> need to provide windows into the PCI memory and IO space, and
> advertises those to the guest.
> 
> However, because qemu can't yet emulate an IOMMU, which is mandatory on
> pseries, PCI devices which use DMA (i.e. most of them) will not work with
> this code alone.  Still, this is enough to support the virtio_pci device
> (which probably _should_ use emulated PCI DMA, but is specced to use
> direct hypervisor access to guest physical memory instead).
> 
> Signed-off-by: Alexey Kardashevskiy <aik@au1.ibm.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied with the following patch folded in:

diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index 4bb8dfb..213340c 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -16,14 +16,16 @@
  * 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/>.
  */
+#if !defined(__HW_SPAPR_H__)
+#error Please include spapr.h before this file!
+#endif
+
 #if !defined(__HW_SPAPR_PCI_H__)
 #define __HW_SPAPR_PCI_H__
 
 #include "hw/pci_host.h"
 #include "hw/xics.h"
 
-typedef struct sPAPREnvironment sPAPREnvironment;
-
 #define SPAPR_PCI_NUM_LSI   16
 
 typedef struct sPAPRPHBState {


Alex

Re: [Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[David Gibson]

Good points below.  I forgot to CC Nish, the original patch author on
my post, so I've added him to the list now.

Nish, can you correct these problems and resend the patch please?

On Mon, Oct 31, 2011 at 04:35:54AM +0100, Alexander Graf wrote:
> 
> On 31.10.2011, at 04:16, David Gibson wrote:
> 
> > From: Nishanth Aravamudan <nacc@us.ibm.com>
> > 
> > When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> > qemu monitor reports "dump_mmu: unimplemented".  However, during
> > bringup work, it can be quite handy to have the SLB entries, which are
> > available in the CPUPPCState.  This patch adds an implementation of
> > info tlb for book3s, which dumps the SLB.
> > 
> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
> > 1 files changed, 27 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> > index 137a494..29c7050 100644
> > --- a/target-ppc/helper.c
> > +++ b/target-ppc/helper.c
> > @@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> >     }
> > }
> > 
> > +static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> > +                              CPUState *env)
> > +{
> > +    int i;
> > +    uint64_t slbe, slbv;
> > +
> > +    cpu_synchronize_state(env);
> > +
> > +    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> > +    for (i = 0; i < env->slb_nr; i++) {
> > +        slbe = env->slb[i].esid;
> > +        slbv = env->slb[i].vsid;
> 
> From cpu.h:
> 
> #if defined(TARGET_PPC64)
>     /* Address space register */
>     target_ulong asr;
>     /* PowerPC 64 SLB area */
>     ppc_slb_t slb[64];
>     int slb_nr;
> #endif
> 
> 
> > +        if (slbe == 0 && slbv == 0) {
> > +            continue;
> > +        }
> > +        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
> > +                    i, slbe, slbv);
> > +    }
> > +}
> > +
> > void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
> > {
> > -    switch (env->mmu_model) {
> > -    case POWERPC_MMU_BOOKE206:
> > +    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
> >         mmubooke206_dump_mmu(f, cpu_fprintf, env);
> > -        break;
> > -    default:
> > -        cpu_fprintf(f, "%s: unimplemented\n", __func__);
> > +    } else {
> > +        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {
> 
> I would actually prefer to explicitly keep the switch and match on all implementations explicitly. Also, have you verified this works without CONFIG_PPC64 set? In cpu.h I see the following:
> 
> #if defined(TARGET_PPC64)
> #define POWERPC_MMU_64       0x00010000
> #define POWERPC_MMU_1TSEG    0x00020000
>     /* 64 bits PowerPC MMU                                     */
>     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
>     /* 620 variant (no segment exceptions)                     */
>     POWERPC_MMU_620        = POWERPC_MMU_64 | 0x00000002,
>     /* Architecture 2.06 variant                               */
>     POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG | 0x00000003,
> #endif /* defined(TARGET_PPC64) */
> 
> So POWERPC_MMU_64B shouldn't be defined for qemu-system-ppc.
> 
> 
> Alex
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[Nishanth Aravamudan]

On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
> Good points below.  I forgot to CC Nish, the original patch author on
> my post, so I've added him to the list now.
> 
> Nish, can you correct these problems and resend the patch please?

Yep, I'll work on this shortly.

> On Mon, Oct 31, 2011 at 04:35:54AM +0100, Alexander Graf wrote:
> > 
> > On 31.10.2011, at 04:16, David Gibson wrote:
> > 
> > > From: Nishanth Aravamudan <nacc@us.ibm.com>
> > > 
> > > When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> > > qemu monitor reports "dump_mmu: unimplemented".  However, during
> > > bringup work, it can be quite handy to have the SLB entries, which are
> > > available in the CPUPPCState.  This patch adds an implementation of
> > > info tlb for book3s, which dumps the SLB.
> > > 
> > > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > > target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
> > > 1 files changed, 27 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> > > index 137a494..29c7050 100644
> > > --- a/target-ppc/helper.c
> > > +++ b/target-ppc/helper.c
> > > @@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> > >     }
> > > }
> > > 
> > > +static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> > > +                              CPUState *env)
> > > +{
> > > +    int i;
> > > +    uint64_t slbe, slbv;
> > > +
> > > +    cpu_synchronize_state(env);
> > > +
> > > +    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> > > +    for (i = 0; i < env->slb_nr; i++) {
> > > +        slbe = env->slb[i].esid;
> > > +        slbv = env->slb[i].vsid;
> > 
> > From cpu.h:
> > 
> > #if defined(TARGET_PPC64)
> >     /* Address space register */
> >     target_ulong asr;
> >     /* PowerPC 64 SLB area */
> >     ppc_slb_t slb[64];
> >     int slb_nr;
> > #endif

Being unfamiliar with qemu's coding style (and not immediately seeing it
in CODING_STYLE), would the right approach be to wrap this definition in
an #if defined(TARGET_PPC64)?

> > > +        if (slbe == 0 && slbv == 0) {
> > > +            continue;
> > > +        }
> > > +        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
> > > +                    i, slbe, slbv);
> > > +    }
> > > +}
> > > +
> > > void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
> > > {
> > > -    switch (env->mmu_model) {
> > > -    case POWERPC_MMU_BOOKE206:
> > > +    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
> > >         mmubooke206_dump_mmu(f, cpu_fprintf, env);
> > > -        break;
> > > -    default:
> > > -        cpu_fprintf(f, "%s: unimplemented\n", __func__);
> > > +    } else {
> > > +        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {
> > 
> > I would actually prefer to explicitly keep the switch and match on
> > all implementations explicitly. Also, have you verified this works
> > without CONFIG_PPC64 set? In cpu.h I see the following:
> > 
> > #if defined(TARGET_PPC64)
> > #define POWERPC_MMU_64       0x00010000
> > #define POWERPC_MMU_1TSEG    0x00020000
> >     /* 64 bits PowerPC MMU                                     */
> >     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
> >     /* 620 variant (no segment exceptions)                     */
> >     POWERPC_MMU_620        = POWERPC_MMU_64 | 0x00000002,
> >     /* Architecture 2.06 variant                               */
> >     POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG | 0x00000003,
> > #endif /* defined(TARGET_PPC64) */
> > 
> > So POWERPC_MMU_64B shouldn't be defined for qemu-system-ppc.

And similarly here, only have the MMU_2_06 and MMU_64B cases in the
switch be defined #if defined(TARGET_PPC64)?

Basically, asking if #ifdefs in the middle of functions are ok :)

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[Alexander Graf]

On 31.10.2011, at 15:53, Nishanth Aravamudan wrote:

> On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
>> Good points below.  I forgot to CC Nish, the original patch author on
>> my post, so I've added him to the list now.
>> 
>> Nish, can you correct these problems and resend the patch please?
> 
> Yep, I'll work on this shortly.
> 
>> On Mon, Oct 31, 2011 at 04:35:54AM +0100, Alexander Graf wrote:
>>> 
>>> On 31.10.2011, at 04:16, David Gibson wrote:
>>> 
>>>> From: Nishanth Aravamudan <nacc@us.ibm.com>
>>>> 
>>>> When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
>>>> qemu monitor reports "dump_mmu: unimplemented".  However, during
>>>> bringup work, it can be quite handy to have the SLB entries, which are
>>>> available in the CPUPPCState.  This patch adds an implementation of
>>>> info tlb for book3s, which dumps the SLB.
>>>> 
>>>> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---
>>>> target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
>>>> 1 files changed, 27 insertions(+), 5 deletions(-)
>>>> 
>>>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
>>>> index 137a494..29c7050 100644
>>>> --- a/target-ppc/helper.c
>>>> +++ b/target-ppc/helper.c
>>>> @@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
>>>>    }
>>>> }
>>>> 
>>>> +static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
>>>> +                              CPUState *env)
>>>> +{
>>>> +    int i;
>>>> +    uint64_t slbe, slbv;
>>>> +
>>>> +    cpu_synchronize_state(env);
>>>> +
>>>> +    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
>>>> +    for (i = 0; i < env->slb_nr; i++) {
>>>> +        slbe = env->slb[i].esid;
>>>> +        slbv = env->slb[i].vsid;
>>> 
>>> From cpu.h:
>>> 
>>> #if defined(TARGET_PPC64)
>>>    /* Address space register */
>>>    target_ulong asr;
>>>    /* PowerPC 64 SLB area */
>>>    ppc_slb_t slb[64];
>>>    int slb_nr;
>>> #endif
> 
> Being unfamiliar with qemu's coding style (and not immediately seeing it
> in CODING_STYLE), would the right approach be to wrap this definition in
> an #if defined(TARGET_PPC64)?

It's the only way to make it compile :)

> 
>>>> +        if (slbe == 0 && slbv == 0) {
>>>> +            continue;
>>>> +        }
>>>> +        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
>>>> +                    i, slbe, slbv);
>>>> +    }
>>>> +}
>>>> +
>>>> void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
>>>> {
>>>> -    switch (env->mmu_model) {
>>>> -    case POWERPC_MMU_BOOKE206:
>>>> +    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
>>>>        mmubooke206_dump_mmu(f, cpu_fprintf, env);
>>>> -        break;
>>>> -    default:
>>>> -        cpu_fprintf(f, "%s: unimplemented\n", __func__);
>>>> +    } else {
>>>> +        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {
>>> 
>>> I would actually prefer to explicitly keep the switch and match on
>>> all implementations explicitly. Also, have you verified this works
>>> without CONFIG_PPC64 set? In cpu.h I see the following:
>>> 
>>> #if defined(TARGET_PPC64)
>>> #define POWERPC_MMU_64       0x00010000
>>> #define POWERPC_MMU_1TSEG    0x00020000
>>>    /* 64 bits PowerPC MMU                                     */
>>>    POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
>>>    /* 620 variant (no segment exceptions)                     */
>>>    POWERPC_MMU_620        = POWERPC_MMU_64 | 0x00000002,
>>>    /* Architecture 2.06 variant                               */
>>>    POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG | 0x00000003,
>>> #endif /* defined(TARGET_PPC64) */
>>> 
>>> So POWERPC_MMU_64B shouldn't be defined for qemu-system-ppc.
> 
> And similarly here, only have the MMU_2_06 and MMU_64B cases in the
> switch be defined #if defined(TARGET_PPC64)?

Yup :).

> Basically, asking if #ifdefs in the middle of functions are ok :)

It's the only valid way of making it work, yes. The alternative would be to drop all the TARGET_PPC64 ifdefery, but let's keep that for later.


Alex

Re: [Qemu-devel] [PATCH 3/3] monitor: add ability to dump SLB entries

[Nishanth Aravamudan]

On 31.10.2011 [17:35:13 -0700], Alexander Graf wrote:
> 
> On 31.10.2011, at 15:53, Nishanth Aravamudan wrote:
> 
> > On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
> >> Good points below.  I forgot to CC Nish, the original patch author on
> >> my post, so I've added him to the list now.
> >> 
> >> Nish, can you correct these problems and resend the patch please?
> > 
> > Yep, I'll work on this shortly.
> > 
> >> On Mon, Oct 31, 2011 at 04:35:54AM +0100, Alexander Graf wrote:
> >>> 
> >>> On 31.10.2011, at 04:16, David Gibson wrote:
> >>> 
> >>>> From: Nishanth Aravamudan <nacc@us.ibm.com>
> >>>> 
> >>>> When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> >>>> qemu monitor reports "dump_mmu: unimplemented".  However, during
> >>>> bringup work, it can be quite handy to have the SLB entries, which are
> >>>> available in the CPUPPCState.  This patch adds an implementation of
> >>>> info tlb for book3s, which dumps the SLB.
> >>>> 
> >>>> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> >>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>>> ---
> >>>> target-ppc/helper.c |   32 +++++++++++++++++++++++++++-----
> >>>> 1 files changed, 27 insertions(+), 5 deletions(-)
> >>>> 
> >>>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> >>>> index 137a494..29c7050 100644
> >>>> --- a/target-ppc/helper.c
> >>>> +++ b/target-ppc/helper.c
> >>>> @@ -1545,14 +1545,36 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> >>>>    }
> >>>> }
> >>>> 
> >>>> +static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> >>>> +                              CPUState *env)
> >>>> +{
> >>>> +    int i;
> >>>> +    uint64_t slbe, slbv;
> >>>> +
> >>>> +    cpu_synchronize_state(env);
> >>>> +
> >>>> +    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> >>>> +    for (i = 0; i < env->slb_nr; i++) {
> >>>> +        slbe = env->slb[i].esid;
> >>>> +        slbv = env->slb[i].vsid;
> >>> 
> >>> From cpu.h:
> >>> 
> >>> #if defined(TARGET_PPC64)
> >>>    /* Address space register */
> >>>    target_ulong asr;
> >>>    /* PowerPC 64 SLB area */
> >>>    ppc_slb_t slb[64];
> >>>    int slb_nr;
> >>> #endif
> > 
> > Being unfamiliar with qemu's coding style (and not immediately seeing it
> > in CODING_STYLE), would the right approach be to wrap this definition in
> > an #if defined(TARGET_PPC64)?
> 
> It's the only way to make it compile :)
> 
> > 
> >>>> +        if (slbe == 0 && slbv == 0) {
> >>>> +            continue;
> >>>> +        }
> >>>> +        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
> >>>> +                    i, slbe, slbv);
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
> >>>> {
> >>>> -    switch (env->mmu_model) {
> >>>> -    case POWERPC_MMU_BOOKE206:
> >>>> +    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
> >>>>        mmubooke206_dump_mmu(f, cpu_fprintf, env);
> >>>> -        break;
> >>>> -    default:
> >>>> -        cpu_fprintf(f, "%s: unimplemented\n", __func__);
> >>>> +    } else {
> >>>> +        if ((env->mmu_model & POWERPC_MMU_64B) != 0) {
> >>> 
> >>> I would actually prefer to explicitly keep the switch and match on
> >>> all implementations explicitly. Also, have you verified this works
> >>> without CONFIG_PPC64 set? In cpu.h I see the following:
> >>> 
> >>> #if defined(TARGET_PPC64)
> >>> #define POWERPC_MMU_64       0x00010000
> >>> #define POWERPC_MMU_1TSEG    0x00020000
> >>>    /* 64 bits PowerPC MMU                                     */
> >>>    POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
> >>>    /* 620 variant (no segment exceptions)                     */
> >>>    POWERPC_MMU_620        = POWERPC_MMU_64 | 0x00000002,
> >>>    /* Architecture 2.06 variant                               */
> >>>    POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG | 0x00000003,
> >>> #endif /* defined(TARGET_PPC64) */
> >>> 
> >>> So POWERPC_MMU_64B shouldn't be defined for qemu-system-ppc.
> > 
> > And similarly here, only have the MMU_2_06 and MMU_64B cases in the
> > switch be defined #if defined(TARGET_PPC64)?
> 
> Yup :).
> 
> > Basically, asking if #ifdefs in the middle of functions are ok :)
> 
> It's the only valid way of making it work, yes. The alternative would
> be to drop all the TARGET_PPC64 ifdefery, but let's keep that for
> later.

Great, thanks for the confirmation.

-Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

[Qemu-devel] [PATCH v2] monitor: add ability to dump SLB entries

[Nishanth Aravamudan]

On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
> Good points below.  I forgot to CC Nish, the original patch author on
> my post, so I've added him to the list now.
> 
> Nish, can you correct these problems and resend the patch please?

When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
qemu monitor reports "dump_mmu: unimplemented".  However, during
bringup work, it can be quite handy to have the SLB entries, which are
available in the CPUPPCState.  This patch adds an implementation of
info tlb for book3s, which dumps the SLB.
    
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

---
v2: Update to build on PPC and PPC64 via suggestion from AGraf.

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 137a494..5847453 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -1545,12 +1545,40 @@ static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
     }
 }
 
+#if defined(TARGET_PPC64)
+static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
+                              CPUState *env)
+{
+    int i;
+    uint64_t slbe, slbv;
+
+    cpu_synchronize_state(env);
+
+    cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
+    for (i = 0; i < env->slb_nr; i++) {
+        slbe = env->slb[i].esid;
+        slbv = env->slb[i].vsid;
+        if (slbe == 0 && slbv == 0) {
+            continue;
+        }
+        cpu_fprintf(f, "%d\t0x%016" PRIx64 "\t0x%016" PRIx64 "\n",
+                    i, slbe, slbv);
+    }
+}
+#endif
+
 void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
 {
     switch (env->mmu_model) {
     case POWERPC_MMU_BOOKE206:
         mmubooke206_dump_mmu(f, cpu_fprintf, env);
         break;
+#if defined(TARGET_PPC64)
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_06:
+        mmubooks_dump_mmu(f, cpu_fprintf, env);
+        break;
+#endif
     default:
         cpu_fprintf(f, "%s: unimplemented\n", __func__);
     }

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH v2] monitor: add ability to dump SLB entries

[Alexander Graf]

On 11/01/2011 08:57 PM, Nishanth Aravamudan wrote:
> On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
>> Good points below.  I forgot to CC Nish, the original patch author on
>> my post, so I've added him to the list now.
>>
>> Nish, can you correct these problems and resend the patch please?
> When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> qemu monitor reports "dump_mmu: unimplemented".  However, during
> bringup work, it can be quite handy to have the SLB entries, which are
> available in the CPUPPCState.  This patch adds an implementation of
> info tlb for book3s, which dumps the SLB.
>
> Signed-off-by: Nishanth Aravamudan<nacc@us.ibm.com>
> Signed-off-by: David Gibson<david@gibson.dropbear.id.au>

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] monitor: add ability to dump SLB entries

[David Gibson]

On Thu, Nov 10, 2011 at 06:11:41PM +0100, Alexander Graf wrote:
> On 11/01/2011 08:57 PM, Nishanth Aravamudan wrote:
> >On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
> >>Good points below.  I forgot to CC Nish, the original patch author on
> >>my post, so I've added him to the list now.
> >>
> >>Nish, can you correct these problems and resend the patch please?
> >When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
> >qemu monitor reports "dump_mmu: unimplemented".  However, during
> >bringup work, it can be quite handy to have the SLB entries, which are
> >available in the CPUPPCState.  This patch adds an implementation of
> >info tlb for book3s, which dumps the SLB.
> >
> >Signed-off-by: Nishanth Aravamudan<nacc@us.ibm.com>
> >Signed-off-by: David Gibson<david@gibson.dropbear.id.au>
> 
> Thanks, applied to ppc-next.

I'm guessing that ppc-next is now the branch destined for post 1.0.
Is that right?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] monitor: add ability to dump SLB entries

[Alexander Graf]

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

> On Thu, Nov 10, 2011 at 06:11:41PM +0100, Alexander Graf wrote:
>> On 11/01/2011 08:57 PM, Nishanth Aravamudan wrote:
>>> On 31.10.2011 [15:14:12 +1100], David Gibson wrote:
>>>> Good points below.  I forgot to CC Nish, the original patch author on
>>>> my post, so I've added him to the list now.
>>>> 
>>>> Nish, can you correct these problems and resend the patch please?
>>> When run with a PPC Book3S (server) CPU Currently 'info tlb' in the
>>> qemu monitor reports "dump_mmu: unimplemented".  However, during
>>> bringup work, it can be quite handy to have the SLB entries, which are
>>> available in the CPUPPCState.  This patch adds an implementation of
>>> info tlb for book3s, which dumps the SLB.
>>> 
>>> Signed-off-by: Nishanth Aravamudan<nacc@us.ibm.com>
>>> Signed-off-by: David Gibson<david@gibson.dropbear.id.au>
>> 
>> Thanks, applied to ppc-next.
> 
> I'm guessing that ppc-next is now the branch destined for post 1.0.
> Is that right?

Yes :). Please mark 1.0 patches as such so I can cherry-pick them there.

Alex

> 
> -- 
> David Gibson            | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au    | minimalist, thank you.  NOT _the_ _other_
>                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>