[Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

This is some cleanup. Please, comment. Thanks!

Changes:
v7:
* cleaned up a bit of QOM use

v6:
* removed kvmppc_fixup_cpu()

v5:
* cleanup
* removed cpustate::kvm_cpu_id
* split into 2 patches - new PPC API and the usage of the API


Alexey Kardashevskiy (2):
  target-ppc: add PowerPCCPU::cpu_dt_id
  target-ppc: spapr: e500: fix to use cpu_dt_id

 hw/intc/openpic_kvm.c       |  2 +-
 hw/intc/xics.c              | 15 +++++++++++++--
 hw/intc/xics_kvm.c          | 10 +++++-----
 hw/ppc/e500.c               |  7 +++++--
 hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
 hw/ppc/spapr.c              |  9 +++++----
 hw/ppc/spapr_hcall.c        |  6 +++---
 hw/ppc/spapr_rtas.c         | 14 +++++++-------
 target-ppc/cpu-qom.h        |  2 ++
 target-ppc/cpu.h            | 18 ++++++++++++++++++
 target-ppc/kvm.c            | 15 +--------------
 target-ppc/kvm_ppc.h        |  6 ------
 target-ppc/translate_init.c | 11 +++++------
 13 files changed, 87 insertions(+), 50 deletions(-)

-- 
1.8.4.rc4

[Qemu-devel] [PATCH v7 1/2] target-ppc: add PowerPCCPU::cpu_dt_id

[Alexey Kardashevskiy]

Normally CPUState::cpu_index is used to pick the right CPU for various
operations. However default consecutive numbering does not always work
for POWERPC.

These indexes are reflected in /proc/device-tree/cpus/PowerPC,POWER7@XX
and used to call KVM VCPU's ioctls. In order to achieve this,
kvmppc_fixup_cpu() was introduced. Roughly speaking, it multiplies
cpu_index by the number of threads per core.

This approach has disadvantages such as:
1. NUMA configuration stays broken after the fixup;
2. CPU-targeted commands from the QEMU Monitor do not work properly as
CPU indexes have been fixed and there is no clear way for the user to
know what the new CPU indexes are.

This introduces a @cpu_dt_id field in the CPUPPCState struct which
is initialized from @cpu_index by default and can be fixed later
to meet the device tree requirements.

This adds an API to handle @cpu_dt_id.

This removes kvmppc_fixup_cpu() as it is not more needed, @cpu_dt_id
is calculated in ppc_cpu_realize().

This will be used later in machine code.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v6: inlined kvmppc_fixup_cpu()
---
 hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
 target-ppc/cpu-qom.h        |  2 ++
 target-ppc/cpu.h            | 18 ++++++++++++++++++
 target-ppc/kvm.c            | 13 -------------
 target-ppc/kvm_ppc.h        |  6 ------
 target-ppc/translate_init.c | 10 ++++------
 6 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 114be64..0e82719 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -26,6 +26,7 @@
 #include "hw/ppc/ppc_e500.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/cpus.h"
 #include "hw/timer/m48t59.h"
 #include "qemu/log.h"
 #include "hw/loader.h"
@@ -1362,3 +1363,24 @@ int PPC_NVRAM_set_params (nvram_t *nvram, uint16_t NVRAM_size,
 
     return 0;
 }
+
+/* CPU device-tree ID helpers */
+int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
+{
+    return cpu->cpu_dt_id;
+}
+
+PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
+{
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+        if (cpu->cpu_dt_id == cpu_dt_id) {
+            return cpu;
+        }
+    }
+
+    return NULL;
+}
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 72b2232..b17c024 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -79,6 +79,7 @@ typedef struct PowerPCCPUClass {
 /**
  * PowerPCCPU:
  * @env: #CPUPPCState
+ * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
  *
  * A PowerPC CPU.
  */
@@ -88,6 +89,7 @@ typedef struct PowerPCCPU {
     /*< public >*/
 
     CPUPPCState env;
+    int cpu_dt_id;
 } PowerPCCPU;
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 51bcd4a..d8577ae 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2154,4 +2154,22 @@ static inline bool cpu_has_work(CPUState *cpu)
 
 void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
 
+/**
+ * ppc_get_vcpu_dt_id:
+ * @cs: a PowerPCCPU struct.
+ *
+ * Returns a device-tree ID for a CPU.
+ */
+int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
+
+/**
+ * ppc_get_vcpu_by_dt_id:
+ * @cpu_dt_id: a device tree id
+ *
+ * Searches for a CPU by @cpu_dt_id.
+ *
+ * Returns: a PowerPCCPU struct
+ */
+PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
+
 #endif /* !defined (__CPU_PPC_H__) */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 781b72f..8bcc5fb 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1766,19 +1766,6 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
     }
 }
 
-int kvmppc_fixup_cpu(PowerPCCPU *cpu)
-{
-    CPUState *cs = CPU(cpu);
-    int smt;
-
-    /* Adjust cpu index for SMT */
-    smt = kvmppc_smt_threads();
-    cs->cpu_index = (cs->cpu_index / smp_threads) * smt
-        + (cs->cpu_index % smp_threads);
-
-    return 0;
-}
-
 bool kvmppc_has_cap_epr(void)
 {
     return cap_epr;
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 5f78e4b..f3afcdb 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -36,7 +36,6 @@ int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
 int kvmppc_reset_htab(int shift_hint);
 uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
 #endif /* !CONFIG_USER_ONLY */
-int kvmppc_fixup_cpu(PowerPCCPU *cpu);
 bool kvmppc_has_cap_epr(void);
 int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);
 int kvmppc_get_htab_fd(bool write);
@@ -155,11 +154,6 @@ static inline int kvmppc_update_sdr1(CPUPPCState *env)
 
 #endif /* !CONFIG_USER_ONLY */
 
-static inline int kvmppc_fixup_cpu(PowerPCCPU *cpu)
-{
-    return -1;
-}
-
 static inline bool kvmppc_has_cap_epr(void)
 {
     return false;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 35470d4..6de7126 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7808,14 +7808,12 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
                    max_smt, kvm_enabled() ? "KVM" : "TCG");
         return;
     }
+
+    cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
+        + (cs->cpu_index % smp_threads);
 #endif
 
-    if (kvm_enabled()) {
-        if (kvmppc_fixup_cpu(cpu) != 0) {
-            error_setg(errp, "Unable to virtualize selected CPU with KVM");
-            return;
-        }
-    } else if (tcg_enabled()) {
+    if (tcg_enabled()) {
         if (ppc_fixup_cpu(cpu) != 0) {
             error_setg(errp, "Unable to emulate selected CPU with TCG");
             return;
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v7 2/2] target-ppc: spapr: e500: fix to use cpu_dt_id

[Alexey Kardashevskiy]

This makes use of @cpu_dt_id and related API in:
1. emulated XICS hypercall handlers as they receive fixed CPU indexes;
2. XICS-KVM to enable in-kernel XICS on right CPU;
3. device-tree renderer.

This removes @cpu_index fixup as @cpu_dt_id is used instead so QEMU monitor
can accept command-line CPU indexes again.

This changes kvm_arch_vcpu_id() to use ppc_get_vcpu_dt_id() as at the moment
KVM CPU id and device tree ID are calculated using the same algorithm.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v7:
* replaced referencing to PowerPCCPU::parent_obj with the CPU macro
---
 hw/intc/openpic_kvm.c       |  2 +-
 hw/intc/xics.c              | 15 +++++++++++++--
 hw/intc/xics_kvm.c          | 10 +++++-----
 hw/ppc/e500.c               |  7 +++++--
 hw/ppc/spapr.c              |  9 +++++----
 hw/ppc/spapr_hcall.c        |  6 +++---
 hw/ppc/spapr_rtas.c         | 14 +++++++-------
 target-ppc/kvm.c            |  2 +-
 target-ppc/translate_init.c |  1 +
 9 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
index c7f7b84..87fdb12 100644
--- a/hw/intc/openpic_kvm.c
+++ b/hw/intc/openpic_kvm.c
@@ -228,7 +228,7 @@ int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs)
 
     encap.cap = KVM_CAP_IRQ_MPIC;
     encap.args[0] = opp->fd;
-    encap.args[1] = cs->cpu_index;
+    encap.args[1] = kvm_arch_vcpu_id(cs);
 
     return kvm_vcpu_ioctl(cs, KVM_ENABLE_CAP, &encap);
 }
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index b437563..64aabe7 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -33,6 +33,17 @@
 #include "qemu/error-report.h"
 #include "qapi/visitor.h"
 
+static int get_cpu_index_by_dt_id(int cpu_dt_id)
+{
+    PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
+
+    if (cpu) {
+        return cpu->parent_obj.cpu_index;
+    }
+
+    return -1;
+}
+
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
@@ -659,7 +670,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPREnvironment *spapr,
 static target_ulong h_ipi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                           target_ulong opcode, target_ulong *args)
 {
-    target_ulong server = args[0];
+    target_ulong server = get_cpu_index_by_dt_id(args[0]);
     target_ulong mfrr = args[1];
 
     if (server >= spapr->icp->nr_servers) {
@@ -728,7 +739,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     }
 
     nr = rtas_ld(args, 0);
-    server = rtas_ld(args, 1);
+    server = get_cpu_index_by_dt_id(rtas_ld(args, 1));
     priority = rtas_ld(args, 2);
 
     if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index c203646..a5bbc24 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -65,7 +65,7 @@ static void icp_get_kvm_state(ICPState *ss)
     ret = kvm_vcpu_ioctl(ss->cs, KVM_GET_ONE_REG, &reg);
     if (ret != 0) {
         error_report("Unable to retrieve KVM interrupt controller state"
-                " for CPU %d: %s", ss->cs->cpu_index, strerror(errno));
+                " for CPU %ld: %s", kvm_arch_vcpu_id(ss->cs), strerror(errno));
         exit(1);
     }
 
@@ -97,7 +97,7 @@ static int icp_set_kvm_state(ICPState *ss, int version_id)
     ret = kvm_vcpu_ioctl(ss->cs, KVM_SET_ONE_REG, &reg);
     if (ret != 0) {
         error_report("Unable to restore KVM interrupt controller state (0x%"
-                PRIx64 ") for CPU %d: %s", state, ss->cs->cpu_index,
+                PRIx64 ") for CPU %ld: %s", state, kvm_arch_vcpu_id(ss->cs),
                 strerror(errno));
         return ret;
     }
@@ -325,15 +325,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
         struct kvm_enable_cap xics_enable_cap = {
             .cap = KVM_CAP_IRQ_XICS,
             .flags = 0,
-            .args = {icpkvm->kernel_xics_fd, cs->cpu_index, 0, 0},
+            .args = {icpkvm->kernel_xics_fd, kvm_arch_vcpu_id(cs), 0, 0},
         };
 
         ss->cs = cs;
 
         ret = kvm_vcpu_ioctl(ss->cs, KVM_ENABLE_CAP, &xics_enable_cap);
         if (ret < 0) {
-            error_report("Unable to connect CPU%d to kernel XICS: %s",
-                    cs->cpu_index, strerror(errno));
+            error_report("Unable to connect CPU%ld to kernel XICS: %s",
+                    kvm_arch_vcpu_id(cs), strerror(errno));
             exit(1);
         }
     }
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index b37ce9d..8a08752 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -238,6 +238,7 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
        the first node as boot node and be happy */
     for (i = smp_cpus - 1; i >= 0; i--) {
         CPUState *cpu;
+        PowerPCCPU *pcpu;
         char cpu_name[128];
         uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
 
@@ -246,14 +247,16 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
             continue;
         }
         env = cpu->env_ptr;
+        pcpu = POWERPC_CPU(cpu);
 
         snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
-                 cpu->cpu_index);
+                 ppc_get_vcpu_dt_id(pcpu));
         qemu_fdt_add_subnode(fdt, cpu_name);
         qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
         qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
         qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
-        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "reg",
+                              ppc_get_vcpu_dt_id(pcpu));
         qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
                               env->dcache_line_size);
         qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5b21562..807f488 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -207,19 +207,20 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
 
     CPU_FOREACH(cpu) {
         DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
         uint32_t associativity[] = {cpu_to_be32(0x5),
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(cpu->numa_node),
-                                    cpu_to_be32(cpu->cpu_index)};
+                                    cpu_to_be32(index)};
 
-        if ((cpu->cpu_index % smt) != 0) {
+        if ((index % smt) != 0) {
             continue;
         }
 
         snprintf(cpu_model, 32, "/cpus/%s@%x", dc->fw_name,
-                 cpu->cpu_index);
+                 index);
 
         offset = fdt_path_offset(fdt, cpu_model);
         if (offset < 0) {
@@ -368,7 +369,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
         CPUPPCState *env = &cpu->env;
         DeviceClass *dc = DEVICE_GET_CLASS(cs);
         PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
-        int index = cs->cpu_index;
+        int index = ppc_get_vcpu_dt_id(cpu);
         uint32_t servers_prop[smp_threads];
         uint32_t gservers_prop[smp_threads * 2];
         char *nodename;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f755a53..c55c356 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -464,13 +464,13 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     target_ulong vpa = args[2];
     target_ulong ret = H_PARAMETER;
     CPUPPCState *tenv;
-    CPUState *tcpu;
+    PowerPCCPU *tcpu;
 
-    tcpu = qemu_get_cpu(procno);
+    tcpu = ppc_get_vcpu_by_dt_id(procno);
     if (!tcpu) {
         return H_PARAMETER;
     }
-    tenv = tcpu->env_ptr;
+    tenv = &tcpu->env;
 
     switch (flags) {
     case FLAGS_REGISTER_VPA:
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 1cb276d..73860d0 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -131,7 +131,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
                                          uint32_t nret, target_ulong rets)
 {
     target_ulong id;
-    CPUState *cpu;
+    PowerPCCPU *cpu;
 
     if (nargs != 1 || nret != 2) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -139,9 +139,9 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
     }
 
     id = rtas_ld(args, 0);
-    cpu = qemu_get_cpu(id);
+    cpu = ppc_get_vcpu_by_dt_id(id);
     if (cpu != NULL) {
-        if (cpu->halted) {
+        if (CPU(cpu)->halted) {
             rtas_st(rets, 1, 0);
         } else {
             rtas_st(rets, 1, 2);
@@ -161,7 +161,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
                            uint32_t nret, target_ulong rets)
 {
     target_ulong id, start, r3;
-    CPUState *cs;
+    PowerPCCPU *cpu;
 
     if (nargs != 3 || nret != 1) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -172,9 +172,9 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
     start = rtas_ld(args, 1);
     r3 = rtas_ld(args, 2);
 
-    cs = qemu_get_cpu(id);
-    if (cs != NULL) {
-        PowerPCCPU *cpu = POWERPC_CPU(cs);
+    cpu = ppc_get_vcpu_by_dt_id(id);
+    if (cpu != NULL) {
+        CPUState *cs = CPU(cpu);
         CPUPPCState *env = &cpu->env;
 
         if (!cs->halted) {
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8bcc5fb..c9b7778 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -401,7 +401,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu)
 
 unsigned long kvm_arch_vcpu_id(CPUState *cpu)
 {
-    return cpu->cpu_index;
+    return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
 }
 
 int kvm_arch_init_vcpu(CPUState *cs)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 6de7126..a67e2b4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8429,6 +8429,7 @@ static void ppc_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(env);
+    cpu->cpu_dt_id = cs->cpu_index;
 
     env->msr_mask = pcc->msr_mask;
     env->mmu_model = pcc->mmu_model;
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

On 02/02/2014 01:45 AM, Alexey Kardashevskiy wrote:
> This is some cleanup. Please, comment. Thanks!

Ping?


> 
> Changes:
> v7:
> * cleaned up a bit of QOM use
> 
> v6:
> * removed kvmppc_fixup_cpu()
> 
> v5:
> * cleanup
> * removed cpustate::kvm_cpu_id
> * split into 2 patches - new PPC API and the usage of the API
> 
> 
> Alexey Kardashevskiy (2):
>   target-ppc: add PowerPCCPU::cpu_dt_id
>   target-ppc: spapr: e500: fix to use cpu_dt_id
> 
>  hw/intc/openpic_kvm.c       |  2 +-
>  hw/intc/xics.c              | 15 +++++++++++++--
>  hw/intc/xics_kvm.c          | 10 +++++-----
>  hw/ppc/e500.c               |  7 +++++--
>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>  hw/ppc/spapr.c              |  9 +++++----
>  hw/ppc/spapr_hcall.c        |  6 +++---
>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>  target-ppc/cpu-qom.h        |  2 ++
>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>  target-ppc/kvm.c            | 15 +--------------
>  target-ppc/kvm_ppc.h        |  6 ------
>  target-ppc/translate_init.c | 11 +++++------
>  13 files changed, 87 insertions(+), 50 deletions(-)
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

On 02/11/2014 06:20 PM, Alexey Kardashevskiy wrote:
> On 02/02/2014 01:45 AM, Alexey Kardashevskiy wrote:
>> This is some cleanup. Please, comment. Thanks!
> 
> Ping?

pingping?


> 
> 
>>
>> Changes:
>> v7:
>> * cleaned up a bit of QOM use
>>
>> v6:
>> * removed kvmppc_fixup_cpu()
>>
>> v5:
>> * cleanup
>> * removed cpustate::kvm_cpu_id
>> * split into 2 patches - new PPC API and the usage of the API
>>
>>
>> Alexey Kardashevskiy (2):
>>   target-ppc: add PowerPCCPU::cpu_dt_id
>>   target-ppc: spapr: e500: fix to use cpu_dt_id
>>
>>  hw/intc/openpic_kvm.c       |  2 +-
>>  hw/intc/xics.c              | 15 +++++++++++++--
>>  hw/intc/xics_kvm.c          | 10 +++++-----
>>  hw/ppc/e500.c               |  7 +++++--
>>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>>  hw/ppc/spapr.c              |  9 +++++----
>>  hw/ppc/spapr_hcall.c        |  6 +++---
>>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>>  target-ppc/cpu-qom.h        |  2 ++
>>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>>  target-ppc/kvm.c            | 15 +--------------
>>  target-ppc/kvm_ppc.h        |  6 ------
>>  target-ppc/translate_init.c | 11 +++++------
>>  13 files changed, 87 insertions(+), 50 deletions(-)
>>
> 
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

On 02/22/2014 12:31 AM, Alexey Kardashevskiy wrote:
> On 02/11/2014 06:20 PM, Alexey Kardashevskiy wrote:
>> On 02/02/2014 01:45 AM, Alexey Kardashevskiy wrote:
>>> This is some cleanup. Please, comment. Thanks!
>>
>> Ping?
> 
> pingping?


Sorry for bothering again. This is quite simple patchset, it does not touch
a lot and just a small rework. This is v7, v1 was 31/10/2013 (~4 months), I
sent "ping" to v5 5 times (took 2 months to get any attention), there was
no comment that it breaks anything, few people commented that the patchset
is ok, we use it internally for quite a while but it is still not in any
tree but mine. I have other patchsets like that and I am not posting new
ones because I do not really see the point until I figure out what is wrong
with my old patches or with the way I posting them and trying to get attention.

If anyone feels ok to "ack" these patches, please do so. Thanks and sorry
again.


>>
>>>
>>> Changes:
>>> v7:
>>> * cleaned up a bit of QOM use
>>>
>>> v6:
>>> * removed kvmppc_fixup_cpu()
>>>
>>> v5:
>>> * cleanup
>>> * removed cpustate::kvm_cpu_id
>>> * split into 2 patches - new PPC API and the usage of the API
>>>
>>>
>>> Alexey Kardashevskiy (2):
>>>   target-ppc: add PowerPCCPU::cpu_dt_id
>>>   target-ppc: spapr: e500: fix to use cpu_dt_id
>>>
>>>  hw/intc/openpic_kvm.c       |  2 +-
>>>  hw/intc/xics.c              | 15 +++++++++++++--
>>>  hw/intc/xics_kvm.c          | 10 +++++-----
>>>  hw/ppc/e500.c               |  7 +++++--
>>>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>>>  hw/ppc/spapr.c              |  9 +++++----
>>>  hw/ppc/spapr_hcall.c        |  6 +++---
>>>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>>>  target-ppc/cpu-qom.h        |  2 ++
>>>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>>>  target-ppc/kvm.c            | 15 +--------------
>>>  target-ppc/kvm_ppc.h        |  6 ------
>>>  target-ppc/translate_init.c | 11 +++++------
>>>  13 files changed, 87 insertions(+), 50 deletions(-)
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Benjamin Herrenschmidt]

On Sat, 2014-02-22 at 13:20 +1100, Alexey Kardashevskiy wrote:

> Sorry for bothering again. This is quite simple patchset, it does not touch
> a lot and just a small rework. This is v7, v1 was 31/10/2013 (~4 months), I
> sent "ping" to v5 5 times (took 2 months to get any attention), there was
> no comment that it breaks anything, few people commented that the patchset

"few people", you should name them. And they should ack it if they are
ok with it.

> is ok, we use it internally for quite a while but it is still not in any
> tree but mine. I have other patchsets like that and I am not posting new
> ones because I do not really see the point until I figure out what is wrong
> with my old patches or with the way I posting them and trying to get attention.
> 
> If anyone feels ok to "ack" these patches, please do so. Thanks and sorry
> again.
> 
> 
> >>
> >>>
> >>> Changes:
> >>> v7:
> >>> * cleaned up a bit of QOM use
> >>>
> >>> v6:
> >>> * removed kvmppc_fixup_cpu()
> >>>
> >>> v5:
> >>> * cleanup
> >>> * removed cpustate::kvm_cpu_id
> >>> * split into 2 patches - new PPC API and the usage of the API
> >>>
> >>>
> >>> Alexey Kardashevskiy (2):
> >>>   target-ppc: add PowerPCCPU::cpu_dt_id
> >>>   target-ppc: spapr: e500: fix to use cpu_dt_id
> >>>
> >>>  hw/intc/openpic_kvm.c       |  2 +-
> >>>  hw/intc/xics.c              | 15 +++++++++++++--
> >>>  hw/intc/xics_kvm.c          | 10 +++++-----
> >>>  hw/ppc/e500.c               |  7 +++++--
> >>>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
> >>>  hw/ppc/spapr.c              |  9 +++++----
> >>>  hw/ppc/spapr_hcall.c        |  6 +++---
> >>>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
> >>>  target-ppc/cpu-qom.h        |  2 ++
> >>>  target-ppc/cpu.h            | 18 ++++++++++++++++++
> >>>  target-ppc/kvm.c            | 15 +--------------
> >>>  target-ppc/kvm_ppc.h        |  6 ------
> >>>  target-ppc/translate_init.c | 11 +++++------
> >>>  13 files changed, 87 insertions(+), 50 deletions(-)
> >>>
> >>
> >>
> > 
> > 
> 
> 

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

On 02/22/2014 01:51 PM, Benjamin Herrenschmidt wrote:
> On Sat, 2014-02-22 at 13:20 +1100, Alexey Kardashevskiy wrote:
> 
>> Sorry for bothering again. This is quite simple patchset, it does not touch
>> a lot and just a small rework. This is v7, v1 was 31/10/2013 (~4 months), I
>> sent "ping" to v5 5 times (took 2 months to get any attention), there was
>> no comment that it breaks anything, few people commented that the patchset
> 
> "few people", you should name them. And they should ack it if they are
> ok with it.


Nobody specifically did "acked-by". Mike Day "would ack them if it would
help", Andreas Färber has "No objection from my side, but I'm not really
familiar with the topic", others commented some older versions and did not
comment newer fixed versions. This is my confusion.

Should I personally ask everyone who commented to re-review new version again?

> 
>> is ok, we use it internally for quite a while but it is still not in any
>> tree but mine. I have other patchsets like that and I am not posting new
>> ones because I do not really see the point until I figure out what is wrong
>> with my old patches or with the way I posting them and trying to get attention.
>>
>> If anyone feels ok to "ack" these patches, please do so. Thanks and sorry
>> again.
>>
>>
>>>>
>>>>>
>>>>> Changes:
>>>>> v7:
>>>>> * cleaned up a bit of QOM use
>>>>>
>>>>> v6:
>>>>> * removed kvmppc_fixup_cpu()
>>>>>
>>>>> v5:
>>>>> * cleanup
>>>>> * removed cpustate::kvm_cpu_id
>>>>> * split into 2 patches - new PPC API and the usage of the API
>>>>>
>>>>>
>>>>> Alexey Kardashevskiy (2):
>>>>>   target-ppc: add PowerPCCPU::cpu_dt_id
>>>>>   target-ppc: spapr: e500: fix to use cpu_dt_id
>>>>>
>>>>>  hw/intc/openpic_kvm.c       |  2 +-
>>>>>  hw/intc/xics.c              | 15 +++++++++++++--
>>>>>  hw/intc/xics_kvm.c          | 10 +++++-----
>>>>>  hw/ppc/e500.c               |  7 +++++--
>>>>>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>>>>>  hw/ppc/spapr.c              |  9 +++++----
>>>>>  hw/ppc/spapr_hcall.c        |  6 +++---
>>>>>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>>>>>  target-ppc/cpu-qom.h        |  2 ++
>>>>>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>>>>>  target-ppc/kvm.c            | 15 +--------------
>>>>>  target-ppc/kvm_ppc.h        |  6 ------
>>>>>  target-ppc/translate_init.c | 11 +++++------
>>>>>  13 files changed, 87 insertions(+), 50 deletions(-)
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
> 
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v7 1/2] target-ppc: add PowerPCCPU::cpu_dt_id

[Mike Day]

On Sat, Feb 1, 2014 at 9:45 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Normally CPUState::cpu_index is used to pick the right CPU for various
> operations. However default consecutive numbering does not always work
> for POWERPC.
>
> These indexes are reflected in /proc/device-tree/cpus/PowerPC,POWER7@XX
> and used to call KVM VCPU's ioctls. In order to achieve this,
> kvmppc_fixup_cpu() was introduced. Roughly speaking, it multiplies
> cpu_index by the number of threads per core.
>
> This approach has disadvantages such as:
> 1. NUMA configuration stays broken after the fixup;
> 2. CPU-targeted commands from the QEMU Monitor do not work properly as
> CPU indexes have been fixed and there is no clear way for the user to
> know what the new CPU indexes are.
>
> This introduces a @cpu_dt_id field in the CPUPPCState struct which
> is initialized from @cpu_index by default and can be fixed later
> to meet the device tree requirements.
>
> This adds an API to handle @cpu_dt_id.
>
> This removes kvmppc_fixup_cpu() as it is not more needed, @cpu_dt_id
> is calculated in ppc_cpu_realize().
>
> This will be used later in machine code.
>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Mike Day <ncmike@ncultra.org>

> ---
> Changes:
> v6: inlined kvmppc_fixup_cpu()
> ---
>  hw/ppc/ppc.c                | 22 ++++++++++++++++++++++
>  target-ppc/cpu-qom.h        |  2 ++
>  target-ppc/cpu.h            | 18 ++++++++++++++++++
>  target-ppc/kvm.c            | 13 -------------
>  target-ppc/kvm_ppc.h        |  6 ------
>  target-ppc/translate_init.c | 10 ++++------
>  6 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 114be64..0e82719 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -26,6 +26,7 @@
>  #include "hw/ppc/ppc_e500.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  #include "hw/timer/m48t59.h"
>  #include "qemu/log.h"
>  #include "hw/loader.h"
> @@ -1362,3 +1363,24 @@ int PPC_NVRAM_set_params (nvram_t *nvram, uint16_t NVRAM_size,
>
>      return 0;
>  }
> +
> +/* CPU device-tree ID helpers */
> +int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
> +{
> +    return cpu->cpu_dt_id;
> +}
> +
> +PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
> +{
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +        if (cpu->cpu_dt_id == cpu_dt_id) {
> +            return cpu;
> +        }
> +    }
> +
> +    return NULL;
> +}
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 72b2232..b17c024 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -79,6 +79,7 @@ typedef struct PowerPCCPUClass {
>  /**
>   * PowerPCCPU:
>   * @env: #CPUPPCState
> + * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
>   *
>   * A PowerPC CPU.
>   */
> @@ -88,6 +89,7 @@ typedef struct PowerPCCPU {
>      /*< public >*/
>
>      CPUPPCState env;
> +    int cpu_dt_id;
>  } PowerPCCPU;
>
>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 51bcd4a..d8577ae 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -2154,4 +2154,22 @@ static inline bool cpu_has_work(CPUState *cpu)
>
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
>
> +/**
> + * ppc_get_vcpu_dt_id:
> + * @cs: a PowerPCCPU struct.
> + *
> + * Returns a device-tree ID for a CPU.
> + */
> +int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
> +
> +/**
> + * ppc_get_vcpu_by_dt_id:
> + * @cpu_dt_id: a device tree id
> + *
> + * Searches for a CPU by @cpu_dt_id.
> + *
> + * Returns: a PowerPCCPU struct
> + */
> +PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
> +
>  #endif /* !defined (__CPU_PPC_H__) */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 781b72f..8bcc5fb 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1766,19 +1766,6 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
>      }
>  }
>
> -int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> -{
> -    CPUState *cs = CPU(cpu);
> -    int smt;
> -
> -    /* Adjust cpu index for SMT */
> -    smt = kvmppc_smt_threads();
> -    cs->cpu_index = (cs->cpu_index / smp_threads) * smt
> -        + (cs->cpu_index % smp_threads);
> -
> -    return 0;
> -}
> -
>  bool kvmppc_has_cap_epr(void)
>  {
>      return cap_epr;
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 5f78e4b..f3afcdb 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -36,7 +36,6 @@ int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
>  int kvmppc_reset_htab(int shift_hint);
>  uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
>  #endif /* !CONFIG_USER_ONLY */
> -int kvmppc_fixup_cpu(PowerPCCPU *cpu);
>  bool kvmppc_has_cap_epr(void);
>  int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);
>  int kvmppc_get_htab_fd(bool write);
> @@ -155,11 +154,6 @@ static inline int kvmppc_update_sdr1(CPUPPCState *env)
>
>  #endif /* !CONFIG_USER_ONLY */
>
> -static inline int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> -{
> -    return -1;
> -}
> -
>  static inline bool kvmppc_has_cap_epr(void)
>  {
>      return false;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 35470d4..6de7126 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7808,14 +7808,12 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>                     max_smt, kvm_enabled() ? "KVM" : "TCG");
>          return;
>      }
> +
> +    cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
> +        + (cs->cpu_index % smp_threads);
>  #endif
>
> -    if (kvm_enabled()) {
> -        if (kvmppc_fixup_cpu(cpu) != 0) {
> -            error_setg(errp, "Unable to virtualize selected CPU with KVM");
> -            return;
> -        }
> -    } else if (tcg_enabled()) {
> +    if (tcg_enabled()) {
>          if (ppc_fixup_cpu(cpu) != 0) {
>              error_setg(errp, "Unable to emulate selected CPU with TCG");
>              return;
> --
> 1.8.4.rc4
>
>

Re: [Qemu-devel] [PATCH v7 2/2] target-ppc: spapr: e500: fix to use cpu_dt_id

[Mike Day]

On Sat, Feb 1, 2014 at 9:45 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> This makes use of @cpu_dt_id and related API in:
> 1. emulated XICS hypercall handlers as they receive fixed CPU indexes;
> 2. XICS-KVM to enable in-kernel XICS on right CPU;
> 3. device-tree renderer.
>
> This removes @cpu_index fixup as @cpu_dt_id is used instead so QEMU monitor
> can accept command-line CPU indexes again.
>
> This changes kvm_arch_vcpu_id() to use ppc_get_vcpu_dt_id() as at the moment
> KVM CPU id and device tree ID are calculated using the same algorithm.
>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Mike Day <ncmike@ncultra.org>

> ---
> Changes:
> v7:
> * replaced referencing to PowerPCCPU::parent_obj with the CPU macro
> ---
>  hw/intc/openpic_kvm.c       |  2 +-
>  hw/intc/xics.c              | 15 +++++++++++++--
>  hw/intc/xics_kvm.c          | 10 +++++-----
>  hw/ppc/e500.c               |  7 +++++--
>  hw/ppc/spapr.c              |  9 +++++----
>  hw/ppc/spapr_hcall.c        |  6 +++---
>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>  target-ppc/kvm.c            |  2 +-
>  target-ppc/translate_init.c |  1 +
>  9 files changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
> index c7f7b84..87fdb12 100644
> --- a/hw/intc/openpic_kvm.c
> +++ b/hw/intc/openpic_kvm.c
> @@ -228,7 +228,7 @@ int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs)
>
>      encap.cap = KVM_CAP_IRQ_MPIC;
>      encap.args[0] = opp->fd;
> -    encap.args[1] = cs->cpu_index;
> +    encap.args[1] = kvm_arch_vcpu_id(cs);
>
>      return kvm_vcpu_ioctl(cs, KVM_ENABLE_CAP, &encap);
>  }
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index b437563..64aabe7 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -33,6 +33,17 @@
>  #include "qemu/error-report.h"
>  #include "qapi/visitor.h"
>
> +static int get_cpu_index_by_dt_id(int cpu_dt_id)
> +{
> +    PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> +
> +    if (cpu) {
> +        return cpu->parent_obj.cpu_index;
> +    }
> +
> +    return -1;
> +}
> +
>  void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>  {
>      CPUState *cs = CPU(cpu);
> @@ -659,7 +670,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>  static target_ulong h_ipi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                            target_ulong opcode, target_ulong *args)
>  {
> -    target_ulong server = args[0];
> +    target_ulong server = get_cpu_index_by_dt_id(args[0]);
>      target_ulong mfrr = args[1];
>
>      if (server >= spapr->icp->nr_servers) {
> @@ -728,7 +739,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>      }
>
>      nr = rtas_ld(args, 0);
> -    server = rtas_ld(args, 1);
> +    server = get_cpu_index_by_dt_id(rtas_ld(args, 1));
>      priority = rtas_ld(args, 2);
>
>      if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index c203646..a5bbc24 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -65,7 +65,7 @@ static void icp_get_kvm_state(ICPState *ss)
>      ret = kvm_vcpu_ioctl(ss->cs, KVM_GET_ONE_REG, &reg);
>      if (ret != 0) {
>          error_report("Unable to retrieve KVM interrupt controller state"
> -                " for CPU %d: %s", ss->cs->cpu_index, strerror(errno));
> +                " for CPU %ld: %s", kvm_arch_vcpu_id(ss->cs), strerror(errno));
>          exit(1);
>      }
>
> @@ -97,7 +97,7 @@ static int icp_set_kvm_state(ICPState *ss, int version_id)
>      ret = kvm_vcpu_ioctl(ss->cs, KVM_SET_ONE_REG, &reg);
>      if (ret != 0) {
>          error_report("Unable to restore KVM interrupt controller state (0x%"
> -                PRIx64 ") for CPU %d: %s", state, ss->cs->cpu_index,
> +                PRIx64 ") for CPU %ld: %s", state, kvm_arch_vcpu_id(ss->cs),
>                  strerror(errno));
>          return ret;
>      }
> @@ -325,15 +325,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>          struct kvm_enable_cap xics_enable_cap = {
>              .cap = KVM_CAP_IRQ_XICS,
>              .flags = 0,
> -            .args = {icpkvm->kernel_xics_fd, cs->cpu_index, 0, 0},
> +            .args = {icpkvm->kernel_xics_fd, kvm_arch_vcpu_id(cs), 0, 0},
>          };
>
>          ss->cs = cs;
>
>          ret = kvm_vcpu_ioctl(ss->cs, KVM_ENABLE_CAP, &xics_enable_cap);
>          if (ret < 0) {
> -            error_report("Unable to connect CPU%d to kernel XICS: %s",
> -                    cs->cpu_index, strerror(errno));
> +            error_report("Unable to connect CPU%ld to kernel XICS: %s",
> +                    kvm_arch_vcpu_id(cs), strerror(errno));
>              exit(1);
>          }
>      }
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index b37ce9d..8a08752 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -238,6 +238,7 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
>         the first node as boot node and be happy */
>      for (i = smp_cpus - 1; i >= 0; i--) {
>          CPUState *cpu;
> +        PowerPCCPU *pcpu;
>          char cpu_name[128];
>          uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
>
> @@ -246,14 +247,16 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
>              continue;
>          }
>          env = cpu->env_ptr;
> +        pcpu = POWERPC_CPU(cpu);
>
>          snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
> -                 cpu->cpu_index);
> +                 ppc_get_vcpu_dt_id(pcpu));
>          qemu_fdt_add_subnode(fdt, cpu_name);
>          qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
>          qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
>          qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
> -        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
> +        qemu_fdt_setprop_cell(fdt, cpu_name, "reg",
> +                              ppc_get_vcpu_dt_id(pcpu));
>          qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
>                                env->dcache_line_size);
>          qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5b21562..807f488 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -207,19 +207,20 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
>
>      CPU_FOREACH(cpu) {
>          DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>          uint32_t associativity[] = {cpu_to_be32(0x5),
>                                      cpu_to_be32(0x0),
>                                      cpu_to_be32(0x0),
>                                      cpu_to_be32(0x0),
>                                      cpu_to_be32(cpu->numa_node),
> -                                    cpu_to_be32(cpu->cpu_index)};
> +                                    cpu_to_be32(index)};
>
> -        if ((cpu->cpu_index % smt) != 0) {
> +        if ((index % smt) != 0) {
>              continue;
>          }
>
>          snprintf(cpu_model, 32, "/cpus/%s@%x", dc->fw_name,
> -                 cpu->cpu_index);
> +                 index);
>
>          offset = fdt_path_offset(fdt, cpu_model);
>          if (offset < 0) {
> @@ -368,7 +369,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>          CPUPPCState *env = &cpu->env;
>          DeviceClass *dc = DEVICE_GET_CLASS(cs);
>          PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> -        int index = cs->cpu_index;
> +        int index = ppc_get_vcpu_dt_id(cpu);
>          uint32_t servers_prop[smp_threads];
>          uint32_t gservers_prop[smp_threads * 2];
>          char *nodename;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index f755a53..c55c356 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -464,13 +464,13 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>      target_ulong vpa = args[2];
>      target_ulong ret = H_PARAMETER;
>      CPUPPCState *tenv;
> -    CPUState *tcpu;
> +    PowerPCCPU *tcpu;
>
> -    tcpu = qemu_get_cpu(procno);
> +    tcpu = ppc_get_vcpu_by_dt_id(procno);
>      if (!tcpu) {
>          return H_PARAMETER;
>      }
> -    tenv = tcpu->env_ptr;
> +    tenv = &tcpu->env;
>
>      switch (flags) {
>      case FLAGS_REGISTER_VPA:
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 1cb276d..73860d0 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -131,7 +131,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
>                                           uint32_t nret, target_ulong rets)
>  {
>      target_ulong id;
> -    CPUState *cpu;
> +    PowerPCCPU *cpu;
>
>      if (nargs != 1 || nret != 2) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -139,9 +139,9 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
>      }
>
>      id = rtas_ld(args, 0);
> -    cpu = qemu_get_cpu(id);
> +    cpu = ppc_get_vcpu_by_dt_id(id);
>      if (cpu != NULL) {
> -        if (cpu->halted) {
> +        if (CPU(cpu)->halted) {
>              rtas_st(rets, 1, 0);
>          } else {
>              rtas_st(rets, 1, 2);
> @@ -161,7 +161,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
>                             uint32_t nret, target_ulong rets)
>  {
>      target_ulong id, start, r3;
> -    CPUState *cs;
> +    PowerPCCPU *cpu;
>
>      if (nargs != 3 || nret != 1) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -172,9 +172,9 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
>      start = rtas_ld(args, 1);
>      r3 = rtas_ld(args, 2);
>
> -    cs = qemu_get_cpu(id);
> -    if (cs != NULL) {
> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    cpu = ppc_get_vcpu_by_dt_id(id);
> +    if (cpu != NULL) {
> +        CPUState *cs = CPU(cpu);
>          CPUPPCState *env = &cpu->env;
>
>          if (!cs->halted) {
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 8bcc5fb..c9b7778 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -401,7 +401,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>
>  unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>  {
> -    return cpu->cpu_index;
> +    return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>  }
>
>  int kvm_arch_init_vcpu(CPUState *cs)
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 6de7126..a67e2b4 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8429,6 +8429,7 @@ static void ppc_cpu_initfn(Object *obj)
>
>      cs->env_ptr = env;
>      cpu_exec_init(env);
> +    cpu->cpu_dt_id = cs->cpu_index;
>
>      env->msr_mask = pcc->msr_mask;
>      env->mmu_model = pcc->mmu_model;
> --
> 1.8.4.rc4
>
>

Re: [Qemu-devel] [PATCH v7 2/2] target-ppc: spapr: e500: fix to use cpu_dt_id

[Alexey Kardashevskiy]

On 03/04/2014 06:57 AM, Mike Day wrote:
> On Sat, Feb 1, 2014 at 9:45 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> This makes use of @cpu_dt_id and related API in:
>> 1. emulated XICS hypercall handlers as they receive fixed CPU indexes;
>> 2. XICS-KVM to enable in-kernel XICS on right CPU;
>> 3. device-tree renderer.
>>
>> This removes @cpu_index fixup as @cpu_dt_id is used instead so QEMU monitor
>> can accept command-line CPU indexes again.
>>
>> This changes kvm_arch_vcpu_id() to use ppc_get_vcpu_dt_id() as at the moment
>> KVM CPU id and device tree ID are calculated using the same algorithm.
>>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Acked-by: Mike Day <ncmike@ncultra.org>


Thanks, Mike!


>> ---
>> Changes:
>> v7:
>> * replaced referencing to PowerPCCPU::parent_obj with the CPU macro
>> ---
>>  hw/intc/openpic_kvm.c       |  2 +-
>>  hw/intc/xics.c              | 15 +++++++++++++--
>>  hw/intc/xics_kvm.c          | 10 +++++-----
>>  hw/ppc/e500.c               |  7 +++++--
>>  hw/ppc/spapr.c              |  9 +++++----
>>  hw/ppc/spapr_hcall.c        |  6 +++---
>>  hw/ppc/spapr_rtas.c         | 14 +++++++-------
>>  target-ppc/kvm.c            |  2 +-
>>  target-ppc/translate_init.c |  1 +
>>  9 files changed, 41 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
>> index c7f7b84..87fdb12 100644
>> --- a/hw/intc/openpic_kvm.c
>> +++ b/hw/intc/openpic_kvm.c
>> @@ -228,7 +228,7 @@ int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs)
>>
>>      encap.cap = KVM_CAP_IRQ_MPIC;
>>      encap.args[0] = opp->fd;
>> -    encap.args[1] = cs->cpu_index;
>> +    encap.args[1] = kvm_arch_vcpu_id(cs);
>>
>>      return kvm_vcpu_ioctl(cs, KVM_ENABLE_CAP, &encap);
>>  }
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index b437563..64aabe7 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -33,6 +33,17 @@
>>  #include "qemu/error-report.h"
>>  #include "qapi/visitor.h"
>>
>> +static int get_cpu_index_by_dt_id(int cpu_dt_id)
>> +{
>> +    PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>> +
>> +    if (cpu) {
>> +        return cpu->parent_obj.cpu_index;
>> +    }
>> +
>> +    return -1;
>> +}
>> +
>>  void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>>  {
>>      CPUState *cs = CPU(cpu);
>> @@ -659,7 +670,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>  static target_ulong h_ipi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>                            target_ulong opcode, target_ulong *args)
>>  {
>> -    target_ulong server = args[0];
>> +    target_ulong server = get_cpu_index_by_dt_id(args[0]);
>>      target_ulong mfrr = args[1];
>>
>>      if (server >= spapr->icp->nr_servers) {
>> @@ -728,7 +739,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>      }
>>
>>      nr = rtas_ld(args, 0);
>> -    server = rtas_ld(args, 1);
>> +    server = get_cpu_index_by_dt_id(rtas_ld(args, 1));
>>      priority = rtas_ld(args, 2);
>>
>>      if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
>> index c203646..a5bbc24 100644
>> --- a/hw/intc/xics_kvm.c
>> +++ b/hw/intc/xics_kvm.c
>> @@ -65,7 +65,7 @@ static void icp_get_kvm_state(ICPState *ss)
>>      ret = kvm_vcpu_ioctl(ss->cs, KVM_GET_ONE_REG, &reg);
>>      if (ret != 0) {
>>          error_report("Unable to retrieve KVM interrupt controller state"
>> -                " for CPU %d: %s", ss->cs->cpu_index, strerror(errno));
>> +                " for CPU %ld: %s", kvm_arch_vcpu_id(ss->cs), strerror(errno));
>>          exit(1);
>>      }
>>
>> @@ -97,7 +97,7 @@ static int icp_set_kvm_state(ICPState *ss, int version_id)
>>      ret = kvm_vcpu_ioctl(ss->cs, KVM_SET_ONE_REG, &reg);
>>      if (ret != 0) {
>>          error_report("Unable to restore KVM interrupt controller state (0x%"
>> -                PRIx64 ") for CPU %d: %s", state, ss->cs->cpu_index,
>> +                PRIx64 ") for CPU %ld: %s", state, kvm_arch_vcpu_id(ss->cs),
>>                  strerror(errno));
>>          return ret;
>>      }
>> @@ -325,15 +325,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>>          struct kvm_enable_cap xics_enable_cap = {
>>              .cap = KVM_CAP_IRQ_XICS,
>>              .flags = 0,
>> -            .args = {icpkvm->kernel_xics_fd, cs->cpu_index, 0, 0},
>> +            .args = {icpkvm->kernel_xics_fd, kvm_arch_vcpu_id(cs), 0, 0},
>>          };
>>
>>          ss->cs = cs;
>>
>>          ret = kvm_vcpu_ioctl(ss->cs, KVM_ENABLE_CAP, &xics_enable_cap);
>>          if (ret < 0) {
>> -            error_report("Unable to connect CPU%d to kernel XICS: %s",
>> -                    cs->cpu_index, strerror(errno));
>> +            error_report("Unable to connect CPU%ld to kernel XICS: %s",
>> +                    kvm_arch_vcpu_id(cs), strerror(errno));
>>              exit(1);
>>          }
>>      }
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index b37ce9d..8a08752 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -238,6 +238,7 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
>>         the first node as boot node and be happy */
>>      for (i = smp_cpus - 1; i >= 0; i--) {
>>          CPUState *cpu;
>> +        PowerPCCPU *pcpu;
>>          char cpu_name[128];
>>          uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
>>
>> @@ -246,14 +247,16 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
>>              continue;
>>          }
>>          env = cpu->env_ptr;
>> +        pcpu = POWERPC_CPU(cpu);
>>
>>          snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
>> -                 cpu->cpu_index);
>> +                 ppc_get_vcpu_dt_id(pcpu));
>>          qemu_fdt_add_subnode(fdt, cpu_name);
>>          qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
>>          qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
>>          qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
>> -        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
>> +        qemu_fdt_setprop_cell(fdt, cpu_name, "reg",
>> +                              ppc_get_vcpu_dt_id(pcpu));
>>          qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
>>                                env->dcache_line_size);
>>          qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 5b21562..807f488 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -207,19 +207,20 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
>>
>>      CPU_FOREACH(cpu) {
>>          DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>> +        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>          uint32_t associativity[] = {cpu_to_be32(0x5),
>>                                      cpu_to_be32(0x0),
>>                                      cpu_to_be32(0x0),
>>                                      cpu_to_be32(0x0),
>>                                      cpu_to_be32(cpu->numa_node),
>> -                                    cpu_to_be32(cpu->cpu_index)};
>> +                                    cpu_to_be32(index)};
>>
>> -        if ((cpu->cpu_index % smt) != 0) {
>> +        if ((index % smt) != 0) {
>>              continue;
>>          }
>>
>>          snprintf(cpu_model, 32, "/cpus/%s@%x", dc->fw_name,
>> -                 cpu->cpu_index);
>> +                 index);
>>
>>          offset = fdt_path_offset(fdt, cpu_model);
>>          if (offset < 0) {
>> @@ -368,7 +369,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>          CPUPPCState *env = &cpu->env;
>>          DeviceClass *dc = DEVICE_GET_CLASS(cs);
>>          PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
>> -        int index = cs->cpu_index;
>> +        int index = ppc_get_vcpu_dt_id(cpu);
>>          uint32_t servers_prop[smp_threads];
>>          uint32_t gservers_prop[smp_threads * 2];
>>          char *nodename;
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index f755a53..c55c356 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -464,13 +464,13 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>      target_ulong vpa = args[2];
>>      target_ulong ret = H_PARAMETER;
>>      CPUPPCState *tenv;
>> -    CPUState *tcpu;
>> +    PowerPCCPU *tcpu;
>>
>> -    tcpu = qemu_get_cpu(procno);
>> +    tcpu = ppc_get_vcpu_by_dt_id(procno);
>>      if (!tcpu) {
>>          return H_PARAMETER;
>>      }
>> -    tenv = tcpu->env_ptr;
>> +    tenv = &tcpu->env;
>>
>>      switch (flags) {
>>      case FLAGS_REGISTER_VPA:
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 1cb276d..73860d0 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -131,7 +131,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
>>                                           uint32_t nret, target_ulong rets)
>>  {
>>      target_ulong id;
>> -    CPUState *cpu;
>> +    PowerPCCPU *cpu;
>>
>>      if (nargs != 1 || nret != 2) {
>>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> @@ -139,9 +139,9 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
>>      }
>>
>>      id = rtas_ld(args, 0);
>> -    cpu = qemu_get_cpu(id);
>> +    cpu = ppc_get_vcpu_by_dt_id(id);
>>      if (cpu != NULL) {
>> -        if (cpu->halted) {
>> +        if (CPU(cpu)->halted) {
>>              rtas_st(rets, 1, 0);
>>          } else {
>>              rtas_st(rets, 1, 2);
>> @@ -161,7 +161,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
>>                             uint32_t nret, target_ulong rets)
>>  {
>>      target_ulong id, start, r3;
>> -    CPUState *cs;
>> +    PowerPCCPU *cpu;
>>
>>      if (nargs != 3 || nret != 1) {
>>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> @@ -172,9 +172,9 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPREnvironment *spapr,
>>      start = rtas_ld(args, 1);
>>      r3 = rtas_ld(args, 2);
>>
>> -    cs = qemu_get_cpu(id);
>> -    if (cs != NULL) {
>> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +    cpu = ppc_get_vcpu_by_dt_id(id);
>> +    if (cpu != NULL) {
>> +        CPUState *cs = CPU(cpu);
>>          CPUPPCState *env = &cpu->env;
>>
>>          if (!cs->halted) {
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 8bcc5fb..c9b7778 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -401,7 +401,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>>
>>  unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>>  {
>> -    return cpu->cpu_index;
>> +    return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>  }
>>
>>  int kvm_arch_init_vcpu(CPUState *cs)
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 6de7126..a67e2b4 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -8429,6 +8429,7 @@ static void ppc_cpu_initfn(Object *obj)
>>
>>      cs->env_ptr = env;
>>      cpu_exec_init(env);
>> +    cpu->cpu_dt_id = cs->cpu_index;
>>
>>      env->msr_mask = pcc->msr_mask;
>>      env->mmu_model = pcc->mmu_model;
>> --
>> 1.8.4.rc4
>>
>>


-- 
Alexey

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexander Graf]

Alexey Kardashevskiy wrote:
> This is some cleanup. Please, comment. Thanks!
>   

Thanks, applied to ppc-next. Please bear in mind that your target CPU
finding loop doesn't scale. So make sure you don't call your new dt
index helper in any code path where you care about performance. Or when
you do make sure to introduce a new table for easier lookup.


Alex

Re: [Qemu-devel] [PATCH v7 0/2] target-ppc: CPU device tree id

[Alexey Kardashevskiy]

On 03/04/2014 11:29 AM, Alexander Graf wrote:
> Alexey Kardashevskiy wrote:
>> This is some cleanup. Please, comment. Thanks!
>>   
> 
> Thanks, applied to ppc-next. Please bear in mind that your target CPU
> finding loop doesn't scale. So make sure you don't call your new dt
> index helper in any code path where you care about performance. Or when
> you do make sure to introduce a new table for easier lookup.

Sure, thanks!


-- 
ps. yay! :)
Alexey