| Message ID | 20230320101035.2214196-4-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: refactor the cpu-exec loop | expand |
On 20/3/23 11:10, Alex Bennée wrote: > We don't want to be polluting the core run loop code with target > specific handling, punt it to sysemu_ops where it belongs. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/hw/core/sysemu-cpu-ops.h | 5 +++++ > target/i386/cpu-internal.h | 1 + > accel/tcg/cpu-exec.c | 14 +++----------- > target/i386/cpu-sysemu.c | 12 ++++++++++++ > target/i386/cpu.c | 1 + > 5 files changed, 22 insertions(+), 11 deletions(-) > diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c > index 28115edf44..e545bf7590 100644 > --- a/target/i386/cpu-sysemu.c > +++ b/target/i386/cpu-sysemu.c > @@ -18,6 +18,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/main-loop.h" > #include "cpu.h" > #include "sysemu/xen.h" > #include "sysemu/whpx.h" Missing "hw/i386/apic.h" which declares apic_poll_irq() ... > @@ -310,6 +311,17 @@ void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > } > } > > +void x86_cpu_handle_halt(CPUState *cpu) > +{ > + if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { > + X86CPU *x86_cpu = X86_CPU(cpu); > + qemu_mutex_lock_iothread(); > + apic_poll_irq(x86_cpu->apic_state); ... used here. > + cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); > + qemu_mutex_unlock_iothread(); > + } > +} Otherwise, Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hi Alex, all, again, this moves TCG-only code to common code, no? Even if this happens to work, the idea is to avoid adding unneeded accel TCG code to a KVM-only binary. We need to keep in mind all dimensions when we do refactorings: user-mode vs sysemu, the architecture, the accel, in particular tcg, non-tcg (which could be not compiled in, built-in, or loaded as separate module). In many cases, testing with --disable-tcg --enable-kvm helps to avoid breakages, but it is possible also to move in unneeded code in a way that does not generate compile or link-time errors, so we need to be a bit alert to that. Ciao, C On 3/20/23 11:10, Alex Bennée wrote: > We don't want to be polluting the core run loop code with target > specific handling, punt it to sysemu_ops where it belongs. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/hw/core/sysemu-cpu-ops.h | 5 +++++ > target/i386/cpu-internal.h | 1 + > accel/tcg/cpu-exec.c | 14 +++----------- > target/i386/cpu-sysemu.c | 12 ++++++++++++ > target/i386/cpu.c | 1 + > 5 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h > index ee169b872c..c9d30172c4 100644 > --- a/include/hw/core/sysemu-cpu-ops.h > +++ b/include/hw/core/sysemu-cpu-ops.h > @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps { > * GUEST_PANICKED events. > */ > GuestPanicInformation* (*get_crash_info)(CPUState *cpu); > + /** > + * @handle_cpu_halt: Callback for special handling during cpu_handle_halt() > + * @cs: The CPUState > + */ > + void (*handle_cpu_halt)(CPUState *cpu); > /** > * @write_elf32_note: Callback for writing a CPU-specific ELF note to a > * 32-bit VM coredump. > diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h > index 9baac5c0b4..75b302fb33 100644 > --- a/target/i386/cpu-internal.h > +++ b/target/i386/cpu-internal.h > @@ -65,6 +65,7 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v, > void x86_cpu_apic_create(X86CPU *cpu, Error **errp); > void x86_cpu_apic_realize(X86CPU *cpu, Error **errp); > void x86_cpu_machine_reset_cb(void *opaque); > +void x86_cpu_handle_halt(CPUState *cs); > #endif /* !CONFIG_USER_ONLY */ > > #endif /* I386_CPU_INTERNAL_H */ > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index c815f2dbfd..5e5906e199 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -22,6 +22,7 @@ > #include "qapi/error.h" > #include "qapi/type-helpers.h" > #include "hw/core/tcg-cpu-ops.h" > +#include "hw/core/sysemu-cpu-ops.h" > #include "trace.h" > #include "disas/disas.h" > #include "exec/exec-all.h" > @@ -30,9 +31,6 @@ > #include "qemu/rcu.h" > #include "exec/log.h" > #include "qemu/main-loop.h" > -#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY) > -#include "hw/i386/apic.h" > -#endif > #include "sysemu/cpus.h" > #include "exec/cpu-all.h" > #include "sysemu/cpu-timers.h" > @@ -650,15 +648,9 @@ static inline bool cpu_handle_halt(CPUState *cpu) > { > #ifndef CONFIG_USER_ONLY > if (cpu->halted) { > -#if defined(TARGET_I386) > - if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { > - X86CPU *x86_cpu = X86_CPU(cpu); > - qemu_mutex_lock_iothread(); > - apic_poll_irq(x86_cpu->apic_state); > - cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); > - qemu_mutex_unlock_iothread(); > + if (cpu->cc->sysemu_ops->handle_cpu_halt) { > + cpu->cc->sysemu_ops->handle_cpu_halt(cpu); > } > -#endif /* TARGET_I386 */ > if (!cpu_has_work(cpu)) { > return true; > } > diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c > index 28115edf44..e545bf7590 100644 > --- a/target/i386/cpu-sysemu.c > +++ b/target/i386/cpu-sysemu.c > @@ -18,6 +18,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/main-loop.h" > #include "cpu.h" > #include "sysemu/xen.h" > #include "sysemu/whpx.h" > @@ -310,6 +311,17 @@ void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > } > } > > +void x86_cpu_handle_halt(CPUState *cpu) > +{ > + if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { > + X86CPU *x86_cpu = X86_CPU(cpu); > + qemu_mutex_lock_iothread(); > + apic_poll_irq(x86_cpu->apic_state); > + cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); > + qemu_mutex_unlock_iothread(); > + } > +} > + > GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs) > { > X86CPU *cpu = X86_CPU(cs); > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 6576287e5b..67027d28b0 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -7241,6 +7241,7 @@ static const struct SysemuCPUOps i386_sysemu_ops = { > .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug, > .asidx_from_attrs = x86_asidx_from_attrs, > .get_crash_info = x86_cpu_get_crash_info, > + .handle_cpu_halt = x86_cpu_handle_halt, > .write_elf32_note = x86_cpu_write_elf32_note, > .write_elf64_note = x86_cpu_write_elf64_note, > .write_elf32_qemunote = x86_cpu_write_elf32_qemunote,
On 20/3/23 16:23, Claudio Fontana wrote: > Hi Alex, all, > > again, this moves TCG-only code to common code, no? Oh, good point. > Even if this happens to work, the idea is to avoid adding unneeded accel TCG code to a KVM-only binary. Could yet another AccelSysemuCPUOps *accel struct in SysemuCPUOps help being stricter? ... > We need to keep in mind all dimensions when we do refactorings: > > user-mode vs sysemu, > the architecture, > the accel, in particular tcg, non-tcg (which could be not compiled in, built-in, or loaded as separate module). > > In many cases, testing with --disable-tcg --enable-kvm helps to avoid breakages, > but it is possible also to move in unneeded code in a way that does not generate compile or link-time errors, so we need to be a bit alert to that. > > Ciao, > > C > > > On 3/20/23 11:10, Alex Bennée wrote: >> We don't want to be polluting the core run loop code with target >> specific handling, punt it to sysemu_ops where it belongs. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/hw/core/sysemu-cpu-ops.h | 5 +++++ >> target/i386/cpu-internal.h | 1 + >> accel/tcg/cpu-exec.c | 14 +++----------- >> target/i386/cpu-sysemu.c | 12 ++++++++++++ >> target/i386/cpu.c | 1 + >> 5 files changed, 22 insertions(+), 11 deletions(-) >> >> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h >> index ee169b872c..c9d30172c4 100644 >> --- a/include/hw/core/sysemu-cpu-ops.h >> +++ b/include/hw/core/sysemu-cpu-ops.h >> @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps { >> * GUEST_PANICKED events. >> */ >> GuestPanicInformation* (*get_crash_info)(CPUState *cpu); >> + /** >> + * @handle_cpu_halt: Callback for special handling during cpu_handle_halt() >> + * @cs: The CPUState >> + */ Perhaps insert within a 'tcg' structure for now. #ifdef CONFIG_TCG struct { >> + void (*handle_cpu_halt)(CPUState *cpu); } tcg; #endif Then we could extract as accel. >> /** >> * @write_elf32_note: Callback for writing a CPU-specific ELF note to a >> * 32-bit VM coredump.
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 20/3/23 11:10, Alex Bennée wrote: >> We don't want to be polluting the core run loop code with target >> specific handling, punt it to sysemu_ops where it belongs. >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/hw/core/sysemu-cpu-ops.h | 5 +++++ >> target/i386/cpu-internal.h | 1 + >> accel/tcg/cpu-exec.c | 14 +++----------- >> target/i386/cpu-sysemu.c | 12 ++++++++++++ >> target/i386/cpu.c | 1 + >> 5 files changed, 22 insertions(+), 11 deletions(-) > > >> diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c >> index 28115edf44..e545bf7590 100644 >> --- a/target/i386/cpu-sysemu.c >> +++ b/target/i386/cpu-sysemu.c >> @@ -18,6 +18,7 @@ >> */ >> #include "qemu/osdep.h" >> +#include "qemu/main-loop.h" >> #include "cpu.h" >> #include "sysemu/xen.h" >> #include "sysemu/whpx.h" > > Missing "hw/i386/apic.h" which declares apic_poll_irq() ... I really need to get to the bottom of why some build hosts get away without the include. Some additional path through the include maze driven by config-host.h? > > >> @@ -310,6 +311,17 @@ void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) >> } >> } >> +void x86_cpu_handle_halt(CPUState *cpu) >> +{ >> + if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { >> + X86CPU *x86_cpu = X86_CPU(cpu); >> + qemu_mutex_lock_iothread(); >> + apic_poll_irq(x86_cpu->apic_state); > > ... used here. > >> + cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); >> + qemu_mutex_unlock_iothread(); >> + } >> +} > > Otherwise, > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Claudio Fontana <cfontana@suse.de> writes: > Hi Alex, all, > > again, this moves TCG-only code to common code, no? > > Even if this happens to work, the idea is to avoid adding unneeded accel TCG code to a KVM-only binary. > > We need to keep in mind all dimensions when we do refactorings: > > user-mode vs sysemu, > the architecture, > the accel, in particular tcg, non-tcg (which could be not compiled in, > built-in, or loaded as separate module). > > In many cases, testing with --disable-tcg --enable-kvm helps to avoid breakages, > but it is possible also to move in unneeded code in a way that does > not generate compile or link-time errors, so we need to be a bit alert > to that. You are right of course, it should be kept tcg only. I guess using tcgops instead of sysemu_ops. > > Ciao, > > C > > > On 3/20/23 11:10, Alex Bennée wrote: >> We don't want to be polluting the core run loop code with target >> specific handling, punt it to sysemu_ops where it belongs. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/hw/core/sysemu-cpu-ops.h | 5 +++++ >> target/i386/cpu-internal.h | 1 + >> accel/tcg/cpu-exec.c | 14 +++----------- >> target/i386/cpu-sysemu.c | 12 ++++++++++++ >> target/i386/cpu.c | 1 + >> 5 files changed, 22 insertions(+), 11 deletions(-) >> >> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h >> index ee169b872c..c9d30172c4 100644 >> --- a/include/hw/core/sysemu-cpu-ops.h >> +++ b/include/hw/core/sysemu-cpu-ops.h >> @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps { >> * GUEST_PANICKED events. >> */ >> GuestPanicInformation* (*get_crash_info)(CPUState *cpu); >> + /** >> + * @handle_cpu_halt: Callback for special handling during cpu_handle_halt() >> + * @cs: The CPUState >> + */ >> + void (*handle_cpu_halt)(CPUState *cpu); >> /** >> * @write_elf32_note: Callback for writing a CPU-specific ELF note to a >> * 32-bit VM coredump. >> diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h >> index 9baac5c0b4..75b302fb33 100644 >> --- a/target/i386/cpu-internal.h >> +++ b/target/i386/cpu-internal.h >> @@ -65,6 +65,7 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v, >> void x86_cpu_apic_create(X86CPU *cpu, Error **errp); >> void x86_cpu_apic_realize(X86CPU *cpu, Error **errp); >> void x86_cpu_machine_reset_cb(void *opaque); >> +void x86_cpu_handle_halt(CPUState *cs); >> #endif /* !CONFIG_USER_ONLY */ >> >> #endif /* I386_CPU_INTERNAL_H */ >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index c815f2dbfd..5e5906e199 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -22,6 +22,7 @@ >> #include "qapi/error.h" >> #include "qapi/type-helpers.h" >> #include "hw/core/tcg-cpu-ops.h" >> +#include "hw/core/sysemu-cpu-ops.h" >> #include "trace.h" >> #include "disas/disas.h" >> #include "exec/exec-all.h" >> @@ -30,9 +31,6 @@ >> #include "qemu/rcu.h" >> #include "exec/log.h" >> #include "qemu/main-loop.h" >> -#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY) >> -#include "hw/i386/apic.h" >> -#endif >> #include "sysemu/cpus.h" >> #include "exec/cpu-all.h" >> #include "sysemu/cpu-timers.h" >> @@ -650,15 +648,9 @@ static inline bool cpu_handle_halt(CPUState *cpu) >> { >> #ifndef CONFIG_USER_ONLY >> if (cpu->halted) { >> -#if defined(TARGET_I386) >> - if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { >> - X86CPU *x86_cpu = X86_CPU(cpu); >> - qemu_mutex_lock_iothread(); >> - apic_poll_irq(x86_cpu->apic_state); >> - cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); >> - qemu_mutex_unlock_iothread(); >> + if (cpu->cc->sysemu_ops->handle_cpu_halt) { >> + cpu->cc->sysemu_ops->handle_cpu_halt(cpu); >> } >> -#endif /* TARGET_I386 */ >> if (!cpu_has_work(cpu)) { >> return true; >> } >> diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c >> index 28115edf44..e545bf7590 100644 >> --- a/target/i386/cpu-sysemu.c >> +++ b/target/i386/cpu-sysemu.c >> @@ -18,6 +18,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qemu/main-loop.h" >> #include "cpu.h" >> #include "sysemu/xen.h" >> #include "sysemu/whpx.h" >> @@ -310,6 +311,17 @@ void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) >> } >> } >> >> +void x86_cpu_handle_halt(CPUState *cpu) >> +{ >> + if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { >> + X86CPU *x86_cpu = X86_CPU(cpu); >> + qemu_mutex_lock_iothread(); >> + apic_poll_irq(x86_cpu->apic_state); >> + cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); >> + qemu_mutex_unlock_iothread(); >> + } >> +} >> + >> GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs) >> { >> X86CPU *cpu = X86_CPU(cs); >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 6576287e5b..67027d28b0 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -7241,6 +7241,7 @@ static const struct SysemuCPUOps i386_sysemu_ops = { >> .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug, >> .asidx_from_attrs = x86_asidx_from_attrs, >> .get_crash_info = x86_cpu_get_crash_info, >> + .handle_cpu_halt = x86_cpu_handle_halt, >> .write_elf32_note = x86_cpu_write_elf32_note, >> .write_elf64_note = x86_cpu_write_elf64_note, >> .write_elf32_qemunote = x86_cpu_write_elf32_qemunote,
On 3/20/23 16:34, Philippe Mathieu-Daudé wrote: > On 20/3/23 16:23, Claudio Fontana wrote: >> Hi Alex, all, >> >> again, this moves TCG-only code to common code, no? > > Oh, good point. > >> Even if this happens to work, the idea is to avoid adding unneeded accel TCG code to a KVM-only binary. > > Could yet another AccelSysemuCPUOps *accel struct in SysemuCPUOps > help being stricter? ... Just a thought, in general I wonder if we could devise a less error prone way to keep things in the right place. Just thinking out loud here, something like a QEMU_ATTRIBUTE_TCG, _KVM, ... to add to symbols to avoid ending up in the wrong binary. Keeping in mind all these dimensions is probably very taxing, maybe getting some support from the build system would be beneficial, checking that a build requested with specific features contains only compatible objects. Any ideas? Ciao, C > >> We need to keep in mind all dimensions when we do refactorings: >> >> user-mode vs sysemu, >> the architecture, >> the accel, in particular tcg, non-tcg (which could be not compiled in, built-in, or loaded as separate module). >> >> In many cases, testing with --disable-tcg --enable-kvm helps to avoid breakages, >> but it is possible also to move in unneeded code in a way that does not generate compile or link-time errors, so we need to be a bit alert to that. >> >> Ciao, >> >> C >> >> >> On 3/20/23 11:10, Alex Bennée wrote: >>> We don't want to be polluting the core run loop code with target >>> specific handling, punt it to sysemu_ops where it belongs. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> include/hw/core/sysemu-cpu-ops.h | 5 +++++ >>> target/i386/cpu-internal.h | 1 + >>> accel/tcg/cpu-exec.c | 14 +++----------- >>> target/i386/cpu-sysemu.c | 12 ++++++++++++ >>> target/i386/cpu.c | 1 + >>> 5 files changed, 22 insertions(+), 11 deletions(-) >>> >>> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h >>> index ee169b872c..c9d30172c4 100644 >>> --- a/include/hw/core/sysemu-cpu-ops.h >>> +++ b/include/hw/core/sysemu-cpu-ops.h >>> @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps { >>> * GUEST_PANICKED events. >>> */ >>> GuestPanicInformation* (*get_crash_info)(CPUState *cpu); >>> + /** >>> + * @handle_cpu_halt: Callback for special handling during cpu_handle_halt() >>> + * @cs: The CPUState >>> + */ > > Perhaps insert within a 'tcg' structure for now. > > #ifdef CONFIG_TCG > struct { > >>> + void (*handle_cpu_halt)(CPUState *cpu); > > } tcg; > #endif > > Then we could extract as accel. > >>> /** >>> * @write_elf32_note: Callback for writing a CPU-specific ELF note to a >>> * 32-bit VM coredump. > >
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h index ee169b872c..c9d30172c4 100644 --- a/include/hw/core/sysemu-cpu-ops.h +++ b/include/hw/core/sysemu-cpu-ops.h @@ -48,6 +48,11 @@ typedef struct SysemuCPUOps { * GUEST_PANICKED events. */ GuestPanicInformation* (*get_crash_info)(CPUState *cpu); + /** + * @handle_cpu_halt: Callback for special handling during cpu_handle_halt() + * @cs: The CPUState + */ + void (*handle_cpu_halt)(CPUState *cpu); /** * @write_elf32_note: Callback for writing a CPU-specific ELF note to a * 32-bit VM coredump. diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h index 9baac5c0b4..75b302fb33 100644 --- a/target/i386/cpu-internal.h +++ b/target/i386/cpu-internal.h @@ -65,6 +65,7 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v, void x86_cpu_apic_create(X86CPU *cpu, Error **errp); void x86_cpu_apic_realize(X86CPU *cpu, Error **errp); void x86_cpu_machine_reset_cb(void *opaque); +void x86_cpu_handle_halt(CPUState *cs); #endif /* !CONFIG_USER_ONLY */ #endif /* I386_CPU_INTERNAL_H */ diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index c815f2dbfd..5e5906e199 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -22,6 +22,7 @@ #include "qapi/error.h" #include "qapi/type-helpers.h" #include "hw/core/tcg-cpu-ops.h" +#include "hw/core/sysemu-cpu-ops.h" #include "trace.h" #include "disas/disas.h" #include "exec/exec-all.h" @@ -30,9 +31,6 @@ #include "qemu/rcu.h" #include "exec/log.h" #include "qemu/main-loop.h" -#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY) -#include "hw/i386/apic.h" -#endif #include "sysemu/cpus.h" #include "exec/cpu-all.h" #include "sysemu/cpu-timers.h" @@ -650,15 +648,9 @@ static inline bool cpu_handle_halt(CPUState *cpu) { #ifndef CONFIG_USER_ONLY if (cpu->halted) { -#if defined(TARGET_I386) - if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { - X86CPU *x86_cpu = X86_CPU(cpu); - qemu_mutex_lock_iothread(); - apic_poll_irq(x86_cpu->apic_state); - cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); - qemu_mutex_unlock_iothread(); + if (cpu->cc->sysemu_ops->handle_cpu_halt) { + cpu->cc->sysemu_ops->handle_cpu_halt(cpu); } -#endif /* TARGET_I386 */ if (!cpu_has_work(cpu)) { return true; } diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c index 28115edf44..e545bf7590 100644 --- a/target/i386/cpu-sysemu.c +++ b/target/i386/cpu-sysemu.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "cpu.h" #include "sysemu/xen.h" #include "sysemu/whpx.h" @@ -310,6 +311,17 @@ void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) } } +void x86_cpu_handle_halt(CPUState *cpu) +{ + if (cpu->interrupt_request & CPU_INTERRUPT_POLL) { + X86CPU *x86_cpu = X86_CPU(cpu); + qemu_mutex_lock_iothread(); + apic_poll_irq(x86_cpu->apic_state); + cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL); + qemu_mutex_unlock_iothread(); + } +} + GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 6576287e5b..67027d28b0 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -7241,6 +7241,7 @@ static const struct SysemuCPUOps i386_sysemu_ops = { .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug, .asidx_from_attrs = x86_asidx_from_attrs, .get_crash_info = x86_cpu_get_crash_info, + .handle_cpu_halt = x86_cpu_handle_halt, .write_elf32_note = x86_cpu_write_elf32_note, .write_elf64_note = x86_cpu_write_elf64_note, .write_elf32_qemunote = x86_cpu_write_elf32_qemunote,
We don't want to be polluting the core run loop code with target specific handling, punt it to sysemu_ops where it belongs. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- include/hw/core/sysemu-cpu-ops.h | 5 +++++ target/i386/cpu-internal.h | 1 + accel/tcg/cpu-exec.c | 14 +++----------- target/i386/cpu-sysemu.c | 12 ++++++++++++ target/i386/cpu.c | 1 + 5 files changed, 22 insertions(+), 11 deletions(-)