* [Qemu-devel] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset()
2017-08-03 6:28 [Qemu-devel] [PATCH 0/4] Cleanup cpu_dt_id Sam Bobroff
@ 2017-08-03 6:28 ` Sam Bobroff
2017-08-03 12:37 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-08-03 6:28 ` [Qemu-devel] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id Sam Bobroff
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2017-08-03 6:28 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: david, agraf
The unicast case in h_signal_sys_reset() seems to be broken:
rather than selecting the target CPU, it looks like it will pick
either the first CPU or fail to find one at all.
Fix it by using the search function rather than open coding the
search.
This was found by inspection; the code appears to be unused because
the Linux kernel only uses the broadcast target.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
hw/ppc/spapr_hcall.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 72ea5a8247..07b3da8dc4 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
} else {
/* Unicast */
- CPU_FOREACH(cs) {
- if (cpu->cpu_dt_id == target) {
- run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
- return H_SUCCESS;
- }
+ cs = CPU(ppc_get_vcpu_by_dt_id(target));
+ if (cs) {
+ run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
+ return H_SUCCESS;
}
return H_PARAMETER;
}
--
2.12.1.382.gc0f9c7058
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset()
2017-08-03 6:28 ` [Qemu-devel] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset() Sam Bobroff
@ 2017-08-03 12:37 ` Greg Kurz
2017-08-04 2:18 ` David Gibson
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2017-08-03 12:37 UTC (permalink / raw)
To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david
[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]
On Thu, 3 Aug 2017 16:28:27 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
> The unicast case in h_signal_sys_reset() seems to be broken:
> rather than selecting the target CPU, it looks like it will pick
> either the first CPU or fail to find one at all.
>
> Fix it by using the search function rather than open coding the
> search.
>
Heh the open coded search is using cpu where it should have been using
POWERPC_CPU(cs) => it can only succeed if a the vCPU is signalling itself.
> This was found by inspection; the code appears to be unused because
> the Linux kernel only uses the broadcast target.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
> hw/ppc/spapr_hcall.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 72ea5a8247..07b3da8dc4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>
> } else {
> /* Unicast */
> - CPU_FOREACH(cs) {
> - if (cpu->cpu_dt_id == target) {
> - run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> - return H_SUCCESS;
> - }
> + cs = CPU(ppc_get_vcpu_by_dt_id(target));
> + if (cs) {
> + run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> + return H_SUCCESS;
> }
> return H_PARAMETER;
> }
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset()
2017-08-03 12:37 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2017-08-04 2:18 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2017-08-04 2:18 UTC (permalink / raw)
To: Greg Kurz; +Cc: Sam Bobroff, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]
On Thu, Aug 03, 2017 at 02:37:15PM +0200, Greg Kurz wrote:
> On Thu, 3 Aug 2017 16:28:27 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
>
> > The unicast case in h_signal_sys_reset() seems to be broken:
> > rather than selecting the target CPU, it looks like it will pick
> > either the first CPU or fail to find one at all.
> >
> > Fix it by using the search function rather than open coding the
> > search.
> >
>
> Heh the open coded search is using cpu where it should have been using
> POWERPC_CPU(cs) => it can only succeed if a the vCPU is signalling itself.
>
> > This was found by inspection; the code appears to be unused because
> > the Linux kernel only uses the broadcast target.
> >
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
Applied to ppc-for-2.11.
>
> > ---
> > hw/ppc/spapr_hcall.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 72ea5a8247..07b3da8dc4 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> >
> > } else {
> > /* Unicast */
> > - CPU_FOREACH(cs) {
> > - if (cpu->cpu_dt_id == target) {
> > - run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > - return H_SUCCESS;
> > - }
> > + cs = CPU(ppc_get_vcpu_by_dt_id(target));
> > + if (cs) {
> > + run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > + return H_SUCCESS;
> > }
> > return H_PARAMETER;
> > }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id
2017-08-03 6:28 [Qemu-devel] [PATCH 0/4] Cleanup cpu_dt_id Sam Bobroff
2017-08-03 6:28 ` [Qemu-devel] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset() Sam Bobroff
@ 2017-08-03 6:28 ` Sam Bobroff
2017-08-03 13:15 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-08-03 6:28 ` [Qemu-devel] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id Sam Bobroff
2017-08-03 6:28 ` [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR Sam Bobroff
3 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2017-08-03 6:28 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: david, agraf
The e500 platform code uses the function ppc_get_vcpu_dt_id() but that
function is actually specific to SPAPR machines, not PPC CPUs, and
will always return the cpu_index in this context.
Simply use the cpu_index instead (which is 'i' in this context
because qemu_get_cpu(i) returns the cpu with cpu_index == i).
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
hw/ppc/e500.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 62f1857206..2a6d34ceb1 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -382,7 +382,6 @@ static int ppce500_load_device_tree(MachineState *machine,
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 = params->spin_base + (i * 0x20);
@@ -391,16 +390,13 @@ static int ppce500_load_device_tree(MachineState *machine,
continue;
}
env = cpu->env_ptr;
- pcpu = POWERPC_CPU(cpu);
- snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
- ppc_get_vcpu_dt_id(pcpu));
+ snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x", i);
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",
- ppc_get_vcpu_dt_id(pcpu));
+ qemu_fdt_setprop_cell(fdt, cpu_name, "reg", i);
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",
--
2.12.1.382.gc0f9c7058
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id
2017-08-03 6:28 ` [Qemu-devel] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id Sam Bobroff
@ 2017-08-03 13:15 ` Greg Kurz
2017-08-04 2:21 ` David Gibson
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2017-08-03 13:15 UTC (permalink / raw)
To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david
[-- Attachment #1: Type: text/plain, Size: 2304 bytes --]
On Thu, 3 Aug 2017 16:28:36 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
> The e500 platform code uses the function ppc_get_vcpu_dt_id() but that
> function is actually specific to SPAPR machines, not PPC CPUs, and
> will always return the cpu_index in this context.
>
ie, e500 compatible CPUs don't support SMT ? Then maybe the e500 machine
should ensure smp_threads == 1, but this can done in a followup patch.
> Simply use the cpu_index instead (which is 'i' in this context
> because qemu_get_cpu(i) returns the cpu with cpu_index == i).
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
> hw/ppc/e500.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 62f1857206..2a6d34ceb1 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -382,7 +382,6 @@ static int ppce500_load_device_tree(MachineState *machine,
> 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 = params->spin_base + (i * 0x20);
>
> @@ -391,16 +390,13 @@ static int ppce500_load_device_tree(MachineState *machine,
> continue;
> }
> env = cpu->env_ptr;
> - pcpu = POWERPC_CPU(cpu);
>
> - snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
> - ppc_get_vcpu_dt_id(pcpu));
> + snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x", i);
> 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",
> - ppc_get_vcpu_dt_id(pcpu));
> + qemu_fdt_setprop_cell(fdt, cpu_name, "reg", i);
> 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",
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id
2017-08-03 13:15 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2017-08-04 2:21 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2017-08-04 2:21 UTC (permalink / raw)
To: Greg Kurz; +Cc: Sam Bobroff, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2838 bytes --]
On Thu, Aug 03, 2017 at 03:15:04PM +0200, Greg Kurz wrote:
> On Thu, 3 Aug 2017 16:28:36 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
>
> > The e500 platform code uses the function ppc_get_vcpu_dt_id() but that
> > function is actually specific to SPAPR machines, not PPC CPUs, and
> > will always return the cpu_index in this context.
> >
>
> ie, e500 compatible CPUs don't support SMT ? Then maybe the e500 machine
> should ensure smp_threads == 1, but this can done in a followup patch.
Uh.. sort of, it's really more about the host than guest restrictions.
I've rewritten the commit message to give a bit more context.
> > Simply use the cpu_index instead (which is 'i' in this context
> > because qemu_get_cpu(i) returns the cpu with cpu_index == i).
> >
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
Applied to ppc-for-2.11
>
> > ---
> > hw/ppc/e500.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > index 62f1857206..2a6d34ceb1 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -382,7 +382,6 @@ static int ppce500_load_device_tree(MachineState *machine,
> > 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 = params->spin_base + (i * 0x20);
> >
> > @@ -391,16 +390,13 @@ static int ppce500_load_device_tree(MachineState *machine,
> > continue;
> > }
> > env = cpu->env_ptr;
> > - pcpu = POWERPC_CPU(cpu);
> >
> > - snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
> > - ppc_get_vcpu_dt_id(pcpu));
> > + snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x", i);
> > 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",
> > - ppc_get_vcpu_dt_id(pcpu));
> > + qemu_fdt_setprop_cell(fdt, cpu_name, "reg", i);
> > 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",
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id
2017-08-03 6:28 [Qemu-devel] [PATCH 0/4] Cleanup cpu_dt_id Sam Bobroff
2017-08-03 6:28 ` [Qemu-devel] [PATCH 1/4] spapr: Fix bug in h_signal_sys_reset() Sam Bobroff
2017-08-03 6:28 ` [Qemu-devel] [PATCH 2/4] e500: Use cpu_index instead of vcpu_dt_id Sam Bobroff
@ 2017-08-03 6:28 ` Sam Bobroff
2017-08-03 13:19 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-08-03 6:28 ` [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR Sam Bobroff
3 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2017-08-03 6:28 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: david, agraf
This field actually records the VCPU ID used by KVM and, although the
value is also used in the device tree it is primarily the VCPU ID so
rename it as such.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
hw/ppc/ppc.c | 8 ++++----
hw/ppc/spapr.c | 16 ++++++++--------
hw/ppc/spapr_hcall.c | 4 ++--
hw/ppc/spapr_rtas.c | 4 ++--
target/ppc/cpu.h | 14 +++++++-------
target/ppc/kvm.c | 2 +-
target/ppc/translate_init.c | 8 ++++----
7 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 224184d66d..4477d4ad89 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1359,19 +1359,19 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
}
/* CPU device-tree ID helpers */
-int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
+int ppc_get_vcpu_id(PowerPCCPU *cpu)
{
- return cpu->cpu_dt_id;
+ return cpu->vcpu_id;
}
-PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
+PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id)
{
CPUState *cs;
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- if (cpu->cpu_dt_id == cpu_dt_id) {
+ if (cpu->vcpu_id == vcpu_id) {
return cpu;
}
}
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f7a19720dc..83c77096a6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -208,7 +208,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
int i, ret = 0;
uint32_t servers_prop[smt_threads];
uint32_t gservers_prop[smt_threads * 2];
- int index = ppc_get_vcpu_dt_id(cpu);
+ int index = ppc_get_vcpu_id(cpu);
if (cpu->compat_pvr) {
ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
@@ -237,7 +237,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
{
- int index = ppc_get_vcpu_dt_id(cpu);
+ int index = ppc_get_vcpu_id(cpu);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -341,7 +341,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
DeviceClass *dc = DEVICE_GET_CLASS(cs);
- int index = ppc_get_vcpu_dt_id(cpu);
+ int index = ppc_get_vcpu_id(cpu);
int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
if ((index % smt) != 0) {
@@ -493,7 +493,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
- int index = ppc_get_vcpu_dt_id(cpu);
+ int index = ppc_get_vcpu_id(cpu);
uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
0xffffffff, 0xffffffff};
uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
@@ -626,7 +626,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
*/
CPU_FOREACH_REVERSE(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- int index = ppc_get_vcpu_dt_id(cpu);
+ int index = ppc_get_vcpu_id(cpu);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
int offset;
@@ -2982,7 +2982,7 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
- int id = ppc_get_vcpu_dt_id(cpu);
+ int id = ppc_get_vcpu_id(cpu);
void *fdt;
int offset, fdt_size;
char *nodename;
@@ -3390,9 +3390,9 @@ static void spapr_ics_resend(XICSFabric *dev)
ics_resend(spapr->ics);
}
-static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
+static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
{
- PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
+ PowerPCCPU *cpu = ppc_get_cpu_by_vcpu_id(vcpu_id);
return cpu ? ICP(cpu->intc) : NULL;
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 07b3da8dc4..4ca233854a 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -999,7 +999,7 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPRMachineState *spapr,
CPUPPCState *tenv;
PowerPCCPU *tcpu;
- tcpu = ppc_get_vcpu_by_dt_id(procno);
+ tcpu = ppc_get_cpu_by_vcpu_id(procno);
if (!tcpu) {
return H_PARAMETER;
}
@@ -1431,7 +1431,7 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
} else {
/* Unicast */
- cs = CPU(ppc_get_vcpu_by_dt_id(target));
+ cs = CPU(ppc_get_cpu_by_vcpu_id(target));
if (cs) {
run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
return H_SUCCESS;
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 94a2799b99..626c06b375 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -104,7 +104,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
}
id = rtas_ld(args, 0);
- cpu = ppc_get_vcpu_by_dt_id(id);
+ cpu = ppc_get_cpu_by_vcpu_id(id);
if (cpu != NULL) {
if (CPU(cpu)->halted) {
rtas_st(rets, 1, 0);
@@ -158,7 +158,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
start = rtas_ld(args, 1);
r3 = rtas_ld(args, 2);
- cpu = ppc_get_vcpu_by_dt_id(id);
+ cpu = ppc_get_cpu_by_vcpu_id(id);
if (cpu != NULL) {
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 6ee2a26a96..89f05542c6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1199,7 +1199,7 @@ struct PowerPCCPU {
/*< public >*/
CPUPPCState env;
- int cpu_dt_id;
+ int vcpu_id;
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
Object *intc;
@@ -2514,22 +2514,22 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
/**
- * ppc_get_vcpu_dt_id:
+ * ppc_get_vcpu_id:
* @cs: a PowerPCCPU struct.
*
* Returns a device-tree ID for a CPU.
*/
-int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
+int ppc_get_vcpu_id(PowerPCCPU *cpu);
/**
- * ppc_get_vcpu_by_dt_id:
- * @cpu_dt_id: a device tree id
+ * ppc_get_cpu_by_vcpu_id:
+ * @vcpu_id: a VCPU ID
*
- * Searches for a CPU by @cpu_dt_id.
+ * Searches for a CPU by @vcpu_id.
*
* Returns: a PowerPCCPU struct
*/
-PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
+PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id);
void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
#endif /* PPC_CPU_H */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 85713795de..7ccb350c5f 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
unsigned long kvm_arch_vcpu_id(CPUState *cpu)
{
- return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
+ return ppc_get_vcpu_id(POWERPC_CPU(cpu));
}
/* e500 supports 2 h/w breakpoint and 2 watchpoint.
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 01723bdfec..a80e9ffdf8 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9828,14 +9828,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
}
#if !defined(CONFIG_USER_ONLY)
- cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
+ cpu->vcpu_id = (cs->cpu_index / smp_threads) * max_smt
+ (cs->cpu_index % smp_threads);
- if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
- error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
+ if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
+ error_setg(errp, "Can't create CPU with id %d in KVM", cpu->vcpu_id);
error_append_hint(errp, "Adjust the number of cpus to %d "
"or try to raise the number of threads per core\n",
- cpu->cpu_dt_id * smp_threads / max_smt);
+ cpu->vcpu_id * smp_threads / max_smt);
goto unrealize;
}
#endif
--
2.12.1.382.gc0f9c7058
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id
2017-08-03 6:28 ` [Qemu-devel] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id Sam Bobroff
@ 2017-08-03 13:19 ` Greg Kurz
2017-08-04 2:25 ` David Gibson
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2017-08-03 13:19 UTC (permalink / raw)
To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david
[-- Attachment #1: Type: text/plain, Size: 9332 bytes --]
On Thu, 3 Aug 2017 16:28:44 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
> This field actually records the VCPU ID used by KVM and, although the
> value is also used in the device tree it is primarily the VCPU ID so
> rename it as such.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
With this patch applied, I still have:
$ git grep cpu_dt_id
target/ppc/cpu.h: * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
With that fixed,
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/ppc.c | 8 ++++----
> hw/ppc/spapr.c | 16 ++++++++--------
> hw/ppc/spapr_hcall.c | 4 ++--
> hw/ppc/spapr_rtas.c | 4 ++--
> target/ppc/cpu.h | 14 +++++++-------
> target/ppc/kvm.c | 2 +-
> target/ppc/translate_init.c | 8 ++++----
> 7 files changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 224184d66d..4477d4ad89 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1359,19 +1359,19 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
> }
>
> /* CPU device-tree ID helpers */
> -int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
> +int ppc_get_vcpu_id(PowerPCCPU *cpu)
> {
> - return cpu->cpu_dt_id;
> + return cpu->vcpu_id;
> }
>
> -PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
> +PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id)
> {
> CPUState *cs;
>
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> - if (cpu->cpu_dt_id == cpu_dt_id) {
> + if (cpu->vcpu_id == vcpu_id) {
> return cpu;
> }
> }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f7a19720dc..83c77096a6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -208,7 +208,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
> int i, ret = 0;
> uint32_t servers_prop[smt_threads];
> uint32_t gservers_prop[smt_threads * 2];
> - int index = ppc_get_vcpu_dt_id(cpu);
> + int index = ppc_get_vcpu_id(cpu);
>
> if (cpu->compat_pvr) {
> ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
> @@ -237,7 +237,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>
> static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
> {
> - int index = ppc_get_vcpu_dt_id(cpu);
> + int index = ppc_get_vcpu_id(cpu);
> uint32_t associativity[] = {cpu_to_be32(0x5),
> cpu_to_be32(0x0),
> cpu_to_be32(0x0),
> @@ -341,7 +341,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> - int index = ppc_get_vcpu_dt_id(cpu);
> + int index = ppc_get_vcpu_id(cpu);
> int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
>
> if ((index % smt) != 0) {
> @@ -493,7 +493,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> - int index = ppc_get_vcpu_dt_id(cpu);
> + int index = ppc_get_vcpu_id(cpu);
> uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> 0xffffffff, 0xffffffff};
> uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
> @@ -626,7 +626,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
> */
> CPU_FOREACH_REVERSE(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> - int index = ppc_get_vcpu_dt_id(cpu);
> + int index = ppc_get_vcpu_id(cpu);
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> int offset;
>
> @@ -2982,7 +2982,7 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> - int id = ppc_get_vcpu_dt_id(cpu);
> + int id = ppc_get_vcpu_id(cpu);
> void *fdt;
> int offset, fdt_size;
> char *nodename;
> @@ -3390,9 +3390,9 @@ static void spapr_ics_resend(XICSFabric *dev)
> ics_resend(spapr->ics);
> }
>
> -static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> +static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
> {
> - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> + PowerPCCPU *cpu = ppc_get_cpu_by_vcpu_id(vcpu_id);
>
> return cpu ? ICP(cpu->intc) : NULL;
> }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4..4ca233854a 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -999,7 +999,7 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> CPUPPCState *tenv;
> PowerPCCPU *tcpu;
>
> - tcpu = ppc_get_vcpu_by_dt_id(procno);
> + tcpu = ppc_get_cpu_by_vcpu_id(procno);
> if (!tcpu) {
> return H_PARAMETER;
> }
> @@ -1431,7 +1431,7 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>
> } else {
> /* Unicast */
> - cs = CPU(ppc_get_vcpu_by_dt_id(target));
> + cs = CPU(ppc_get_cpu_by_vcpu_id(target));
> if (cs) {
> run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> return H_SUCCESS;
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 94a2799b99..626c06b375 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -104,7 +104,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
> }
>
> id = rtas_ld(args, 0);
> - cpu = ppc_get_vcpu_by_dt_id(id);
> + cpu = ppc_get_cpu_by_vcpu_id(id);
> if (cpu != NULL) {
> if (CPU(cpu)->halted) {
> rtas_st(rets, 1, 0);
> @@ -158,7 +158,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> start = rtas_ld(args, 1);
> r3 = rtas_ld(args, 2);
>
> - cpu = ppc_get_vcpu_by_dt_id(id);
> + cpu = ppc_get_cpu_by_vcpu_id(id);
> if (cpu != NULL) {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 6ee2a26a96..89f05542c6 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1199,7 +1199,7 @@ struct PowerPCCPU {
> /*< public >*/
>
> CPUPPCState env;
> - int cpu_dt_id;
> + int vcpu_id;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> Object *intc;
> @@ -2514,22 +2514,22 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
> void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
>
> /**
> - * ppc_get_vcpu_dt_id:
> + * ppc_get_vcpu_id:
> * @cs: a PowerPCCPU struct.
> *
> * Returns a device-tree ID for a CPU.
> */
> -int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
> +int ppc_get_vcpu_id(PowerPCCPU *cpu);
>
> /**
> - * ppc_get_vcpu_by_dt_id:
> - * @cpu_dt_id: a device tree id
> + * ppc_get_cpu_by_vcpu_id:
> + * @vcpu_id: a VCPU ID
> *
> - * Searches for a CPU by @cpu_dt_id.
> + * Searches for a CPU by @vcpu_id.
> *
> * Returns: a PowerPCCPU struct
> */
> -PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
> +PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id);
>
> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> #endif /* PPC_CPU_H */
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 85713795de..7ccb350c5f 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
>
> unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> {
> - return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
> + return ppc_get_vcpu_id(POWERPC_CPU(cpu));
> }
>
> /* e500 supports 2 h/w breakpoint and 2 watchpoint.
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 01723bdfec..a80e9ffdf8 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -9828,14 +9828,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
> }
>
> #if !defined(CONFIG_USER_ONLY)
> - cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
> + cpu->vcpu_id = (cs->cpu_index / smp_threads) * max_smt
> + (cs->cpu_index % smp_threads);
>
> - if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
> - error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
> + if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
> + error_setg(errp, "Can't create CPU with id %d in KVM", cpu->vcpu_id);
> error_append_hint(errp, "Adjust the number of cpus to %d "
> "or try to raise the number of threads per core\n",
> - cpu->cpu_dt_id * smp_threads / max_smt);
> + cpu->vcpu_id * smp_threads / max_smt);
> goto unrealize;
> }
> #endif
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id
2017-08-03 13:19 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2017-08-04 2:25 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2017-08-04 2:25 UTC (permalink / raw)
To: Greg Kurz; +Cc: Sam Bobroff, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 10155 bytes --]
On Thu, Aug 03, 2017 at 03:19:40PM +0200, Greg Kurz wrote:
> On Thu, 3 Aug 2017 16:28:44 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
>
> > This field actually records the VCPU ID used by KVM and, although the
> > value is also used in the device tree it is primarily the VCPU ID so
> > rename it as such.
> >
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> > ---
>
> With this patch applied, I still have:
>
> $ git grep cpu_dt_id
> target/ppc/cpu.h: * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
>
> With that fixed,
Updated as you suggested and..
> Reviewed-by: Greg Kurz <groug@kaod.org>
.. applied to ppc-for-2.11.
>
> > hw/ppc/ppc.c | 8 ++++----
> > hw/ppc/spapr.c | 16 ++++++++--------
> > hw/ppc/spapr_hcall.c | 4 ++--
> > hw/ppc/spapr_rtas.c | 4 ++--
> > target/ppc/cpu.h | 14 +++++++-------
> > target/ppc/kvm.c | 2 +-
> > target/ppc/translate_init.c | 8 ++++----
> > 7 files changed, 28 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> > index 224184d66d..4477d4ad89 100644
> > --- a/hw/ppc/ppc.c
> > +++ b/hw/ppc/ppc.c
> > @@ -1359,19 +1359,19 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
> > }
> >
> > /* CPU device-tree ID helpers */
> > -int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
> > +int ppc_get_vcpu_id(PowerPCCPU *cpu)
> > {
> > - return cpu->cpu_dt_id;
> > + return cpu->vcpu_id;
> > }
> >
> > -PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
> > +PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id)
> > {
> > CPUState *cs;
> >
> > CPU_FOREACH(cs) {
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> >
> > - if (cpu->cpu_dt_id == cpu_dt_id) {
> > + if (cpu->vcpu_id == vcpu_id) {
> > return cpu;
> > }
> > }
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index f7a19720dc..83c77096a6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -208,7 +208,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
> > int i, ret = 0;
> > uint32_t servers_prop[smt_threads];
> > uint32_t gservers_prop[smt_threads * 2];
> > - int index = ppc_get_vcpu_dt_id(cpu);
> > + int index = ppc_get_vcpu_id(cpu);
> >
> > if (cpu->compat_pvr) {
> > ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
> > @@ -237,7 +237,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
> >
> > static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
> > {
> > - int index = ppc_get_vcpu_dt_id(cpu);
> > + int index = ppc_get_vcpu_id(cpu);
> > uint32_t associativity[] = {cpu_to_be32(0x5),
> > cpu_to_be32(0x0),
> > cpu_to_be32(0x0),
> > @@ -341,7 +341,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> > CPUPPCState *env = &cpu->env;
> > DeviceClass *dc = DEVICE_GET_CLASS(cs);
> > - int index = ppc_get_vcpu_dt_id(cpu);
> > + int index = ppc_get_vcpu_id(cpu);
> > int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
> >
> > if ((index % smt) != 0) {
> > @@ -493,7 +493,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> > CPUPPCState *env = &cpu->env;
> > PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> > - int index = ppc_get_vcpu_dt_id(cpu);
> > + int index = ppc_get_vcpu_id(cpu);
> > uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> > 0xffffffff, 0xffffffff};
> > uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
> > @@ -626,7 +626,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
> > */
> > CPU_FOREACH_REVERSE(cs) {
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> > - int index = ppc_get_vcpu_dt_id(cpu);
> > + int index = ppc_get_vcpu_id(cpu);
> > DeviceClass *dc = DEVICE_GET_CLASS(cs);
> > int offset;
> >
> > @@ -2982,7 +2982,7 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> > {
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> > DeviceClass *dc = DEVICE_GET_CLASS(cs);
> > - int id = ppc_get_vcpu_dt_id(cpu);
> > + int id = ppc_get_vcpu_id(cpu);
> > void *fdt;
> > int offset, fdt_size;
> > char *nodename;
> > @@ -3390,9 +3390,9 @@ static void spapr_ics_resend(XICSFabric *dev)
> > ics_resend(spapr->ics);
> > }
> >
> > -static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> > +static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
> > {
> > - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> > + PowerPCCPU *cpu = ppc_get_cpu_by_vcpu_id(vcpu_id);
> >
> > return cpu ? ICP(cpu->intc) : NULL;
> > }
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 07b3da8dc4..4ca233854a 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -999,7 +999,7 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > CPUPPCState *tenv;
> > PowerPCCPU *tcpu;
> >
> > - tcpu = ppc_get_vcpu_by_dt_id(procno);
> > + tcpu = ppc_get_cpu_by_vcpu_id(procno);
> > if (!tcpu) {
> > return H_PARAMETER;
> > }
> > @@ -1431,7 +1431,7 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> >
> > } else {
> > /* Unicast */
> > - cs = CPU(ppc_get_vcpu_by_dt_id(target));
> > + cs = CPU(ppc_get_cpu_by_vcpu_id(target));
> > if (cs) {
> > run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > return H_SUCCESS;
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 94a2799b99..626c06b375 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -104,7 +104,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
> > }
> >
> > id = rtas_ld(args, 0);
> > - cpu = ppc_get_vcpu_by_dt_id(id);
> > + cpu = ppc_get_cpu_by_vcpu_id(id);
> > if (cpu != NULL) {
> > if (CPU(cpu)->halted) {
> > rtas_st(rets, 1, 0);
> > @@ -158,7 +158,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> > start = rtas_ld(args, 1);
> > r3 = rtas_ld(args, 2);
> >
> > - cpu = ppc_get_vcpu_by_dt_id(id);
> > + cpu = ppc_get_cpu_by_vcpu_id(id);
> > if (cpu != NULL) {
> > CPUState *cs = CPU(cpu);
> > CPUPPCState *env = &cpu->env;
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 6ee2a26a96..89f05542c6 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1199,7 +1199,7 @@ struct PowerPCCPU {
> > /*< public >*/
> >
> > CPUPPCState env;
> > - int cpu_dt_id;
> > + int vcpu_id;
> > uint32_t compat_pvr;
> > PPCVirtualHypervisor *vhyp;
> > Object *intc;
> > @@ -2514,22 +2514,22 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
> > void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
> >
> > /**
> > - * ppc_get_vcpu_dt_id:
> > + * ppc_get_vcpu_id:
> > * @cs: a PowerPCCPU struct.
> > *
> > * Returns a device-tree ID for a CPU.
> > */
> > -int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
> > +int ppc_get_vcpu_id(PowerPCCPU *cpu);
> >
> > /**
> > - * ppc_get_vcpu_by_dt_id:
> > - * @cpu_dt_id: a device tree id
> > + * ppc_get_cpu_by_vcpu_id:
> > + * @vcpu_id: a VCPU ID
> > *
> > - * Searches for a CPU by @cpu_dt_id.
> > + * Searches for a CPU by @vcpu_id.
> > *
> > * Returns: a PowerPCCPU struct
> > */
> > -PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
> > +PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id);
> >
> > void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> > #endif /* PPC_CPU_H */
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 85713795de..7ccb350c5f 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> >
> > unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> > {
> > - return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
> > + return ppc_get_vcpu_id(POWERPC_CPU(cpu));
> > }
> >
> > /* e500 supports 2 h/w breakpoint and 2 watchpoint.
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index 01723bdfec..a80e9ffdf8 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -9828,14 +9828,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
> > }
> >
> > #if !defined(CONFIG_USER_ONLY)
> > - cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
> > + cpu->vcpu_id = (cs->cpu_index / smp_threads) * max_smt
> > + (cs->cpu_index % smp_threads);
> >
> > - if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
> > - error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
> > + if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
> > + error_setg(errp, "Can't create CPU with id %d in KVM", cpu->vcpu_id);
> > error_append_hint(errp, "Adjust the number of cpus to %d "
> > "or try to raise the number of threads per core\n",
> > - cpu->cpu_dt_id * smp_threads / max_smt);
> > + cpu->vcpu_id * smp_threads / max_smt);
> > goto unrealize;
> > }
> > #endif
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR
2017-08-03 6:28 [Qemu-devel] [PATCH 0/4] Cleanup cpu_dt_id Sam Bobroff
` (2 preceding siblings ...)
2017-08-03 6:28 ` [Qemu-devel] [PATCH 3/4] ppc: spapr: Rename cpu_dt_id to vcpu_id Sam Bobroff
@ 2017-08-03 6:28 ` Sam Bobroff
2017-08-03 13:23 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-08-04 3:10 ` [Qemu-devel] " David Gibson
3 siblings, 2 replies; 14+ messages in thread
From: Sam Bobroff @ 2017-08-03 6:28 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: david, agraf
The concept of a VCPU ID that differs from the CPU's index
(cpu->cpu_index) exists only within SPAPR machines so, move the
functions ppc_get_vcpu_id() and ppc_get_cpu_by_vcpu_id() into spapr.c
and rename them appropriately.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
hw/ppc/ppc.c | 21 ---------------------
hw/ppc/spapr.c | 34 +++++++++++++++++++++++++++-------
hw/ppc/spapr_hcall.c | 4 ++--
hw/ppc/spapr_rtas.c | 4 ++--
include/hw/ppc/spapr.h | 3 +++
target/ppc/cpu.h | 18 ------------------
target/ppc/kvm.c | 2 +-
7 files changed, 35 insertions(+), 51 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 4477d4ad89..f76886f4d3 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1358,27 +1358,6 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
}
}
-/* CPU device-tree ID helpers */
-int ppc_get_vcpu_id(PowerPCCPU *cpu)
-{
- return cpu->vcpu_id;
-}
-
-PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id)
-{
- CPUState *cs;
-
- CPU_FOREACH(cs) {
- PowerPCCPU *cpu = POWERPC_CPU(cs);
-
- if (cpu->vcpu_id == vcpu_id) {
- return cpu;
- }
- }
-
- return NULL;
-}
-
void ppc_cpu_parse_features(const char *cpu_model)
{
CPUClass *cc;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 83c77096a6..0b5ffc5e9b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -208,7 +208,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
int i, ret = 0;
uint32_t servers_prop[smt_threads];
uint32_t gservers_prop[smt_threads * 2];
- int index = ppc_get_vcpu_id(cpu);
+ int index = spapr_vcpu_id(cpu);
if (cpu->compat_pvr) {
ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
@@ -237,7 +237,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
{
- int index = ppc_get_vcpu_id(cpu);
+ int index = spapr_vcpu_id(cpu);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -341,7 +341,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
DeviceClass *dc = DEVICE_GET_CLASS(cs);
- int index = ppc_get_vcpu_id(cpu);
+ int index = spapr_vcpu_id(cpu);
int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
if ((index % smt) != 0) {
@@ -493,7 +493,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
- int index = ppc_get_vcpu_id(cpu);
+ int index = spapr_vcpu_id(cpu);
uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
0xffffffff, 0xffffffff};
uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
@@ -626,7 +626,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
*/
CPU_FOREACH_REVERSE(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- int index = ppc_get_vcpu_id(cpu);
+ int index = spapr_vcpu_id(cpu);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
int offset;
@@ -2982,7 +2982,7 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
- int id = ppc_get_vcpu_id(cpu);
+ int id = spapr_vcpu_id(cpu);
void *fdt;
int offset, fdt_size;
char *nodename;
@@ -3392,7 +3392,7 @@ static void spapr_ics_resend(XICSFabric *dev)
static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
{
- PowerPCCPU *cpu = ppc_get_cpu_by_vcpu_id(vcpu_id);
+ PowerPCCPU *cpu = spapr_find_cpu(vcpu_id);
return cpu ? ICP(cpu->intc) : NULL;
}
@@ -3412,6 +3412,26 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
ics_pic_print_info(spapr->ics, mon);
}
+int spapr_vcpu_id(PowerPCCPU *cpu)
+{
+ return cpu->vcpu_id;
+}
+
+PowerPCCPU *spapr_find_cpu(int vcpu_id)
+{
+ CPUState *cs;
+
+ CPU_FOREACH(cs) {
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+ if (cpu->vcpu_id == vcpu_id) {
+ return cpu;
+ }
+ }
+
+ return NULL;
+}
+
static void spapr_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4ca233854a..7cf0993800 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -999,7 +999,7 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPRMachineState *spapr,
CPUPPCState *tenv;
PowerPCCPU *tcpu;
- tcpu = ppc_get_cpu_by_vcpu_id(procno);
+ tcpu = spapr_find_cpu(procno);
if (!tcpu) {
return H_PARAMETER;
}
@@ -1431,7 +1431,7 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
} else {
/* Unicast */
- cs = CPU(ppc_get_cpu_by_vcpu_id(target));
+ cs = CPU(spapr_find_cpu(target));
if (cs) {
run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
return H_SUCCESS;
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 626c06b375..cdf0b607a0 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -104,7 +104,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
}
id = rtas_ld(args, 0);
- cpu = ppc_get_cpu_by_vcpu_id(id);
+ cpu = spapr_find_cpu(id);
if (cpu != NULL) {
if (CPU(cpu)->halted) {
rtas_st(rets, 1, 0);
@@ -158,7 +158,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
start = rtas_ld(args, 1);
r3 = rtas_ld(args, 2);
- cpu = ppc_get_cpu_by_vcpu_id(id);
+ cpu = spapr_find_cpu(id);
if (cpu != NULL) {
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 2a303a705c..86c982cf2c 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -704,4 +704,7 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg);
#define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift))
+int spapr_vcpu_id(PowerPCCPU *cpu);
+PowerPCCPU *spapr_find_cpu(int vcpu_id);
+
#endif /* HW_SPAPR_H */
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 89f05542c6..a5c7ace266 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2513,23 +2513,5 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
-/**
- * ppc_get_vcpu_id:
- * @cs: a PowerPCCPU struct.
- *
- * Returns a device-tree ID for a CPU.
- */
-int ppc_get_vcpu_id(PowerPCCPU *cpu);
-
-/**
- * ppc_get_cpu_by_vcpu_id:
- * @vcpu_id: a VCPU ID
- *
- * Searches for a CPU by @vcpu_id.
- *
- * Returns: a PowerPCCPU struct
- */
-PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id);
-
void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
#endif /* PPC_CPU_H */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 7ccb350c5f..2bf2727860 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
unsigned long kvm_arch_vcpu_id(CPUState *cpu)
{
- return ppc_get_vcpu_id(POWERPC_CPU(cpu));
+ return spapr_vcpu_id(POWERPC_CPU(cpu));
}
/* e500 supports 2 h/w breakpoint and 2 watchpoint.
--
2.12.1.382.gc0f9c7058
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR
2017-08-03 6:28 ` [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR Sam Bobroff
@ 2017-08-03 13:23 ` Greg Kurz
2017-08-04 3:10 ` [Qemu-devel] " David Gibson
1 sibling, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2017-08-03 13:23 UTC (permalink / raw)
To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david
[-- Attachment #1: Type: text/plain, Size: 8503 bytes --]
On Thu, 3 Aug 2017 16:28:52 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
> The concept of a VCPU ID that differs from the CPU's index
> (cpu->cpu_index) exists only within SPAPR machines so, move the
> functions ppc_get_vcpu_id() and ppc_get_cpu_by_vcpu_id() into spapr.c
> and rename them appropriately.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
Nice cleanup ! :)
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/ppc.c | 21 ---------------------
> hw/ppc/spapr.c | 34 +++++++++++++++++++++++++++-------
> hw/ppc/spapr_hcall.c | 4 ++--
> hw/ppc/spapr_rtas.c | 4 ++--
> include/hw/ppc/spapr.h | 3 +++
> target/ppc/cpu.h | 18 ------------------
> target/ppc/kvm.c | 2 +-
> 7 files changed, 35 insertions(+), 51 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 4477d4ad89..f76886f4d3 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1358,27 +1358,6 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
> }
> }
>
> -/* CPU device-tree ID helpers */
> -int ppc_get_vcpu_id(PowerPCCPU *cpu)
> -{
> - return cpu->vcpu_id;
> -}
> -
> -PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id)
> -{
> - CPUState *cs;
> -
> - CPU_FOREACH(cs) {
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> -
> - if (cpu->vcpu_id == vcpu_id) {
> - return cpu;
> - }
> - }
> -
> - return NULL;
> -}
> -
> void ppc_cpu_parse_features(const char *cpu_model)
> {
> CPUClass *cc;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 83c77096a6..0b5ffc5e9b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -208,7 +208,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
> int i, ret = 0;
> uint32_t servers_prop[smt_threads];
> uint32_t gservers_prop[smt_threads * 2];
> - int index = ppc_get_vcpu_id(cpu);
> + int index = spapr_vcpu_id(cpu);
>
> if (cpu->compat_pvr) {
> ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
> @@ -237,7 +237,7 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>
> static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
> {
> - int index = ppc_get_vcpu_id(cpu);
> + int index = spapr_vcpu_id(cpu);
> uint32_t associativity[] = {cpu_to_be32(0x5),
> cpu_to_be32(0x0),
> cpu_to_be32(0x0),
> @@ -341,7 +341,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> - int index = ppc_get_vcpu_id(cpu);
> + int index = spapr_vcpu_id(cpu);
> int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
>
> if ((index % smt) != 0) {
> @@ -493,7 +493,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> - int index = ppc_get_vcpu_id(cpu);
> + int index = spapr_vcpu_id(cpu);
> uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> 0xffffffff, 0xffffffff};
> uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
> @@ -626,7 +626,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
> */
> CPU_FOREACH_REVERSE(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> - int index = ppc_get_vcpu_id(cpu);
> + int index = spapr_vcpu_id(cpu);
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> int offset;
>
> @@ -2982,7 +2982,7 @@ static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> DeviceClass *dc = DEVICE_GET_CLASS(cs);
> - int id = ppc_get_vcpu_id(cpu);
> + int id = spapr_vcpu_id(cpu);
> void *fdt;
> int offset, fdt_size;
> char *nodename;
> @@ -3392,7 +3392,7 @@ static void spapr_ics_resend(XICSFabric *dev)
>
> static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
> {
> - PowerPCCPU *cpu = ppc_get_cpu_by_vcpu_id(vcpu_id);
> + PowerPCCPU *cpu = spapr_find_cpu(vcpu_id);
>
> return cpu ? ICP(cpu->intc) : NULL;
> }
> @@ -3412,6 +3412,26 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
> ics_pic_print_info(spapr->ics, mon);
> }
>
> +int spapr_vcpu_id(PowerPCCPU *cpu)
> +{
> + return cpu->vcpu_id;
> +}
> +
> +PowerPCCPU *spapr_find_cpu(int vcpu_id)
> +{
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> + if (cpu->vcpu_id == vcpu_id) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
> +
> static void spapr_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4ca233854a..7cf0993800 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -999,7 +999,7 @@ static target_ulong h_register_vpa(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> CPUPPCState *tenv;
> PowerPCCPU *tcpu;
>
> - tcpu = ppc_get_cpu_by_vcpu_id(procno);
> + tcpu = spapr_find_cpu(procno);
> if (!tcpu) {
> return H_PARAMETER;
> }
> @@ -1431,7 +1431,7 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>
> } else {
> /* Unicast */
> - cs = CPU(ppc_get_cpu_by_vcpu_id(target));
> + cs = CPU(spapr_find_cpu(target));
> if (cs) {
> run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> return H_SUCCESS;
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 626c06b375..cdf0b607a0 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -104,7 +104,7 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
> }
>
> id = rtas_ld(args, 0);
> - cpu = ppc_get_cpu_by_vcpu_id(id);
> + cpu = spapr_find_cpu(id);
> if (cpu != NULL) {
> if (CPU(cpu)->halted) {
> rtas_st(rets, 1, 0);
> @@ -158,7 +158,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> start = rtas_ld(args, 1);
> r3 = rtas_ld(args, 2);
>
> - cpu = ppc_get_cpu_by_vcpu_id(id);
> + cpu = spapr_find_cpu(id);
> if (cpu != NULL) {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 2a303a705c..86c982cf2c 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -704,4 +704,7 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg);
>
> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift))
>
> +int spapr_vcpu_id(PowerPCCPU *cpu);
> +PowerPCCPU *spapr_find_cpu(int vcpu_id);
> +
> #endif /* HW_SPAPR_H */
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 89f05542c6..a5c7ace266 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2513,23 +2513,5 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
>
> void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
>
> -/**
> - * ppc_get_vcpu_id:
> - * @cs: a PowerPCCPU struct.
> - *
> - * Returns a device-tree ID for a CPU.
> - */
> -int ppc_get_vcpu_id(PowerPCCPU *cpu);
> -
> -/**
> - * ppc_get_cpu_by_vcpu_id:
> - * @vcpu_id: a VCPU ID
> - *
> - * Searches for a CPU by @vcpu_id.
> - *
> - * Returns: a PowerPCCPU struct
> - */
> -PowerPCCPU *ppc_get_cpu_by_vcpu_id(int vcpu_id);
> -
> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> #endif /* PPC_CPU_H */
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 7ccb350c5f..2bf2727860 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
>
> unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> {
> - return ppc_get_vcpu_id(POWERPC_CPU(cpu));
> + return spapr_vcpu_id(POWERPC_CPU(cpu));
> }
>
> /* e500 supports 2 h/w breakpoint and 2 watchpoint.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR
2017-08-03 6:28 ` [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR Sam Bobroff
2017-08-03 13:23 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2017-08-04 3:10 ` David Gibson
2017-08-07 6:07 ` Sam Bobroff
1 sibling, 1 reply; 14+ messages in thread
From: David Gibson @ 2017-08-04 3:10 UTC (permalink / raw)
To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, agraf
[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]
On Thu, Aug 03, 2017 at 04:28:52PM +1000, Sam Bobroff wrote:
> The concept of a VCPU ID that differs from the CPU's index
> (cpu->cpu_index) exists only within SPAPR machines so, move the
> functions ppc_get_vcpu_id() and ppc_get_cpu_by_vcpu_id() into spapr.c
> and rename them appropriately.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Mostly good, but...
[snip]
> +int spapr_vcpu_id(PowerPCCPU *cpu)
> +{
> + return cpu->vcpu_id;
> +}
> +
> +PowerPCCPU *spapr_find_cpu(int vcpu_id)
> +{
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> + if (cpu->vcpu_id == vcpu_id) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
[...]
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 7ccb350c5f..2bf2727860 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
>
> unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> {
> - return ppc_get_vcpu_id(POWERPC_CPU(cpu));
> + return spapr_vcpu_id(POWERPC_CPU(cpu));
> }
Here you've replaced an implicit dependency on spapr details in the
generic code with an explicit dependency on spapr details. That's the
wrong direction.
Instead _this_ one should directly reference vcpu_id, the spapr one
should be something like:
if (kvm)
return kvm_arch_vcpu_id(...)
else
return cpu_index;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] ppc: spapr: Make VCPU ID handling private to SPAPR
2017-08-04 3:10 ` [Qemu-devel] " David Gibson
@ 2017-08-07 6:07 ` Sam Bobroff
0 siblings, 0 replies; 14+ messages in thread
From: Sam Bobroff @ 2017-08-07 6:07 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, agraf
On Fri, Aug 04, 2017 at 01:10:19PM +1000, David Gibson wrote:
> On Thu, Aug 03, 2017 at 04:28:52PM +1000, Sam Bobroff wrote:
> > The concept of a VCPU ID that differs from the CPU's index
> > (cpu->cpu_index) exists only within SPAPR machines so, move the
> > functions ppc_get_vcpu_id() and ppc_get_cpu_by_vcpu_id() into spapr.c
> > and rename them appropriately.
> >
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>
> Mostly good, but...
>
> [snip]
> > +int spapr_vcpu_id(PowerPCCPU *cpu)
> > +{
> > + return cpu->vcpu_id;
> > +}
> > +
> > +PowerPCCPU *spapr_find_cpu(int vcpu_id)
> > +{
> > + CPUState *cs;
> > +
> > + CPU_FOREACH(cs) {
> > + PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +
> > + if (cpu->vcpu_id == vcpu_id) {
> > + return cpu;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
>
> [...]
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 7ccb350c5f..2bf2727860 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> >
> > unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> > {
> > - return ppc_get_vcpu_id(POWERPC_CPU(cpu));
> > + return spapr_vcpu_id(POWERPC_CPU(cpu));
> > }
>
> Here you've replaced an implicit dependency on spapr details in the
> generic code with an explicit dependency on spapr details. That's the
> wrong direction.
Ah right, I'll flip it around.
> Instead _this_ one should directly reference vcpu_id, the spapr one
> should be something like:
>
> if (kvm)
> return kvm_arch_vcpu_id(...)
> else
> return cpu_index;
OK.
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread