Am 16.10.2012 02:49, schrieb Igor Mammedov:
> On Fri, 12 Oct 2012 03:26:40 +0200
> Andreas Färber <afaerber@xxxxxxx> wrote:
>
>> Change return type to bool, move to include/qemu/cpu.h and
>> add documentation.
>>
>> Signed-off-by: Andreas Färber <afaerber@xxxxxxx>
>> ---
>> cpus.c | 10 ++++------
>> exec.c | 3 ++-
>> hw/apic.c | 6 ++++--
>> include/qemu/cpu.h | 10 ++++++++++
>> kvm-all.c | 4 +++-
>> qemu-common.h | 1 -
>> target-i386/kvm.c | 6 ++++--
>> 7 Dateien geändert, 27 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 750a76f..849ea8a 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -638,9 +638,10 @@ void qemu_init_cpu_loop(void)
>>
>> void run_on_cpu(CPUArchState *env, void (*func)(void *data), void *data)
>> {
>> + CPUState *cpu = ENV_GET_CPU(env);
>> struct qemu_work_item wi;
>>
>> - if (qemu_cpu_is_self(env)) {
>> + if (qemu_cpu_is_self(cpu)) {
>> func(data);
>> return;
>> }
>> @@ -855,7 +856,7 @@ static void qemu_cpu_kick_thread(CPUArchState *env)
>> exit(1);
>> }
>> #else /* _WIN32 */
>> - if (!qemu_cpu_is_self(env)) {
>> + if (!qemu_cpu_is_self(cpu)) {
>> SuspendThread(cpu->hThread);
>> cpu_signal(0);
>> ResumeThread(cpu->hThread);
>> @@ -890,11 +891,8 @@ void qemu_cpu_kick_self(void)
>> #endif
>> }
>>
>> -int qemu_cpu_is_self(void *_env)
>> +bool qemu_cpu_is_self(CPUState *cpu)
>> {
>> - CPUArchState *env = _env;
>> - CPUState *cpu = ENV_GET_CPU(env);
>> -
>> return qemu_thread_is_self(cpu->thread);
>> }
>>
>> diff --git a/exec.c b/exec.c
>> index 7899042..e21be32 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1685,6 +1685,7 @@ static void cpu_unlink_tb(CPUArchState *env)
>> /* mask must never be zero, except for A20 change call */
>> static void tcg_handle_interrupt(CPUArchState *env, int mask)
>> {
>> + CPUState *cpu = ENV_GET_CPU(env);
> Is there any chance to get rid of expensive cast on this call path?
I've spent some time investigating this and it would be rather complicated:
* We can't change the argument to CPUState yet (pushing the
responsibility to the caller) since interrupt_request is being accessed
til the not-yet-committable end of original part 4 series, the TB is
being flushed, icount being handled, and it is being used for the first
if, so not movable to a deeper block.
* A pointer to this function (or the KVM version below) is stored as
cpu_interrupt_handler and accessed through cpu_interrupt(), which for
softmmu calls the handler and for *-user does something similar to the
TCG version. This function is being used in a lot of places, so adding a
CPUState argument in addition to the CPUArchState would touch a lot of
places for an interim solution.
* QOM makes no guarantees about subclass layout (.env placement), so
going from CPUState to CPUArchState involves quite some ugly #ifdef'fery
that I am trying to avoid, going only the I-know-my-type direction.
I'll therefore accept your Reviewed-by to apply this and will keep
working during the Soft Freeze on the follow-ups that allow changing
this and others completely over to CPUState. Alex has just given green
light for the ppc prerequisites so I can apply them together with xtensa
to qom-cpu and post the next batch if I haven't already.
Regards,
Andreas
>
>> int old_mask;
>>
>> old_mask = env->interrupt_request;
>> @@ -1694,7 +1695,7 @@ static void tcg_handle_interrupt(CPUArchState *env, int mask)
>> * If called from iothread context, wake the target cpu in
>> * case its halted.
>> */
>> - if (!qemu_cpu_is_self(env)) {
>> + if (!qemu_cpu_is_self(cpu)) {
>> qemu_cpu_kick(env);
>> return;
>> }
>> diff --git a/hw/apic.c b/hw/apic.c
>> index ccf2819..1b4cd2f 100644
>> --- a/hw/apic.c
>> +++ b/hw/apic.c
>> @@ -107,7 +107,7 @@ static void apic_sync_vapic(APICCommonState *s, int sync_type)
>> length = offsetof(VAPICState, enabled) - offsetof(VAPICState, isr);
>>
>> if (sync_type & SYNC_TO_VAPIC) {
>> - assert(qemu_cpu_is_self(&s->cpu->env));
>> + assert(qemu_cpu_is_self(CPU(s->cpu)));
>>
>> vapic_state.tpr = s->tpr;
>> vapic_state.enabled = 1;
>> @@ -363,10 +363,12 @@ static int apic_irq_pending(APICCommonState *s)
>> /* signal the CPU if an irq is pending */
>> static void apic_update_irq(APICCommonState *s)
>> {
>> + CPUState *cpu = CPU(s->cpu);
>> +
>> if (!(s->spurious_vec & APIC_SV_ENABLE)) {
>> return;
>> }
>> - if (!qemu_cpu_is_self(&s->cpu->env)) {
>> + if (!qemu_cpu_is_self(cpu)) {
>> cpu_interrupt(&s->cpu->env, CPU_INTERRUPT_POLL);
>> } else if (apic_irq_pending(s) > 0) {
>> cpu_interrupt(&s->cpu->env, CPU_INTERRUPT_HARD);
>> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
>> index ad706a6..7be983d 100644
>> --- a/include/qemu/cpu.h
>> +++ b/include/qemu/cpu.h
>> @@ -78,5 +78,15 @@ struct CPUState {
>> */
>> void cpu_reset(CPUState *cpu);
>>
>> +/**
>> + * qemu_cpu_is_self:
>> + * @cpu: The vCPU to check against.
>> + *
>> + * Checks whether the caller is executing on the vCPU thread.
>> + *
>> + * Returns: %true if called from @cpu's thread, %false otherwise.
>> + */
>> +bool qemu_cpu_is_self(CPUState *cpu);
>> +
>>
>> #endif
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 92a7137..db01aeb 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -854,9 +854,11 @@ static MemoryListener kvm_memory_listener = {
>>
>> static void kvm_handle_interrupt(CPUArchState *env, int mask)
>> {
>> + CPUState *cpu = ENV_GET_CPU(env);
>> +
>> env->interrupt_request |= mask;
>>
>> - if (!qemu_cpu_is_self(env)) {
>> + if (!qemu_cpu_is_self(cpu)) {
>> qemu_cpu_kick(env);
>> }
>> }
>> diff --git a/qemu-common.h b/qemu-common.h
>> index b54612b..2094742 100644
>> --- a/qemu-common.h
>> +++ b/qemu-common.h
>> @@ -326,7 +326,6 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id);
>> /* Unblock cpu */
>> void qemu_cpu_kick(void *env);
>> void qemu_cpu_kick_self(void);
>> -int qemu_cpu_is_self(void *env);
>>
>> /* work queue */
>> struct qemu_work_item {
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 5b18383..cf3d2f1 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -1552,9 +1552,10 @@ static int kvm_get_debugregs(CPUX86State *env)
>>
>> int kvm_arch_put_registers(CPUX86State *env, int level)
>> {
>> + CPUState *cpu = ENV_GET_CPU(env);
>> int ret;
>>
>> - assert(cpu_is_stopped(env) || qemu_cpu_is_self(env));
>> + assert(cpu_is_stopped(env) || qemu_cpu_is_self(cpu));
>>
>> ret = kvm_getput_regs(env, 1);
>> if (ret < 0) {
>> @@ -1609,9 +1610,10 @@ int kvm_arch_put_registers(CPUX86State *env, int level)
>>
>> int kvm_arch_get_registers(CPUX86State *env)
>> {
>> + CPUState *cpu = ENV_GET_CPU(env);
>> int ret;
>>
>> - assert(cpu_is_stopped(env) || qemu_cpu_is_self(env));
>> + assert(cpu_is_stopped(env) || qemu_cpu_is_self(cpu));
>>
>> ret = kvm_getput_regs(env, 0);
>> if (ret < 0) {
>> --
>> 1.7.10.4
>>
>>
> Reviewed-by: Igor Mammedov <imammedo@xxxxxxxxxx>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
