On 28/11/2024 9:03 am, Roger Pau Monné wrote:
> On Thu, Nov 28, 2024 at 12:47:36AM +0000, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>> index 3363926b487b..98394ed26a52 100644
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -108,7 +108,7 @@ static void vlapic_error(struct vlapic *vlapic, unsigned
>> int errmask)
>> uint32_t esr;
>>
>> spin_lock_irqsave(&vlapic->esr_lock, flags);
>> - esr = vlapic_get_reg(vlapic, APIC_ESR);
>> + esr = vlapic->hw.pending_esr;
>> if ( (esr & errmask) != errmask )
>> {
>> uint32_t lvterr = vlapic_get_reg(vlapic, APIC_LVTERR);
>> @@ -127,7 +127,7 @@ static void vlapic_error(struct vlapic *vlapic, unsigned
>> int errmask)
>> errmask |= APIC_ESR_RECVILL;
>> }
>>
>> - vlapic_set_reg(vlapic, APIC_ESR, esr | errmask);
>> + vlapic->hw.pending_esr |= errmask;
>>
>> if ( inj )
>> vlapic_set_irq(vlapic, lvterr & APIC_VECTOR_MASK, 0);
> The SDM also contains:
>
> "This write also rearms the APIC error interrupt triggering
> mechanism."
>
> Where "this write" is a write to the ESR register.
Correct.
> My understanding
> is that the error vector will only be injected for the first reported
> error. I think the logic regarding whether to inject the lvterr vector
> needs to additionally be gated on whether vlapic->hw.pending_esr ==
> 0.
I think it's clumsy wording.
Bits being set mask subsequent LVTERR's of the same type. That's what
the "if ( (esr & errmask) != errmask )" guard is doing above.
What I think it's referring to is that writing APIC_ESR will zero
pending_esr and thus any subsequent error will cause LVTERR to deliver.
Having said all that, I can't find anything in the current SDM/APM which
states this. I think I need to go back to the older manuals.
~Andrew
