Re: [PATCH] Search the LAPIC's for one that will accept a PIC interrupt.

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jun 21, 2010 at 11:29:40AM -0400, Chris Lalancette wrote:
> Older versions of 32-bit linux have a "Checking 'hlt' instruction"
> test where they repeatedly call the 'hlt' instruction, and then
> expect a timer interrupt to kick the CPU out of halt.  This happens
> before any LAPIC or IOAPIC setup happens, which means that all of
> the APIC's are in virtual wire mode at this point.  Unfortunately,
> the current implementation of virtual wire mode is hardcoded to
> only kick the BSP, so if a crash+kexec occurs on a different
> vcpu, it will never get kicked.
> 
> This patch makes pic_unlock() do the equivalent of
> kvm_irq_delivery_to_apic() for the IOAPIC code.  That is, it runs
> through all of the vcpus looking for one that is in virtual wire
> mode.  In the normal case where LAPICs and IOAPICs are configured,
> this won't be used at all.  In the bootstrap phase of a modern
> OS, before the LAPICs and IOAPICs are configured, this will have
> exactly the same behavior as today; VCPU0 is always looked at
> first, so it will always get out of the loop after the first
> iteration.  This will only go through the loop more than once
> during a kexec/kdump, in which case it will only do it a few times
> until the kexec'ed kernel programs the LAPIC and IOAPIC.
> 
> Signed-off-by: Chris Lalancette <clalance@redhat.com>
> ---
>  arch/x86/kvm/i8259.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index 2c73f44..85ecabc 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -44,16 +44,25 @@ static void pic_unlock(struct kvm_pic *s)
>  	__releases(&s->lock)
>  {
>  	bool wakeup = s->wakeup_needed;
> -	struct kvm_vcpu *vcpu;
> +	struct kvm_vcpu *vcpu, *found = NULL;
> +	int i;
>  
>  	s->wakeup_needed = false;
>  
>  	raw_spin_unlock(&s->lock);
>  
>  	if (wakeup) {
> -		vcpu = s->kvm->bsp_vcpu;
> -		if (vcpu)
> -			kvm_vcpu_kick(vcpu);
> +		kvm_for_each_vcpu(i, vcpu, s->kvm) {
> +			if (kvm_apic_accept_pic_intr(vcpu)) {
> +				found = vcpu;
> +				break;
> +			}
> +		}
Shouldn't we kick all vcpus that are in virtual write mode, not just
first one found?

> +
> +		if (!found)
> +			found = s->kvm->bsp_vcpu;
> +
> +		kvm_vcpu_kick(found);
>  	}
>  }
>  
> -- 
> 1.6.6.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.