[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-4.14 6/8] x86/vpt: fix injection to remote vCPU



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.