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!
