[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/hvm: Drop get_shadow_gs_base() hook and use hvm_get_reg()
On 21.01.2022 12:22, Andrew Cooper wrote: > This is a trivial accessor for an MSR, so use hvm_get_reg() rather than a > dedicated hook. In arch_get_info_guest(), rework the logic to read GS_SHADOW > only once. > > get_hvm_registers() is called on current, meaning that diagnostics print a > stale GS_SHADOW from the previous vcpu context switch. Adjust both > implementations to obtain the correct value. > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> Just one minor request for consideration at the end. > If we care to backport the bugfix aspect, a far less invasive option would be > to read MSR_SHADOW_GS_BASE directly. > > The only case where that goes wrong is when vmcb->kerngsbase has been modified > and is pending a VMLOAD. I'm fairly sure this can only occur when we need > vcpu diagnostics, after an emulated write of MSR_SHADOW_GS_BASE. Hmm. Maybe best to leave alone in stable trees? > @@ -2417,6 +2413,15 @@ static uint64_t vmx_get_reg(struct vcpu *v, unsigned > int reg) > domain_crash(d); > } > return val; > + > + case MSR_SHADOW_GS_BASE: > + if ( v == curr ) > + { > + rdmsrl(MSR_SHADOW_GS_BASE, val); > + return val; > + } > + else > + return v->arch.hvm.vmx.shadow_gs; > } I think it wasn't too long ago that I saw you ask for an "else" like this one to be dropped (in someone else's patch). May I ask that you consider doing so here, perhaps going straight to the more compact case MSR_SHADOW_GS_BASE: if ( v != curr ) return v->arch.hvm.vmx.shadow_gs; rdmsrl(MSR_SHADOW_GS_BASE, val); return val; ? Actually, as I notice only now: Would you mind making "curr" here and in the VMX equivalent pointer-to-const? Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |