* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 6:31 ` Paolo Bonzini
@ 2024-02-16 17:41 ` Xin Li
2024-02-16 17:47 ` Xin Li
2024-02-16 21:45 ` Thomas Gleixner
2024-02-16 21:46 ` Borislav Petkov
2 siblings, 1 reply; 15+ messages in thread
From: Xin Li @ 2024-02-16 17:41 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Max Kellermann
Cc: hpa, x86, linux-kernel, Stephen Rothwell, kvm
On 2/15/2024 10:31 PM, Paolo Bonzini wrote:
> On 2/16/24 03:10, Xin Li wrote:
>> On 2/15/2024 11:55 AM, Sean Christopherson wrote:
>>> +Paolo and Stephen
>>>
>>> FYI, there's a build failure in -next due to a collision between
>>> kvm/next and
>>> tip/x86/fred. The above makes everything happy.
>>>
>>> On Thu, Feb 15, 2024, Max Kellermann wrote:
>>>> When KVM is disabled, the POSTED_INTR_* macros do not exist, and the
>>>> build fails.
>>>>
>>>> Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
>>>> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
>>>> ---
>>>> arch/x86/entry/entry_fred.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
>>>> index ac120cbdaaf2..660b7f7f9a79 100644
>>>> --- a/arch/x86/entry/entry_fred.c
>>>> +++ b/arch/x86/entry/entry_fred.c
>>>> @@ -114,9 +114,11 @@ static idtentry_t
>>>> sysvec_table[NR_SYSTEM_VECTORS] __ro_after_init = {
>>>> SYSVEC(IRQ_WORK_VECTOR, irq_work),
>>>> +#if IS_ENABLED(CONFIG_KVM)
>>>> SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
>>>> SYSVEC(POSTED_INTR_WAKEUP_VECTOR, kvm_posted_intr_wakeup_ipi),
>>>> SYSVEC(POSTED_INTR_NESTED_VECTOR, kvm_posted_intr_nested_ipi),
>>>> +#endif
>>>> };
>>>> static bool fred_setup_done __initdata;
>>>> --
>>>> 2.39.2
>>
>> We want to minimize #ifdeffery (which is why we didn't add any to
>> sysvec_table[]), would it be better to simply remove "#if
>> IS_ENABLED(CONFIG_KVM)" around the the POSTED_INTR_* macros from the
>> Linux-next tree?
>>
>> BTW, kvm_posted_intr_*() are defined to NULL if !IS_ENABLED(CONFIG_KVM).
>
> It is intentional that KVM-related things are compiled out completely
> if !IS_ENABLED(CONFIG_KVM),
In arch/x86/include/asm/irq_vectors.h, most vector definitions are not
under any #ifdeffery, e.g., THERMAL_APIC_VECTOR not under
CONFIG_X86_THERMAL_VECTOR and IRQ_WORK_VECTOR not under CONFIG_IRQ_WORK.
We'd better make all of them consistent, and the question is that should
we add #ifdefs or not.
> because then it's also not necessary to have
>
> # define fred_sysvec_kvm_posted_intr_ipi NULL
> # define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
> # define fred_sysvec_kvm_posted_intr_nested_ipi NULL
>
> in arch/x86/include/asm/idtentry.h. The full conflict resultion is
>
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index ac120cbdaaf2..660b7f7f9a79 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -114,9 +114,11 @@ static idtentry_t sysvec_table[NR_SYSTEM_VECTORS]
> __ro_after_init = {
>
> SYSVEC(IRQ_WORK_VECTOR, irq_work),
>
> +#if IS_ENABLED(CONFIG_KVM)
> SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
> SYSVEC(POSTED_INTR_WAKEUP_VECTOR, kvm_posted_intr_wakeup_ipi),
> SYSVEC(POSTED_INTR_NESTED_VECTOR, kvm_posted_intr_nested_ipi),
> +#endif
> };
>
> static bool fred_setup_done __initdata;
> diff --git a/arch/x86/include/asm/idtentry.h
> b/arch/x86/include/asm/idtentry.h
> index 749c7411d2f1..758f6a2838a8 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -745,10 +745,6 @@ DECLARE_IDTENTRY_SYSVEC(IRQ_WORK_VECTOR,
> sysvec_irq_work);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_VECTOR,
> sysvec_kvm_posted_intr_ipi);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_WAKEUP_VECTOR,
> sysvec_kvm_posted_intr_wakeup_ipi);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_NESTED_VECTOR,
> sysvec_kvm_posted_intr_nested_ipi);
> -#else
> -# define fred_sysvec_kvm_posted_intr_ipi NULL
> -# define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
> -# define fred_sysvec_kvm_posted_intr_nested_ipi NULL
> #endif
>
> #if IS_ENABLED(CONFIG_HYPERV)
>
> and it seems to be a net improvement to me. The #ifs match in
> the .h and .c files, and there are no unnecessary initializers
> in the sysvec_table.
>
I somehow get an impression that the x86 maintainers don't like #ifs in
the .c files, but I could be just wrong.
Thanks!
Xin
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 17:41 ` Xin Li
@ 2024-02-16 17:47 ` Xin Li
0 siblings, 0 replies; 15+ messages in thread
From: Xin Li @ 2024-02-16 17:47 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Max Kellermann
Cc: hpa, x86, linux-kernel, Stephen Rothwell, kvm
On 2/16/2024 9:41 AM, Xin Li wrote:
> On 2/15/2024 10:31 PM, Paolo Bonzini wrote:
>> On 2/16/24 03:10, Xin Li wrote:
>>> On 2/15/2024 11:55 AM, Sean Christopherson wrote:
>>>> +Paolo and Stephen
>>>>
>>>> FYI, there's a build failure in -next due to a collision between
>>>> kvm/next and
>>>> tip/x86/fred. The above makes everything happy.
>>>>
>>>> On Thu, Feb 15, 2024, Max Kellermann wrote:
>>>>> When KVM is disabled, the POSTED_INTR_* macros do not exist, and the
>>>>> build fails.
>>>>>
>>>>> Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
>>>>> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
>>>>> ---
>>>>> arch/x86/entry/entry_fred.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
>>>>> index ac120cbdaaf2..660b7f7f9a79 100644
>>>>> --- a/arch/x86/entry/entry_fred.c
>>>>> +++ b/arch/x86/entry/entry_fred.c
>>>>> @@ -114,9 +114,11 @@ static idtentry_t
>>>>> sysvec_table[NR_SYSTEM_VECTORS] __ro_after_init = {
>>>>> SYSVEC(IRQ_WORK_VECTOR, irq_work),
>>>>> +#if IS_ENABLED(CONFIG_KVM)
>>>>> SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
>>>>> SYSVEC(POSTED_INTR_WAKEUP_VECTOR,
>>>>> kvm_posted_intr_wakeup_ipi),
>>>>> SYSVEC(POSTED_INTR_NESTED_VECTOR,
>>>>> kvm_posted_intr_nested_ipi),
>>>>> +#endif
>>>>> };
>>>>> static bool fred_setup_done __initdata;
>>>>> --
>>>>> 2.39.2
>>>
>>> We want to minimize #ifdeffery (which is why we didn't add any to
>>> sysvec_table[]), would it be better to simply remove "#if
>>> IS_ENABLED(CONFIG_KVM)" around the the POSTED_INTR_* macros from the
>>> Linux-next tree?
>>>
>>> BTW, kvm_posted_intr_*() are defined to NULL if !IS_ENABLED(CONFIG_KVM).
>>
>> It is intentional that KVM-related things are compiled out completely
>> if !IS_ENABLED(CONFIG_KVM),
>
> In arch/x86/include/asm/irq_vectors.h, most vector definitions are not
> under any #ifdeffery, e.g., THERMAL_APIC_VECTOR not under
> CONFIG_X86_THERMAL_VECTOR and IRQ_WORK_VECTOR not under CONFIG_IRQ_WORK.
>
> We'd better make all of them consistent, and the question is that should
> we add #ifdefs or not.
>
>> because then it's also not necessary to have
>>
>> # define fred_sysvec_kvm_posted_intr_ipi NULL
>> # define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
>> # define fred_sysvec_kvm_posted_intr_nested_ipi NULL
>>
>> in arch/x86/include/asm/idtentry.h. The full conflict resultion is
>>
>> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
>> index ac120cbdaaf2..660b7f7f9a79 100644
>> --- a/arch/x86/entry/entry_fred.c
>> +++ b/arch/x86/entry/entry_fred.c
>> @@ -114,9 +114,11 @@ static idtentry_t sysvec_table[NR_SYSTEM_VECTORS]
>> __ro_after_init = {
>>
>> SYSVEC(IRQ_WORK_VECTOR, irq_work),
>>
>> +#if IS_ENABLED(CONFIG_KVM)
>> SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
>> SYSVEC(POSTED_INTR_WAKEUP_VECTOR, kvm_posted_intr_wakeup_ipi),
>> SYSVEC(POSTED_INTR_NESTED_VECTOR, kvm_posted_intr_nested_ipi),
>> +#endif
>> };
>>
>> static bool fred_setup_done __initdata;
>> diff --git a/arch/x86/include/asm/idtentry.h
>> b/arch/x86/include/asm/idtentry.h
>> index 749c7411d2f1..758f6a2838a8 100644
>> --- a/arch/x86/include/asm/idtentry.h
>> +++ b/arch/x86/include/asm/idtentry.h
>> @@ -745,10 +745,6 @@ DECLARE_IDTENTRY_SYSVEC(IRQ_WORK_VECTOR,
>> sysvec_irq_work);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_VECTOR,
>> sysvec_kvm_posted_intr_ipi);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_WAKEUP_VECTOR,
>> sysvec_kvm_posted_intr_wakeup_ipi);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_NESTED_VECTOR,
>> sysvec_kvm_posted_intr_nested_ipi);
>> -#else
>> -# define fred_sysvec_kvm_posted_intr_ipi NULL
>> -# define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
>> -# define fred_sysvec_kvm_posted_intr_nested_ipi NULL
>> #endif
>>
>> #if IS_ENABLED(CONFIG_HYPERV)
>>
>> and it seems to be a net improvement to me. The #ifs match in
>> the .h and .c files, and there are no unnecessary initializers
>> in the sysvec_table.
>>
>
> I somehow get an impression that the x86 maintainers don't like #ifs in
> the .c files, but I could be just wrong.
>
Here is an example, but again my interpretation could just be wrong:
#ifdef CONFIG_X86_FRED
void fred_install_sysvec(unsigned int vector, const idtentry_t function);
#else
static inline void fred_install_sysvec(unsigned int vector, const
idtentry_t function) { }
#endif
#define sysvec_install(vector, function) { \
if (cpu_feature_enabled(X86_FEATURE_FRED)) \
fred_install_sysvec(vector, function); \
else \
idt_install_sysvec(vector, asm_##function); \
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 6:31 ` Paolo Bonzini
2024-02-16 17:41 ` Xin Li
@ 2024-02-16 21:45 ` Thomas Gleixner
2024-02-16 23:00 ` Max Kellermann
2024-02-17 9:52 ` Paolo Bonzini
2024-02-16 21:46 ` Borislav Petkov
2 siblings, 2 replies; 15+ messages in thread
From: Thomas Gleixner @ 2024-02-16 21:45 UTC (permalink / raw)
To: Paolo Bonzini, Xin Li, Sean Christopherson, Max Kellermann
Cc: hpa, x86, linux-kernel, Stephen Rothwell, kvm
On Fri, Feb 16 2024 at 07:31, Paolo Bonzini wrote:
> On 2/16/24 03:10, Xin Li wrote:
>
> It is intentional that KVM-related things are compiled out completely
> if !IS_ENABLED(CONFIG_KVM), because then it's also not necessary to
> have
That's a matter of taste. In both cases _ALL_ KVM related things are
compiled out.
#ifdeffing out the vector numbers is silly to begin with because these
vector numbers stay assigned to KVM whether KVM is enabled or not.
And no, I don't think it's a net win to have the #ifdeffery in that
table. Look at apic_idts[] in arch/x86/kernel/idt.c how this ends up
looking. It's unreadable gunk.
The few NULL defines in a header file next to the real stuff
#if IS_ENABLED(CONFIG_KVM)
.....
#else
# define fred_sysvec_kvm_posted_intr_ipi NULL
# define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
# define fred_sysvec_kvm_posted_intr_nested_ipi NULL
#endif
are not hurting at all and they are at a place where #ifdeffery is
required anyway. That's a very common pattern all over the kernel and it
limits the #ifdef horror to _ONE_ place.
With your change you propagate the #ifdefffery to the multiple and the
very wrong places for absolutely zero practical value. The resulting
binary code is exactly the same for the price of tasteless #ifdeffery in
places where it matters.
Please get rid of this #ifdef in the vector header and don't inflict
bad taste on everyone.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 21:45 ` Thomas Gleixner
@ 2024-02-16 23:00 ` Max Kellermann
2024-02-17 0:11 ` Thomas Gleixner
2024-02-17 9:52 ` Paolo Bonzini
1 sibling, 1 reply; 15+ messages in thread
From: Max Kellermann @ 2024-02-16 23:00 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Xin Li, Sean Christopherson, hpa, x86,
linux-kernel, Stephen Rothwell, kvm
On Fri, Feb 16, 2024 at 10:45 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> #ifdeffing out the vector numbers is silly to begin with because these
> vector numbers stay assigned to KVM whether KVM is enabled or not.
There could be one non-silly use of this: if the macros are not
defined in absence of the feature, any use of it will lead to a
compiler error, which is good, because it may reveal certain kinds of
bugs.
(Though I agree that this isn't worth the code ugliness. I prefer to
avoid the preprocessor whenever possible. I hate how much the kernel
uses macros instead of inline functions.)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 23:00 ` Max Kellermann
@ 2024-02-17 0:11 ` Thomas Gleixner
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2024-02-17 0:11 UTC (permalink / raw)
To: Max Kellermann
Cc: Paolo Bonzini, Xin Li, Sean Christopherson, hpa, x86,
linux-kernel, Stephen Rothwell, kvm
On Sat, Feb 17 2024 at 00:00, Max Kellermann wrote:
> On Fri, Feb 16, 2024 at 10:45 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> #ifdeffing out the vector numbers is silly to begin with because these
>> vector numbers stay assigned to KVM whether KVM is enabled or not.
>
> There could be one non-silly use of this: if the macros are not
> defined in absence of the feature, any use of it will lead to a
> compiler error, which is good, because it may reveal certain kinds of
> bugs.
I generally agree with this sentiment, but for constants like those in
the case at hand I really draw the line.
> (Though I agree that this isn't worth the code ugliness. I prefer to
> avoid the preprocessor whenever possible. I hate how much the kernel
> uses macros instead of inline functions.)
No argument about that. I'm urging people to use inlines instead of
macros where ever possible, but there are things which can only solved
by macros.
I'm well aware that I wrote some of the more ugly ones myself. Though
the end justifies the means. If the ugly macro from hell which you
verify once safes you from the horrors of copy & pasta error hell then
they are making the code better and there are plenty of options to make
them reasonably (type) safe if done right.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 21:45 ` Thomas Gleixner
2024-02-16 23:00 ` Max Kellermann
@ 2024-02-17 9:52 ` Paolo Bonzini
2024-02-17 22:25 ` Thomas Gleixner
1 sibling, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2024-02-17 9:52 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Xin Li, Sean Christopherson, Max Kellermann, hpa, x86,
linux-kernel, Stephen Rothwell, kvm
On Fri, Feb 16, 2024 at 10:45 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Fri, Feb 16 2024 at 07:31, Paolo Bonzini wrote:
> > On 2/16/24 03:10, Xin Li wrote:
> >
> > It is intentional that KVM-related things are compiled out completely
> > if !IS_ENABLED(CONFIG_KVM), because then it's also not necessary to
> > have
>
> That's a matter of taste. In both cases _ALL_ KVM related things are
> compiled out.
>
> #ifdeffing out the vector numbers is silly to begin with because these
> vector numbers stay assigned to KVM whether KVM is enabled or not.
No problem---it seems that I misunderstood or read too much into the
usage of CONFIG_HAVE_KVM up to 6.8, so I'm happy to follow whatever
FRED support did for thermal vector and the like, and remove the
#ifdef for the vector numbers.
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-17 9:52 ` Paolo Bonzini
@ 2024-02-17 22:25 ` Thomas Gleixner
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2024-02-17 22:25 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Xin Li, Sean Christopherson, Max Kellermann, hpa, x86,
linux-kernel, Stephen Rothwell, kvm
On Sat, Feb 17 2024 at 10:52, Paolo Bonzini wrote:
> On Fri, Feb 16, 2024 at 10:45 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> #ifdeffing out the vector numbers is silly to begin with because these
>> vector numbers stay assigned to KVM whether KVM is enabled or not.
>
> No problem---it seems that I misunderstood or read too much into the
> usage of CONFIG_HAVE_KVM up to 6.8, so I'm happy to follow whatever
> FRED support did for thermal vector and the like, and remove the
> #ifdef for the vector numbers.
Thank you! Appreciated!
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 6:31 ` Paolo Bonzini
2024-02-16 17:41 ` Xin Li
2024-02-16 21:45 ` Thomas Gleixner
@ 2024-02-16 21:46 ` Borislav Petkov
2024-02-16 22:29 ` Thomas Gleixner
2 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2024-02-16 21:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Xin Li, Sean Christopherson, Max Kellermann, hpa, x86,
linux-kernel, Stephen Rothwell, kvm, Arnd Bergmann
+ Arnd for
https://lore.kernel.org/r/20240216202527.2493264-1-arnd@kernel.org
On Fri, Feb 16, 2024 at 07:31:46AM +0100, Paolo Bonzini wrote:
> On 2/16/24 03:10, Xin Li wrote:
> > On 2/15/2024 11:55 AM, Sean Christopherson wrote:
> > > +Paolo and Stephen
> > >
> > > FYI, there's a build failure in -next due to a collision between
> > > kvm/next and
> > > tip/x86/fred. The above makes everything happy.
> > >
> > > On Thu, Feb 15, 2024, Max Kellermann wrote:
> > > > When KVM is disabled, the POSTED_INTR_* macros do not exist, and the
> > > > build fails.
> > > >
> > > > Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
> > > > Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> > > > ---
> > > > arch/x86/entry/entry_fred.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> > > > index ac120cbdaaf2..660b7f7f9a79 100644
> > > > --- a/arch/x86/entry/entry_fred.c
> > > > +++ b/arch/x86/entry/entry_fred.c
> > > > @@ -114,9 +114,11 @@ static idtentry_t
> > > > sysvec_table[NR_SYSTEM_VECTORS] __ro_after_init = {
> > > > SYSVEC(IRQ_WORK_VECTOR, irq_work),
> > > > +#if IS_ENABLED(CONFIG_KVM)
> > > > SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
> > > > SYSVEC(POSTED_INTR_WAKEUP_VECTOR, kvm_posted_intr_wakeup_ipi),
> > > > SYSVEC(POSTED_INTR_NESTED_VECTOR, kvm_posted_intr_nested_ipi),
> > > > +#endif
> > > > };
> > > > static bool fred_setup_done __initdata;
> > > > --
> > > > 2.39.2
> >
> > We want to minimize #ifdeffery (which is why we didn't add any to
> > sysvec_table[]), would it be better to simply remove "#if
> > IS_ENABLED(CONFIG_KVM)" around the the POSTED_INTR_* macros from the
> > Linux-next tree?
> >
> > BTW, kvm_posted_intr_*() are defined to NULL if !IS_ENABLED(CONFIG_KVM).
>
> It is intentional that KVM-related things are compiled out completely
> if !IS_ENABLED(CONFIG_KVM), because then it's also not necessary to have
>
> # define fred_sysvec_kvm_posted_intr_ipi NULL
> # define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
> # define fred_sysvec_kvm_posted_intr_nested_ipi NULL
>
> in arch/x86/include/asm/idtentry.h. The full conflict resultion is
>
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index ac120cbdaaf2..660b7f7f9a79 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -114,9 +114,11 @@ static idtentry_t sysvec_table[NR_SYSTEM_VECTORS] __ro_after_init = {
> SYSVEC(IRQ_WORK_VECTOR, irq_work),
> +#if IS_ENABLED(CONFIG_KVM)
> SYSVEC(POSTED_INTR_VECTOR, kvm_posted_intr_ipi),
> SYSVEC(POSTED_INTR_WAKEUP_VECTOR, kvm_posted_intr_wakeup_ipi),
> SYSVEC(POSTED_INTR_NESTED_VECTOR, kvm_posted_intr_nested_ipi),
> +#endif
> };
> static bool fred_setup_done __initdata;
> diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
> index 749c7411d2f1..758f6a2838a8 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -745,10 +745,6 @@ DECLARE_IDTENTRY_SYSVEC(IRQ_WORK_VECTOR, sysvec_irq_work);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_VECTOR, sysvec_kvm_posted_intr_ipi);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_WAKEUP_VECTOR, sysvec_kvm_posted_intr_wakeup_ipi);
> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_NESTED_VECTOR, sysvec_kvm_posted_intr_nested_ipi);
> -#else
> -# define fred_sysvec_kvm_posted_intr_ipi NULL
> -# define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
> -# define fred_sysvec_kvm_posted_intr_nested_ipi NULL
> #endif
> #if IS_ENABLED(CONFIG_HYPERV)
>
> and it seems to be a net improvement to me. The #ifs match in
> the .h and .c files, and there are no unnecessary initializers
> in the sysvec_table.
Ok, I'll pick up Max' patch tomorrow and we must remember to tell Linus
during the merge window about this.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] arch/x86/entry_fred: don't set up KVM IRQs if KVM is disabled
2024-02-16 21:46 ` Borislav Petkov
@ 2024-02-16 22:29 ` Thomas Gleixner
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2024-02-16 22:29 UTC (permalink / raw)
To: Borislav Petkov, Paolo Bonzini
Cc: Xin Li, Sean Christopherson, Max Kellermann, hpa, x86,
linux-kernel, Stephen Rothwell, kvm, Arnd Bergmann
On Fri, Feb 16 2024 at 22:46, Borislav Petkov wrote:
> On Fri, Feb 16, 2024 at 07:31:46AM +0100, Paolo Bonzini wrote:
>> and it seems to be a net improvement to me. The #ifs match in
>> the .h and .c files, and there are no unnecessary initializers
>> in the sysvec_table.
>
> Ok, I'll pick up Max' patch tomorrow and we must remember to tell Linus
> during the merge window about this.
No. Don't.
This pointless #ifdeffery in the vector header needs to vanish from the
KVM tree.
Why would you take the #ifdef mess into tasteful code just because
someone decided that #ifdeffing out constants in a header which is
maintained by other people is a brilliant idea?
The #ifdeffery in the idtentry header is unavoidable and the extra NULL
defines are at the right place and not making the actual code
unreadable.
Thanks,
tglx
^ permalink raw reply [flat|nested] 15+ messages in thread