linux-kernel - Re: [PATCH v8 03/33] x86/traps: add a system interrupt table for system interrupt dispatch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877csi8fyl.ffs@tglx>
Date:   Mon, 05 Jun 2023 10:34:58 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Xin Li <xin3.li@...el.com>, linux-kernel@...r.kernel.org,
        x86@...nel.org, kvm@...r.kernel.org
Cc:     mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        hpa@...or.com, peterz@...radead.org, andrew.cooper3@...rix.com,
        seanjc@...gle.com, pbonzini@...hat.com, ravi.v.shankar@...el.com,
        jiangshanlai@...il.com, shan.kang@...el.com
Subject: Re: [PATCH v8 03/33] x86/traps: add a system interrupt table for
 system interrupt dispatch

On Mon, Apr 10 2023 at 01:14, Xin Li wrote:
>  
>  /**
>   * DECLARE_IDTENTRY_IRQ - Declare functions for device interrupt IDT entry
> - *			  points (common/spurious)
> + *			  points (common/spurious) and their corresponding
> + *			  software based dispatch handlers in the non-noinstr
> + *			  text section
>   * @vector:	Vector number (ignored for C)
>   * @func:	Function name of the entry point
>   *
> - * Maps to DECLARE_IDTENTRY_ERRORCODE()
> + * Maps to DECLARE_IDTENTRY_ERRORCODE(), plus a dispatch function prototype
>   */
>  #define DECLARE_IDTENTRY_IRQ(vector, func)				\
> -	DECLARE_IDTENTRY_ERRORCODE(vector, func)
> +	DECLARE_IDTENTRY_ERRORCODE(vector, func);			\
> +	void dispatch_##func(struct pt_regs *regs, unsigned long
>  error_code)

Can these IDTENTRY changes please be separate from the actual table
implementation?
  
>  
> +#ifdef CONFIG_X86_64
> +
> +#ifndef CONFIG_X86_LOCAL_APIC

Seriously? You _cannot_ disable local APIC on x8664 builds.

> +/*
> + * Used when local APIC is not configured to build into the kernel, but
> + * dispatch_table_spurious_interrupt() needs
> dispatch_spurious_interrupt().

What? If you there is something which is not used in a certain
configuration then just exclude it via #ifdef in the table or provide a

#define foo   NULL

if you want to spare the #ifdeffery.

> + */
> +DEFINE_IDTENTRY_IRQ(spurious_interrupt)
> +{
> +	pr_info("Spurious interrupt (vector 0x%x) on CPU#%d, should never happen.\n",
> +		vector, smp_processor_id());
> +}

But mindlessly copying code which is even never compiled is a pretty
pointless exercise.

> +#endif
> +
> +static void dispatch_table_spurious_interrupt(struct pt_regs *regs)
> +{
> +	dispatch_spurious_interrupt(regs, regs->vector);
> +}
> +
> +#define SYSV(x,y) [(x) - FIRST_SYSTEM_VECTOR] = y
> +
> +static system_interrupt_handler system_interrupt_handlers[NR_SYSTEM_VECTORS] = {
> +	[0 ... NR_SYSTEM_VECTORS-1]		= dispatch_table_spurious_interrupt,
> +#ifdef CONFIG_SMP
> +	SYSV(RESCHEDULE_VECTOR,			dispatch_table_sysvec_reschedule_ipi),
> +	SYSV(CALL_FUNCTION_VECTOR,		dispatch_table_sysvec_call_function),
> +	SYSV(CALL_FUNCTION_SINGLE_VECTOR,	dispatch_table_sysvec_call_function_single),
> +	SYSV(REBOOT_VECTOR,			dispatch_table_sysvec_reboot),
> +#endif
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +	SYSV(THERMAL_APIC_VECTOR,		dispatch_table_sysvec_thermal),
> +#endif
> +
> +#ifdef CONFIG_X86_MCE_THRESHOLD
> +	SYSV(THRESHOLD_APIC_VECTOR,		dispatch_table_sysvec_threshold),
> +#endif
> +
> +#ifdef CONFIG_X86_MCE_AMD
> +	SYSV(DEFERRED_ERROR_VECTOR,		dispatch_table_sysvec_deferred_error),
> +#endif
> +
> +#ifdef CONFIG_X86_LOCAL_APIC
> +	SYSV(LOCAL_TIMER_VECTOR,		dispatch_table_sysvec_apic_timer_interrupt),
> +	SYSV(X86_PLATFORM_IPI_VECTOR,		dispatch_table_sysvec_x86_platform_ipi),
> +# ifdef CONFIG_HAVE_KVM
> +	SYSV(POSTED_INTR_VECTOR,		dispatch_table_sysvec_kvm_posted_intr_ipi),
> +	SYSV(POSTED_INTR_WAKEUP_VECTOR,		dispatch_table_sysvec_kvm_posted_intr_wakeup_ipi),
> +	SYSV(POSTED_INTR_NESTED_VECTOR,		dispatch_table_sysvec_kvm_posted_intr_nested_ipi),
> +# endif
> +# ifdef CONFIG_IRQ_WORK
> +	SYSV(IRQ_WORK_VECTOR,			dispatch_table_sysvec_irq_work),
> +# endif
> +	SYSV(SPURIOUS_APIC_VECTOR,		dispatch_table_sysvec_spurious_apic_interrupt),

This is clearly in the #ifdef CONFIG_X86_LOCAL_APIC, so what is the
above hackery useful for?

> +	SYSV(ERROR_APIC_VECTOR,			dispatch_table_sysvec_error_interrupt),
> +#endif
> +};

Thanks,

        tglx