* [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat @ 2023-02-27 18:46 Michael Kelley 2023-03-20 20:37 ` Michael Kelley (LINUX) 2023-03-22 18:07 ` Dave Hansen 0 siblings, 2 replies; 8+ messages in thread From: Michael Kelley @ 2023-02-27 18:46 UTC (permalink / raw) To: tglx, mingo, bp, dave.hansen, hpa, x86, linux-hyperv, linux-kernel; +Cc: mikelley Some hypervisor interrupts (such as for Hyper-V VMbus and Hyper-V timers) have hardcoded interrupt vectors on x86 and don't have Linux IRQs assigned. These interrupts are shown in /proc/interrupts, but are not reported in the first field of the "intr" line in /proc/stat because the x86 version of arch_irq_stat_cpu() doesn't include them. Fix this by adding code to arch_irq_stat_cpu() to include these interrupts, similar to existing interrupts that don't have Linux IRQs. Signed-off-by: Michael Kelley <mikelley@microsoft.com> --- arch/x86/kernel/irq.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 766ffe3..9f668d2 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) #ifdef CONFIG_X86_MCE_THRESHOLD sum += irq_stats(cpu)->irq_threshold_count; #endif +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR + sum += irq_stats(cpu)->irq_hv_callback_count; +#endif +#if IS_ENABLED(CONFIG_HYPERV) + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; + sum += irq_stats(cpu)->hyperv_stimer0_count; +#endif #ifdef CONFIG_X86_MCE sum += per_cpu(mce_exception_count, cpu); sum += per_cpu(mce_poll_count, cpu); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-02-27 18:46 [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat Michael Kelley @ 2023-03-20 20:37 ` Michael Kelley (LINUX) 2023-03-22 18:07 ` Dave Hansen 1 sibling, 0 replies; 8+ messages in thread From: Michael Kelley (LINUX) @ 2023-03-20 20:37 UTC (permalink / raw) To: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org From: Michael Kelley (LINUX) <mikelley@microsoft.com> Sent: Monday, February 27, 2023 10:46 AM > > Some hypervisor interrupts (such as for Hyper-V VMbus and Hyper-V timers) > have hardcoded interrupt vectors on x86 and don't have Linux IRQs assigned. > These interrupts are shown in /proc/interrupts, but are not reported in > the first field of the "intr" line in /proc/stat because the x86 version > of arch_irq_stat_cpu() doesn't include them. > > Fix this by adding code to arch_irq_stat_cpu() to include these interrupts, > similar to existing interrupts that don't have Linux IRQs. > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> Gentle ping. Any comments on this patch? Seems pretty straightforward to me .... Michael > --- > arch/x86/kernel/irq.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index 766ffe3..9f668d2 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > #ifdef CONFIG_X86_MCE_THRESHOLD > sum += irq_stats(cpu)->irq_threshold_count; > #endif > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > + sum += irq_stats(cpu)->irq_hv_callback_count; > +#endif > +#if IS_ENABLED(CONFIG_HYPERV) > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > + sum += irq_stats(cpu)->hyperv_stimer0_count; > +#endif > #ifdef CONFIG_X86_MCE > sum += per_cpu(mce_exception_count, cpu); > sum += per_cpu(mce_poll_count, cpu); > -- > 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-02-27 18:46 [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat Michael Kelley 2023-03-20 20:37 ` Michael Kelley (LINUX) @ 2023-03-22 18:07 ` Dave Hansen 2023-03-22 19:52 ` Michael Kelley (LINUX) 2023-06-08 15:20 ` Michael Kelley (LINUX) 1 sibling, 2 replies; 8+ messages in thread From: Dave Hansen @ 2023-03-22 18:07 UTC (permalink / raw) To: Michael Kelley, tglx, mingo, bp, dave.hansen, hpa, x86, linux-hyperv, linux-kernel On 2/27/23 10:46, Michael Kelley wrote: > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index 766ffe3..9f668d2 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > #ifdef CONFIG_X86_MCE_THRESHOLD > sum += irq_stats(cpu)->irq_threshold_count; > #endif > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > + sum += irq_stats(cpu)->irq_hv_callback_count; > +#endif > +#if IS_ENABLED(CONFIG_HYPERV) > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > + sum += irq_stats(cpu)->hyperv_stimer0_count; > +#endif > #ifdef CONFIG_X86_MCE > sum += per_cpu(mce_exception_count, cpu); > sum += per_cpu(mce_poll_count, cpu); This seems fine, especially since arch_show_interrupts() has them. But, what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some difference I'm missing? Why not just be consistent with the other code and use a plain #ifdef for both? ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-03-22 18:07 ` Dave Hansen @ 2023-03-22 19:52 ` Michael Kelley (LINUX) 2023-03-23 1:26 ` Sean Christopherson 2023-06-08 15:20 ` Michael Kelley (LINUX) 1 sibling, 1 reply; 8+ messages in thread From: Michael Kelley (LINUX) @ 2023-03-22 19:52 UTC (permalink / raw) To: Dave Hansen, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org From: Dave Hansen <dave.hansen@intel.com> Sent: Wednesday, March 22, 2023 11:07 AM > > On 2/27/23 10:46, Michael Kelley wrote: > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > index 766ffe3..9f668d2 100644 > > --- a/arch/x86/kernel/irq.c > > +++ b/arch/x86/kernel/irq.c > > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > > #ifdef CONFIG_X86_MCE_THRESHOLD > > sum += irq_stats(cpu)->irq_threshold_count; > > #endif > > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > > + sum += irq_stats(cpu)->irq_hv_callback_count; > > +#endif > > +#if IS_ENABLED(CONFIG_HYPERV) > > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > > + sum += irq_stats(cpu)->hyperv_stimer0_count; > > +#endif > > #ifdef CONFIG_X86_MCE > > sum += per_cpu(mce_exception_count, cpu); > > sum += per_cpu(mce_poll_count, cpu); > > This seems fine, especially since arch_show_interrupts() has them. But, > what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some > difference I'm missing? Why not just be consistent with the other code > and use a plain #ifdef for both? I'm following the coding pattern in arch_show_interrupts(), in irq_cpustat_t, and most other places that test CONFIG_HYPERV. Maybe all those existing cases are a mis-application of Documentation/process/coding-style.rst Section 21, which prefers "if (IS_ENABLED(CONFIG_HYPERV))" over "#ifdef CONFIG_HYPERV". "#if IS_ENABLED()" is not the same as "if (IS_ENABLED())". :-) Net, I don't have a strong preference either way. Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-03-22 19:52 ` Michael Kelley (LINUX) @ 2023-03-23 1:26 ` Sean Christopherson 2023-03-23 17:36 ` Michael Kelley (LINUX) 0 siblings, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2023-03-23 1:26 UTC (permalink / raw) To: Michael Kelley (LINUX) Cc: Dave Hansen, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Mar 22, 2023, Michael Kelley (LINUX) wrote: > From: Dave Hansen <dave.hansen@intel.com> Sent: Wednesday, March 22, 2023 11:07 AM > > > > On 2/27/23 10:46, Michael Kelley wrote: > > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > > index 766ffe3..9f668d2 100644 > > > --- a/arch/x86/kernel/irq.c > > > +++ b/arch/x86/kernel/irq.c > > > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > > > #ifdef CONFIG_X86_MCE_THRESHOLD > > > sum += irq_stats(cpu)->irq_threshold_count; > > > #endif > > > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > > > + sum += irq_stats(cpu)->irq_hv_callback_count; > > > +#endif > > > +#if IS_ENABLED(CONFIG_HYPERV) > > > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > > > + sum += irq_stats(cpu)->hyperv_stimer0_count; > > > +#endif > > > #ifdef CONFIG_X86_MCE > > > sum += per_cpu(mce_exception_count, cpu); > > > sum += per_cpu(mce_poll_count, cpu); > > > > This seems fine, especially since arch_show_interrupts() has them. But, > > what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some > > difference I'm missing? Why not just be consistent with the other code > > and use a plain #ifdef for both? > > I'm following the coding pattern in arch_show_interrupts(), in irq_cpustat_t, > and most other places that test CONFIG_HYPERV. Maybe all those existing > cases are a mis-application of Documentation/process/coding-style.rst > Section 21, which prefers "if (IS_ENABLED(CONFIG_HYPERV))" over > "#ifdef CONFIG_HYPERV". "#if IS_ENABLED()" is not the same as > "if (IS_ENABLED())". :-) > > Net, I don't have a strong preference either way. Using IS_ENABLED() is mandatory because CONFIG_HYPERV is a tri-state, i.e. can be a module and thus #define CONFIG_HYPER_MODULE instead of CONFIG_HYPERV. ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-03-23 1:26 ` Sean Christopherson @ 2023-03-23 17:36 ` Michael Kelley (LINUX) 2023-04-13 23:19 ` Michael Kelley (LINUX) 0 siblings, 1 reply; 8+ messages in thread From: Michael Kelley (LINUX) @ 2023-03-23 17:36 UTC (permalink / raw) To: Sean Christopherson Cc: Dave Hansen, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, March 22, 2023 6:27 PM > > On Wed, Mar 22, 2023, Michael Kelley (LINUX) wrote: > > From: Dave Hansen <dave.hansen@intel.com> Sent: Wednesday, March 22, 2023 > 11:07 AM > > > > > > On 2/27/23 10:46, Michael Kelley wrote: > > > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > > > index 766ffe3..9f668d2 100644 > > > > --- a/arch/x86/kernel/irq.c > > > > +++ b/arch/x86/kernel/irq.c > > > > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > > > > #ifdef CONFIG_X86_MCE_THRESHOLD > > > > sum += irq_stats(cpu)->irq_threshold_count; > > > > #endif > > > > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > > > > + sum += irq_stats(cpu)->irq_hv_callback_count; > > > > +#endif > > > > +#if IS_ENABLED(CONFIG_HYPERV) > > > > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > > > > + sum += irq_stats(cpu)->hyperv_stimer0_count; > > > > +#endif > > > > #ifdef CONFIG_X86_MCE > > > > sum += per_cpu(mce_exception_count, cpu); > > > > sum += per_cpu(mce_poll_count, cpu); > > > > > > This seems fine, especially since arch_show_interrupts() has them. But, > > > what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some > > > difference I'm missing? Why not just be consistent with the other code > > > and use a plain #ifdef for both? > > > > I'm following the coding pattern in arch_show_interrupts(), in irq_cpustat_t, > > and most other places that test CONFIG_HYPERV. Maybe all those existing > > cases are a mis-application of Documentation/process/coding-style.rst > > Section 21, which prefers "if (IS_ENABLED(CONFIG_HYPERV))" over > > "#ifdef CONFIG_HYPERV". "#if IS_ENABLED()" is not the same as > > "if (IS_ENABLED())". :-) > > > > Net, I don't have a strong preference either way. > > Using IS_ENABLED() is mandatory because CONFIG_HYPERV is a tri-state, i.e. can > be a module and thus #define CONFIG_HYPER_MODULE instead of CONFIG_HYPERV. Ah, right. Thanks. Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-03-23 17:36 ` Michael Kelley (LINUX) @ 2023-04-13 23:19 ` Michael Kelley (LINUX) 0 siblings, 0 replies; 8+ messages in thread From: Michael Kelley (LINUX) @ 2023-04-13 23:19 UTC (permalink / raw) To: Michael Kelley (LINUX), tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com, bp@alien8.de Cc: Dave Hansen, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, Sean Christopherson From: Michael Kelley (LINUX) <mikelley@microsoft.com> Sent: Thursday, March 23, 2023 10:36 AM > > From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, March 22, 2023 > 6:27 PM > > > > On Wed, Mar 22, 2023, Michael Kelley (LINUX) wrote: > > > From: Dave Hansen <dave.hansen@intel.com> Sent: Wednesday, March 22, 2023 > > 11:07 AM > > > > > > > > On 2/27/23 10:46, Michael Kelley wrote: > > > > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > > > > index 766ffe3..9f668d2 100644 > > > > > --- a/arch/x86/kernel/irq.c > > > > > +++ b/arch/x86/kernel/irq.c > > > > > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > > > > > #ifdef CONFIG_X86_MCE_THRESHOLD > > > > > sum += irq_stats(cpu)->irq_threshold_count; > > > > > #endif > > > > > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > > > > > + sum += irq_stats(cpu)->irq_hv_callback_count; > > > > > +#endif > > > > > +#if IS_ENABLED(CONFIG_HYPERV) > > > > > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > > > > > + sum += irq_stats(cpu)->hyperv_stimer0_count; > > > > > +#endif > > > > > #ifdef CONFIG_X86_MCE > > > > > sum += per_cpu(mce_exception_count, cpu); > > > > > sum += per_cpu(mce_poll_count, cpu); > > > > > > > > This seems fine, especially since arch_show_interrupts() has them. But, > > > > what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some > > > > difference I'm missing? Why not just be consistent with the other code > > > > and use a plain #ifdef for both? > > > > > > I'm following the coding pattern in arch_show_interrupts(), in irq_cpustat_t, > > > and most other places that test CONFIG_HYPERV. Maybe all those existing > > > cases are a mis-application of Documentation/process/coding-style.rst > > > Section 21, which prefers "if (IS_ENABLED(CONFIG_HYPERV))" over > > > "#ifdef CONFIG_HYPERV". "#if IS_ENABLED()" is not the same as > > > "if (IS_ENABLED())". :-) > > > > > > Net, I don't have a strong preference either way. > > > > Using IS_ENABLED() is mandatory because CONFIG_HYPERV is a tri-state, i.e. can > > be a module and thus #define CONFIG_HYPER_MODULE instead of CONFIG_HYPERV. > > Ah, right. Thanks. > x86 maintainers: Any issues with picking up this patch for the 6.4 merge window? Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat 2023-03-22 18:07 ` Dave Hansen 2023-03-22 19:52 ` Michael Kelley (LINUX) @ 2023-06-08 15:20 ` Michael Kelley (LINUX) 1 sibling, 0 replies; 8+ messages in thread From: Michael Kelley (LINUX) @ 2023-06-08 15:20 UTC (permalink / raw) To: Dave Hansen Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org From: Dave Hansen <dave.hansen@intel.com> Sent: Wednesday, March 22, 2023 11:07 AM > > On 2/27/23 10:46, Michael Kelley wrote: > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > index 766ffe3..9f668d2 100644 > > --- a/arch/x86/kernel/irq.c > > +++ b/arch/x86/kernel/irq.c > > @@ -211,6 +211,13 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > > #ifdef CONFIG_X86_MCE_THRESHOLD > > sum += irq_stats(cpu)->irq_threshold_count; > > #endif > > +#ifdef CONFIG_X86_HV_CALLBACK_VECTOR > > + sum += irq_stats(cpu)->irq_hv_callback_count; > > +#endif > > +#if IS_ENABLED(CONFIG_HYPERV) > > + sum += irq_stats(cpu)->irq_hv_reenlightenment_count; > > + sum += irq_stats(cpu)->hyperv_stimer0_count; > > +#endif > > #ifdef CONFIG_X86_MCE > > sum += per_cpu(mce_exception_count, cpu); > > sum += per_cpu(mce_poll_count, cpu); > > This seems fine, especially since arch_show_interrupts() has them. But, > what's with the "#if IS_ENABLED" versus the plain #ifdef? Is there some > difference I'm missing? Why not just be consistent with the other code > and use a plain #ifdef for both? Dave -- With Sean's explanation for #if IS_ENABLED, are you OK with giving this an ACK as an x86 maintainer? This patch has been hanging around for a while now ... Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-08 15:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-27 18:46 [PATCH 1/1] x86/irq: Add hardcoded hypervisor interrupts to /proc/stat Michael Kelley 2023-03-20 20:37 ` Michael Kelley (LINUX) 2023-03-22 18:07 ` Dave Hansen 2023-03-22 19:52 ` Michael Kelley (LINUX) 2023-03-23 1:26 ` Sean Christopherson 2023-03-23 17:36 ` Michael Kelley (LINUX) 2023-04-13 23:19 ` Michael Kelley (LINUX) 2023-06-08 15:20 ` Michael Kelley (LINUX)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox