[vbox-dev] Question regarding - tr.flags and TSS_BUSY

[Alexander Boettcher]

Hello,

in src/recompiler/VBoxRecompiler.c the following code snippet

2352     /*
2353      * Sync TR unconditionally to make life simpler.
2354      */
2355     pVM->rem.s.Env.tr.selector    = pCtx->tr.Sel;
2356     pVM->rem.s.Env.tr.newselector = 0;
2357     pVM->rem.s.Env.tr.fVBoxFlags  = pCtx->tr.fFlags;
2358     pVM->rem.s.Env.tr.base        = pCtx->tr.u64Base;
2359     pVM->rem.s.Env.tr.limit       = pCtx->tr.u32Limit;
2360     pVM->rem.s.Env.tr.flags       = (pCtx->tr.Attr.u &
SEL_FLAGS_SMASK) << SEL_FLAGS_SHIFT;
2361     /* Note! do_interrupt will fault if the busy flag is still
set... */ /** @todo so fix do_interrupt then! */
2362     pVM->rem.s.Env.tr.flags      &= ~DESC_TSS_BUSY_MASK;
2363

un-set the TSS BUSY flag unconditionally.

Later on there are two code snippet (showing only one, since they are
very similar)

2910     if (    pCtx->tr.Sel      != pVM->rem.s.Env.tr.selector
2911         ||  pCtx->tr.ValidSel != pVM->rem.s.Env.tr.selector
2912         ||  pCtx->tr.u64Base  != pVM->rem.s.Env.tr.base
2913         ||  pCtx->tr.u32Limit != pVM->rem.s.Env.tr.limit
2914             /* Qemu and AMD/Intel have different ideas about the
busy flag ... */
2915         ||  pCtx->tr.Attr.u   != (  (pVM->rem.s.Env.tr.flags >>
SEL_FLAGS_SHIFT) & (SEL_FLAGS_SMASK & ~DESC_INTEL_UNUSABLE)
2916                                   ? (pVM->rem.s.Env.tr.flags |
DESC_TSS_BUSY_MASK) >> SEL_FLAGS_SHIFT
2917                                   : 0)
2918         ||  !(pCtx->tr.fFlags & CPUMSELREG_FLAGS_VALID)
2919        )
2920     {

where it is checked whether the tr register content changed when leaving
the REM compiler. For the tr.Attr.u check the DESC_TSS_BUSY_MASK flag is
set ever (in the one case, LINE 2916), even when it was not set when it
entered the REM compiler.

This leads in one of our use cases to the phenomena, that the tr
register was not changed by the REM, but the pCtx->tr vs
pVM->rem.s.ENv.tr comparison as shown above concluded that the tr
register was changed. Because of the result the code enters the body of
the if statement and there set wrongly the TSS_BUSY bit.

This leads later on in our use-case to an invalid tss segment error.

With the following patch attached, we adjust the comparison to also
account if the BUSY_BIT actually was really set when it entered the REM
compiler. Due to the patch the comparison now detects the cases where
the tr register was not changed and let it stay intact, which solves the
issue about invalid tss segments for us.

What do you think, makes the observation/fix sense to you or maybe we
overlooked something ?

The issue with the invalid tss segments happens for us in 4.3.40 as in
5.1.14, the patch applies to both versions.

Cheers,

-- 
Alexander Boettcher
Genode Labs

http://www.genode-labs.com - http://www.genode.org

Genode Labs GmbH - Amtsgericht Dresden - HRB 28424 - Sitz Dresden
Geschäftsführer: Dr.-Ing. Norman Feske, Christian Helmuth
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rem_tss.patch
Type: text/x-patch
Size: 1936 bytes
Desc: not available
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20170310/374a8d6c/attachment.bin>