All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest
@ 2018-12-10 10:03 Xin Li
  2019-01-14 12:43 ` Igor Druzhinin
  0 siblings, 1 reply; 7+ messages in thread
From: Xin Li @ 2018-12-10 10:03 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 capability bit instead of using fake cpuid
value, the EXTD should always be off for PV guest without depending
on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
always clear the X2APIC_ENABLE bit.

Signed-off-by: Talons Lee <xin.li@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.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>

v2:
don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
the EXTD bit.
---
 arch/x86/xen/enlighten_pv.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4b20082..17cf92b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -900,10 +900,7 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
 	val = native_read_msr_safe(msr, err);
 	switch (msr) {
 	case MSR_IA32_APICBASE:
-#ifdef CONFIG_X86_X2APIC
-		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
-#endif
-			val &= ~X2APIC_ENABLE;
+		val &= ~X2APIC_ENABLE;
 		break;
 	}
 	return val;
-- 
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] 7+ messages in thread
* Re: [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest
  2018-12-10 10:03 [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest Xin Li
@ 2019-01-14 12:43 ` Igor Druzhinin
  2019-01-14 18:09   ` Boris Ostrovsky
  2019-01-14 18:09   ` [Xen-devel] " Boris Ostrovsky
  0 siblings, 2 replies; 7+ messages in thread
From: Igor Druzhinin @ 2019-01-14 12:43 UTC (permalink / raw)
  To: xen-devel, Juergen Gross; +Cc: Xin Li, Talons Lee, Andrew Cooper, Sergey Dyasli

ping?

On 10/12/2018 10:03, Xin Li wrote:
> From: Talons Lee <xin.li@citrix.com>
> 
> Commit e657fcc clears cpu capability bit instead of using fake cpuid
> value, the EXTD should always be off for PV guest without depending
> on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
> always clear the X2APIC_ENABLE bit.
> 
> Signed-off-by: Talons Lee <xin.li@citrix.com>
> Reviewed-by: Juergen Gross <jgross@suse.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>
> 
> v2:
> don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
> the EXTD bit.
> ---
>  arch/x86/xen/enlighten_pv.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 4b20082..17cf92b 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -900,10 +900,7 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
>  	val = native_read_msr_safe(msr, err);
>  	switch (msr) {
>  	case MSR_IA32_APICBASE:
> -#ifdef CONFIG_X86_X2APIC
> -		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
> -#endif
> -			val &= ~X2APIC_ENABLE;
> +		val &= ~X2APIC_ENABLE;
>  		break;
>  	}
>  	return val;
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [Xen-devel] [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest
  2019-01-14 12:43 ` Igor Druzhinin
  2019-01-14 18:09   ` Boris Ostrovsky
@ 2019-01-14 18:09   ` Boris Ostrovsky
  1 sibling, 0 replies; 7+ messages in thread
From: Boris Ostrovsky @ 2019-01-14 18:09 UTC (permalink / raw)
  To: Igor Druzhinin, xen-devel, Juergen Gross
  Cc: Xin Li, Talons Lee, Andrew Cooper, Sergey Dyasli,
	Linux Kernel Mailing List

On 1/14/19 7:43 AM, Igor Druzhinin wrote:
> ping?

Applied to for-linus-4.21 (nka 5.0)

(this should have been copied to linux-kernel@vger.kernel.org and to me,
which is partly the reason why it was missed)

-boris


>
> On 10/12/2018 10:03, Xin Li wrote:
>> From: Talons Lee <xin.li@citrix.com>
>>
>> Commit e657fcc clears cpu capability bit instead of using fake cpuid
>> value, the EXTD should always be off for PV guest without depending
>> on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
>> always clear the X2APIC_ENABLE bit.
>>
>> Signed-off-by: Talons Lee <xin.li@citrix.com>
>> Reviewed-by: Juergen Gross <jgross@suse.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>
>>
>> v2:
>> don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
>> the EXTD bit.
>> ---
>>  arch/x86/xen/enlighten_pv.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>> index 4b20082..17cf92b 100644
>> --- a/arch/x86/xen/enlighten_pv.c
>> +++ b/arch/x86/xen/enlighten_pv.c
>> @@ -900,10 +900,7 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
>>  	val = native_read_msr_safe(msr, err);
>>  	switch (msr) {
>>  	case MSR_IA32_APICBASE:
>> -#ifdef CONFIG_X86_X2APIC
>> -		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
>> -#endif
>> -			val &= ~X2APIC_ENABLE;
>> +		val &= ~X2APIC_ENABLE;
>>  		break;
>>  	}
>>  	return val;
>>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest
  2019-01-14 12:43 ` Igor Druzhinin
@ 2019-01-14 18:09   ` Boris Ostrovsky
  2019-01-14 18:09   ` [Xen-devel] " Boris Ostrovsky
  1 sibling, 0 replies; 7+ messages in thread
From: Boris Ostrovsky @ 2019-01-14 18:09 UTC (permalink / raw)
  To: Igor Druzhinin, xen-devel, Juergen Gross
  Cc: Xin Li, Talons Lee, Sergey Dyasli, Linux Kernel Mailing List,
	Andrew Cooper

On 1/14/19 7:43 AM, Igor Druzhinin wrote:
> ping?

Applied to for-linus-4.21 (nka 5.0)

(this should have been copied to linux-kernel@vger.kernel.org and to me,
which is partly the reason why it was missed)

-boris


>
> On 10/12/2018 10:03, Xin Li wrote:
>> From: Talons Lee <xin.li@citrix.com>
>>
>> Commit e657fcc clears cpu capability bit instead of using fake cpuid
>> value, the EXTD should always be off for PV guest without depending
>> on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
>> always clear the X2APIC_ENABLE bit.
>>
>> Signed-off-by: Talons Lee <xin.li@citrix.com>
>> Reviewed-by: Juergen Gross <jgross@suse.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>
>>
>> v2:
>> don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
>> the EXTD bit.
>> ---
>>  arch/x86/xen/enlighten_pv.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>> index 4b20082..17cf92b 100644
>> --- a/arch/x86/xen/enlighten_pv.c
>> +++ b/arch/x86/xen/enlighten_pv.c
>> @@ -900,10 +900,7 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
>>  	val = native_read_msr_safe(msr, err);
>>  	switch (msr) {
>>  	case MSR_IA32_APICBASE:
>> -#ifdef CONFIG_X86_X2APIC
>> -		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
>> -#endif
>> -			val &= ~X2APIC_ENABLE;
>> +		val &= ~X2APIC_ENABLE;
>>  		break;
>>  	}
>>  	return val;
>>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] always clear the X2APIC_ENABLE bit for PV guest
  2018-12-04 10:25 [PATCH v2] " Xin Li
  2018-12-04 10:29 ` Juergen Gross
@ 2018-12-04 10:45 ` Andrew Cooper
  1 sibling, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2018-12-04 10:45 UTC (permalink / raw)
  To: Xin Li, xen-devel
  Cc: Juergen Gross, Talons Lee, Sergey Dyasli, Igor Druzhinin

On 04/12/2018 10:25, Xin Li wrote:
> From: Talons Lee <xin.li@citrix.com>
>
> Commit e657fcc clears cpu capability bit instead of using fake cpuid
> value, the EXID should always be off for PV guest without depending

EXTD

> on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
> always clear the X2APIC_ENABLE bit.
>
> 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>
>
> v2:
> don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
> the EXIT bit.
> ---
>  arch/x86/xen/enlighten_pv.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 4b20082..6ad312d 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -900,9 +900,6 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
>  	val = native_read_msr_safe(msr, err);
>  	switch (msr) {
>  	case MSR_IA32_APICBASE:
> -#ifdef CONFIG_X86_X2APIC
> -		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
> -#endif
>  			val &= ~X2APIC_ENABLE;

While this probably does bodge the issue for now, I can't help but think
its going to cause problems for larger PV guests.

The problem isn't the visibility (or not) of x2APIC per say - it's that
Linux goes and tries to use the APIC.  Ideally, PV guests should never
have been able to see the APIC at all, and should be safe in the
knowledge that, because its virtualised, it has sane capabilities.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] always clear the X2APIC_ENABLE bit for PV guest
  2018-12-04 10:25 [PATCH v2] " Xin Li
@ 2018-12-04 10:29 ` Juergen Gross
  2018-12-04 10:45 ` Andrew Cooper
  1 sibling, 0 replies; 7+ messages in thread
From: Juergen Gross @ 2018-12-04 10:29 UTC (permalink / raw)
  To: Xin Li, xen-devel
  Cc: Talons Lee, Sergey Dyasli, Andrew Cooper, Igor Druzhinin

On 04/12/2018 11:25, Xin Li wrote:
> From: Talons Lee <xin.li@citrix.com>
> 
> Commit e657fcc clears cpu capability bit instead of using fake cpuid
> value, the EXID should always be off for PV guest without depending
> on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
> always clear the X2APIC_ENABLE bit.
> 
> 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>
> 
> v2:
> don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
> the EXIT bit.
> ---
>  arch/x86/xen/enlighten_pv.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 4b20082..6ad312d 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -900,9 +900,6 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
>  	val = native_read_msr_safe(msr, err);
>  	switch (msr) {
>  	case MSR_IA32_APICBASE:
> -#ifdef CONFIG_X86_X2APIC
> -		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
> -#endif
>  			val &= ~X2APIC_ENABLE;

Could you please correct indentation?

With that addressed you can have my

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH v2] always clear the X2APIC_ENABLE bit for PV guest
@ 2018-12-04 10:25 Xin Li
  2018-12-04 10:29 ` Juergen Gross
  2018-12-04 10:45 ` Andrew Cooper
  0 siblings, 2 replies; 7+ messages in thread
From: Xin Li @ 2018-12-04 10:25 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 capability bit instead of using fake cpuid
value, the EXID should always be off for PV guest without depending
on cpuid value. So remove the cpuid check in xen_read_msr_safe() to
always clear the X2APIC_ENABLE bit.

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>

v2:
don't use fake cpuid to cheat xen_read_msr_safe(), just always clear
the EXIT bit.
---
 arch/x86/xen/enlighten_pv.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4b20082..6ad312d 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -900,9 +900,6 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
 	val = native_read_msr_safe(msr, err);
 	switch (msr) {
 	case MSR_IA32_APICBASE:
-#ifdef CONFIG_X86_X2APIC
-		if (!(cpuid_ecx(1) & (1 << (X86_FEATURE_X2APIC & 31))))
-#endif
 			val &= ~X2APIC_ENABLE;
 		break;
 	}
-- 
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] 7+ messages in thread
end of thread, other threads:[~2019-01-14 18:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 10:03 [PATCH v2 ] always clear the X2APIC_ENABLE bit for PV guest Xin Li
2019-01-14 12:43 ` Igor Druzhinin
2019-01-14 18:09   ` Boris Ostrovsky
2019-01-14 18:09   ` [Xen-devel] " Boris Ostrovsky
  -- strict thread matches above, loose matches on Subject: below --
2018-12-04 10:25 [PATCH v2] " Xin Li
2018-12-04 10:29 ` Juergen Gross
2018-12-04 10:45 ` Andrew Cooper

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.