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

[Michal Necasek]

    Hi Alexander,

  Could you please describe your use case, or rather how we can 
reproduce the problem? This is not code we want to change without 
precisely understanding why it needs to change and testing the behavior.

  Also, why are you using the recompiler at all? On a typical system 
with hardware virtualization, it should be used very little or not at all.

      Regards,
         Michal

On 3/10/2017 11:52 AM, Alexander Boettcher wrote:
> 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,
>
>
>
> _______________________________________________
> vbox-dev mailing list
> vbox-dev at virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>