On Mon, Apr 10 2023 at 01:14, Xin Li wrote: > Some kernel components install system interrupt handlers into the IDT, > and we need to do the same for system_interrupt_handlers. We need to the same? This sentence does not make any sense at all. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html > +void install_system_interrupt_handler(unsigned int n, const void *asm_addr, const void *addr); Why is this void *? > > +void __init install_system_interrupt_handler(unsigned int n, const void *asm_addr, const void *addr) > +{ > + BUG_ON(n < FIRST_SYSTEM_VECTOR); > + > +#ifdef CONFIG_X86_64 > + system_interrupt_handlers[n - FIRST_SYSTEM_VECTOR] = (system_interrupt_handler)addr; Just that you can add a silly typecast here, right? Oh well. > - alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback); > + install_system_interrupt_handler(HYPERVISOR_CALLBACK_VECTOR, > + asm_sysvec_xen_hvm_callback, > + sysvec_xen_hvm_callback); Can we please make this less convoluted? #ifdef CONFIG_X86_64 static inline void sysvec_setup_fred(unsigned int vector, void (*func)(struct pt_regs*)) { ... } #else static inline void sysvec_setup_fred(unsigned int vector, void (*func)(struct pt_regs*)) { } #endif #define sysvec_install(vector, func) { \ sysvec_setup_fred(vector, func); \ alloc_intr_gate(vector, asm_##func); \ } - alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, asm_sysvec_xen_hvm_callback); + sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_xen_hvm_callback); No? Thanks, tglx
