* [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE
@ 2016-03-07 14:27 Andrew Cooper
2016-03-07 14:45 ` Boris Ostrovsky
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2016-03-07 14:27 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Boris Ostrovsky, Jan Beulich
The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS and PEBS
UNAVAIL bits.
Some 64bit Windows include IA32_MISC_ENABLE in the set of items checked by
PatchGuard, and will suffer a BSOD 0x109 CRITICAL_STRUCTURE_CORRUPTION if the
contents change on migrate.
The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
This appears to have been broken since the vPMU code was first introduced. It
appears to have lurked this log due to a hole (now fixed) in XenServers
upgrade testing. The BSODs occur ~80% of the time on Win 8 thru 10, but
appear very hard to provoke on Windows 7.
This MSR still leaks mostly host state through into the guest. Therefore
migration of windows is still liable to crash if moving between two
non-identical servers. I need to get proper MSR levelling sorted before this
issue can be resolved fully.
---
xen/arch/x86/cpu/vpmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 237b5ff..2f9ddf6 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
return ret;
nop:
- if ( !is_write )
+ if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
*msr_content = 0;
return 0;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE
2016-03-07 14:27 [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE Andrew Cooper
@ 2016-03-07 14:45 ` Boris Ostrovsky
2016-03-07 14:58 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Boris Ostrovsky @ 2016-03-07 14:45 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
On 03/07/2016 09:27 AM, Andrew Cooper wrote:
> The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
> vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS and PEBS
> UNAVAIL bits.
>
> Some 64bit Windows include IA32_MISC_ENABLE in the set of items checked by
> PatchGuard, and will suffer a BSOD 0x109 CRITICAL_STRUCTURE_CORRUPTION if the
> contents change on migrate.
>
> The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> This appears to have been broken since the vPMU code was first introduced. It
> appears to have lurked this log due to a hole (now fixed) in XenServers
> upgrade testing. The BSODs occur ~80% of the time on Win 8 thru 10, but
> appear very hard to provoke on Windows 7.
>
> This MSR still leaks mostly host state through into the guest. Therefore
> migration of windows is still liable to crash if moving between two
> non-identical servers. I need to get proper MSR levelling sorted before this
> issue can be resolved fully.
> ---
> xen/arch/x86/cpu/vpmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index 237b5ff..2f9ddf6 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
> return ret;
>
> nop:
> - if ( !is_write )
> + if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
> *msr_content = 0;
>
> return 0;
This is Intel-specific register so the test should really be happening
in vpmu_intel.c. Of course then you'd need to always dereference
vcpu_vpmu() and possibly add more checks to read/write ops (to mirror
the one at the top of vpmu_do_msr()).
So maybe at least have the vendor check too??
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE
2016-03-07 14:45 ` Boris Ostrovsky
@ 2016-03-07 14:58 ` Andrew Cooper
2016-03-07 15:11 ` Boris Ostrovsky
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2016-03-07 14:58 UTC (permalink / raw)
To: Boris Ostrovsky, Xen-devel; +Cc: Jan Beulich
On 07/03/16 14:45, Boris Ostrovsky wrote:
> On 03/07/2016 09:27 AM, Andrew Cooper wrote:
>> The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
>> vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS
>> and PEBS
>> UNAVAIL bits.
>>
>> Some 64bit Windows include IA32_MISC_ENABLE in the set of items
>> checked by
>> PatchGuard, and will suffer a BSOD 0x109
>> CRITICAL_STRUCTURE_CORRUPTION if the
>> contents change on migrate.
>>
>> The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>
>> This appears to have been broken since the vPMU code was first
>> introduced. It
>> appears to have lurked this log due to a hole (now fixed) in XenServers
>> upgrade testing. The BSODs occur ~80% of the time on Win 8 thru 10, but
>> appear very hard to provoke on Windows 7.
>>
>> This MSR still leaks mostly host state through into the guest.
>> Therefore
>> migration of windows is still liable to crash if moving between two
>> non-identical servers. I need to get proper MSR levelling sorted
>> before this
>> issue can be resolved fully.
>> ---
>> xen/arch/x86/cpu/vpmu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
>> index 237b5ff..2f9ddf6 100644
>> --- a/xen/arch/x86/cpu/vpmu.c
>> +++ b/xen/arch/x86/cpu/vpmu.c
>> @@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t
>> *msr_content,
>> return ret;
>> nop:
>> - if ( !is_write )
>> + if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
>> *msr_content = 0;
>> return 0;
>
>
> This is Intel-specific register so the test should really be happening
> in vpmu_intel.c. Of course then you'd need to always dereference
> vcpu_vpmu() and possibly add more checks to read/write ops (to mirror
> the one at the top of vpmu_do_msr()).
>
> So maybe at least have the vendor check too??
Strictly speaking, if we were to do a vendor check, it should be a guest
vendor check, not a host vendor check.
OTOH, we won't get here on a non-Intel host system, and emulating a
cross-vendor vPMU is going to end in disaster. I personally don't think
its worth it.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE
2016-03-07 14:58 ` Andrew Cooper
@ 2016-03-07 15:11 ` Boris Ostrovsky
0 siblings, 0 replies; 4+ messages in thread
From: Boris Ostrovsky @ 2016-03-07 15:11 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
On 03/07/2016 09:58 AM, Andrew Cooper wrote:
> On 07/03/16 14:45, Boris Ostrovsky wrote:
>> On 03/07/2016 09:27 AM, Andrew Cooper wrote:
>>> The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
>>> vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS
>>> and PEBS
>>> UNAVAIL bits.
>>>
>>> Some 64bit Windows include IA32_MISC_ENABLE in the set of items
>>> checked by
>>> PatchGuard, and will suffer a BSOD 0x109
>>> CRITICAL_STRUCTURE_CORRUPTION if the
>>> contents change on migrate.
>>>
>>> The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>
>>> This appears to have been broken since the vPMU code was first
>>> introduced. It
>>> appears to have lurked this log due to a hole (now fixed) in XenServers
>>> upgrade testing. The BSODs occur ~80% of the time on Win 8 thru 10, but
>>> appear very hard to provoke on Windows 7.
>>>
>>> This MSR still leaks mostly host state through into the guest.
>>> Therefore
>>> migration of windows is still liable to crash if moving between two
>>> non-identical servers. I need to get proper MSR levelling sorted
>>> before this
>>> issue can be resolved fully.
>>> ---
>>> xen/arch/x86/cpu/vpmu.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
>>> index 237b5ff..2f9ddf6 100644
>>> --- a/xen/arch/x86/cpu/vpmu.c
>>> +++ b/xen/arch/x86/cpu/vpmu.c
>>> @@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t
>>> *msr_content,
>>> return ret;
>>> nop:
>>> - if ( !is_write )
>>> + if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
>>> *msr_content = 0;
>>> return 0;
>>
>> This is Intel-specific register so the test should really be happening
>> in vpmu_intel.c. Of course then you'd need to always dereference
>> vcpu_vpmu() and possibly add more checks to read/write ops (to mirror
>> the one at the top of vpmu_do_msr()).
>>
>> So maybe at least have the vendor check too??
> Strictly speaking, if we were to do a vendor check, it should be a guest
> vendor check, not a host vendor check.
>
> OTOH, we won't get here on a non-Intel host system, and emulating a
> cross-vendor vPMU is going to end in disaster. I personally don't think
> its worth it.
I wasn't thinking about cross-vendor cases. But I forgot that we now go
to VPMU code only for VPMU registers.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-07 15:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-07 14:27 [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE Andrew Cooper
2016-03-07 14:45 ` Boris Ostrovsky
2016-03-07 14:58 ` Andrew Cooper
2016-03-07 15:11 ` Boris Ostrovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).