* [PATCH v1] restore the fake x2apic value for cpuid
@ 2018-11-22 7:18 Xin Li
2018-12-03 7:00 ` Juergen Gross
0 siblings, 1 reply; 5+ messages in thread
From: Xin Li @ 2018-11-22 7:18 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Talons Lee, Sergey Dyasli, Andrew Cooper, Igor Druzhinin
From: Talons Lee <xin.li@citrix.com>
Commit e657fcc clears cpu x2apic capability bit instead of using fake
cpuid value. However, with cpuid x2apic bit on, xen_read_msr_safe() will
not clear the EXTD bit, which leads to uncessary msr write trying to
disable x2apic in __x2apic_disable(). So restore the fake x2apic value.
Signed-off-by: Talons Lee <xin.li@citrix.com>
---
CC: Igor Druzhinin <igor.druzhinin@citrix.com>
CC: Sergey Dyasli <sergey.dyasli@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
arch/x86/xen/enlighten_pv.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4b20082..156c408 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -165,12 +165,17 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
unsigned int *cx, unsigned int *dx)
{
unsigned maskebx = ~0;
+ unsigned maskecx = ~0;
/*
* Mask out inconvenient features, to try and disable as many
* unsupported kernel subsystems as possible.
*/
switch (*ax) {
+ case 1:
+ maskecx &= ~(1 << (X86_FEATURE_X2APIC % 32));
+ break;
+
case CPUID_MWAIT_LEAF:
/* Synthesize the values.. */
*ax = 0;
@@ -193,6 +198,7 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
: "0" (*ax), "2" (*cx));
*bx &= maskebx;
+ *cx &= maskecx;
}
STACK_FRAME_NON_STANDARD(xen_cpuid); /* XEN_EMULATE_PREFIX */
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1] restore the fake x2apic value for cpuid
2018-11-22 7:18 [PATCH v1] restore the fake x2apic value for cpuid Xin Li
@ 2018-12-03 7:00 ` Juergen Gross
2018-12-03 7:03 ` Juergen Gross
0 siblings, 1 reply; 5+ messages in thread
From: Juergen Gross @ 2018-12-03 7:00 UTC (permalink / raw)
To: Xin Li, xen-devel
Cc: Talons Lee, Sergey Dyasli, Andrew Cooper, Igor Druzhinin
On 22/11/2018 08:18, Xin Li wrote:
> From: Talons Lee <xin.li@citrix.com>
>
> Commit e657fcc clears cpu x2apic capability bit instead of using fake
> cpuid value. However, with cpuid x2apic bit on, xen_read_msr_safe() will
> not clear the EXTD bit, which leads to uncessary msr write trying to
> disable x2apic in __x2apic_disable(). So restore the fake x2apic value.
>
> Signed-off-by: Talons Lee <xin.li@citrix.com>
Wouldn't it be easier to use just rdmsr_safe() in __x2apic_disable()
instead?
Juergen
>
> ---
> CC: Igor Druzhinin <igor.druzhinin@citrix.com>
> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
>
> ---
> arch/x86/xen/enlighten_pv.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 4b20082..156c408 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -165,12 +165,17 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
> unsigned int *cx, unsigned int *dx)
> {
> unsigned maskebx = ~0;
> + unsigned maskecx = ~0;
>
> /*
> * Mask out inconvenient features, to try and disable as many
> * unsupported kernel subsystems as possible.
> */
> switch (*ax) {
> + case 1:
> + maskecx &= ~(1 << (X86_FEATURE_X2APIC % 32));
> + break;
> +
> case CPUID_MWAIT_LEAF:
> /* Synthesize the values.. */
> *ax = 0;
> @@ -193,6 +198,7 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
> : "0" (*ax), "2" (*cx));
>
> *bx &= maskebx;
> + *cx &= maskecx;
> }
> STACK_FRAME_NON_STANDARD(xen_cpuid); /* XEN_EMULATE_PREFIX */
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] restore the fake x2apic value for cpuid
2018-12-03 7:00 ` Juergen Gross
@ 2018-12-03 7:03 ` Juergen Gross
2018-12-03 11:15 ` Xin Li (Talons)
0 siblings, 1 reply; 5+ messages in thread
From: Juergen Gross @ 2018-12-03 7:03 UTC (permalink / raw)
To: Xin Li, xen-devel
Cc: Talons Lee, Igor Druzhinin, Andrew Cooper, Sergey Dyasli
On 03/12/2018 08:00, Juergen Gross wrote:
> On 22/11/2018 08:18, Xin Li wrote:
>> From: Talons Lee <xin.li@citrix.com>
>>
>> Commit e657fcc clears cpu x2apic capability bit instead of using fake
>> cpuid value. However, with cpuid x2apic bit on, xen_read_msr_safe() will
>> not clear the EXTD bit, which leads to uncessary msr write trying to
>> disable x2apic in __x2apic_disable(). So restore the fake x2apic value.
>>
>> Signed-off-by: Talons Lee <xin.li@citrix.com>
>
> Wouldn't it be easier to use just rdmsr_safe() in __x2apic_disable()
> instead?
Sorry, just seeing it now: using apic_is_x2apic_enabled() might be even
better. Same in __x2apic_enable().
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] restore the fake x2apic value for cpuid
2018-12-03 7:03 ` Juergen Gross
@ 2018-12-03 11:15 ` Xin Li (Talons)
2018-12-03 11:40 ` Juergen Gross
0 siblings, 1 reply; 5+ messages in thread
From: Xin Li (Talons) @ 2018-12-03 11:15 UTC (permalink / raw)
To: Juergen Gross
Cc: Xin Li, Sergey Dyasli, Igor Druzhinin, xen-devel, Andrew Cooper
Hi Juergen,
thanks for your comments.
Are you suggesting to call apic_is_x2apic_enabled() in __x2apic_disable()?
but I think that's exact what changed due to the fake xen_cpuid value.
Doing so will probably still see the EXTD bit on, and call wrmsrl to disable x2apic.
log for linux4.4:
check_x2apic()
[ 0.000000] xen_read_msr_safe native read: fee00d00
[ 0.000000] clear X2APIC_ENABLE
[ 0.000000] xen_read_msr_safe cpuid: fee00900
[ 0.000000] x2apic_enabled cpu_has_x2apic: 0 apic_is_x2apic_enabled: 0
log for linux4.19:
[ 0.000817] xen_read_msr_safe native read: fee00d00
[ 0.000819] xen_read_msr_safe cpuid: *fee00d00 * //X2APIC_ENABLE not cleared
[ 0.000820] x2apic_enabled X86_FEATURE_X2APIC: 0 apic_is_x2apic_enabled: 1
________________________________________
From: Juergen Gross <jgross@suse.com>
Sent: Monday, December 3, 2018 3:03 PM
To: Xin Li; xen-devel@lists.xen.org
Cc: Xin Li (Talons); Sergey Dyasli; Andrew Cooper; Igor Druzhinin
Subject: Re: [Xen-devel] [PATCH v1] restore the fake x2apic value for cpuid
On 03/12/2018 08:00, Juergen Gross wrote:
> On 22/11/2018 08:18, Xin Li wrote:
>> From: Talons Lee <xin.li@citrix.com>
>>
>> Commit e657fcc clears cpu x2apic capability bit instead of using fake
>> cpuid value. However, with cpuid x2apic bit on, xen_read_msr_safe() will
>> not clear the EXTD bit, which leads to uncessary msr write trying to
>> disable x2apic in __x2apic_disable(). So restore the fake x2apic value.
>>
>> Signed-off-by: Talons Lee <xin.li@citrix.com>
>
> Wouldn't it be easier to use just rdmsr_safe() in __x2apic_disable()
> instead?
Sorry, just seeing it now: using apic_is_x2apic_enabled() might be even
better. Same in __x2apic_enable().
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] restore the fake x2apic value for cpuid
2018-12-03 11:15 ` Xin Li (Talons)
@ 2018-12-03 11:40 ` Juergen Gross
0 siblings, 0 replies; 5+ messages in thread
From: Juergen Gross @ 2018-12-03 11:40 UTC (permalink / raw)
To: Xin Li (Talons)
Cc: Xin Li, Sergey Dyasli, Igor Druzhinin, xen-devel, Andrew Cooper
On 03/12/2018 12:15, Xin Li (Talons) wrote:
> Hi Juergen,
> thanks for your comments.
>
> Are you suggesting to call apic_is_x2apic_enabled() in __x2apic_disable()?
> but I think that's exact what changed due to the fake xen_cpuid value.
> Doing so will probably still see the EXTD bit on, and call wrmsrl to disable x2apic.
>
> log for linux4.4:
> check_x2apic()
> [ 0.000000] xen_read_msr_safe native read: fee00d00
> [ 0.000000] clear X2APIC_ENABLE
> [ 0.000000] xen_read_msr_safe cpuid: fee00900
> [ 0.000000] x2apic_enabled cpu_has_x2apic: 0 apic_is_x2apic_enabled: 0
>
> log for linux4.19:
> [ 0.000817] xen_read_msr_safe native read: fee00d00
> [ 0.000819] xen_read_msr_safe cpuid: *fee00d00 * //X2APIC_ENABLE not cleared
> [ 0.000820] x2apic_enabled X86_FEATURE_X2APIC: 0 apic_is_x2apic_enabled: 1
Oh, you are right, sorry.
I overlooked rdmsr() is already using xen_read_msr_safe().
OTOH looking into xen_read_msr_safe() I can't see why the X2APIC_ENABLE
bit should be cleared depending on the cpuid value returned. We always
do setup_clear_cpu_cap(X86_FEATURE_X2APIC) for PV domains, so there is
no reason to check the cpuid return data for deciding whether to clear
X2APIC_ENABLE or not. I think we can do that always.
So why don't we remove the #ifdef CONFIG_X86_X2APIC block from
xen_read_msr_safe() ?
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-03 11:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-22 7:18 [PATCH v1] restore the fake x2apic value for cpuid Xin Li
2018-12-03 7:00 ` Juergen Gross
2018-12-03 7:03 ` Juergen Gross
2018-12-03 11:15 ` Xin Li (Talons)
2018-12-03 11:40 ` Juergen Gross
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.