Re: [PATCH] percpu/module resevation: change resevation size iff X86_VSMP is set
Greetings Paul, On 3/15/19 12:19 AM, Paul E. McKenney wrote: > On Thu, Mar 14, 2019 at 10:36:19AM -0700, Tejun Heo wrote: >> On Wed, Mar 13, 2019 at 04:11:55PM -0700, Paul E. McKenney wrote: >>> commit 34f67df09cc0c6bf082a7cfca435373caeeb8d82 >>> Author: Paul E. McKenney >>> Date: Wed Mar 13 16:06:22 2019 -0700 >>> >>> srcu: Forbid DEFINE{,_STATIC}_SRCU() from modules >>> >>> Adding DEFINE_SRCU() or DEFINE_STATIC_SRCU() to a loadable module >>> requires that the size of the reserved region be increased, which is >>> not something we want to be doing all that often. Instead, loadable >>> modules should define an srcu_struct and invoke init_srcu_struct() >>> from their module_init function and cleanup_srcu_struct() from their >>> module_exit function. Note that modules using call_srcu() will also >>> need to invoke srcu_barrier() from their module_exit function. >>> >>> This commit enforces this advice by refusing to define DEFINE_SRCU() >>> and DEFINE_STATIC_SRCU() within loadable modules. >>> >>> Suggested-by: Barret Rhoden >>> Signed-off-by: Paul E. McKenney >> >> Looks-great-to-me-by: Tejun Heo > > Applied. ;-) > > Thanx, Paul > >> Thanks. :) >> >> -- >> tejun >> > > when can this patch be found in the kernel mainline git repo? I'd like to test and see if the patch that started this mail thread still occurs. Thanks, Eial.
Re: [PATCH] percpu/module resevation: change resevation size iff X86_VSMP is set
Greetings Barret, On 3/1/19 8:30 PM, Barret Rhoden wrote: > Hi - > > On 01/21/2019 06:47 AM, Eial Czerwacki wrote: >> > > Your main issue was that you only sent this patch to LKML, but not the > maintainers of the file. If you don't, your patch might get lost. To > get the appropriate people and lists, run: > > scripts/get_maintainer.pl YOUR_PATCH.patch. > > For this patch, you'll get this: > > Dennis Zhou (maintainer:PER-CPU MEMORY ALLOCATOR) > Tejun Heo (maintainer:PER-CPU MEMORY ALLOCATOR) > Christoph Lameter (maintainer:PER-CPU MEMORY ALLOCATOR) > linux-kernel@vger.kernel.org (open list) > > I added the three maintainers to this email. > > I have a few minor comments below. > thanks, I did not knew that, I'll use it next time. >> [PATCH] percpu/module resevation: change resevation size iff X86_VSMP > is set > > You misspelled 'reservation'. Also, I'd just say: "percpu: increase > module reservation size if X86_VSMP is set". ('change' -> 'increase'), > only says 'reservation' once.) > >> as reported in bug #201339 >> (https://bugzilla.kernel.org/show_bug.cgi?id=201339) > > I think you can add a tag for this right above your Signed-off-by tags. > e.g.: > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201339 > >> by enabling X86_VSMP, INTERNODE_CACHE_BYTES's definition differs from >> the default one >> causing the struct size to exceed the size ok 8KB. > ^of > will fix, thanks. > Which struct are you talking about? I have one in mind, but others > might not know from reading the commit message. > > I ran into this on https://bugzilla.kernel.org/show_bug.cgi?id=202511. > In that case, it was because modules (drm and amdkfd) were using > DEFINE_SRCU, which does a DEFINE_PER_CPU on struct srcu_data, and that > used cacheline_internodealigned_in_smp. you are correct, the structure in question is struct srcu_data. > >> >> in order to avoid such issue, increse PERCPU_MODULE_RESERVE to 64KB if >> CONFIG_X86_VSMP is set. > ^increase > >> >> the value was caculated on linux 4.20.3, make allmodconfig all and the >> following oneliner: > ^calculated > will fix, thanks. >> for f in `find -name *.ko`; do echo $f; readelf -S $f |grep perc; >> done |grep data..percpu -B 1 |grep ko |while read r; do echo -n "$r: >> "; objdump --syms --section=.data..percpu $r|grep data |sort -n |awk >> '{c++; d=strtonum("0x" $1) + strtonum("0x" $5); if (m < d) m = d;} END >> {printf("%d vars-> last addr 0x%x ( %d )\n", c, m, m)}' ; done |column >> -t |sort -k 8 -n | awk '{print $8}'| paste -sd+ | bc > > Not sure how useful the one-liner is, versus a description of what > you're doing. i.e. "the size of all module percpu data sections, or > something." I thought an easy reproducing will suffice, I'll take that into account. > > Also, how close was that calculated value to 64K? If more modules start > using DEFINE_SRCU, each of which uses 8K, then that 64K might run out. the biggest module was 12472 bytes in size, as multiple modules uses the same percpu, more is needed, the only way I was able to make it fit was 64K. of course there is a possibility that at a specific scenario 64K will not be enough but we have yet to encounter such scenario. > > Thanks, > Barret > >> Signed-off-by: Eial Czerwacki >> Signed-off-by: Shai Fultheim >> Signed-off-by: Oren Twaig >> --- >> include/linux/percpu.h | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/include/linux/percpu.h b/include/linux/percpu.h >> index 70b7123..6b79693 100644 >> --- a/include/linux/percpu.h >> +++ b/include/linux/percpu.h >> @@ -14,7 +14,11 @@ >> /* enough to cover all DEFINE_PER_CPUs in modules */ >> #ifdef CONFIG_MODULES >> +#ifdef X86_VSMP >> +#define PERCPU_MODULE_RESERVE (1 << 16) >> +#else >> #define PERCPU_MODULE_RESERVE (8 << 10) >> +#endif >> #else >> #define PERCPU_MODULE_RESERVE 0 >> #endif >> > >
Re: [PATCH] percpu/module resevation: change resevation size iff X86_VSMP is set
Greetings, On 1/21/19 1:47 PM, Eial Czerwacki wrote: > as reported in bug #201339 > (https://bugzilla.kernel.org/show_bug.cgi?id=201339) > by enabling X86_VSMP, INTERNODE_CACHE_BYTES's definition differs from the > default one > causing the struct size to exceed the size ok 8KB. > > in order to avoid such issue, increse PERCPU_MODULE_RESERVE to 64KB if > CONFIG_X86_VSMP is set. > > the value was caculated on linux 4.20.3, make allmodconfig all and the > following oneliner: > for f in `find -name *.ko`; do echo $f; readelf -S $f |grep perc; done |grep > data..percpu -B 1 |grep ko |while read r; do echo -n "$r: "; objdump --syms > --section=.data..percpu $r|grep data |sort -n |awk '{c++; d=strtonum("0x" > $1) + strtonum("0x" $5); if (m < d) m = d;} END {printf("%d vars-> last addr > 0x%x ( %d )\n", c, m, m)}' ; done |column -t |sort -k 8 -n | awk '{print > $8}'| paste -sd+ | bc > > Signed-off-by: Eial Czerwacki > Signed-off-by: Shai Fultheim > Signed-off-by: Oren Twaig > --- > include/linux/percpu.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/include/linux/percpu.h b/include/linux/percpu.h > index 70b7123..6b79693 100644 > --- a/include/linux/percpu.h > +++ b/include/linux/percpu.h > @@ -14,7 +14,11 @@ > > /* enough to cover all DEFINE_PER_CPUs in modules */ > #ifdef CONFIG_MODULES > +#ifdef X86_VSMP > +#define PERCPU_MODULE_RESERVE(1 << 16) > +#else > #define PERCPU_MODULE_RESERVE(8 << 10) > +#endif > #else > #define PERCPU_MODULE_RESERVE0 > #endif > is it possible to push this patch to mainline? it seems like no objections/comment regarding it exists. we'd like to fix the bug mentioned above.
[PATCH] percpu/module resevation: change resevation size iff X86_VSMP is set
as reported in bug #201339 (https://bugzilla.kernel.org/show_bug.cgi?id=201339) by enabling X86_VSMP, INTERNODE_CACHE_BYTES's definition differs from the default one causing the struct size to exceed the size ok 8KB. in order to avoid such issue, increse PERCPU_MODULE_RESERVE to 64KB if CONFIG_X86_VSMP is set. the value was caculated on linux 4.20.3, make allmodconfig all and the following oneliner: for f in `find -name *.ko`; do echo $f; readelf -S $f |grep perc; done |grep data..percpu -B 1 |grep ko |while read r; do echo -n "$r: "; objdump --syms --section=.data..percpu $r|grep data |sort -n |awk '{c++; d=strtonum("0x" $1) + strtonum("0x" $5); if (m < d) m = d;} END {printf("%d vars-> last addr 0x%x ( %d )\n", c, m, m)}' ; done |column -t |sort -k 8 -n | awk '{print $8}'| paste -sd+ | bc Signed-off-by: Eial Czerwacki Signed-off-by: Shai Fultheim Signed-off-by: Oren Twaig --- include/linux/percpu.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/percpu.h b/include/linux/percpu.h index 70b7123..6b79693 100644 --- a/include/linux/percpu.h +++ b/include/linux/percpu.h @@ -14,7 +14,11 @@ /* enough to cover all DEFINE_PER_CPUs in modules */ #ifdef CONFIG_MODULES +#ifdef X86_VSMP +#define PERCPU_MODULE_RESERVE (1 << 16) +#else #define PERCPU_MODULE_RESERVE (8 << 10) +#endif #else #define PERCPU_MODULE_RESERVE 0 #endif -- 2.7.4
Re: [PATCH v3] x86/vsmp_64.c: Remove dependency on pv_irq_ops
Greetings Ingo, On 11/05/2018 07:59 PM, Ingo Molnar wrote: > > * Eial Czerwacki wrote: > >> +#if defined(CONFIG_PCI) > > This is shorter: > >#ifdef CONFIG_PCI > > Thanks, > > Ingo > you are absolutely right, looks like Thomas have handled it already. Eial.
Re: [PATCH v3] x86/vsmp_64.c: Remove dependency on pv_irq_ops
Greetings Ingo, On 11/05/2018 07:59 PM, Ingo Molnar wrote: > > * Eial Czerwacki wrote: > >> +#if defined(CONFIG_PCI) > > This is shorter: > >#ifdef CONFIG_PCI > > Thanks, > > Ingo > you are absolutely right, looks like Thomas have handled it already. Eial.
[tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Commit-ID: a48777fdda7d13179979a889e1fb87655a783cc0 Gitweb: https://git.kernel.org/tip/a48777fdda7d13179979a889e1fb87655a783cc0 Author: Eial Czerwacki AuthorDate: Mon, 5 Nov 2018 19:31:54 +0200 Committer: Thomas Gleixner CommitDate: Tue, 6 Nov 2018 21:35:11 +0100 x86/vsmp: Remove dependency on pv_irq_ops vSMP dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. Signed-off-by: Eial Czerwacki Signed-off-by: Thomas Gleixner Acked-by: Shai Fultheim Cc: Juergen Gross Link: https://lkml.kernel.org/r/1541439114-28297-1-git-send-email-e...@scalemp.com --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e3464ee92..9d734f3c8234 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af491c2..891a75dbc131 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#ifdef CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; }
[tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Commit-ID: a48777fdda7d13179979a889e1fb87655a783cc0 Gitweb: https://git.kernel.org/tip/a48777fdda7d13179979a889e1fb87655a783cc0 Author: Eial Czerwacki AuthorDate: Mon, 5 Nov 2018 19:31:54 +0200 Committer: Thomas Gleixner CommitDate: Tue, 6 Nov 2018 21:35:11 +0100 x86/vsmp: Remove dependency on pv_irq_ops vSMP dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. Signed-off-by: Eial Czerwacki Signed-off-by: Thomas Gleixner Acked-by: Shai Fultheim Cc: Juergen Gross Link: https://lkml.kernel.org/r/1541439114-28297-1-git-send-email-e...@scalemp.com --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e3464ee92..9d734f3c8234 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af491c2..891a75dbc131 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#ifdef CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; }
[PATCH v3] x86/vsmp_64.c: Remove dependency on pv_irq_ops
vSMP dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim Cc: Juergen Gross Cc: Thomas Gleixner Cc: Ingo Molnar --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e346..9d734f3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..3099aaf 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined(CONFIG_PCI) +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
[PATCH v3] x86/vsmp_64.c: Remove dependency on pv_irq_ops
vSMP dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim Cc: Juergen Gross Cc: Thomas Gleixner Cc: Ingo Molnar --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e346..9d734f3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..3099aaf 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined(CONFIG_PCI) +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
Re: [tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Greetings, On 11/05/2018 05:44 PM, Thomas Gleixner wrote: > On Mon, 5 Nov 2018, Ingo Molnar wrote: > >> >> * tip-bot for Eial Czerwacki wrote: >> >>> Commit-ID: 7f2d7f8190d856949d8aaab55d3902cd10ea1048 >>> Gitweb: >>> https://git.kernel.org/tip/7f2d7f8190d856949d8aaab55d3902cd10ea1048 >>> Author: Eial Czerwacki >>> AuthorDate: Mon, 5 Nov 2018 10:01:04 +0200 >>> Committer: Thomas Gleixner >>> CommitDate: Mon, 5 Nov 2018 12:33:47 +0100 >>> >>> x86/vsmp: Remove dependency on pv_irq_ops >>> >>> vsmp dependency on pv_irq_ops has been removed some years ago, but the code >>> still deals with pv_irq_ops. >> >> s/vsmp >> /vSMP >> >>> +#if defined CONFIG_PCI >> >> I don't think code was ever tested with PCI disabled? >> >> The above typoed sequence of code accidentally evaluates to 'true' >> regardless of CONFIG_PCI. > > Grr. Indeed, missed that > > actually I've tested it with CONFIG_PCI unset but evidently I've managed to mess up the test somehow. thanks for pointing this out, I'll send a patch that fixes it. Eial.
Re: [tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Greetings, On 11/05/2018 05:44 PM, Thomas Gleixner wrote: > On Mon, 5 Nov 2018, Ingo Molnar wrote: > >> >> * tip-bot for Eial Czerwacki wrote: >> >>> Commit-ID: 7f2d7f8190d856949d8aaab55d3902cd10ea1048 >>> Gitweb: >>> https://git.kernel.org/tip/7f2d7f8190d856949d8aaab55d3902cd10ea1048 >>> Author: Eial Czerwacki >>> AuthorDate: Mon, 5 Nov 2018 10:01:04 +0200 >>> Committer: Thomas Gleixner >>> CommitDate: Mon, 5 Nov 2018 12:33:47 +0100 >>> >>> x86/vsmp: Remove dependency on pv_irq_ops >>> >>> vsmp dependency on pv_irq_ops has been removed some years ago, but the code >>> still deals with pv_irq_ops. >> >> s/vsmp >> /vSMP >> >>> +#if defined CONFIG_PCI >> >> I don't think code was ever tested with PCI disabled? >> >> The above typoed sequence of code accidentally evaluates to 'true' >> regardless of CONFIG_PCI. > > Grr. Indeed, missed that > > actually I've tested it with CONFIG_PCI unset but evidently I've managed to mess up the test somehow. thanks for pointing this out, I'll send a patch that fixes it. Eial.
[tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Commit-ID: 7f2d7f8190d856949d8aaab55d3902cd10ea1048 Gitweb: https://git.kernel.org/tip/7f2d7f8190d856949d8aaab55d3902cd10ea1048 Author: Eial Czerwacki AuthorDate: Mon, 5 Nov 2018 10:01:04 +0200 Committer: Thomas Gleixner CommitDate: Mon, 5 Nov 2018 12:33:47 +0100 x86/vsmp: Remove dependency on pv_irq_ops vsmp dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. [ tglx: Minor changelog tweaks ] Signed-off-by: Eial Czerwacki Signed-off-by: Thomas Gleixner Acked-by: Shai Fultheim Cc: Juergen Gross Link: https://lkml.kernel.org/r/1541404864-31603-1-git-send-email-e...@scalemp.com --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e3464ee92..9d734f3c8234 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af491c2..48625766c7a4 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift,
[tip:x86/urgent] x86/vsmp: Remove dependency on pv_irq_ops
Commit-ID: 7f2d7f8190d856949d8aaab55d3902cd10ea1048 Gitweb: https://git.kernel.org/tip/7f2d7f8190d856949d8aaab55d3902cd10ea1048 Author: Eial Czerwacki AuthorDate: Mon, 5 Nov 2018 10:01:04 +0200 Committer: Thomas Gleixner CommitDate: Mon, 5 Nov 2018 12:33:47 +0100 x86/vsmp: Remove dependency on pv_irq_ops vsmp dependency on pv_irq_ops has been removed some years ago, but the code still deals with pv_irq_ops. In short, "cap & ctl & (1 << 4)" is always returning 0, so all PARAVIRT/PARAVIRT_XXL code related to that can be removed. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Rename set_vsmp_pv_ops to set_vsmp_ctl as the original name does not make sense anymore. [ tglx: Minor changelog tweaks ] Signed-off-by: Eial Czerwacki Signed-off-by: Thomas Gleixner Acked-by: Shai Fultheim Cc: Juergen Gross Link: https://lkml.kernel.org/r/1541404864-31603-1-git-send-email-e...@scalemp.com --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e3464ee92..9d734f3c8234 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af491c2..48625766c7a4 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift,
[PATCH] x86/vsmp: Remove dependency on pv_irq_ops
vsmp dependency on pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim Cc: Juergen Gross Cc: Thomas Gleixner --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e346..9d734f3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..4862576 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
[PATCH] x86/vsmp: Remove dependency on pv_irq_ops
vsmp dependency on pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim Cc: Juergen Gross Cc: Thomas Gleixner --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba7e346..9d734f3 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -525,7 +525,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..4862576 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
Re: [PATCH v2] x86/build: Build VSMP support only if CONFIG_PCI is selected
Greetings Thomas, On 11/04/2018 11:05 PM, Thomas Gleixner wrote: > Eial, > > On Thu, 1 Nov 2018, Eial Czerwacki wrote: > >> Subject: x86/build: Build VSMP support only if CONFIG_PCI is selected > > That's not what the patch does, right? > you are correct, I'll resend it with a more appropriate subject. >> vsmp dependency on pv_irq_ops removed some years ago, so now let's clean >> it up from vsmp_64.c. >> >> In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can >> remove all the PARAVIRT/PARAVIRT_XXL code handling that. >> >> However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. >> in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. >> >> Signed-off-by: Eial Czerwacki >> Acked-by: Shai Fultheim > > Unfortunately that patch does not apply. It's white space damaged, i.e. all > tabs are converted to spaces. > > Thanks, > > tglx > weird, it got applied without any issues on latest git, I'll verify it applies well. Eial.
Re: [PATCH v2] x86/build: Build VSMP support only if CONFIG_PCI is selected
Greetings Thomas, On 11/04/2018 11:05 PM, Thomas Gleixner wrote: > Eial, > > On Thu, 1 Nov 2018, Eial Czerwacki wrote: > >> Subject: x86/build: Build VSMP support only if CONFIG_PCI is selected > > That's not what the patch does, right? > you are correct, I'll resend it with a more appropriate subject. >> vsmp dependency on pv_irq_ops removed some years ago, so now let's clean >> it up from vsmp_64.c. >> >> In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can >> remove all the PARAVIRT/PARAVIRT_XXL code handling that. >> >> However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. >> in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. >> >> Signed-off-by: Eial Czerwacki >> Acked-by: Shai Fultheim > > Unfortunately that patch does not apply. It's white space damaged, i.e. all > tabs are converted to spaces. > > Thanks, > > tglx > weird, it got applied without any issues on latest git, I'll verify it applies well. Eial.
[PATCH v2] x86/build: Build VSMP support only if CONFIG_PCI is selected
vsmp dependency on pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index c51c989..4b187ca 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -524,7 +524,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..4862576 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
[PATCH v2] x86/build: Build VSMP support only if CONFIG_PCI is selected
vsmp dependency on pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. in addition, rename set_vsmp_pv_ops to set_vsmp_ctl. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 84 --- 2 files changed, 7 insertions(+), 78 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index c51c989..4b187ca 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -524,7 +524,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..4862576 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,65 +26,8 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - -static void __init set_vsmp_pv_ops(void) +#if defined CONFIG_PCI +static void __init set_vsmp_ctl(void) { void __iomem *address; unsigned int cap, ctl, cfg; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(void) { return 0; } +static void __init set_vsmp_ctl(void) +{ +} #endif static void __init vsmp_cap_cpus(void) { -#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP) && defined(CONFIG_PCI) void __iomem *address; unsigned int cfg, topology, node_shift, maxcpus; @@ -221,6 +151,6 @@ void __init vsmp_init(void) vsmp_cap_cpus(); - set_vsmp_pv_ops(); + set_vsmp_ctl(); return; } -- 2.7.4
Re: [PATCH] x86/build: Build VSMP support only if selected
Greetings, On 11/01/2018 03:45 PM, Juergen Gross wrote: > On 01/11/2018 14:10, Eial Czerwacki wrote: >> Greetings, >> >> On 11/01/2018 12:39 PM, Shai Fultheim (s...@scalemp.com) wrote: >>> On 01/11/18 11:37, Thomas Gleixner wrote: >>> >>>> VSMP support is built even if CONFIG_X86_VSMP is not set. This leads to a >>>> build >>>> breakage when CONFIG_PCI is disabled as well. >>>> >>>> Build VSMP code only when selected. >>> >>> This patch disables detect_vsmp_box() on systems without CONFIG_X86_VSMP, >>> due to >>> the recent 6da63eb241a05b0e676d68975e793c0521387141. This is significant >>> regression that will affect significant number of deployments. >>> >>> We will reply shortly with an updated patch that fix the dependency on >>> pv_irq_ops, >>> and revert to CONFIG_PARAVIRT, with proper protection for CONFIG_PCI. >>> >> >> here is the proper patch which fixes the issue on hand: >> From ebff534f8cfa55d7c3ab798c44abe879f3fbe2b8 Mon Sep 17 00:00:00 2001 >> From: Eial Czerwacki >> Date: Thu, 1 Nov 2018 15:08:32 +0200 >> Subject: [PATCH] x86/build: Build VSMP support only if CONFIG_PCI is >> selected >> >> vsmp dependency of pv_irq_ops removed some years ago, so now let's clean >> it up from vsmp_64.c. >> >> In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can >> remove all the PARAVIRT/PARAVIRT_XXL code handling that. >> >> However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. >> >> Signed-off-by: Eial Czerwacki >> Acked-by: Shai Fultheim >> --- >> arch/x86/Kconfig | 1 - >> arch/x86/kernel/vsmp_64.c | 80 >> +++ >> 2 files changed, 5 insertions(+), 76 deletions(-) >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index c51c989..4b187ca 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -524,7 +524,6 @@ config X86_VSMP >> bool "ScaleMP vSMP" >> select HYPERVISOR_GUEST >> select PARAVIRT > > Do you really still need PARAVIRT and HYPERVISOR_GUEST? > Maybe you want IRQ_REMAP instead? > Better performance is achieved with PARAVIRTed kernel. Hence we keep them both in. >> - select PARAVIRT_XXL >> depends on X86_64 && PCI >> depends on X86_EXTENDED_PLATFORM >> depends on SMP >> diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c >> index 1eae5af..c6d2b76 100644 >> --- a/arch/x86/kernel/vsmp_64.c >> +++ b/arch/x86/kernel/vsmp_64.c >> @@ -26,64 +26,7 @@ >> >> #define TOPOLOGY_REGISTER_OFFSET 0x10 >> >> -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL >> -/* >> - * Interrupt control on vSMPowered systems: >> - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' >> - * and vice versa. >> - */ >> - >> -asmlinkage __visible unsigned long vsmp_save_fl(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) >> - flags &= ~X86_EFLAGS_IF; >> - return flags; >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); >> - >> -__visible void vsmp_restore_fl(unsigned long flags) >> -{ >> - if (flags & X86_EFLAGS_IF) >> - flags &= ~X86_EFLAGS_AC; >> - else >> - flags |= X86_EFLAGS_AC; >> - native_restore_fl(flags); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); >> - >> -asmlinkage __visible void vsmp_irq_disable(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); >> - >> -asmlinkage __visible void vsmp_irq_enable(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); >> - >> -static unsigned __init vsmp_patch(u8 type, void *ibuf, >> - unsigned long addr, unsigned len) >> -{ >> - switch (type) { >> - case PARAVIRT_PATCH(irq.irq_enable): >> - case PARAVIRT_PATCH(irq.irq_disable): >> - case PARAVIRT_PATCH(irq.save_fl): >> - case PARAVIRT_PATCH(irq.restore_fl): >> - return paravirt_patch_default(type, ibuf, addr, len); >> - default: >> - return native_patch(type, ibuf, addr, len); >> - } >> - >> -} >> - >> +#if defined CONFIG_PCI >> static void __init set_vsmp_pv_ops(void) > > Wouldn't be a rename of the function be appropriate now? you are correct, I'll fix and resend the patch. > > > Juergen >
Re: [PATCH] x86/build: Build VSMP support only if selected
Greetings, On 11/01/2018 03:45 PM, Juergen Gross wrote: > On 01/11/2018 14:10, Eial Czerwacki wrote: >> Greetings, >> >> On 11/01/2018 12:39 PM, Shai Fultheim (s...@scalemp.com) wrote: >>> On 01/11/18 11:37, Thomas Gleixner wrote: >>> >>>> VSMP support is built even if CONFIG_X86_VSMP is not set. This leads to a >>>> build >>>> breakage when CONFIG_PCI is disabled as well. >>>> >>>> Build VSMP code only when selected. >>> >>> This patch disables detect_vsmp_box() on systems without CONFIG_X86_VSMP, >>> due to >>> the recent 6da63eb241a05b0e676d68975e793c0521387141. This is significant >>> regression that will affect significant number of deployments. >>> >>> We will reply shortly with an updated patch that fix the dependency on >>> pv_irq_ops, >>> and revert to CONFIG_PARAVIRT, with proper protection for CONFIG_PCI. >>> >> >> here is the proper patch which fixes the issue on hand: >> From ebff534f8cfa55d7c3ab798c44abe879f3fbe2b8 Mon Sep 17 00:00:00 2001 >> From: Eial Czerwacki >> Date: Thu, 1 Nov 2018 15:08:32 +0200 >> Subject: [PATCH] x86/build: Build VSMP support only if CONFIG_PCI is >> selected >> >> vsmp dependency of pv_irq_ops removed some years ago, so now let's clean >> it up from vsmp_64.c. >> >> In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can >> remove all the PARAVIRT/PARAVIRT_XXL code handling that. >> >> However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. >> >> Signed-off-by: Eial Czerwacki >> Acked-by: Shai Fultheim >> --- >> arch/x86/Kconfig | 1 - >> arch/x86/kernel/vsmp_64.c | 80 >> +++ >> 2 files changed, 5 insertions(+), 76 deletions(-) >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index c51c989..4b187ca 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -524,7 +524,6 @@ config X86_VSMP >> bool "ScaleMP vSMP" >> select HYPERVISOR_GUEST >> select PARAVIRT > > Do you really still need PARAVIRT and HYPERVISOR_GUEST? > Maybe you want IRQ_REMAP instead? > Better performance is achieved with PARAVIRTed kernel. Hence we keep them both in. >> - select PARAVIRT_XXL >> depends on X86_64 && PCI >> depends on X86_EXTENDED_PLATFORM >> depends on SMP >> diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c >> index 1eae5af..c6d2b76 100644 >> --- a/arch/x86/kernel/vsmp_64.c >> +++ b/arch/x86/kernel/vsmp_64.c >> @@ -26,64 +26,7 @@ >> >> #define TOPOLOGY_REGISTER_OFFSET 0x10 >> >> -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL >> -/* >> - * Interrupt control on vSMPowered systems: >> - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' >> - * and vice versa. >> - */ >> - >> -asmlinkage __visible unsigned long vsmp_save_fl(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) >> - flags &= ~X86_EFLAGS_IF; >> - return flags; >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); >> - >> -__visible void vsmp_restore_fl(unsigned long flags) >> -{ >> - if (flags & X86_EFLAGS_IF) >> - flags &= ~X86_EFLAGS_AC; >> - else >> - flags |= X86_EFLAGS_AC; >> - native_restore_fl(flags); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); >> - >> -asmlinkage __visible void vsmp_irq_disable(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); >> - >> -asmlinkage __visible void vsmp_irq_enable(void) >> -{ >> - unsigned long flags = native_save_fl(); >> - >> - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); >> -} >> -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); >> - >> -static unsigned __init vsmp_patch(u8 type, void *ibuf, >> - unsigned long addr, unsigned len) >> -{ >> - switch (type) { >> - case PARAVIRT_PATCH(irq.irq_enable): >> - case PARAVIRT_PATCH(irq.irq_disable): >> - case PARAVIRT_PATCH(irq.save_fl): >> - case PARAVIRT_PATCH(irq.restore_fl): >> - return paravirt_patch_default(type, ibuf, addr, len); >> - default: >> - return native_patch(type, ibuf, addr, len); >> - } >> - >> -} >> - >> +#if defined CONFIG_PCI >> static void __init set_vsmp_pv_ops(void) > > Wouldn't be a rename of the function be appropriate now? you are correct, I'll fix and resend the patch. > > > Juergen >
Re: [PATCH] x86/build: Build VSMP support only if selected
Greetings, On 11/01/2018 12:39 PM, Shai Fultheim (s...@scalemp.com) wrote: > On 01/11/18 11:37, Thomas Gleixner wrote: > >> VSMP support is built even if CONFIG_X86_VSMP is not set. This leads to a >> build >> breakage when CONFIG_PCI is disabled as well. >> >> Build VSMP code only when selected. > > This patch disables detect_vsmp_box() on systems without CONFIG_X86_VSMP, due > to > the recent 6da63eb241a05b0e676d68975e793c0521387141. This is significant > regression that will affect significant number of deployments. > > We will reply shortly with an updated patch that fix the dependency on > pv_irq_ops, > and revert to CONFIG_PARAVIRT, with proper protection for CONFIG_PCI. > here is the proper patch which fixes the issue on hand: >From ebff534f8cfa55d7c3ab798c44abe879f3fbe2b8 Mon Sep 17 00:00:00 2001 From: Eial Czerwacki Date: Thu, 1 Nov 2018 15:08:32 +0200 Subject: [PATCH] x86/build: Build VSMP support only if CONFIG_PCI is selected vsmp dependency of pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 80 +++ 2 files changed, 5 insertions(+), 76 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index c51c989..4b187ca 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -524,7 +524,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..c6d2b76 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,64 +26,7 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - +#if defined CONFIG_PCI static void __init set_vsmp_pv_ops(void) { void __iomem *address; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(v
Re: [PATCH] x86/build: Build VSMP support only if selected
Greetings, On 11/01/2018 12:39 PM, Shai Fultheim (s...@scalemp.com) wrote: > On 01/11/18 11:37, Thomas Gleixner wrote: > >> VSMP support is built even if CONFIG_X86_VSMP is not set. This leads to a >> build >> breakage when CONFIG_PCI is disabled as well. >> >> Build VSMP code only when selected. > > This patch disables detect_vsmp_box() on systems without CONFIG_X86_VSMP, due > to > the recent 6da63eb241a05b0e676d68975e793c0521387141. This is significant > regression that will affect significant number of deployments. > > We will reply shortly with an updated patch that fix the dependency on > pv_irq_ops, > and revert to CONFIG_PARAVIRT, with proper protection for CONFIG_PCI. > here is the proper patch which fixes the issue on hand: >From ebff534f8cfa55d7c3ab798c44abe879f3fbe2b8 Mon Sep 17 00:00:00 2001 From: Eial Czerwacki Date: Thu, 1 Nov 2018 15:08:32 +0200 Subject: [PATCH] x86/build: Build VSMP support only if CONFIG_PCI is selected vsmp dependency of pv_irq_ops removed some years ago, so now let's clean it up from vsmp_64.c. In short, "cap & ctl & (1 << 4)" was always returning 0, as such we can remove all the PARAVIRT/PARAVIRT_XXL code handling that. However, the rest of the code depends on CONFIG_PCI, so fix it accordingly. Signed-off-by: Eial Czerwacki Acked-by: Shai Fultheim --- arch/x86/Kconfig | 1 - arch/x86/kernel/vsmp_64.c | 80 +++ 2 files changed, 5 insertions(+), 76 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index c51c989..4b187ca 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -524,7 +524,6 @@ config X86_VSMP bool "ScaleMP vSMP" select HYPERVISOR_GUEST select PARAVIRT - select PARAVIRT_XXL depends on X86_64 && PCI depends on X86_EXTENDED_PLATFORM depends on SMP diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c index 1eae5af..c6d2b76 100644 --- a/arch/x86/kernel/vsmp_64.c +++ b/arch/x86/kernel/vsmp_64.c @@ -26,64 +26,7 @@ #define TOPOLOGY_REGISTER_OFFSET 0x10 -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT_XXL -/* - * Interrupt control on vSMPowered systems: - * ~AC is a shadow of IF. If IF is 'on' AC should be 'off' - * and vice versa. - */ - -asmlinkage __visible unsigned long vsmp_save_fl(void) -{ - unsigned long flags = native_save_fl(); - - if (!(flags & X86_EFLAGS_IF) || (flags & X86_EFLAGS_AC)) - flags &= ~X86_EFLAGS_IF; - return flags; -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_save_fl); - -__visible void vsmp_restore_fl(unsigned long flags) -{ - if (flags & X86_EFLAGS_IF) - flags &= ~X86_EFLAGS_AC; - else - flags |= X86_EFLAGS_AC; - native_restore_fl(flags); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_restore_fl); - -asmlinkage __visible void vsmp_irq_disable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags & ~X86_EFLAGS_IF) | X86_EFLAGS_AC); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_disable); - -asmlinkage __visible void vsmp_irq_enable(void) -{ - unsigned long flags = native_save_fl(); - - native_restore_fl((flags | X86_EFLAGS_IF) & (~X86_EFLAGS_AC)); -} -PV_CALLEE_SAVE_REGS_THUNK(vsmp_irq_enable); - -static unsigned __init vsmp_patch(u8 type, void *ibuf, - unsigned long addr, unsigned len) -{ - switch (type) { - case PARAVIRT_PATCH(irq.irq_enable): - case PARAVIRT_PATCH(irq.irq_disable): - case PARAVIRT_PATCH(irq.save_fl): - case PARAVIRT_PATCH(irq.restore_fl): - return paravirt_patch_default(type, ibuf, addr, len); - default: - return native_patch(type, ibuf, addr, len); - } - -} - +#if defined CONFIG_PCI static void __init set_vsmp_pv_ops(void) { void __iomem *address; @@ -109,28 +52,12 @@ static void __init set_vsmp_pv_ops(void) } #endif - if (cap & ctl & (1 << 4)) { - /* Setup irq ops and turn on vSMP IRQ fastpath handling */ - pv_ops.irq.irq_disable = PV_CALLEE_SAVE(vsmp_irq_disable); - pv_ops.irq.irq_enable = PV_CALLEE_SAVE(vsmp_irq_enable); - pv_ops.irq.save_fl = PV_CALLEE_SAVE(vsmp_save_fl); - pv_ops.irq.restore_fl = PV_CALLEE_SAVE(vsmp_restore_fl); - pv_ops.init.patch = vsmp_patch; - ctl &= ~(1 << 4); - } writel(ctl, address + 4); ctl = readl(address + 4); pr_info("vSMP CTL: control set to:0x%08x\n", ctl); early_iounmap(address, 8); } -#else -static void __init set_vsmp_pv_ops(void) -{ -} -#endif - -#ifdef CONFIG_PCI static int is_vsmp = -1; static void __init detect_vsmp_box(void) @@ -164,11 +91,14 @@ static int is_vsmp_box(v