Re: [Xen-devel] [PATCH v4 4/7] x86/HVM: implement memory read caching for insn emulation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.02.2020 20:48, Andrew Cooper wrote:
> On 31/01/2020 16:44, Jan Beulich wrote:
>> TBD: In principle the caching here yields unnecessary the one used for
>>      insn bytes (vio->mmio_insn{,_bytes}. However, to seed the cache
>>      with the data SVM may have made available, we'd have to also know
>>      the corresponding GPA. It's not safe, however, to re-walk the page
>>      tables to find out, as the page tables may have changed in the
>>      meantime. Therefore I guess we need to keep the duplicate
>>      functionality for now. A possible solution to this could be to use
>>      a physical-address-based cache for page table accesses (and looking
>>      forward also e.g. SVM/VMX insn emulation), and a linear-address-
>>      based one for all other reads.
> 
> Splitting caching like that will re-introduce the same bugs I pointed
> out in earlier revisions of this series.  It is not correct to have
> multiple (and therefore, non-coherent) caches of memory.
> 
> The AMD instruction stream bytes support is by no means perfect either. 
> It doesn't function properly when SMAP is active (Erratum #1096), which
> is actually caused by the instruction stream being re-fetched at vmexit
> time, with a data side access (hence the interaction with SMAP).

I've looked into this some more, and I'm afraid the text is too
ambiguous to draw conclusions as to possible actions on our
part. It mentions a "GuestInstrBytes field of the VMCB", but the
PM doesn't use such naming afaics. Hence it's not clear whether
the entire 16-byte block is meant, or just the high 120 bits of
it. In the former case we're fine, but in the latter case we'd
mistakenly use the zeros and interpret them as "add %al, (%rax)"
(in 64-bit guest mode, different memory address in others), and
hence we'd need to suppress setting or use of
v->arch.hvm.svm.cached_insn_len. (The used wording comes closer
to the latter case, with the VMCB field at 0xD0 described as
"Number of bytes fetched" and then "Guest instruction bytes".)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel