Re: [Xen-devel] guest being crashed from viridian_start_apic_assist()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.06.16 at 14:11, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 03 June 2016 10:59
>> To: Paul Durrant
>> Cc: xen-devel
>> Subject: RE: guest being crashed from viridian_start_apic_assist()
>> 
>> >>> On 03.06.16 at 11:39, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: 03 June 2016 10:04
>> >>
>> >> to test a guest ACPI change I went through all Windows versions that
>> >> I have installed anywhere, to find 32-bit Win7 dying (the other
>> >> variants I tried worked fine), albeit retrying a couple of times I then
>> >> didn't see this a second time. Is this something you've observed
>> >> elsewhere? The APIC assist logic isn't that complicated, and I can't
>> >> really spot any race in there...
>> >
>> > How did it die?
>> 
>> From the single domain_crash() in that function. No other information
>> was available (apart from the register dump, which doesn't look like
>> it would be very useful for analysis here).
>> 
> 
> There is an inconsistency between the test to abort a previous assist, which 
> is based on:
> 
>     irr = vlapic_find_highest_irr(vlapic);
> 
>     isr = vlapic_find_highest_isr(vlapic);
>     if ( (isr & 0xf0) >= (irr & 0xf0) )
> 
> and the test to start a new one:
> 
>     isr = vlapic_find_lowest_vector(&vlapic->regs->data[APIC_ISR]);
>     if ( isr >= 0 && isr < vector )
> 
> I suspect that may be the problem you hit.

I don't think so. Of the second if(), the right side could actually be
dropped, as the logic in vlapic_has_pending_irq() guarantees
(vector & 0xf0) > (highest_isr & 0xf0), i.e. vector > highest_isr
and hence necessarily vector > lowest_isr. That is, if
vlapic_has_pending_irq() finds _some_ ISR bit set, it must be one
below vector. Which means viridian_start_apic_assist() gets called
solely when there's _no_ other ISR bit set.

What I find more problematic looking at those functions (but
unrelated to the problem here afaict) is the
vlapic_virtual_intr_delivery_enabled() related logic and its
interaction with the Viridian APIC assist (leaving aside nested
mode for the moment): vlapic_has_pending_irq() won't do
anything with the APIC assist in that case, but if force_ack is
true in vlapic_ack_pending_irq() there will be an interaction.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel