diff mbox series

x86/xen: simplify sysenter and syscall setup

Message ID 20221020113619.17204-1-jgross@suse.com
State New
Headers
Series x86/xen: simplify sysenter and syscall setup |

Commit Message

Juergen Gross Oct. 20, 2022, 11:36 a.m. UTC 
  xen_enable_sysenter() and xen_enable_syscall() can be simplified a lot.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/setup.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

Comments

Borislav Petkov Oct. 20, 2022, 11:39 a.m. UTC | #1 
On Thu, Oct 20, 2022 at 01:36:19PM +0200, Juergen Gross wrote:
> xen_enable_sysenter() and xen_enable_syscall() can be simplified a lot.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/xen/setup.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index cfa99e8f054b..0f33ed6d3a7b 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -910,17 +910,9 @@ static int register_callback(unsigned type, const void *func)
>  
>  void xen_enable_sysenter(void)
>  {
> -	int ret;
> -	unsigned sysenter_feature;
> -
> -	sysenter_feature = X86_FEATURE_SYSENTER32;
> -
> -	if (!boot_cpu_has(sysenter_feature))
> -		return;
> -
> -	ret = register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat);
> -	if(ret != 0)
> -		setup_clear_cpu_cap(sysenter_feature);
> +	if (boot_cpu_has(X86_FEATURE_SYSENTER32) &&

Can you switch that and below to cpu_feature_enabled() while at it, pls?

> +	if (boot_cpu_has(X86_FEATURE_SYSCALL32) &&
	^^^^^^^^^^^^^^^^^^

Thx.
Juergen Gross Oct. 20, 2022, 11:41 a.m. UTC | #2 
On 20.10.22 13:39, Borislav Petkov wrote:
> On Thu, Oct 20, 2022 at 01:36:19PM +0200, Juergen Gross wrote:
>> xen_enable_sysenter() and xen_enable_syscall() can be simplified a lot.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/xen/setup.c | 23 ++++++-----------------
>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
>> index cfa99e8f054b..0f33ed6d3a7b 100644
>> --- a/arch/x86/xen/setup.c
>> +++ b/arch/x86/xen/setup.c
>> @@ -910,17 +910,9 @@ static int register_callback(unsigned type, const void *func)
>>   
>>   void xen_enable_sysenter(void)
>>   {
>> -	int ret;
>> -	unsigned sysenter_feature;
>> -
>> -	sysenter_feature = X86_FEATURE_SYSENTER32;
>> -
>> -	if (!boot_cpu_has(sysenter_feature))
>> -		return;
>> -
>> -	ret = register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat);
>> -	if(ret != 0)
>> -		setup_clear_cpu_cap(sysenter_feature);
>> +	if (boot_cpu_has(X86_FEATURE_SYSENTER32) &&
> 
> Can you switch that and below to cpu_feature_enabled() while at it, pls?
> 
>> +	if (boot_cpu_has(X86_FEATURE_SYSCALL32) &&
> 	^^^^^^^^^^^^^^^^^^

Yes, of course.


Juergen
Juergen Gross Oct. 21, 2022, 8:21 a.m. UTC | #3 
On 21.10.22 10:06, Andrew Cooper wrote:
> On 20/10/2022 12:39, Borislav Petkov wrote:
>> On Thu, Oct 20, 2022 at 01:36:19PM +0200, Juergen Gross wrote:
>>> xen_enable_sysenter() and xen_enable_syscall() can be simplified a lot.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>   arch/x86/xen/setup.c | 23 ++++++-----------------
>>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
>>> index cfa99e8f054b..0f33ed6d3a7b 100644
>>> --- a/arch/x86/xen/setup.c
>>> +++ b/arch/x86/xen/setup.c
>>> @@ -910,17 +910,9 @@ static int register_callback(unsigned type, const void *func)
>>>   
>>>   void xen_enable_sysenter(void)
>>>   {
>>> -	int ret;
>>> -	unsigned sysenter_feature;
>>> -
>>> -	sysenter_feature = X86_FEATURE_SYSENTER32;
>>> -
>>> -	if (!boot_cpu_has(sysenter_feature))
>>> -		return;
>>> -
>>> -	ret = register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat);
>>> -	if(ret != 0)
>>> -		setup_clear_cpu_cap(sysenter_feature);
>>> +	if (boot_cpu_has(X86_FEATURE_SYSENTER32) &&
>> Can you switch that and below to cpu_feature_enabled() while at it, pls?
> 
> Why?
> 
> This function (should) be called on the BSP only (because Xen's API lets
> this be specified when starting APs).

No, this is true for the syscall callback only. the sysenter and the syscall32
callbacks can only be set via Xen tools or on the local cpu by registering.

> 
> Whether it's once, or one per cpu, it doesn't matter.

It does.


Juergen
diff mbox series

Patch

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index cfa99e8f054b..0f33ed6d3a7b 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -910,17 +910,9 @@  static int register_callback(unsigned type, const void *func)
 
 void xen_enable_sysenter(void)
 {
-	int ret;
-	unsigned sysenter_feature;
-
-	sysenter_feature = X86_FEATURE_SYSENTER32;
-
-	if (!boot_cpu_has(sysenter_feature))
-		return;
-
-	ret = register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat);
-	if(ret != 0)
-		setup_clear_cpu_cap(sysenter_feature);
+	if (boot_cpu_has(X86_FEATURE_SYSENTER32) &&
+	    register_callback(CALLBACKTYPE_sysenter, xen_entry_SYSENTER_compat))
+		setup_clear_cpu_cap(X86_FEATURE_SYSENTER32);
 }
 
 void xen_enable_syscall(void)
@@ -934,12 +926,9 @@  void xen_enable_syscall(void)
 		   mechanism for syscalls. */
 	}
 
-	if (boot_cpu_has(X86_FEATURE_SYSCALL32)) {
-		ret = register_callback(CALLBACKTYPE_syscall32,
-					xen_entry_SYSCALL_compat);
-		if (ret != 0)
-			setup_clear_cpu_cap(X86_FEATURE_SYSCALL32);
-	}
+	if (boot_cpu_has(X86_FEATURE_SYSCALL32) &&
+	    register_callback(CALLBACKTYPE_syscall32, xen_entry_SYSCALL_compat))
+		setup_clear_cpu_cap(X86_FEATURE_SYSCALL32);
 }
 
 static void __init xen_pvmmu_arch_setup(void)