> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] > Sent: Monday, February 20, 2017 7:17 PM > To: Tian, Kevin > Cc: Jan Beulich; Suravee Suthikulpanit; Nakajima, Jun; Xen-devel; Boris > Ostrovsky > Subject: Re: [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM > guests > > On 14/02/17 10:13, Jan Beulich wrote: > >>>> On 14.02.17 at 09:40, <kevin.t...@intel.com> wrote: > >>> From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of Andrew > Cooper > >>> Sent: Tuesday, February 14, 2017 4:19 PM > >>> > >>> On 14/02/2017 08:04, Tian, Kevin wrote: > >>>>> From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of Andrew > >>> Cooper > >>>>> Sent: Tuesday, February 14, 2017 3:59 PM > >>>>> > >>>>> On 14/02/2017 02:52, Tian, Kevin wrote: > >>>>>>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] > >>>>>>> Sent: Monday, February 13, 2017 10:32 PM > >>>>>>> > >>>>>>> hvm_hw_cpu->msr_flags is in fact the VMX dirty bitmap of MSRs needing > >>>>>>> to > be > >>>>>>> restored when switching into guest context. It should never have > >>>>>>> been part > of > >>>>>>> the migration state to start with, and Xen must not make any > >>>>>>> decisions based > >>>>>>> on the value seen during restore. > >>>>>>> > >>>>>>> Identify it as obsolete in the header files, consistently save it as > >>>>>>> zero and > >>>>>>> ignore it on restore. > >>>>>>> > >>>>>>> The MSRs must be considered dirty during VMCS creation to cause the > >>>>>>> proper > >>>>>>> defaults of 0 to be visible to the guest. > >>>>>>> > >>>>>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > >>>>>> Reviewed-by: Kevin Tian <kevin.t...@intel.com>, with one small comment. > >>>>>> > >>>>>> the effect of this patch should be more than not leaking syscall MSR. > >>>>>> what about making the subject clearer when check-in? > >>>>> What other effects do you think are going on here? Yes, we now context > >>>>> switch the MSRs right from the start of the domain, but that is > >>>>> necessary to avoid leaking them. > >>>>> > >>>> If just looking at this patch, it's for general MSR save/restore policy, > >>>> nothing specific to syscall MSR. > >>> The only three MSRs which use this infrastructure are LSTAR, STAR and > >>> FMASK. What if I were to clarify that in the first paragraph? > >> I meant the subject line (talk about syscall MSR leakage) mismatches the > >> commit message (for general MSR load) > > I'm with Andrew here: The title seems perfectly fine to me, considering > > that the generic mechanism is only used for the syscall MSRs. Hence I > > would think his offer to clarify the change in the first paragraph of the > > commit message ought to suffice. Otherwise, may I ask that you make > > a concrete suggestion as to what you'd like to see? > > Kevin: Do you have any other concerns? > > ~Andrew
fine to me. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel