Re: [PATCH v2 4/7] x86/vmx: add do_vmtrace_op

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.06.2020 04:56, Michał Leszczyński wrote:
> ----- 19 cze 2020 o 1:41, Michał Leszczyński michal.leszczynski@xxxxxxx 
> napisał(a):
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -508,11 +508,25 @@ static void vmx_restore_host_msrs(void)
>>
>> static void vmx_save_guest_msrs(struct vcpu *v)
>> {
>> +    uint64_t rtit_ctl;
>> +
>>     /*
>>      * We cannot cache SHADOW_GS_BASE while the VCPU runs, as it can
>>      * be updated at any time via SWAPGS, which we cannot trap.
>>      */
>>     v->arch.hvm.vmx.shadow_gs = rdgsshadow();
>> +
>> +    if ( unlikely(v->arch.hvm.vmx.ipt_state &&
>> v->arch.hvm.vmx.ipt_state->active) )
>> +    {
>> +        smp_rmb();
>> +        rdmsrl(MSR_RTIT_CTL, rtit_ctl);
>> +
>> +        if ( rtit_ctl & RTIT_CTL_TRACEEN )
>> +            BUG();
>> +
>> +        rdmsrl(MSR_RTIT_STATUS, v->arch.hvm.vmx.ipt_state->status);
>> +        rdmsrl(MSR_RTIT_OUTPUT_MASK,
>> v->arch.hvm.vmx.ipt_state->output_mask.raw);
>> +    }
>> }
> 
> 
> It was suggested to move this piece of code from vm-entry/vm-exit handling to
> vmx_save_guest_msrs/vmx_restore_guest_msrs functions.
> 
> Please note that we do need to periodically read MSR_RTIT_OUTPUT_MASK in order
> to know how much data was written into the buffer by the processor. During 
> tests,
> I've spotted that in some cases vCPUs get out of scope very rarely.
> 
> For instance: when IPT gets enabled, xc_vmtrace_ipt_get_offset() is returning 
> zero
> value for the first few seconds, because it's relying on the value of
> v->arch.hvm.vmx.ipt_state->output_mask which we only read in 
> vmx_save_guest_msrs()
> and in some cases this occurs very rarely.
> 
> Could you guys suggest some solution to the problem? If we would leave this 
> value
> dirty in hardware, AFAIK we have no possibility to directly access it,
> but observing this particular value is very important in the runtime.

Much depends on what state the vCPU in question is in when you need
to "periodically read" the value. If it's paused, you may need to
invoke sync_vcpu_execstate(). If it's actively running you can't
get at the value anyway except when you're on the CPU that this vCPU
is running on (and then you can RDMSR it quite easily). Any
intermediate state between paused and running is prone to change
immediately after you've established it anyway, so you need to get
the vCPU into one of the two "stable" states in order to get at
the register.

Also (and I think I said so before) - please trim your replies.

Jan