On Thu, Jan 28, 2021, Paolo Bonzini wrote:

On 28/01/21 18:56, Sean Christopherson wrote:

...

On Thu, Jan 28, 2021, Paolo Bonzini wrote:

...

• 	vmx->guest_uret_msrs[j].mask = ~(u64)TSX_CTRL_CPUID_CLEAR;
  

• 	if (boot_cpu_has(X86_FEATURE_RTM))
  

• 		vmx->guest_uret_msrs[j].mask = ~(u64)TSX_CTRL_CPUID_CLEAR;
  

• 	else
  

• 		vmx->guest_uret_msrs[j].mask = 0;
  

IMO, this is an unnecessarily confusing way to "remove" the user return MSR. Changing the ordering to do a 'continue' would also provide a separate chunk of code for the new comment. And maybe replace the switch with an if-statement to avoid a 'continue' buried in a switch?

You still need the slot in vmx->guest_uret_msrs to store the guest value, even though the two available bits are both no-ops. It's ugly but it makes sense: you don't want to ever re-enable TSX, so you use the ignore the guest value and run unconditionally with the host value.

Ugh, didn't think about the guest wanting to read back the value it wrote.

I'll rephrase everything and resend.

Thanks!