On 06/04/2023 10:31 am, Jan Beulich wrote:
> On 05.04.2023 22:44, Andrew Cooper wrote:
>> This removes raw number manipulation, and makes the logic easier to follow.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
>
> One remark though:
>
>> --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>> +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>> @@ -450,6 +450,11 @@ struct vmcb_struct {
>>
>> uint64_t nrip;
>> } io;
>> + struct {
>> + uint64_t gpr:4;
>> + uint64_t :59;
>> + bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc
>> */
>> + } mov;
> The field being named just "mov" makes it apparently applicable to DRn
> moves, too (and the title supports this), yet the top bit doesn't have
> this meaning there. So perhaps say "MOV-CR" (or alike) in the comment?
Hmm - I'd not spotted that distinction.
Xen never decodes the exitinfo for a DR access - we just resync dr
state, drop the intercept and reenter the guest.
Therefore I think it would be better to rename "mov" to "mov_cr" so you
can't use the mov_insn field in a context that plausibly looks like a dr
access.
Thoughts?
~Andrew
