| Message ID | 20250410193106.16353-1-jason.andryuk@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 34d1957b6b6fea958bb6fb78e0c181cd91620243 |
| Headers | show |
| Series | x86/xen: Fix __xen_hypercall_setfunc | expand |
On 10.04.25 21:31, Jason Andryuk wrote: > Hypercall detection is failing with xen_hypercall_intel() chosen even on > an AMD processor. Looking at the disassembly, the call to > xen_get_vendor() was removed. > > The check for boot_cpu_has(X86_FEATURE_CPUID) was used as a proxy for > the x86_vendor having been set. When > CONFIG_X86_REQUIRED_FEATURE_CPUID=y (the default value), DCE eliminates > the call to xen_get_vendor(). An uninitialized value 0 means > X86_VENDOR_INTEL, so the Intel function is always returned. > > Remove the if and always call xen_get_vendor() to avoid this issue. > > Fixes: 3d37d9396eb3 ("x86/cpufeatures: Add {REQUIRED,DISABLED} feature configs") > Suggested-by: Juergen Gross <jgross@suse.com> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
* Juergen Gross <jgross@suse.com> wrote: > On 10.04.25 21:31, Jason Andryuk wrote: > > Hypercall detection is failing with xen_hypercall_intel() chosen even on > > an AMD processor. Looking at the disassembly, the call to > > xen_get_vendor() was removed. > > > > The check for boot_cpu_has(X86_FEATURE_CPUID) was used as a proxy for > > the x86_vendor having been set. When > > CONFIG_X86_REQUIRED_FEATURE_CPUID=y (the default value), DCE eliminates > > the call to xen_get_vendor(). An uninitialized value 0 means > > X86_VENDOR_INTEL, so the Intel function is always returned. > > > > Remove the if and always call xen_get_vendor() to avoid this issue. > > > > Fixes: 3d37d9396eb3 ("x86/cpufeatures: Add {REQUIRED,DISABLED} feature configs") > > Suggested-by: Juergen Gross <jgross@suse.com> > > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > > Reviewed-by: Juergen Gross <jgross@suse.com> Wanna merge this via the Xen tree, or should it go to x86/urgent? The bug was *caused* by the x86 tree so we'd be glad to merge, but your call. If you'll merge it: Acked-by: Ingo Molnar <mingo@kernel.org> Thanks, Ingo
On 11.04.25 11:27, Ingo Molnar wrote: > > * Juergen Gross <jgross@suse.com> wrote: > >> On 10.04.25 21:31, Jason Andryuk wrote: >>> Hypercall detection is failing with xen_hypercall_intel() chosen even on >>> an AMD processor. Looking at the disassembly, the call to >>> xen_get_vendor() was removed. >>> >>> The check for boot_cpu_has(X86_FEATURE_CPUID) was used as a proxy for >>> the x86_vendor having been set. When >>> CONFIG_X86_REQUIRED_FEATURE_CPUID=y (the default value), DCE eliminates >>> the call to xen_get_vendor(). An uninitialized value 0 means >>> X86_VENDOR_INTEL, so the Intel function is always returned. >>> >>> Remove the if and always call xen_get_vendor() to avoid this issue. >>> >>> Fixes: 3d37d9396eb3 ("x86/cpufeatures: Add {REQUIRED,DISABLED} feature configs") >>> Suggested-by: Juergen Gross <jgross@suse.com> >>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> >> >> Reviewed-by: Juergen Gross <jgross@suse.com> > > Wanna merge this via the Xen tree, or should it go to x86/urgent? > > The bug was *caused* by the x86 tree so we'd be glad to merge, > but your call. x86/urgent is fine for me. Juergen
* Jürgen Groß <jgross@suse.com> wrote: > On 11.04.25 11:27, Ingo Molnar wrote: > > > > * Juergen Gross <jgross@suse.com> wrote: > > > > > On 10.04.25 21:31, Jason Andryuk wrote: > > > > Hypercall detection is failing with xen_hypercall_intel() chosen even on > > > > an AMD processor. Looking at the disassembly, the call to > > > > xen_get_vendor() was removed. > > > > > > > > The check for boot_cpu_has(X86_FEATURE_CPUID) was used as a proxy for > > > > the x86_vendor having been set. When > > > > CONFIG_X86_REQUIRED_FEATURE_CPUID=y (the default value), DCE eliminates > > > > the call to xen_get_vendor(). An uninitialized value 0 means > > > > X86_VENDOR_INTEL, so the Intel function is always returned. > > > > > > > > Remove the if and always call xen_get_vendor() to avoid this issue. > > > > > > > > Fixes: 3d37d9396eb3 ("x86/cpufeatures: Add {REQUIRED,DISABLED} feature configs") > > > > Suggested-by: Juergen Gross <jgross@suse.com> > > > > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> > > > > > > Reviewed-by: Juergen Gross <jgross@suse.com> > > > > Wanna merge this via the Xen tree, or should it go to x86/urgent? > > > > The bug was *caused* by the x86 tree so we'd be glad to merge, > > but your call. > > x86/urgent is fine for me. Applied, thanks! Ingo
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index 43dcd8c7badc..6a5cae915eb1 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -100,10 +100,6 @@ noinstr void *__xen_hypercall_setfunc(void) void (*func)(void); /* - * Xen is supported only on CPUs with CPUID, so testing for - * X86_FEATURE_CPUID is a test for early_cpu_init() having been - * run. - * * Note that __xen_hypercall_setfunc() is noinstr only due to a nasty * dependency chain: it is being called via the xen_hypercall static * call when running as a PVH or HVM guest. Hypercalls need to be @@ -115,8 +111,7 @@ noinstr void *__xen_hypercall_setfunc(void) */ instrumentation_begin(); - if (!boot_cpu_has(X86_FEATURE_CPUID)) - xen_get_vendor(); + xen_get_vendor(); if ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD || boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))
Hypercall detection is failing with xen_hypercall_intel() chosen even on an AMD processor. Looking at the disassembly, the call to xen_get_vendor() was removed. The check for boot_cpu_has(X86_FEATURE_CPUID) was used as a proxy for the x86_vendor having been set. When CONFIG_X86_REQUIRED_FEATURE_CPUID=y (the default value), DCE eliminates the call to xen_get_vendor(). An uninitialized value 0 means X86_VENDOR_INTEL, so the Intel function is always returned. Remove the if and always call xen_get_vendor() to avoid this issue. Fixes: 3d37d9396eb3 ("x86/cpufeatures: Add {REQUIRED,DISABLED} feature configs") Suggested-by: Juergen Gross <jgross@suse.com> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com> --- I think the Fixes is correct in that it default enables requiring the feature. There were a lot of other changes to how headers were generated as well. --- arch/x86/xen/enlighten.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)