* [Qemu-devel] [PATCH 0/1] s390-cpu: qom interface for S390 cpu states array @ 2014-03-07 16:29 Jason J. Herne 2014-03-07 16:29 ` [Qemu-devel] [PATCH 1/1] " Jason J. Herne 0 siblings, 1 reply; 4+ messages in thread From: Jason J. Herne @ 2014-03-07 16:29 UTC (permalink / raw) To: afaerber, borntraeger, agraf, qemu-devel; +Cc: Jason J. Herne From: "Jason J. Herne" <jjherne@us.ibm.com> This patch is a result of changes requested by Andreas Färber during the S390 cpu hotplug code review. Andreas, I am unsure if you were asking for the complete removal of the ipi_states array, or the removal of the set/get functions used to encapsulate access to the array, or if you are ok with the current scheme and you just wanted the data accesible within the qom tree as well. The approach I took with this patch was to leave the ipi_states array in place, although it has been renamed for clarity. Because object_property_add_link() expects Object** when pasing the link target, we must have a lasting reference to our S390CPU objects. I'm not sure where we would store these pointers, if not in the currently existing aray. I decided to encapsulate the details in set/get functions instead of (eventually) repeating all of the link related details when we allow the creation of cpus via hotplug. If you are unhappy with how I did this, please assist me in understanding what you would like changed. I am happy to change it as needed. Jason J. Herne (1): s390-cpu: qom interface for S390 cpu states array hw/s390x/s390-virtio.c | 32 +++++++++++++++++++++++--------- target-s390x/cpu.h | 1 + 2 files changed, 24 insertions(+), 9 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array 2014-03-07 16:29 [Qemu-devel] [PATCH 0/1] s390-cpu: qom interface for S390 cpu states array Jason J. Herne @ 2014-03-07 16:29 ` Jason J. Herne 2014-03-07 17:03 ` Paolo Bonzini 0 siblings, 1 reply; 4+ messages in thread From: Jason J. Herne @ 2014-03-07 16:29 UTC (permalink / raw) To: afaerber, borntraeger, agraf, qemu-devel; +Cc: Jason J. Herne From: "Jason J. Herne" <jjherne@us.ibm.com> Rename the S390 ipi_states array to cpu_states to better reflect its contents. Create machine/cpu[cpu_addr] links within the qom tree when creating a new cpu. Encapsulate the qom tree linking process and the management of the cpu_states array into helper functions. Signed-off-by: Jason J. Herne <jjherne@us.ibm.com> --- hw/s390x/s390-virtio.c | 30 ++++++++++++++++++++++++------ target-s390x/cpu.h | 1 + 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c index 9eeda97..82411e7 100644 --- a/hw/s390x/s390-virtio.c +++ b/hw/s390x/s390-virtio.c @@ -52,15 +52,33 @@ #define ZIPL_FILENAME "s390-zipl.rom" static VirtIOS390Bus *s390_bus; -static S390CPU **ipi_states; +static S390CPU **cpu_states; S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) { + gchar *name; + Object *cpu; + if (cpu_addr >= smp_cpus) { return NULL; } - return ipi_states[cpu_addr]; + name = g_strdup_printf("cpu[%i]", cpu_addr); + cpu = object_property_get_link(qdev_get_machine(), name, NULL); + + g_free(name); + return S390_CPU(cpu); +} + +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state) +{ + gchar *name; + + cpu_states[cpu_addr] = state; + name = g_strdup_printf("cpu[%i]", cpu_addr); + object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU, + (Object **) &cpu_states[cpu_addr], NULL); + g_free(name); } static int s390_virtio_hcall_notify(const uint64_t *args) @@ -184,16 +202,16 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) cpu_model = "host"; } - ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus); + cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus); for (i = 0; i < smp_cpus; i++) { - S390CPU *cpu; + S390CPU* cpu; CPUState *cs; cpu = cpu_s390x_init(cpu_model); - cs = CPU(cpu); + s390_cpu_set_cpustate(i, cpu); - ipi_states[i] = cpu; + cs = CPU(cpu_states[i]); cs->halted = 1; cpu->env.exception_index = EXCP_HLT; cpu->env.storage_keys = storage_keys; diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 96c2b4a..6ce3b64 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -370,6 +370,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type, } #endif S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state); void s390_add_running_cpu(S390CPU *cpu); unsigned s390_del_running_cpu(S390CPU *cpu); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array 2014-03-07 16:29 ` [Qemu-devel] [PATCH 1/1] " Jason J. Herne @ 2014-03-07 17:03 ` Paolo Bonzini 2014-03-11 12:51 ` Jason J. Herne 0 siblings, 1 reply; 4+ messages in thread From: Paolo Bonzini @ 2014-03-07 17:03 UTC (permalink / raw) To: Jason J. Herne, afaerber, borntraeger, agraf, qemu-devel Il 07/03/2014 17:29, Jason J. Herne ha scritto: > From: "Jason J. Herne" <jjherne@us.ibm.com> > > Rename the S390 ipi_states array to cpu_states to better reflect its contents. > > Create machine/cpu[cpu_addr] links within the qom tree when creating a new cpu. > > Encapsulate the qom tree linking process and the management of the cpu_states > array into helper functions. > > Signed-off-by: Jason J. Herne <jjherne@us.ibm.com> > --- > hw/s390x/s390-virtio.c | 30 ++++++++++++++++++++++++------ > target-s390x/cpu.h | 1 + > 2 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c > index 9eeda97..82411e7 100644 > --- a/hw/s390x/s390-virtio.c > +++ b/hw/s390x/s390-virtio.c > @@ -52,15 +52,33 @@ > #define ZIPL_FILENAME "s390-zipl.rom" > > static VirtIOS390Bus *s390_bus; > -static S390CPU **ipi_states; > +static S390CPU **cpu_states; > > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > { > + gchar *name; > + Object *cpu; > + > if (cpu_addr >= smp_cpus) { > return NULL; > } > > - return ipi_states[cpu_addr]; > + name = g_strdup_printf("cpu[%i]", cpu_addr); > + cpu = object_property_get_link(qdev_get_machine(), name, NULL); > + > + g_free(name); > + return S390_CPU(cpu); > +} QOM is too slow to be used in the data path. I don't think you want a malloc + a linear scan of an array in css_inject_io_interrupt, so you should keep using cpu_states here. Paolo > +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state) > +{ > + gchar *name; > + > + cpu_states[cpu_addr] = state; > + name = g_strdup_printf("cpu[%i]", cpu_addr); > + object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU, > + (Object **) &cpu_states[cpu_addr], NULL); > + g_free(name); > } > > static int s390_virtio_hcall_notify(const uint64_t *args) > @@ -184,16 +202,16 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) > cpu_model = "host"; > } > > - ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus); > + cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus); > > for (i = 0; i < smp_cpus; i++) { > - S390CPU *cpu; > + S390CPU* cpu; > CPUState *cs; > > cpu = cpu_s390x_init(cpu_model); > - cs = CPU(cpu); > + s390_cpu_set_cpustate(i, cpu); > > - ipi_states[i] = cpu; > + cs = CPU(cpu_states[i]); > cs->halted = 1; > cpu->env.exception_index = EXCP_HLT; > cpu->env.storage_keys = storage_keys; > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index 96c2b4a..6ce3b64 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -370,6 +370,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU *cpu, int type, > } > #endif > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); > +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state); > void s390_add_running_cpu(S390CPU *cpu); > unsigned s390_del_running_cpu(S390CPU *cpu); > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array 2014-03-07 17:03 ` Paolo Bonzini @ 2014-03-11 12:51 ` Jason J. Herne 0 siblings, 0 replies; 4+ messages in thread From: Jason J. Herne @ 2014-03-11 12:51 UTC (permalink / raw) To: Paolo Bonzini, Jason J. Herne, afaerber, borntraeger, agraf, qemu-devel On 03/07/2014 12:03 PM, Paolo Bonzini wrote: > Il 07/03/2014 17:29, Jason J. Herne ha scritto: >> From: "Jason J. Herne" <jjherne@us.ibm.com> >> >> Rename the S390 ipi_states array to cpu_states to better reflect its >> contents. >> >> Create machine/cpu[cpu_addr] links within the qom tree when creating a >> new cpu. >> >> Encapsulate the qom tree linking process and the management of the >> cpu_states >> array into helper functions. >> >> Signed-off-by: Jason J. Herne <jjherne@us.ibm.com> >> --- >> hw/s390x/s390-virtio.c | 30 ++++++++++++++++++++++++------ >> target-s390x/cpu.h | 1 + >> 2 files changed, 25 insertions(+), 6 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c >> index 9eeda97..82411e7 100644 >> --- a/hw/s390x/s390-virtio.c >> +++ b/hw/s390x/s390-virtio.c >> @@ -52,15 +52,33 @@ >> #define ZIPL_FILENAME "s390-zipl.rom" >> >> static VirtIOS390Bus *s390_bus; >> -static S390CPU **ipi_states; >> +static S390CPU **cpu_states; >> >> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) >> { >> + gchar *name; >> + Object *cpu; >> + >> if (cpu_addr >= smp_cpus) { >> return NULL; >> } >> >> - return ipi_states[cpu_addr]; >> + name = g_strdup_printf("cpu[%i]", cpu_addr); >> + cpu = object_property_get_link(qdev_get_machine(), name, NULL); >> + >> + g_free(name); >> + return S390_CPU(cpu); >> +} > > QOM is too slow to be used in the data path. > > I don't think you want a malloc + a linear scan of an array in > css_inject_io_interrupt, so you should keep using cpu_states here. > Hi Paolo, I agree. Provided Andreas also agrees I'll include this change in my next patch. Thanks. -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-11 12:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-07 16:29 [Qemu-devel] [PATCH 0/1] s390-cpu: qom interface for S390 cpu states array Jason J. Herne 2014-03-07 16:29 ` [Qemu-devel] [PATCH 1/1] " Jason J. Herne 2014-03-07 17:03 ` Paolo Bonzini 2014-03-11 12:51 ` Jason J. Herne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).