On 22/07/2020 10:16, Jan Beulich wrote: > On 21.07.2020 19:22, Andrew Cooper wrote: >> ... to simplify the default cases. >> >> There are multiple errors with the handling of these three MSRs, but they are >> deliberately not addressed this point. >> >> This removes the dance converting -1/0/1 into X86EMUL_*, allowing for the >> removal of the 'ret' variable. >> >> While cleaning this up, drop the gdprintk()'s for #GP conditions, and the >> 'result' variable from svm_msr_write_intercept() is it is never modified. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <[email protected]> > Reviewed-by: Jan Beulich <[email protected]> > > However, ... > >> @@ -2085,6 +2091,22 @@ static int svm_msr_write_intercept(unsigned int msr, >> uint64_t msr_content) >> goto gpf; >> break; >> >> + case MSR_K8_VM_CR: >> + /* ignore write. handle all bits as read-only. */ >> + break; >> + >> + case MSR_K8_VM_HSAVE_PA: >> + if ( (msr_content & ~PAGE_MASK) || msr_content > 0xfd00000000ULL ) > ... while you're moving this code here, wouldn't it be worthwhile > to at least fix the > to be >= ?
I'd prefer not to, to avoid breaking the "No Functional Change" aspect. In reality, this needs to be a path which takes an extra ref on the nominated frame and globally maps it, seeing as we memcpy to/from it on every virtual vmentry/exit. The check against the HT range is quite bogus. ~Andrew
