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

Re: [Xen-devel] [PATCH v2] SVM: Add union intstat_t for offset 68h in vmcb struct

On 25/03/2020 15:22, Pu Wen wrote:
> On 2020/3/24 20:28, Andrew Cooper wrote:
>> Hmm - this field doesn't appear to be part of AVIC, which makes me
>> wonder what we're doing without it.
>> It appears to be a shadow copy of EFLAGS.IF which is only written on
>> vmexit, and never consumed, but this is based on Appendix B which is the
>> only reference I can find to the field at all.  Neither the
>> VMRUN/#VMEXIT descriptions discuss it at all.
>> Given its position next to the (ambiguous) INTERRUPT_SHADOW, it just
>> might actually distinguish the STI shadow from the MovSS shadow, but it
>> could only do that by not behaving as described, and being asymmetric
>> with EFLAGS.  I don't have time to investigate this right now.
>> We need the field described in Xen to set it appropriately for virtual
>> vmexit, but I think that is the extent of what we need to do.
> We encountered problem while running xen with new firmware which
> implement the bit[1] of the VMCB offset 68h: the DomU stopped when
> running seabios. We debugged the seabios code and found that the
> seabios hung after call16_int10().
> Then we debugged the xen code, and found the cause at this place in
> svm_get_interrupt_shadow():
>     if ( vmcb->interrupt_shadow )
>          intr_shadow |= HVM_INTR_SHADOW_MOV_SS | HVM_INTR_SHADOW_STI;
> the conditional is true if bit[1] is 1 even though bit[0] is zero.
> If just only use bit[0] in the conditional, the problem is solved, DomU
> will run successfully.

Oh - now you point this out, the issue is obvious.

The above content would make a far more informative commit message.  How
about extending the middle paragraph with:

"...part of interrupt_shadow, causing svm_get_interrupt_shadow() to
mistake the guest having interrupts enabled as being in an interrupt
shadow.  This has been observed to cause SeaBIOS to hang on boot."

or words to that effect.  The "it definitely breaks a guest" is the most
important piece of information here.

Do you happen to know call16_int10() was doing, exactly?  We've
presumably trapped for emulation to be using svm_get_interrupt_shadow()
in the first place.




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