On 18.06.2020 19:14, Roger Pau Monné wrote:
> On Thu, Jun 18, 2020 at 05:12:17PM +0200, Jan Beulich wrote:
>> On 12.06.2020 17:56, Roger Pau Monne wrote:
>>> case PTSRC_ioapic:
>>> pt_vector = hvm_ioapic_assert(v->domain, irq, level);
>>> - if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
>>> - {
>>> - pt_vector = -1;
>>> - if ( level )
>>> + if ( pt_vector < 0 )
>>> + return pt_vector;
>>> +
>>> + break;
>>> + }
>>> +
>>> + ASSERT(pt_vector >= 0);
>>> + if ( !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
>>> + {
>>> + time_cb *cb = NULL;
>>> + void *cb_priv;
>>> +
>>> + /*
>>> + * Vector has been injected to a different vCPU, call pt_irq_fired
>>> and
>>> + * execute the callback, since the destination vCPU(s) won't call
>>> + * pt_intr_post for it.
>>
>> ... this isn't the only reason to come here. Beyond what the comment
>> says there is the hvm_domain_use_pirq() check in assert_gsi() which
>> would similarly result in the IRR bit not observed set here. At the
>> very least these cases want mentioning; I have to admit that I'm not
>> entirely clear yet whether your handling is correct for both, or
>> whether the information needs to be propagated into here.
>
> I always forget about that weird pirq stuff (and I'm refraining from
> using other adjectives) that we have for HVM.
>
> AFAICT vpt is already broken when trying to inject interrupts
> generated from it over an event channel. hvm_ioapic_assert will return
> whatever garbage is in the IO-APIC entry, which will likely not be
> initialized because the GSI is routed over an event channel.
>
> I really have no idea what hvm_ioapic_assert should return in that
> case, the event channel callback vector maybe?
>
> Maybe just returning -1 would be fine, a guest using this routing of
> pirqs over event channels shouldn't be using any of the emulated
> timers, and hence vpt is not required to be functional in that case?
I would guess(!) that -1 ought to be fine. But this whole thing
escapes me as well, so let's ask Stefano, who iirc was who
introduced this.
Jan
