|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/3] x86/virt: rename x2apic_available to x2apic_without_ir_available
Hi x86 and virt folks,
I'd like some feedback on this patch. I realise that just updating the
name to x2apic_without_ir_available() with no indication in the code
suggesting that the hypervisor implementations may not be answering the
question "is x2apic availalble without IR?" is bad.
I suppose the options are:
1. Check seven hypervisor's x2apic_available() implementation to see if
the "x2apic_without_ir_available" semantic matches, and then do the
renaming
Problem is, I don't know enough about the hypervisors to check
the implementations. Some help from the virt folks would be
great!
2. Add TODOs on the hypervisor implementations, hoping they'll be
audited in the future
There's a chance the TODOs will just sit there rotting. It's
ugly, even I don't like it
So how do we proceed?
On Mon, Feb 02, 2026 at 06:51:04PM +0900, Shashank Balaji wrote:
> No functional change.
>
> x86_init.hyper.x2apic_available is used only in try_to_enable_x2apic to check
> if
> x2apic needs to be disabled if interrupt remapping support isn't present. But
> the name x2apic_available doesn't reflect that usage.
>
> This is what x2apic_available is set to for various hypervisors:
>
> acrn boot_cpu_has(X86_FEATURE_X2APIC)
> mshyperv boot_cpu_has(X86_FEATURE_X2APIC)
> xen boot_cpu_has(X86_FEATURE_X2APIC) or false
> vmware vmware_legacy_x2apic_available
> kvm kvm_cpuid_base() != 0
> jailhouse x2apic_enabled()
> bhyve true
> default false
>
> Bare metal and vmware correctly check if x2apic is available without interrupt
> remapping. The rest of them check if x2apic is enabled/supported, and kvm just
> checks if the kernel is running on kvm. The other hypervisors may have to have
> their checks audited.
>
> Also fix the backwards pr_info message printed on disabling x2apic because of
> lack of irq remapping support.
>
> Compile tested with all the hypervisor guest support enabled.
>
> Co-developed-by: Rahul Bukte <rahul.bukte@xxxxxxxx>
> Signed-off-by: Rahul Bukte <rahul.bukte@xxxxxxxx>
> Signed-off-by: Shashank Balaji <shashank.mahadasyam@xxxxxxxx>
> ---
> arch/x86/include/asm/x86_init.h | 4 ++--
> arch/x86/kernel/apic/apic.c | 4 ++--
> arch/x86/kernel/cpu/acrn.c | 2 +-
> arch/x86/kernel/cpu/bhyve.c | 2 +-
> arch/x86/kernel/cpu/mshyperv.c | 2 +-
> arch/x86/kernel/cpu/vmware.c | 2 +-
> arch/x86/kernel/jailhouse.c | 2 +-
> arch/x86/kernel/kvm.c | 2 +-
> arch/x86/kernel/x86_init.c | 12 ++++++------
> arch/x86/xen/enlighten_hvm.c | 4 ++--
> 10 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 6c8a6ead84f6..b270d9eed755 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -116,7 +116,7 @@ struct x86_init_pci {
> * struct x86_hyper_init - x86 hypervisor init functions
> * @init_platform: platform setup
> * @guest_late_init: guest late init
> - * @x2apic_available: X2APIC detection
> + * @x2apic_without_ir_available: is x2apic available without irq remap?
> * @msi_ext_dest_id: MSI supports 15-bit APIC IDs
> * @init_mem_mapping: setup early mappings during
> init_mem_mapping()
> * @init_after_bootmem: guest init after boot allocator is
> finished
> @@ -124,7 +124,7 @@ struct x86_init_pci {
> struct x86_hyper_init {
> void (*init_platform)(void);
> void (*guest_late_init)(void);
> - bool (*x2apic_available)(void);
> + bool (*x2apic_without_ir_available)(void);
> bool (*msi_ext_dest_id)(void);
> void (*init_mem_mapping)(void);
> void (*init_after_bootmem)(void);
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index cc64d61f82cf..8820b631f8a2 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1836,8 +1836,8 @@ static __init void try_to_enable_x2apic(int remap_mode)
> * Using X2APIC without IR is not architecturally supported
> * on bare metal but may be supported in guests.
> */
> - if (!x86_init.hyper.x2apic_available()) {
> - pr_info("x2apic: IRQ remapping doesn't support X2APIC
> mode\n");
> + if (!x86_init.hyper.x2apic_without_ir_available()) {
> + pr_info("x2apic: Not supported without IRQ
> remapping\n");
> x2apic_disable();
> return;
> }
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index 2c5b51aad91a..9204b98d4786 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -77,5 +77,5 @@ const __initconst struct hypervisor_x86 x86_hyper_acrn = {
> .detect = acrn_detect,
> .type = X86_HYPER_ACRN,
> .init.init_platform = acrn_init_platform,
> - .init.x2apic_available = acrn_x2apic_available,
> + .init.x2apic_without_ir_available = acrn_x2apic_available,
> };
> diff --git a/arch/x86/kernel/cpu/bhyve.c b/arch/x86/kernel/cpu/bhyve.c
> index f1a8ca3dd1ed..91a90a7459ce 100644
> --- a/arch/x86/kernel/cpu/bhyve.c
> +++ b/arch/x86/kernel/cpu/bhyve.c
> @@ -61,6 +61,6 @@ const struct hypervisor_x86 x86_hyper_bhyve __refconst = {
> .name = "Bhyve",
> .detect = bhyve_detect,
> .init.init_platform = x86_init_noop,
> - .init.x2apic_available = bhyve_x2apic_available,
> + .init.x2apic_without_ir_available = bhyve_x2apic_available,
> .init.msi_ext_dest_id = bhyve_ext_dest_id,
> };
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 579fb2c64cfd..61458855094a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -760,7 +760,7 @@ const __initconst struct hypervisor_x86
> x86_hyper_ms_hyperv = {
> .name = "Microsoft Hyper-V",
> .detect = ms_hyperv_platform,
> .type = X86_HYPER_MS_HYPERV,
> - .init.x2apic_available = ms_hyperv_x2apic_available,
> + .init.x2apic_without_ir_available = ms_hyperv_x2apic_available,
> .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
> .init.init_platform = ms_hyperv_init_platform,
> .init.guest_late_init = ms_hyperv_late_init,
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index cb3f900c46fc..46d325818797 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -585,7 +585,7 @@ const __initconst struct hypervisor_x86 x86_hyper_vmware
> = {
> .detect = vmware_platform,
> .type = X86_HYPER_VMWARE,
> .init.init_platform = vmware_platform_setup,
> - .init.x2apic_available = vmware_legacy_x2apic_available,
> + .init.x2apic_without_ir_available = vmware_legacy_x2apic_available,
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> .runtime.sev_es_hcall_prepare = vmware_sev_es_hcall_prepare,
> .runtime.sev_es_hcall_finish = vmware_sev_es_hcall_finish,
> diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
> index 9e9a591a5fec..84a0bbe15989 100644
> --- a/arch/x86/kernel/jailhouse.c
> +++ b/arch/x86/kernel/jailhouse.c
> @@ -291,6 +291,6 @@ const struct hypervisor_x86 x86_hyper_jailhouse
> __refconst = {
> .name = "Jailhouse",
> .detect = jailhouse_detect,
> .init.init_platform = jailhouse_init_platform,
> - .init.x2apic_available = jailhouse_x2apic_available,
> + .init.x2apic_without_ir_available = jailhouse_x2apic_available,
> .ignore_nopv = true,
> };
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 37dc8465e0f5..709eba87d58e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -1042,7 +1042,7 @@ const __initconst struct hypervisor_x86 x86_hyper_kvm =
> {
> .detect = kvm_detect,
> .type = X86_HYPER_KVM,
> .init.guest_late_init = kvm_guest_init,
> - .init.x2apic_available = kvm_para_available,
> + .init.x2apic_without_ir_available = kvm_para_available,
> .init.msi_ext_dest_id = kvm_msi_ext_dest_id,
> .init.init_platform = kvm_init_platform,
> #if defined(CONFIG_AMD_MEM_ENCRYPT)
> diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
> index ebefb77c37bb..9ddf8c901ac6 100644
> --- a/arch/x86/kernel/x86_init.c
> +++ b/arch/x86/kernel/x86_init.c
> @@ -112,12 +112,12 @@ struct x86_init_ops x86_init __initdata = {
> },
>
> .hyper = {
> - .init_platform = x86_init_noop,
> - .guest_late_init = x86_init_noop,
> - .x2apic_available = bool_x86_init_noop,
> - .msi_ext_dest_id = bool_x86_init_noop,
> - .init_mem_mapping = x86_init_noop,
> - .init_after_bootmem = x86_init_noop,
> + .init_platform = x86_init_noop,
> + .guest_late_init = x86_init_noop,
> + .x2apic_without_ir_available = bool_x86_init_noop,
> + .msi_ext_dest_id = bool_x86_init_noop,
> + .init_mem_mapping = x86_init_noop,
> + .init_after_bootmem = x86_init_noop,
> },
>
> .acpi = {
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index fe57ff85d004..42f3d21f313d 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -311,7 +311,7 @@ static uint32_t __init xen_platform_hvm(void)
> * detect PVH and panic there.
> */
> h->init_platform = x86_init_noop;
> - h->x2apic_available = bool_x86_init_noop;
> + h->x2apic_without_ir_available = bool_x86_init_noop;
> h->init_mem_mapping = x86_init_noop;
> h->init_after_bootmem = x86_init_noop;
> h->guest_late_init = xen_hvm_guest_late_init;
> @@ -325,7 +325,7 @@ struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
> .detect = xen_platform_hvm,
> .type = X86_HYPER_XEN_HVM,
> .init.init_platform = xen_hvm_guest_init,
> - .init.x2apic_available = xen_x2apic_available,
> + .init.x2apic_without_ir_available = xen_x2apic_available,
> .init.init_mem_mapping = xen_hvm_init_mem_mapping,
> .init.guest_late_init = xen_hvm_guest_late_init,
> .init.msi_ext_dest_id = msi_ext_dest_id,
>
> --
> 2.43.0
>
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |