* [PATCH] x86/xen: simplify sysenter and syscall setup
@ 2022-10-20 11:36 Juergen Gross
2022-10-20 11:39 ` Borislav Petkov
0 siblings, 1 reply; 5+ messages in thread
From: Juergen Gross @ 2022-10-20 11:36 UTC (permalink / raw)
To: linux-kernel, x86
Cc: Juergen Gross, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, xen-devel
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) &&
+ 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)
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] x86/xen: simplify sysenter and syscall setup
2022-10-20 11:36 [PATCH] x86/xen: simplify sysenter and syscall setup Juergen Gross
@ 2022-10-20 11:39 ` Borislav Petkov
2022-10-20 11:41 ` Juergen Gross
2022-10-21 8:06 ` Andrew Cooper
0 siblings, 2 replies; 5+ messages in thread
From: Borislav Petkov @ 2022-10-20 11:39 UTC (permalink / raw)
To: Juergen Gross
Cc: linux-kernel, x86, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Dave Hansen, H. Peter Anvin, xen-devel
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.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/xen: simplify sysenter and syscall setup
2022-10-20 11:39 ` Borislav Petkov
@ 2022-10-20 11:41 ` Juergen Gross
2022-10-21 8:06 ` Andrew Cooper
1 sibling, 0 replies; 5+ messages in thread
From: Juergen Gross @ 2022-10-20 11:41 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, x86, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Dave Hansen, H. Peter Anvin, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 1226 bytes --]
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
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/xen: simplify sysenter and syscall setup
2022-10-20 11:39 ` Borislav Petkov
2022-10-20 11:41 ` Juergen Gross
@ 2022-10-21 8:06 ` Andrew Cooper
2022-10-21 8:21 ` Juergen Gross
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2022-10-21 8:06 UTC (permalink / raw)
To: Borislav Petkov, Juergen Gross
Cc: linux-kernel, x86, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Dave Hansen, H. Peter Anvin, xen-devel, Peter Zijlstra (Intel)
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).
Whether it's once, or one per cpu, it doesn't matter.
cpu_feature_enabled() puts in an out-of-line thunk (which is what
actually gets used), and a patchable code section.
Text patching will happen at least once to orphan the out-of-line thunk,
probably after the last time it gets used, then then maybe again later
to clear the feature. Even if you had several million CPUs, there's no
way the overhead of cpu_feature_enabled() is worth it here.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/xen: simplify sysenter and syscall setup
2022-10-21 8:06 ` Andrew Cooper
@ 2022-10-21 8:21 ` Juergen Gross
0 siblings, 0 replies; 5+ messages in thread
From: Juergen Gross @ 2022-10-21 8:21 UTC (permalink / raw)
To: Andrew Cooper, Borislav Petkov
Cc: linux-kernel, x86, Boris Ostrovsky, Thomas Gleixner, Ingo Molnar,
Dave Hansen, H. Peter Anvin, xen-devel, Peter Zijlstra (Intel)
[-- Attachment #1.1.1: Type: text/plain, Size: 1568 bytes --]
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
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-21 8:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20 11:36 [PATCH] x86/xen: simplify sysenter and syscall setup Juergen Gross
2022-10-20 11:39 ` Borislav Petkov
2022-10-20 11:41 ` Juergen Gross
2022-10-21 8:06 ` Andrew Cooper
2022-10-21 8:21 ` 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.