Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
> 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, 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 > >> Reviewed-by: Kevin Tian , 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
Re: [Xen-devel] [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, 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 >> Reviewed-by: Kevin Tian , 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
>>> On 14.02.17 at 09:40, 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 >> >>> Reviewed-by: Kevin Tian , 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? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
> 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 > >>> Reviewed-by: Kevin Tian , 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? > > ~Andrew I meant the subject line (talk about syscall MSR leakage) mismatches the commit message (for general MSR load) Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
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 >>> Reviewed-by: Kevin Tian , 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? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
> 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 > > Reviewed-by: Kevin Tian , 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. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
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 > Reviewed-by: Kevin Tian , 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. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
> 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 Reviewed-by: Kevin Tian , 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? Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests
>>> On 13.02.17 at 15:32, wrote: > 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 Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel