On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote:
> On 2011-01-31 14:04, Jan Kiszka wrote:
> > On 2011-01-31 12:36, Jan Kiszka wrote:
> >> On 2011-01-31 11:08, Avi Kivity wrote:
> >>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
> >>>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
> >>>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
> >>>> this service processes will first cause an exit from kvm_cpu_exec
> >>>> anyway. And we will have to reenter the kernel on IO exits
> >>>> unconditionally, something that the current logic prevents.
> >>>>
> >>>> Signed-off-by: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
> >>>> ---
> >>>> kvm-all.c | 11 ++++++-----
> >>>> 1 files changed, 6 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/kvm-all.c b/kvm-all.c
> >>>> index 5bfa8c0..46ecc1c 100644
> >>>> --- a/kvm-all.c
> >>>> +++ b/kvm-all.c
> >>>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
> >>>>
> >>>> DPRINTF("kvm_cpu_exec()\n");
> >>>>
> >>>> + if (kvm_arch_process_irqchip_events(env)) {
> >>>> + env->exit_request = 0;
> >>>> + env->exception_index = EXCP_HLT;
> >>>> + return 0;
> >>>> + }
> >>>> +
> >>>> do {
> >>>> #ifndef CONFIG_IOTHREAD
> >>>> if (env->exit_request) {
> >>>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
> >>>> }
> >>>
> >>> We check for ->exit_request here
> >>>
> >>>> #endif
> >>>>
> >>>> - if (kvm_arch_process_irqchip_events(env)) {
> >>>> - ret = 0;
> >>>> - break;
> >>>> - }
> >>>> -
> >>>
> >>> But this checks for ->interrupt_request. What ensures that we exit when
> >>> ->interrupt_request is set?
> >>
> >> Good question, need to check again. But if that turns out to be an
> >> issue, qemu-kvm would be broken as well. I'm just aligning the code here.
> >>
> >
> > The only thing we miss by moving process_irqchip_events is a self-INIT
> > of an AP - if such thing exists in real life. In that case, the AP would
> > cause a reset of itself, followed by a transition to HALT state.
>
> I checked again with the Intel spec, and a self-INIT is invalid (at
> least when specified via shorthand). So I'm under the impression now
> that we can safely ignore this case and leave the patch as is.
>
> Any different views?
>
IIRC if you don't use shorthand you can send INIT to self.
--
Gleb.
--
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
