Re: [PATCH for-4.14 3/8] x86/hvm: fix ISA IRQ 0 handling when set as lowest priority mode in IO APIC

From: Jan Beulich <jbeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	paul@xen.org
Subject: Re: [PATCH for-4.14 3/8] x86/hvm: fix ISA IRQ 0 handling when set as lowest priority mode in IO APIC
Date: Thu, 18 Jun 2020 16:26:08 +0200	[thread overview]
Message-ID: <ec8e6328-59d6-8f6e-53db-dc6410897c2e@suse.com> (raw)
In-Reply-To: <20200612155640.4101-4-roger.pau@citrix.com>

On 12.06.2020 17:56, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -422,12 +422,13 @@ static void vioapic_deliver(struct hvm_vioapic *vioapic, unsigned int pin)
>      case dest_LowestPrio:
>      {
>  #ifdef IRQ0_SPECIAL_ROUTING
> -        /* Force round-robin to pick VCPU 0 */
> -        if ( (irq == hvm_isa_irq_to_gsi(0)) && pit_channel0_enabled() )
> -        {
> -            v = d->vcpu ? d->vcpu[0] : NULL;
> -            target = v ? vcpu_vlapic(v) : NULL;
> -        }
> +        struct vlapic *lapic0 = vcpu_vlapic(d->vcpu[0]);
> +
> +        /* Force to pick vCPU 0 if part of the destination list */
> +        if ( (irq == hvm_isa_irq_to_gsi(0)) && pit_channel0_enabled() &&
> +             vlapic_match_dest(lapic0, NULL, 0, dest, dest_mode) &&
> +             vlapic_enabled(lapic0) )

The vlapic_enabled() part needs justification in the commit message
(if it is to stay), the more that the other path that patch 2 touched
doesn't have / gain it. I'm unconvinced this is a helpful check here
(or anywhere when it's not current's LAPIC that gets probed), as its
result may be stale right after probing.

Having thought about this (including patch 2) some more, I also wonder
whether, if no destination match was found, the IRQ0_SPECIAL_ROUTING
hack should become to nevertheless deliver to CPU0.

Jan