* Re: [PATCH v6 10/28] powerpc: Introduce functions for instruction equality
From: Jordan Niethe @ 2020-04-29 2:52 UTC (permalink / raw)
To: Alistair Popple
Cc: Nicholas Piggin, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <7085187.DCEHQqzOHC@townsend>
On Wed, Apr 29, 2020 at 11:59 AM Alistair Popple <alistair@popple.id.au> wrote:
>
> There seems to be a minor typo which breaks compilation when
> CONFIG_MPROFILE_KERNEL is not enabled. See the fix below.
>
> ---
> arch/powerpc/kernel/trace/ftrace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/
> ftrace.c
> index a6064e1977ca..0ad2c9d4ab49 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -499,7 +499,7 @@ expected_nop_sequence(void *ip, struct ppc_inst op0,
> struct ppc_inst op1)
> * The load offset is different depending on the ABI. For simplicity
> * just mask it out when doing the compare.
> */
> - if ((!ppc_inst_equal(op0), ppc_inst(0x48000008)) || (ppc_inst_val(op1) &
> 0xffff0000) != 0xe8410000)
> + if ((!ppc_inst_equal(op0, ppc_inst(0x48000008))) || (ppc_inst_val(op1) &
> 0xffff0000) != 0xe8410000)
> return 0;
> return 1;
> }
Thank you.
> --
> 2.20.1
>
>
>
>
^ permalink raw reply
* Re: [PATCH v6 00/28] Initial Prefixed Instruction support
From: Jordan Niethe @ 2020-04-29 2:51 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <621da706df99146dbe47bbb3243b4f6ed7f3a15c.camel@linux.ibm.com>
On Tue, Apr 28, 2020 at 8:07 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Tue, 2020-04-28 at 11:57 +1000, Jordan Niethe wrote:
> > A future revision of the ISA will introduce prefixed instructions. A
> > prefixed instruction is composed of a 4-byte prefix followed by a
> > 4-byte suffix.
> >
> > All prefixes have the major opcode 1. A prefix will never be a valid
> > word instruction. A suffix may be an existing word instruction or a
> > new instruction.
> >
> > This series enables prefixed instructions and extends the instruction
> > emulation to support them. Then the places where prefixed instructions
> > might need to be emulated are updated.
>
> Hi Jordan,
>
> I tried to test Kprobes with prefixed instruction on this patchset and
> observed that kprobe/uprobe enablement patches are missing from v4.
> Till v3 it were available,
>
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200211053355.21574-11-jniethe5@gmail.com/
>
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200211053355.21574-12-jniethe5@gmail.com/
>
> was it missed for any dependencies/reason ? or will you plan it include in next
> version ?
V4 was when I introduced the data type of prefixed instructions, that
along with helper functions covered a lot of what those enablement
patches were doing.
All of the stuff in that uprobes patch gets done without needing a
specific patch now. I'll add back in the checks for prefix
instructions in arch_prepare_kprobe().
> please let me know if you need help on it.
>
> -- Bala
>
> >
> > v6 is based on feedback from Balamuruhan Suriyakumar, Alistair Popple,
> > Christophe Leroy and Segher Boessenkool.
> > The major changes:
> > - Use the instruction type in more places that had been missed before
> > - Fix issues with ppc32
> > - Introduce new self tests for code patching and feature fixups
> >
> > v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> > Suriyakumar and Alistair Popple.
> > The major changes:
> > - The ppc instruction type is now a struct
> > - Series now just based on next
> > - ppc_inst_masked() dropped
> > - Space for xmon breakpoints allocated in an assembly file
> > - "Add prefixed instructions to instruction data type" patch seperated in
> > to smaller patches
> > - Calling convention for create_branch() is changed
> > - Some places which had not been updated to use the data type are now
> > updated
> >
> > v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel
> > Axtens.
> > The major changes:
> > - Move xmon breakpoints from data section to text section
> > - Introduce a data type for instructions on powerpc
> >
> > v3 is based on feedback from Christophe Leroy. The major changes:
> > - Completely replacing store_inst() with patch_instruction() in
> > xmon
> > - Improve implementation of mread_instr() to not use mread().
> > - Base the series on top of
> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> > kprobes.
> > - Some renaming and simplification of conditionals.
> >
> > v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> > S. The major changes are:
> > - Squashing together all commits about SRR1 bits
> > - Squashing all commits for supporting prefixed load stores
> > - Changing abbreviated references to sufx/prfx -> suffix/prefix
> > - Introducing macros for returning the length of an instruction
> > - Removing sign extension flag from pstd/pld in sstep.c
> > - Dropping patch "powerpc/fault: Use analyse_instr() to check for
> > store with updates to sp" from the series, it did not really fit
> > with prefixed enablement in the first place and as reported by Greg
> > Kurz did not work correctly.
> >
> > Alistair Popple (1):
> > powerpc: Enable Prefixed Instructions
> >
> > Jordan Niethe (27):
> > powerpc/xmon: Remove store_inst() for patch_instruction()
> > powerpc/xmon: Move breakpoint instructions to own array
> > powerpc/xmon: Move breakpoints to text section
> > powerpc/xmon: Use bitwise calculations in_breakpoint_table()
> > powerpc: Change calling convention for create_branch() et. al.
> > powerpc: Use a macro for creating instructions from u32s
> > powerpc: Use an accessor for instructions
> > powerpc: Use a function for getting the instruction op code
> > powerpc: Use a function for byte swapping instructions
> > powerpc: Introduce functions for instruction equality
> > powerpc: Use a datatype for instructions
> > powerpc: Use a function for reading instructions
> > powerpc: Add a probe_user_read_inst() function
> > powerpc: Add a probe_kernel_read_inst() function
> > powerpc/kprobes: Use patch_instruction()
> > powerpc: Define and use __get_user_instr{,inatomic}()
> > powerpc: Introduce a function for reporting instruction length
> > powerpc/xmon: Use a function for reading instructions
> > powerpc/xmon: Move insertion of breakpoint for xol'ing
> > powerpc: Make test_translate_branch() independent of instruction
> > length
> > powerpc: Define new SRR1 bits for a future ISA version
> > powerpc: Add prefixed instructions to instruction data type
> > powerpc: Test prefixed code patching
> > powerpc: Test prefixed instructions in feature fixups
> > powerpc: Support prefixed instructions in alignment handler
> > powerpc sstep: Add support for prefixed load/stores
> > powerpc sstep: Add support for prefixed fixed-point arithmetic
> >
> > arch/powerpc/include/asm/code-patching.h | 37 +-
> > arch/powerpc/include/asm/inst.h | 106 ++++++
> > arch/powerpc/include/asm/kprobes.h | 2 +-
> > arch/powerpc/include/asm/reg.h | 7 +-
> > arch/powerpc/include/asm/sstep.h | 15 +-
> > arch/powerpc/include/asm/uaccess.h | 35 ++
> > arch/powerpc/include/asm/uprobes.h | 7 +-
> > arch/powerpc/kernel/align.c | 13 +-
> > arch/powerpc/kernel/asm-offsets.c | 8 +
> > arch/powerpc/kernel/epapr_paravirt.c | 7 +-
> > arch/powerpc/kernel/hw_breakpoint.c | 5 +-
> > arch/powerpc/kernel/jump_label.c | 5 +-
> > arch/powerpc/kernel/kgdb.c | 9 +-
> > arch/powerpc/kernel/kprobes.c | 24 +-
> > arch/powerpc/kernel/mce_power.c | 5 +-
> > arch/powerpc/kernel/module_64.c | 3 +-
> > arch/powerpc/kernel/optprobes.c | 91 +++--
> > arch/powerpc/kernel/optprobes_head.S | 3 +
> > arch/powerpc/kernel/security.c | 9 +-
> > arch/powerpc/kernel/setup_32.c | 8 +-
> > arch/powerpc/kernel/trace/ftrace.c | 160 ++++----
> > arch/powerpc/kernel/traps.c | 20 +-
> > arch/powerpc/kernel/uprobes.c | 5 +-
> > arch/powerpc/kernel/vecemu.c | 20 +-
> > arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> > arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> > arch/powerpc/lib/Makefile | 2 +-
> > arch/powerpc/lib/code-patching.c | 305 ++++++++-------
> > arch/powerpc/lib/feature-fixups-test.S | 68 ++++
> > arch/powerpc/lib/feature-fixups.c | 159 ++++++--
> > arch/powerpc/lib/inst.c | 69 ++++
> > arch/powerpc/lib/sstep.c | 461 ++++++++++++++++-------
> > arch/powerpc/lib/test_code-patching.S | 19 +
> > arch/powerpc/lib/test_emulate_step.c | 56 +--
> > arch/powerpc/mm/fault.c | 15 +-
> > arch/powerpc/perf/core-book3s.c | 4 +-
> > arch/powerpc/xmon/Makefile | 2 +-
> > arch/powerpc/xmon/xmon.c | 94 +++--
> > arch/powerpc/xmon/xmon_bpts.S | 11 +
> > arch/powerpc/xmon/xmon_bpts.h | 14 +
> > 41 files changed, 1307 insertions(+), 582 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/inst.h
> > create mode 100644 arch/powerpc/lib/inst.c
> > create mode 100644 arch/powerpc/lib/test_code-patching.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
>
^ permalink raw reply
* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Jordan Niethe @ 2020-04-29 2:45 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <46af7f6e-00b2-dab4-5657-6f5e67dc3582@c-s.fr>
On Tue, Apr 28, 2020 at 3:36 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 28/04/2020 à 07:30, Jordan Niethe a écrit :
> > On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
> > <christophe.leroy@c-s.fr> wrote:
> >>
> >>
> >>
> >> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
> >>> The instructions for xmon's breakpoint are stored bpt_table[] which is in
> >>> the data section. This is problematic as the data section may be marked
> >>> as no execute. Move bpt_table[] to the text section.
> >>>
> >>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> >>> ---
> >>> v6: - New to series. Was part of the previous patch.
> >>> - Make BPT_SIZE available in assembly
> >>> ---
> >>> arch/powerpc/kernel/asm-offsets.c | 8 ++++++++
> >>> arch/powerpc/xmon/Makefile | 2 +-
> >>> arch/powerpc/xmon/xmon.c | 6 +-----
> >>> arch/powerpc/xmon/xmon_bpts.S | 9 +++++++++
> >>> arch/powerpc/xmon/xmon_bpts.h | 14 ++++++++++++++
> >>> 5 files changed, 33 insertions(+), 6 deletions(-)
> >>> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> >>> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >>>
> >>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> >>> index c25e562f1cd9..2401f415f423 100644
> >>> --- a/arch/powerpc/kernel/asm-offsets.c
> >>> +++ b/arch/powerpc/kernel/asm-offsets.c
> >>> @@ -70,6 +70,10 @@
> >>> #include <asm/fixmap.h>
> >>> #endif
> >>>
> >>> +#ifdef CONFIG_XMON
> >>> +#include "../xmon/xmon_bpts.h"
> >>> +#endif
> >>> +
> >>> #define STACK_PT_REGS_OFFSET(sym, val) \
> >>> DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
> >>>
> >>> @@ -783,5 +787,9 @@ int main(void)
> >>> DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
> >>> #endif
> >>>
> >>> +#ifdef CONFIG_XMON
> >>> + DEFINE(BPT_SIZE, BPT_SIZE);
> >>> +#endif
> >>> +
> >>> return 0;
> >>> }
> >>> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> >>> index c3842dbeb1b7..515a13ea6f28 100644
> >>> --- a/arch/powerpc/xmon/Makefile
> >>> +++ b/arch/powerpc/xmon/Makefile
> >>> @@ -21,7 +21,7 @@ endif
> >>>
> >>> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >>>
> >>> -obj-y += xmon.o nonstdio.o spr_access.o
> >>> +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
> >>>
> >>> ifdef CONFIG_XMON_DISASSEMBLY
> >>> obj-y += ppc-dis.o ppc-opc.o
> >>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> >>> index a064392df1b8..f7ce3ea8694c 100644
> >>> --- a/arch/powerpc/xmon/xmon.c
> >>> +++ b/arch/powerpc/xmon/xmon.c
> >>> @@ -62,6 +62,7 @@
> >>>
> >>> #include "nonstdio.h"
> >>> #include "dis-asm.h"
> >>> +#include "xmon_bpts.h"
> >>>
> >>> #ifdef CONFIG_SMP
> >>> static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> >>> @@ -108,7 +109,6 @@ struct bpt {
> >>> #define BP_TRAP 2
> >>> #define BP_DABR 4
> >>>
> >>> -#define NBPTS 256
> >>> static struct bpt bpts[NBPTS];
> >>> static struct bpt dabr;
> >>> static struct bpt *iabr;
> >>> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
> >>>
> >>> #define BP_NUM(bp) ((bp) - bpts + 1)
> >>>
> >>> -#define BPT_SIZE (sizeof(unsigned int) * 2)
> >>> -#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> >>> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
> >>> -
> >>> /* Prototypes */
> >>> static int cmds(struct pt_regs *);
> >>> static int mread(unsigned long, void *, int);
> >>> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> >>> new file mode 100644
> >>> index 000000000000..f3ad0ab50854
> >>> --- /dev/null
> >>> +++ b/arch/powerpc/xmon/xmon_bpts.S
> >>> @@ -0,0 +1,9 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +#include <asm/ppc_asm.h>
> >>> +#include <asm/asm-compat.h>
> >>> +#include <asm/asm-offsets.h>
> >>> +#include "xmon_bpts.h"
> >>> +
> >>> +.global bpt_table
> >>> +bpt_table:
> >>> + .space NBPTS * BPT_SIZE
> >>
> >> No alignment required ? Standard alignment (probably 4 bytes ?) is
> >> acceptable ?
> > No, I'll add a .balign 4 to be sure.
>
> I think it is aligned to 4 by default. My question was to know whether 4
> is enough.
> I see BPT_SIZE is 8, should the alignment be at least 8 ?
At this point each breakpoint entry is two instructions: the
instruction that has been replaced by a breakpoint and a trap
instruction after that. So I think it should be fine aligned to 4. In
the patch that introduces prefixed instructions to the data type, the
alignment changes.
>
> >>
> >>
> >>> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> >>> new file mode 100644
> >>> index 000000000000..b7e94375db86
> >>> --- /dev/null
> >>> +++ b/arch/powerpc/xmon/xmon_bpts.h
> >>> @@ -0,0 +1,14 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +#ifndef XMON_BPTS_H
> >>> +#define XMON_BPTS_H
> >>> +
> >>> +#define NBPTS 256
> >>> +#ifndef __ASSEMBLY__
> >>> +#define BPT_SIZE (sizeof(unsigned int) * 2)
> >>> +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> >>> +
> >>> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> >>> +
> >>> +#endif /* __ASSEMBLY__ */
> >>> +
> >>> +#endif /* XMON_BPTS_H */
> >>>
> >>
> >> Christophe
^ permalink raw reply
* Re: New powerpc vdso calling convention
From: Nicholas Piggin @ 2020-04-29 2:39 UTC (permalink / raw)
To: Adhemerval Zanella, Rich Felker
Cc: libc-alpha, musl, binutils, Andy Lutomirski, libc-dev,
Thomas Gleixner, Vincenzo Frascino, linuxppc-dev
In-Reply-To: <e8a9d3d9-18a7-2cb9-db70-74f7b107f744@linaro.org>
Excerpts from Adhemerval Zanella's message of April 27, 2020 11:09 pm:
>
>
> On 26/04/2020 00:41, Nicholas Piggin wrote:
>> Excerpts from Rich Felker's message of April 26, 2020 9:11 am:
>>> On Sun, Apr 26, 2020 at 08:58:19AM +1000, Nicholas Piggin wrote:
>>>> Excerpts from Christophe Leroy's message of April 25, 2020 10:20 pm:
>>>>>
>>>>>
>>>>> Le 25/04/2020 à 12:56, Nicholas Piggin a écrit :
>>>>>> Excerpts from Christophe Leroy's message of April 25, 2020 5:47 pm:
>>>>>>>
>>>>>>>
>>>>>>> Le 25/04/2020 à 07:22, Nicholas Piggin a écrit :
>>>>>>>> As noted in the 'scv' thread, powerpc's vdso calling convention does not
>>>>>>>> match the C ELF ABI calling convention (or the proposed scv convention).
>>>>>>>> I think we could implement a new ABI by basically duplicating function
>>>>>>>> entry points with different names.
>>>>>>>
>>>>>>> I think doing this is a real good idea.
>>>>>>>
>>>>>>> I've been working at porting powerpc VDSO to the GENERIC C VDSO, and the
>>>>>>> main pitfall has been that our vdso calling convention is not compatible
>>>>>>> with C calling convention, so we have go through an ASM entry/exit.
>>>>>>>
>>>>>>> See https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=171469
>>>>>>>
>>>>>>> We should kill this error flag return through CR[SO] and get it the
>>>>>>> "modern" way like other architectectures implementing the C VDSO: return
>>>>>>> 0 when successfull, return -err when failed.
>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>>>> The ELF v2 ABI convention would suit it well, because the caller already
>>>>>>>> requires the function address for ctr, so having it in r12 will
>>>>>>>> eliminate the need for address calculation, which suits the vdso data
>>>>>>>> page access.
>>>>>>>>
>>>>>>>> Is there a need for ELF v1 specific calls as well, or could those just be
>>>>>>>> deprecated and remain on existing functions or required to use the ELF
>>>>>>>> v2 calls using asm wrappers?
>>>>>>>
>>>>>>> What's ELF v1 and ELF v2 ? Is ELF v1 what PPC32 uses ? If so, I'd say
>>>>>>> yes, it would be good to have it to avoid going through ASM in the middle.
>>>>>>
>>>>>> I'm not sure about PPC32. On PPC64, ELFv2 functions must be called with
>>>>>> their address in r12 if called at their global entry point. ELFv1 have a
>>>>>> function descriptor with call address and TOC in it, caller has to load
>>>>>> the TOC if it's global.
>>>>>>
>>>>>> The vdso doesn't have TOC, it has one global address (the vdso data
>>>>>> page) which it loads by calculating its own address.
>>>>>>
>>>>>> The kernel doesn't change the vdso based on whether it's called by a v1
>>>>>> or v2 userspace (it doesn't really know itself and would have to export
>>>>>> different functions). glibc has a hack to create something:
>>>>>>
>>>>>> # define VDSO_IFUNC_RET(value) \
>>>>>> ({ \
>>>>>> static Elf64_FuncDesc vdso_opd = { .fd_toc = ~0x0 }; \
>>>>>> vdso_opd.fd_func = (Elf64_Addr)value; \
>>>>>> &vdso_opd; \
>>>>>> })
>>>>>>
>>>>>> If we could make something which links more like any other dso with
>>>>>> ELFv1, that would be good. Otherwise I think v2 is preferable so it
>>>>>> doesn't have to calculate its own address.
>>>>>
>>>>> I see the following in glibc. So looks like PPC32 is like PPC64 elfv1.
>>>>> By the way, they are talking about something not completely finished in
>>>>> the kernel. Can we finish it ?
>>>>
>>>> Possibly can. It seems like a good idea to fix all loose ends if we are
>>>> going to add new versions. Will have to check with the toolchain people
>>>> to make sure we're doing the right thing.
>>>
>>> "ELFv1" and "ELFv2" are PPC64-specific names for the old and new
>>> version of the ELF psABI for PPC64. They have nothing at all to do
>>> with PPC32 which is a completely different ABI from either.
>>
>> Right, I'm just talking about those comments -- it seems like the kernel
>> vdso should contain an .opd section with function descriptors in it for
>> elfv1 calls, rather than the hack it has now of creating one in the
>> caller's .data section.
>>
>> But all that function descriptor code is gated by
>>
>> #if (defined(__PPC64__) || defined(__powerpc64__)) && _CALL_ELF != 2
>>
>> So it seems PPC32 does not use function descriptors but a direct pointer
>> to the entry point like PPC64 with ELFv2.
>
> Yes, this hack is only for ELFv1. The missing ODP has not been an issue
> or glibc because it has been using the inline assembly to emulate the
> functions call since initial vDSO support (INTERNAL_VSYSCALL_CALL_TYPE).
> It just has become an issue when I added a ifunc optimization to
> gettimeofday so it can bypass the libc.so and make plt branch to vDSO
> directly.
I can't understand if it's actually a problem for you or not.
Regardless if you can hack around it, it seems to me that if we're going
to add sane calling conventions to the vdso, then we should also just
have a .opd section for it as well, whether or not a particular libc
requires it.
> Recently on some y2038 refactoring it was suggested to get rid of this
> and make gettimeofday call clock_gettime regardless. But some felt that
> the performance degradation was not worth for a symbol that is still used
> extensibility, so we stuck with the hack.
>
> And I think having this synthetic opd entry is not an issue, since for
> full relro the program's will be used and correctly set as read-only.
I'm not quite sure what this means, I don't really know how glibc ifunc
works. How do you set r2 if you have no opd?
> The issue is more for glibc itself, and I wouldn't mind to just remove the
> gettimeofday and time optimizations and use the default vDSO support
> (which might increase in function latency though).
>
> As Rich has put, it would be simpler to just have powerpc vDSO symbols
> to have a default function call semantic so we could issue a function
> call directly. But for powerpc64, we glibc will need to continue to
> support this non-standard call on older kernels and I am not sure if
> adding new symbols with a different semantic will help much.
Yeah, we will add entry points with default function call semantics.
At which point we make the things look like any other dso unless there
is good reason otherwise.
> GLibc already hides this powerpc semantic on INTERNAL_VSYSCALL_CALL_TYPE,
> so internally all syscalls are assumed to have the new semantic (-errno
> on error, 0 on success). Adding another ELFv1 would require to add
> more logic to handle multiple symbol version for vDSO setup
> (sysdeps/unix/sysv/linux/dl-vdso-setup.h), which would mostly likely to
> require an arch-specific implementation to handle it.
Is it not a build-time choice? The arch can set its own vdso symbol
names AFAIKS.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Tianjia Zhang @ 2020-04-29 2:20 UTC (permalink / raw)
To: Thomas Huth, Christian Borntraeger, Cornelia Huck
Cc: wanpengli, kvm, david, heiko.carstens, peterx, linux-mips, hpa,
kvmarm, linux-s390, frankja, maz, joro, x86, mingo,
julien.thierry.kdev, gor, suzuki.poulose, kvm-ppc, bp, tglx,
linux-arm-kernel, jmattson, tsbogend, christoffer.dall,
sean.j.christopherson, linux-kernel, james.morse, pbonzini,
vkuznets, linuxppc-dev
In-Reply-To: <73f6ecd0-ac47-eaad-0e4f-2d41c2b34450@redhat.com>
On 2020/4/26 20:59, Thomas Huth wrote:
> On 23/04/2020 13.00, Christian Borntraeger wrote:
>>
>>
>> On 23.04.20 12:58, Tianjia Zhang wrote:
>>>
>>>
>>> On 2020/4/23 18:39, Cornelia Huck wrote:
>>>> On Thu, 23 Apr 2020 11:01:43 +0800
>>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>>
>>>>> On 2020/4/23 0:04, Cornelia Huck wrote:
>>>>>> On Wed, 22 Apr 2020 17:58:04 +0200
>>>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>>>>
>>>>>>> On 22.04.20 15:45, Cornelia Huck wrote:
>>>>>>>> On Wed, 22 Apr 2020 20:58:04 +0800
>>>>>>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>>>>>>
>>>>>>>>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>>>>>>>>> structure. Earlier than historical reasons, many kvm-related function
>>>>>>>>
>>>>>>>> s/Earlier than/For/ ?
>>>>>>>>
>>>>>>>>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>>>>>>>>> This patch does a unified cleanup of these remaining redundant parameters.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>>>>>>> ---
>>>>>>>>> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++---------------
>>>>>>>>> 1 file changed, 22 insertions(+), 15 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>>>> index e335a7e5ead7..d7bb2e7a07ff 100644
>>>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>>>> @@ -4176,8 +4176,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>>>>>>>>> return rc;
>>>>>>>>> }
>>>>>>>>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>>>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>>>>>>>>> {
>>>>>>>>> + struct kvm_run *kvm_run = vcpu->run;
>>>>>>>>> struct runtime_instr_cb *riccb;
>>>>>>>>> struct gs_cb *gscb;
>>>>>>>>> @@ -4235,7 +4236,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>>>> }
>>>>>>>>> if (vcpu->arch.gs_enabled) {
>>>>>>>>> current->thread.gs_cb = (struct gs_cb *)
>>>>>>>>> - &vcpu->run->s.regs.gscb;
>>>>>>>>> + &kvm_run->s.regs.gscb;
>>>>>>>>
>>>>>>>> Not sure if these changes (vcpu->run-> => kvm_run->) are really worth
>>>>>>>> it. (It seems they amount to at least as much as the changes advertised
>>>>>>>> in the patch description.)
>>>>>>>>
>>>>>>>> Other opinions?
>>>>>>>
>>>>>>> Agreed. It feels kind of random. Maybe just do the first line (move kvm_run from the
>>>>>>> function parameter list into the variable declaration)? Not sure if this is better.
>>>>>>>
>>>>>>
>>>>>> There's more in this patch that I cut... but I think just moving
>>>>>> kvm_run from the parameter list would be much less disruptive.
>>>>>>
>>>>>
>>>>> I think there are two kinds of code(`vcpu->run->` and `kvm_run->`), but
>>>>> there will be more disruptive, not less.
>>>>
>>>> I just fail to see the benefit; sure, kvm_run-> is convenient, but the
>>>> current code is just fine, and any rework should be balanced against
>>>> the cost (e.g. cluttering git annotate).
>>>>
>>>
>>> cluttering git annotate ? Does it mean Fix xxxx ("comment"). Is it possible to solve this problem by splitting this patch?
>>
>> No its about breaking git blame (and bugfix backports) for just a cosmetic improvement.
>
> It could be slightly more than a cosmetic improvement (depending on the
> smartness of the compiler): vcpu->run-> are two dereferences, while
> kvm_run-> is only one dereference. So it could be slightly more compact
> and faster code.
>
> Thomas
>
If the compiler is smart enough, this place can be automatically
optimized, but we can't just rely on the compiler, if not? This requires
a trade-off between code cleanliness readability and breaking git blame.
In addition, I have removed the changes here and sent a v4 patch. Please
also help review it.
Thanks and best,
Tianjia
^ permalink raw reply
* [RFC PATCH v2 1/5] powerpc/mm: Introduce temporary mm
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
In-Reply-To: <20200429020531.20684-1-cmr@informatik.wtf>
x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
said mappings to other CPUs. A side benefit is that other CPU TLBs do
not need to be flushed when the temporary mm is torn down.
Mappings in the temporary mm can be set in the userspace portion of the
address-space.
Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when unloading
the temporary mm. All HW breakpoints are indiscriminately disabled while
the temporary mm is in use.
Based on x86 implementation:
commit cefa929c034e
("x86/mm: Introduce temporary mm structs")
Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
---
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/include/asm/mmu_context.h | 54 ++++++++++++++++++++++++++
arch/powerpc/kernel/process.c | 5 +++
3 files changed, 60 insertions(+)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..b945bc16c932 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,6 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs) { return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
+void __get_breakpoint(struct arch_hw_breakpoint *brk);
void __set_breakpoint(struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 360367c579de..57a8695fe63f 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -10,6 +10,7 @@
#include <asm/mmu.h>
#include <asm/cputable.h>
#include <asm/cputhreads.h>
+#include <asm/debug.h>
/*
* Most if the context management is out of line
@@ -270,5 +271,58 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
return 0;
}
+struct temp_mm {
+ struct mm_struct *temp;
+ struct mm_struct *prev;
+ bool is_kernel_thread;
+ struct arch_hw_breakpoint brk;
+};
+
+static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
+{
+ temp_mm->temp = mm;
+ temp_mm->prev = NULL;
+ temp_mm->is_kernel_thread = false;
+ memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
+}
+
+static inline void use_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ temp_mm->is_kernel_thread = current->mm == NULL;
+ if (temp_mm->is_kernel_thread)
+ temp_mm->prev = current->active_mm;
+ else
+ temp_mm->prev = current->mm;
+
+ /*
+ * Hash requires a non-NULL current->mm to allocate a userspace address
+ * when handling a page fault. Does not appear to hurt in Radix either.
+ */
+ current->mm = temp_mm->temp;
+ switch_mm_irqs_off(NULL, temp_mm->temp, current);
+
+ if (ppc_breakpoint_available()) {
+ __get_breakpoint(&temp_mm->brk);
+ if (temp_mm->brk.type != 0)
+ hw_breakpoint_disable();
+ }
+}
+
+static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ if (temp_mm->is_kernel_thread)
+ current->mm = NULL;
+ else
+ current->mm = temp_mm->prev;
+ switch_mm_irqs_off(NULL, temp_mm->prev, current);
+
+ if (ppc_breakpoint_available() && temp_mm->brk.type != 0)
+ __set_breakpoint(&temp_mm->brk);
+}
+
#endif /* __KERNEL__ */
#endif /* __ASM_POWERPC_MMU_CONTEXT_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 9c21288f8645..ec4cf890d92c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -800,6 +800,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
+void __get_breakpoint(struct arch_hw_breakpoint *brk)
+{
+ memcpy(brk, this_cpu_ptr(¤t_brk), sizeof(*brk));
+}
+
void __set_breakpoint(struct arch_hw_breakpoint *brk)
{
memcpy(this_cpu_ptr(¤t_brk), brk, sizeof(*brk));
--
2.26.1
^ permalink raw reply related
* [RFC PATCH v2 4/5] powerpc/lib: Add LKDTM accessor for patching addr
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
In-Reply-To: <20200429020531.20684-1-cmr@informatik.wtf>
When live patching a STRICT_RWX kernel, a mapping is installed at a
"patching address" with temporary write permissions. Provide a
LKDTM-only accessor function for this address in preparation for a LKDTM
test which attempts to "hijack" this mapping by writing to it from
another CPU.
Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
---
arch/powerpc/lib/code-patching.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 26f06cdb5d7e..cfbdef90384e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -46,6 +46,13 @@ int raw_patch_instruction(unsigned int *addr, unsigned int instr)
static struct mm_struct *patching_mm __ro_after_init;
static unsigned long patching_addr __ro_after_init;
+#ifdef CONFIG_LKDTM
+unsigned long read_cpu_patching_addr(unsigned int cpu)
+{
+ return patching_addr;
+}
+#endif
+
void __init poking_init(void)
{
spinlock_t *ptl; /* for protecting pte table */
--
2.26.1
^ permalink raw reply related
* [RFC PATCH v2 2/5] powerpc/lib: Initialize a temporary mm for code patching
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
In-Reply-To: <20200429020531.20684-1-cmr@informatik.wtf>
When code patching a STRICT_KERNEL_RWX kernel the page containing the
address to be patched is temporarily mapped with permissive memory
protections. Currently, a per-cpu vmalloc patch area is used for this
purpose. While the patch area is per-cpu, the temporary page mapping is
inserted into the kernel page tables for the duration of the patching.
The mapping is exposed to CPUs other than the patching CPU - this is
undesirable from a hardening perspective.
Use the `poking_init` init hook to prepare a temporary mm and patching
address. Initialize the temporary mm by copying the init mm. Choose a
randomized patching address inside the temporary mm userspace address
portion. The next patch uses the temporary mm and patching address for
code patching.
Based on x86 implementation:
commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")
Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
---
arch/powerpc/lib/code-patching.c | 33 ++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 3345f039a876..259c19480a85 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -11,6 +11,8 @@
#include <linux/cpuhotplug.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/sched/task.h>
+#include <linux/random.h>
#include <asm/pgtable.h>
#include <asm/tlbflush.h>
@@ -39,6 +41,37 @@ int raw_patch_instruction(unsigned int *addr, unsigned int instr)
}
#ifdef CONFIG_STRICT_KERNEL_RWX
+
+static struct mm_struct *patching_mm __ro_after_init;
+static unsigned long patching_addr __ro_after_init;
+
+void __init poking_init(void)
+{
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+
+ /*
+ * Some parts of the kernel (static keys for example) depend on
+ * successful code patching. Code patching under STRICT_KERNEL_RWX
+ * requires this setup - otherwise we cannot patch at all. We use
+ * BUG_ON() here and later since an early failure is preferred to
+ * buggy behavior and/or strange crashes later.
+ */
+ patching_mm = copy_init_mm();
+ BUG_ON(!patching_mm);
+
+ /*
+ * In hash we cannot go above DEFAULT_MAP_WINDOW easily.
+ * XXX: Do we want additional bits of entropy for radix?
+ */
+ patching_addr = (get_random_long() & PAGE_MASK) %
+ (DEFAULT_MAP_WINDOW - PAGE_SIZE);
+
+ ptep = get_locked_pte(patching_mm, patching_addr, &ptl);
+ BUG_ON(!ptep);
+ pte_unmap_unlock(ptep, ptl);
+}
+
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
static int text_area_cpu_up(unsigned int cpu)
--
2.26.1
^ permalink raw reply related
* [RFC PATCH v2 5/5] powerpc: Add LKDTM test to hijack a patch mapping
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
In-Reply-To: <20200429020531.20684-1-cmr@informatik.wtf>
When live patching with STRICT_KERNEL_RWX, the CPU doing the patching
must use a temporary mapping which allows for writing to kernel text.
During the entire window of time when this temporary mapping is in use,
another CPU could write to the same mapping and maliciously alter kernel
text. Implement a LKDTM test to attempt to exploit such a openings when
a CPU is patching under STRICT_KERNEL_RWX. The test is only implemented
on powerpc for now.
The LKDTM "hijack" test works as follows:
1. A CPU executes an infinite loop to patch an instruction.
This is the "patching" CPU.
2. Another CPU attempts to write to the address of the temporary
mapping used by the "patching" CPU. This other CPU is the
"hijacker" CPU. The hijack either fails with a segfault or
succeeds, in which case some kernel text is now overwritten.
How to run the test:
mount -t debugfs none /sys/kernel/debug
(echo HIJACK_PATCH > /sys/kernel/debug/provoke-crash/DIRECT)
Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
---
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 99 ++++++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+)
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index a5e344df9166..482e72f6a1e1 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -145,6 +145,7 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(WRITE_RO),
CRASHTYPE(WRITE_RO_AFTER_INIT),
CRASHTYPE(WRITE_KERN),
+ CRASHTYPE(HIJACK_PATCH),
CRASHTYPE(REFCOUNT_INC_OVERFLOW),
CRASHTYPE(REFCOUNT_ADD_OVERFLOW),
CRASHTYPE(REFCOUNT_INC_NOT_ZERO_OVERFLOW),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 601a2156a0d4..bfcf3542370d 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -62,6 +62,7 @@ void lkdtm_EXEC_USERSPACE(void);
void lkdtm_EXEC_NULL(void);
void lkdtm_ACCESS_USERSPACE(void);
void lkdtm_ACCESS_NULL(void);
+void lkdtm_HIJACK_PATCH(void);
/* lkdtm_refcount.c */
void lkdtm_REFCOUNT_INC_OVERFLOW(void);
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 62f76d506f04..547ce16e03e5 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -9,6 +9,7 @@
#include <linux/vmalloc.h>
#include <linux/mman.h>
#include <linux/uaccess.h>
+#include <linux/kthread.h>
#include <asm/cacheflush.h>
/* Whether or not to fill the target memory area with do_nothing(). */
@@ -213,6 +214,104 @@ void lkdtm_ACCESS_NULL(void)
*ptr = tmp;
}
+#if defined(CONFIG_PPC) && defined(CONFIG_STRICT_KERNEL_RWX)
+#include <include/asm/code-patching.h>
+
+extern unsigned long read_cpu_patching_addr(unsigned int cpu);
+
+static unsigned int * const patch_site = (unsigned int * const)&do_nothing;
+
+static int lkdtm_patching_cpu(void *data)
+{
+ int err = 0;
+
+ pr_info("starting patching_cpu=%d\n", smp_processor_id());
+ do {
+ err = patch_instruction(patch_site, 0xdeadbeef);
+ } while (*READ_ONCE(patch_site) == 0xdeadbeef &&
+ !err && !kthread_should_stop());
+
+ if (err)
+ pr_warn("patch_instruction returned error: %d\n", err);
+
+ set_current_state(TASK_INTERRUPTIBLE);
+ while (!kthread_should_stop()) {
+ schedule();
+ set_current_state(TASK_INTERRUPTIBLE);
+ }
+
+ return err;
+}
+
+void lkdtm_HIJACK_PATCH(void)
+{
+ struct task_struct *patching_kthrd;
+ int patching_cpu, hijacker_cpu, original_insn, attempts;
+ unsigned long addr;
+ bool hijacked;
+
+ if (num_online_cpus() < 2) {
+ pr_warn("need at least two cpus\n");
+ return;
+ }
+
+ original_insn = *READ_ONCE(patch_site);
+
+ hijacker_cpu = smp_processor_id();
+ patching_cpu = cpumask_any_but(cpu_online_mask, hijacker_cpu);
+
+ patching_kthrd = kthread_create_on_node(&lkdtm_patching_cpu, NULL,
+ cpu_to_node(patching_cpu),
+ "lkdtm_patching_cpu");
+ kthread_bind(patching_kthrd, patching_cpu);
+ wake_up_process(patching_kthrd);
+
+ addr = offset_in_page(patch_site) | read_cpu_patching_addr(patching_cpu);
+
+ pr_info("starting hijacker_cpu=%d\n", hijacker_cpu);
+ for (attempts = 0; attempts < 100000; ++attempts) {
+ /* Use __put_user to catch faults without an Oops */
+ hijacked = !__put_user(0xbad00bad, (unsigned int *)addr);
+
+ if (hijacked) {
+ if (kthread_stop(patching_kthrd))
+ goto out;
+ break;
+ }
+ }
+ pr_info("hijack attempts: %d\n", attempts);
+
+ if (hijacked) {
+ if (*READ_ONCE(patch_site) == 0xbad00bad)
+ pr_err("overwrote kernel text\n");
+ /*
+ * There are window conditions where the hijacker cpu manages to
+ * write to the patch site but the site gets overwritten again by
+ * the patching cpu. We still consider that a "successful" hijack
+ * since the hijacker cpu did not fault on the write.
+ */
+ pr_err("FAIL: wrote to another cpu's patching area\n");
+ } else {
+ kthread_stop(patching_kthrd);
+ }
+
+out:
+ /* Restore the original insn for any future lkdtm tests */
+ patch_instruction(patch_site, original_insn);
+}
+
+#else
+
+void lkdtm_HIJACK_PATCH(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC))
+ pr_err("XFAIL: this test is powerpc-only\n");
+ if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+ pr_err("XFAIL: this test requires CONFIG_STRICT_KERNEL_RWX\n");
+}
+
+#endif /* CONFIG_PPC && CONFIG_STRICT_KERNEL_RWX */
+
void __init lkdtm_perms_init(void)
{
/* Make sure we can write to __ro_after_init values during __init */
--
2.26.1
^ permalink raw reply related
* [RFC PATCH v2 3/5] powerpc/lib: Use a temporary mm for code patching
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
In-Reply-To: <20200429020531.20684-1-cmr@informatik.wtf>
Currently, code patching a STRICT_KERNEL_RWX exposes the temporary
mappings to other CPUs. These mappings should be kept local to the CPU
doing the patching. Use the pre-initialized temporary mm and patching
address for this purpose. Also add a check after patching to ensure the
patch succeeded.
Use the KUAP functions on non-BOOKS3_64 platforms since the temporary
mapping for patching uses a userspace address (to keep the mapping
local). On BOOKS3_64 platforms hash does not implement KUAP and on radix
the use of PAGE_KERNEL sets EAA[0] for the PTE which means the AMR
(KUAP) protection is ignored (see PowerISA v3.0b, Fig, 35).
Based on x86 implementation:
commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")
Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
---
arch/powerpc/lib/code-patching.c | 149 ++++++++++++-------------------
1 file changed, 55 insertions(+), 94 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 259c19480a85..26f06cdb5d7e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -19,6 +19,7 @@
#include <asm/page.h>
#include <asm/code-patching.h>
#include <asm/setup.h>
+#include <asm/mmu_context.h>
static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
unsigned int *patch_addr)
@@ -72,101 +73,58 @@ void __init poking_init(void)
pte_unmap_unlock(ptep, ptl);
}
-static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
-
-static int text_area_cpu_up(unsigned int cpu)
-{
- struct vm_struct *area;
-
- area = get_vm_area(PAGE_SIZE, VM_ALLOC);
- if (!area) {
- WARN_ONCE(1, "Failed to create text area for cpu %d\n",
- cpu);
- return -1;
- }
- this_cpu_write(text_poke_area, area);
-
- return 0;
-}
-
-static int text_area_cpu_down(unsigned int cpu)
-{
- free_vm_area(this_cpu_read(text_poke_area));
- return 0;
-}
-
-/*
- * Run as a late init call. This allows all the boot time patching to be done
- * simply by patching the code, and then we're called here prior to
- * mark_rodata_ro(), which happens after all init calls are run. Although
- * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
- * it as being preferable to a kernel that will crash later when someone tries
- * to use patch_instruction().
- */
-static int __init setup_text_poke_area(void)
-{
- BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
- "powerpc/text_poke:online", text_area_cpu_up,
- text_area_cpu_down));
-
- return 0;
-}
-late_initcall(setup_text_poke_area);
+struct patch_mapping {
+ spinlock_t *ptl; /* for protecting pte table */
+ pte_t *ptep;
+ struct temp_mm temp_mm;
+};
/*
* This can be called for kernel text or a module.
*/
-static int map_patch_area(void *addr, unsigned long text_poke_addr)
+static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
{
- unsigned long pfn;
- int err;
+ struct page *page;
+ pte_t pte;
+ pgprot_t pgprot;
if (is_vmalloc_addr(addr))
- pfn = vmalloc_to_pfn(addr);
+ page = vmalloc_to_page(addr);
else
- pfn = __pa_symbol(addr) >> PAGE_SHIFT;
+ page = virt_to_page(addr);
- err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
+ if (radix_enabled())
+ pgprot = PAGE_KERNEL;
+ else
+ pgprot = PAGE_SHARED;
- pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err);
- if (err)
+ patch_mapping->ptep = get_locked_pte(patching_mm, patching_addr,
+ &patch_mapping->ptl);
+ if (unlikely(!patch_mapping->ptep)) {
+ pr_warn("map patch: failed to allocate pte for patching\n");
return -1;
+ }
+
+ pte = mk_pte(page, pgprot);
+ if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
+ pte = pte_mkdirty(pte);
+ set_pte_at(patching_mm, patching_addr, patch_mapping->ptep, pte);
+
+ init_temp_mm(&patch_mapping->temp_mm, patching_mm);
+ use_temporary_mm(&patch_mapping->temp_mm);
return 0;
}
-static inline int unmap_patch_area(unsigned long addr)
+static void unmap_patch(struct patch_mapping *patch_mapping)
{
- pte_t *ptep;
- pmd_t *pmdp;
- pud_t *pudp;
- pgd_t *pgdp;
-
- pgdp = pgd_offset_k(addr);
- if (unlikely(!pgdp))
- return -EINVAL;
-
- pudp = pud_offset(pgdp, addr);
- if (unlikely(!pudp))
- return -EINVAL;
-
- pmdp = pmd_offset(pudp, addr);
- if (unlikely(!pmdp))
- return -EINVAL;
-
- ptep = pte_offset_kernel(pmdp, addr);
- if (unlikely(!ptep))
- return -EINVAL;
+ /* In hash, pte_clear flushes the tlb */
+ pte_clear(patching_mm, patching_addr, patch_mapping->ptep);
+ unuse_temporary_mm(&patch_mapping->temp_mm);
- pr_devel("clearing mm %p, pte %p, addr %lx\n", &init_mm, ptep, addr);
-
- /*
- * In hash, pte_clear flushes the tlb, in radix, we have to
- */
- pte_clear(&init_mm, addr, ptep);
- flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
-
- return 0;
+ /* In radix, we have to explicitly flush the tlb (no-op in hash) */
+ local_flush_tlb_mm(patching_mm);
+ pte_unmap_unlock(patch_mapping->ptep, patch_mapping->ptl);
}
static int do_patch_instruction(unsigned int *addr, unsigned int instr)
@@ -174,33 +132,36 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
int err;
unsigned int *patch_addr = NULL;
unsigned long flags;
- unsigned long text_poke_addr;
- unsigned long kaddr = (unsigned long)addr;
+ struct patch_mapping patch_mapping;
/*
- * During early early boot patch_instruction is called
- * when text_poke_area is not ready, but we still need
- * to allow patching. We just do the plain old patching
+ * The patching_mm is initialized before calling mark_rodata_ro. Prior
+ * to this, patch_instruction is called when we don't have (and don't
+ * need) the patching_mm so just do plain old patching.
*/
- if (!this_cpu_read(text_poke_area))
+ if (!patching_mm)
return raw_patch_instruction(addr, instr);
local_irq_save(flags);
- text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
- if (map_patch_area(addr, text_poke_addr)) {
- err = -1;
+ err = map_patch(addr, &patch_mapping);
+ if (err)
goto out;
- }
- patch_addr = (unsigned int *)(text_poke_addr) +
- ((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
+ patch_addr = (unsigned int *)(patching_addr | offset_in_page(addr));
- __patch_instruction(addr, instr, patch_addr);
+ if (!radix_enabled())
+ allow_write_to_user(patch_addr, sizeof(instr));
+ err = __patch_instruction(addr, instr, patch_addr);
+ if (!radix_enabled())
+ prevent_write_to_user(patch_addr, sizeof(instr));
- err = unmap_patch_area(text_poke_addr);
- if (err)
- pr_warn("failed to unmap %lx\n", text_poke_addr);
+ unmap_patch(&patch_mapping);
+ /*
+ * Something is wrong if what we just wrote doesn't match what we
+ * think we just wrote.
+ */
+ WARN_ON(*addr != instr);
out:
local_irq_restore(flags);
--
2.26.1
^ permalink raw reply related
* [RFC PATCH v2 0/5] Use per-CPU temporary mappings for patching
From: Christopher M. Riedl @ 2020-04-29 2:05 UTC (permalink / raw)
To: linuxppc-dev, kernel-hardening
When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
temporary mappings when patching itself. These mappings temporarily
override the strict RWX text protections to permit a write. Currently,
powerpc allocates a per-CPU VM area for patching. Patching occurs as
follows:
1. Map page of text to be patched to per-CPU VM area w/
PAGE_KERNEL protection
2. Patch text
3. Remove the temporary mapping
While the VM area is per-CPU, the mapping is actually inserted into the
kernel page tables. Presumably, this could allow another CPU to access
the normally write-protected text - either malicously or accidentally -
via this same mapping if the address of the VM area is known. Ideally,
the mapping should be kept local to the CPU doing the patching (or any
other sensitive operations requiring temporarily overriding memory
protections) [0].
x86 introduced "temporary mm" structs which allow the creation of
mappings local to a particular CPU [1]. This series intends to bring the
notion of a temporary mm to powerpc and harden powerpc by using such a
mapping for patching a kernel with strict RWX permissions.
The first patch introduces the temporary mm struct and API for powerpc
along with a new function to retrieve a current hw breakpoint.
The second patch uses the `poking_init` init hook added by the x86
patches to initialize a temporary mm and patching address. The patching
address is randomized between 0 and DEFAULT_MAP_WINDOW-PAGE_SIZE. The
upper limit is necessary due to how the hash MMU operates - by default
the space above DEFAULT_MAP_WINDOW is not available. For now, both hash
and radix randomize inside this range. The number of possible random
addresses is dependent on PAGE_SIZE and limited by DEFAULT_MAP_WINDOW.
Bits of entropy with 64K page size on BOOK3S_64:
bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
bits of entropy = log2(128TB / 64K)
bits of entropy = 31
Randomization occurs only once during initialization at boot.
The third patch replaces the VM area with the temporary mm in the
patching code. The page for patching has to be mapped PAGE_SHARED with
the hash MMU since hash prevents the kernel from accessing userspace
pages with PAGE_PRIVILEGED bit set. On the radix MMU the page is mapped with
PAGE_KERNEL which has the added benefit that we can skip KUAP.
The fourth and fifth patches implement an LKDTM test "proof-of-concept" which
exploits the previous vulnerability (ie. the mapping during patching is exposed
in kernel page tables and accessible by other CPUS). The LKDTM test is somewhat
"rough" in that it uses a brute-force approach - I am open to any suggestions
and/or ideas to improve this. Currently, the LKDTM test passes with this series
on POWER8 (hash) and POWER9 (radix, hash) and fails without this series (ie.
the temporary mapping for patching is exposed to CPUs other than the patching
CPU).
The test can be applied to a tree without this new series by first
adding this in /arch/powerpc/lib/code-patching.c:
@@ -41,6 +41,13 @@ int raw_patch_instruction(unsigned int *addr, unsigned int instr)
#ifdef CONFIG_STRICT_KERNEL_RWX
static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
+#ifdef CONFIG_LKDTM
+unsigned long read_cpu_patching_addr(unsigned int cpu)
+{
+ return (unsigned long)(per_cpu(text_poke_area, cpu))->addr;
+}
+#endif
+
static int text_area_cpu_up(unsigned int cpu)
{
struct vm_struct *area;
And then applying the last patch of this series which adds the LKDTM test,
(powerpc: Add LKDTM test to hijack a patch mapping).
Tested on QEMU (POWER8, POWER9), POWER8 VM, and a Blackbird (8-core POWER9).
v2: Many fixes and improvements mostly based on extensive feedback and testing
by Christophe Leroy (thanks!).
* Make patching_mm and patching_addr static and mode '__ro_after_init'
to after the variable name (more common in other parts of the kernel)
* Use 'asm/debug.h' header instead of 'asm/hw_breakpoint.h' to fix
PPC64e compile
* Add comment explaining why we use BUG_ON() during the init call to
setup for patching later
* Move ptep into patch_mapping to avoid walking page tables a second
time when unmapping the temporary mapping
* Use KUAP under non-radix, also manually dirty the PTE for patch
mapping on non-BOOK3S_64 platforms
* Properly return any error from __patch_instruction
* Do not use 'memcmp' where a simple comparison is appropriate
* Simplify expression for patch address by removing pointer maths
* Add LKDTM test
[0]: https://github.com/linuxppc/issues/issues/224
[1]: https://lore.kernel.org/kernel-hardening/20190426232303.28381-1-nadav.amit@gmail.com/
Christopher M. Riedl (5):
powerpc/mm: Introduce temporary mm
powerpc/lib: Initialize a temporary mm for code patching
powerpc/lib: Use a temporary mm for code patching
powerpc/lib: Add LKDTM accessor for patching addr
powerpc: Add LKDTM test to hijack a patch mapping
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/include/asm/mmu_context.h | 54 ++++++++
arch/powerpc/kernel/process.c | 5 +
arch/powerpc/lib/code-patching.c | 173 +++++++++++++------------
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 99 ++++++++++++++
7 files changed, 248 insertions(+), 86 deletions(-)
--
2.26.1
^ permalink raw reply
* Re: [PATCH v6 11/28] powerpc: Use a datatype for instructions
From: Alistair Popple @ 2020-04-29 2:02 UTC (permalink / raw)
To: Jordan Niethe; +Cc: npiggin, bala24, linuxppc-dev, dja
In-Reply-To: <20200428015814.15380-12-jniethe5@gmail.com>
Hi Jordan,
I needed the below fix for building with CONFIG_STRICT_KERNEL_RWX enabled.
Hopefully it's correct, I have not yet had a chance to test it beyond building
it.
- Alistair
---
arch/powerpc/lib/code-patching.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
patching.c
index ad5754c5f007..a8c8ffdb1ccd 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -166,8 +166,8 @@ static int do_patch_instruction(struct ppc_inst *addr,
struct ppc_inst instr)
goto out;
}
- patch_addr = (unsigned int *)(text_poke_addr) +
- ((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
+ patch_addr = (struct ppc_inst *)(text_poke_addr) +
+ ((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
__patch_instruction(addr, instr, patch_addr);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v6 10/28] powerpc: Introduce functions for instruction equality
From: Alistair Popple @ 2020-04-29 1:59 UTC (permalink / raw)
To: Jordan Niethe; +Cc: npiggin, bala24, linuxppc-dev, dja
In-Reply-To: <20200428015814.15380-11-jniethe5@gmail.com>
There seems to be a minor typo which breaks compilation when
CONFIG_MPROFILE_KERNEL is not enabled. See the fix below.
---
arch/powerpc/kernel/trace/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/
ftrace.c
index a6064e1977ca..0ad2c9d4ab49 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -499,7 +499,7 @@ expected_nop_sequence(void *ip, struct ppc_inst op0,
struct ppc_inst op1)
* The load offset is different depending on the ABI. For simplicity
* just mask it out when doing the compare.
*/
- if ((!ppc_inst_equal(op0), ppc_inst(0x48000008)) || (ppc_inst_val(op1) &
0xffff0000) != 0xe8410000)
+ if ((!ppc_inst_equal(op0, ppc_inst(0x48000008))) || (ppc_inst_val(op1) &
0xffff0000) != 0xe8410000)
return 0;
return 1;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Srikar Dronamraju @ 2020-04-29 1:41 UTC (permalink / raw)
To: Andrew Morton
Cc: Michal Hocko, Linus Torvalds, linux-kernel, linux-mm, Mel Gorman,
Kirill A. Shutemov, Christopher Lameter, linuxppc-dev,
Vlastimil Babka
In-Reply-To: <20200428165912.ca1eadefbac56d740e6e8fd1@linux-foundation.org>
> >
> > By marking, N_ONLINE as NODE_MASK_NONE, lets stop assuming that Node 0 is
> > always online.
> >
> > ...
> >
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -116,8 +116,10 @@ EXPORT_SYMBOL(latent_entropy);
> > */
> > nodemask_t node_states[NR_NODE_STATES] __read_mostly = {
> > [N_POSSIBLE] = NODE_MASK_ALL,
> > +#ifdef CONFIG_NUMA
> > + [N_ONLINE] = NODE_MASK_NONE,
> > +#else
> > [N_ONLINE] = { { [0] = 1UL } },
> > -#ifndef CONFIG_NUMA
> > [N_NORMAL_MEMORY] = { { [0] = 1UL } },
> > #ifdef CONFIG_HIGHMEM
> > [N_HIGH_MEMORY] = { { [0] = 1UL } },
>
> So on all other NUMA machines, when does node 0 get marked online?
>
> This change means that for some time during boot, such machines will
> now be running with node 0 marked as offline. What are the
> implications of this? Will something break?
Till the nodes are detected, marking Node 0 as online tends to be redundant.
Because the system doesn't know if its a NUMA or a non-NUMA system.
Once we detect the nodes, we online them immediately. Hence I don't see any
side-effects or negative implications of this change.
However if I am missing anything, please do let me know.
From my part, I have tested this on
1. Non-NUMA Single node but CPUs and memory coming from zero node.
2. Non-NUMA Single node but CPUs and memory coming from non-zero node.
3. NUMA Multi node but with CPUs and memory from node 0.
4. NUMA Multi node but with no CPUs and memory from node 0.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [RFC PATCH] powerpc/spufs: fix copy_to_user while atomic
From: Jeremy Kerr @ 2020-04-29 1:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linuxppc-dev, linux-kernel, Eric W . Biederman,
linux-fsdevel, Andrew Morton, Linus Torvalds, Alexander Viro
In-Reply-To: <20200428171133.GA17445@lst.de>
Hi Christoph,
> FYI, these little hunks reduce the difference to my version, maybe
> you can fold them in?
Sure, no problem.
How do you want to coordinate these? I can submit mine through mpe, but
that may make it tricky to synchronise with your changes. Or, you can
include this change in your series if you prefer.
Cheers,
Jeremy
^ permalink raw reply
* [PATCH v3] powerpc/64: Option to use ELF V2 ABI for big-endian kernels
From: Nicholas Piggin @ 2020-04-29 1:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Provide an option to build big-endian kernels using the ELF V2 ABI. This works
on GCC and clang (since about 2014). it is is not officially supported by the
GNU toolchain, but it can give big-endian kernels some useful advantages of
the V2 ABI (e.g., less stack usage).
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v1:
- Improved the override flavour name suggested by Segher.
- Improved changelog wording.
Since v2:
- Improved changelog, help text, to use the name ELF V2 ABI in the spec,
and clarify things a bit more, suggested by Segher.
- For option name, match the ELF_ABI_v1/2 which is already in the kernel.
- Prefix options with PPC64_ to avoid arch clashes or confusion.
- "elfv2" is the toolchain name of the ABI, so I kept that in the crypto
perl scripts.
arch/powerpc/Kconfig | 21 +++++++++++++++++++++
arch/powerpc/Makefile | 15 ++++++++++-----
arch/powerpc/boot/Makefile | 4 ++++
drivers/crypto/vmx/Makefile | 8 ++++++--
drivers/crypto/vmx/ppc-xlate.pl | 10 ++++++----
5 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 924c541a9260..444867e07039 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -147,6 +147,7 @@ config PPC
select ARCH_WEAK_RELEASE_ACQUIRE
select BINFMT_ELF
select BUILDTIME_TABLE_SORT
+ select PPC64_BUILD_ELF_ABI_V2 if PPC64 && CPU_LITTLE_ENDIAN
select CLONE_BACKWARDS
select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN
select DYNAMIC_FTRACE if FUNCTION_TRACER
@@ -541,6 +542,26 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
+config PPC64_BUILD_ELF_ABI_V2
+ bool
+
+config PPC64_BUILD_BIG_ENDIAN_ELF_ABI_V2
+ bool "Build big-endian kernel using ELF V2 ABI (EXPERIMENTAL)"
+ depends on PPC64 && CPU_BIG_ENDIAN && EXPERT
+ default n
+ select PPC64_BUILD_ELF_ABI_V2
+ help
+ This builds the kernel image using the "Power Architecture 64-Bit ELF
+ V2 ABI Specification", which has a reduced stack overhead and faster
+ function calls. This internal kernel ABI option does not affect
+ userspace compatibility.
+
+ The V2 ABI is standard for 64-bit little-endian, but for big-endian
+ it is less well tested by kernel and toolchain. However some distros
+ build userspace this way, and it can produce a functioning kernel.
+
+ This requires gcc 4.9 or newer and binutils 2.24 or newer.
+
config RELOCATABLE
bool "Build a relocatable kernel"
depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index f310c32e88a4..baf477d100b2 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -92,10 +92,14 @@ endif
ifdef CONFIG_PPC64
ifndef CONFIG_CC_IS_CLANG
-cflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mabi=elfv1)
-cflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mcall-aixdesc)
-aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mabi=elfv1)
-aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
+ifdef CONFIG_PPC64_BUILD_ELF_ABI_V2
+cflags-y += $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
+aflags-y += $(call cc-option,-mabi=elfv2)
+else
+cflags-y += $(call cc-option,-mabi=elfv1)
+cflags-y += $(call cc-option,-mcall-aixdesc)
+aflags-y += $(call cc-option,-mabi=elfv1)
+endif
endif
endif
@@ -144,7 +148,7 @@ endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
ifndef CONFIG_CC_IS_CLANG
-ifdef CONFIG_CPU_LITTLE_ENDIAN
+ifdef CONFIG_PPC64_BUILD_ELF_ABI_V2
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
else
@@ -153,6 +157,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
endif
+
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index c53a1b8bba8b..d27e85413c84 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -41,6 +41,10 @@ endif
BOOTCFLAGS += -isystem $(shell $(BOOTCC) -print-file-name=include)
+ifdef CONFIG_PPC64_BUILD_ELF_ABI_V2
+BOOTCFLAGS += $(call cc-option,-mabi=elfv2)
+endif
+
ifdef CONFIG_CPU_BIG_ENDIAN
BOOTCFLAGS += -mbig-endian
else
diff --git a/drivers/crypto/vmx/Makefile b/drivers/crypto/vmx/Makefile
index 709670d2b553..751ffca694aa 100644
--- a/drivers/crypto/vmx/Makefile
+++ b/drivers/crypto/vmx/Makefile
@@ -5,18 +5,22 @@ vmx-crypto-objs := vmx.o aesp8-ppc.o ghashp8-ppc.o aes.o aes_cbc.o aes_ctr.o aes
ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
override flavour := linux-ppc64le
else
+ifdef CONFIG_PPC64_BUILD_ELF_ABI_V2
+override flavour := linux-ppc64-elfv2
+else
override flavour := linux-ppc64
endif
+endif
quiet_cmd_perl = PERL $@
cmd_perl = $(PERL) $(<) $(flavour) > $(@)
targets += aesp8-ppc.S ghashp8-ppc.S
-$(obj)/aesp8-ppc.S: $(src)/aesp8-ppc.pl FORCE
+$(obj)/aesp8-ppc.S: $(src)/aesp8-ppc.pl $(src)/ppc-xlate.pl FORCE
$(call if_changed,perl)
-$(obj)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl FORCE
+$(obj)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl $(src)/ppc-xlate.pl FORCE
$(call if_changed,perl)
clean-files := aesp8-ppc.S ghashp8-ppc.S
diff --git a/drivers/crypto/vmx/ppc-xlate.pl b/drivers/crypto/vmx/ppc-xlate.pl
index 36db2ef09e5b..b583898c11ae 100644
--- a/drivers/crypto/vmx/ppc-xlate.pl
+++ b/drivers/crypto/vmx/ppc-xlate.pl
@@ -9,6 +9,8 @@ open STDOUT,">$output" || die "can't open $output: $!";
my %GLOBALS;
my $dotinlocallabels=($flavour=~/linux/)?1:0;
+my $elfv2abi=(($flavour =~ /linux-ppc64le/) or ($flavour =~ /linux-ppc64-elfv2/))?1:0;
+my $dotfunctions=($elfv2abi=~1)?0:1;
################################################################
# directives which need special treatment on different platforms
@@ -40,7 +42,7 @@ my $globl = sub {
};
my $text = sub {
my $ret = ($flavour =~ /aix/) ? ".csect\t.text[PR],7" : ".text";
- $ret = ".abiversion 2\n".$ret if ($flavour =~ /linux.*64le/);
+ $ret = ".abiversion 2\n".$ret if ($elfv2abi);
$ret;
};
my $machine = sub {
@@ -56,8 +58,8 @@ my $size = sub {
if ($flavour =~ /linux/)
{ shift;
my $name = shift; $name =~ s|^[\.\_]||;
- my $ret = ".size $name,.-".($flavour=~/64$/?".":"").$name;
- $ret .= "\n.size .$name,.-.$name" if ($flavour=~/64$/);
+ my $ret = ".size $name,.-".($dotfunctions?".":"").$name;
+ $ret .= "\n.size .$name,.-.$name" if ($dotfunctions);
$ret;
}
else
@@ -142,7 +144,7 @@ my $vmr = sub {
# Some ABIs specify vrsave, special-purpose register #256, as reserved
# for system use.
-my $no_vrsave = ($flavour =~ /linux-ppc64le/);
+my $no_vrsave = ($elfv2abi);
my $mtspr = sub {
my ($f,$idx,$ra) = @_;
if ($idx == 256 && $no_vrsave) {
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v3 00/29] Convert files to ReST - part 2
From: Jan Harkes @ 2020-04-28 19:25 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-usb,
linux-kernel, linux-xfs, linux-cachefs, linux-fsdevel,
linuxppc-dev
In-Reply-To: <20200428130128.22c4b973@lwn.net>
On Tue, Apr 28, 2020 at 03:09:51PM -0400, Jonathan Corbet wrote:
> So I'm happy to merge this set, but there is one thing that worries me a
> bit...
>
> > fs/coda/Kconfig | 2 +-
>
> I'd feel a bit better if I could get an ack or two from filesystem folks
> before I venture that far out of my own yard...what say you all?
I acked the Coda parts on the first iteration of this patch. I have no
problem with you merging them.
Jan
^ permalink raw reply
* Re: [PATCH v2] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Nicholas Piggin @ 2020-04-29 0:57 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <20200428234046.GP17645@gate.crashing.org>
Excerpts from Segher Boessenkool's message of April 29, 2020 9:40 am:
> Hi!
>
> On Tue, Apr 28, 2020 at 09:25:17PM +1000, Nicholas Piggin wrote:
>> +config BUILD_BIG_ENDIAN_ELF_V2
>> + bool "Build big-endian kernel using ELFv2 ABI (EXPERIMENTAL)"
>> + depends on PPC64 && CPU_BIG_ENDIAN && EXPERT
>> + default n
>> + select BUILD_ELF_V2
>> + help
>> + This builds the kernel image using the ELFv2 ABI, which has a
>> + reduced stack overhead and faster function calls. This does not
>> + affect the userspace ABIs.
>> +
>> + ELFv2 is the standard ABI for little-endian, but for big-endian
>> + this is an experimental option that is less tested (kernel and
>> + toolchain). This requires gcc 4.9 or newer and binutils 2.24 or
>> + newer.
>
> Is it clear that this is only for 64-bit? Maybe this text should fit
> that in somewhere?
Don't know if it's necessary, the option only appears when 64-bit is
selected.
> It's not obvious to people who do not already know that ELFv2 is just
> the (nick-)name of a particular ABI, not a new kind of ELF (it is just
> version 1 ELF in fact), and that ABI is for 64-bit Power only.
I blame toolchain for -mabi=elfv2 ! And also some blame on ABI document
which is called ELF V2 ABI rather than ELF ABI V2 which would have been
unambiguous.
Kernel mostly gets it right (in the code I didn't write), and uses these
#if defined(_CALL_ELF) && _CALL_ELF == 2
#define PPC64_ELF_ABI_v2
#else
#define PPC64_ELF_ABI_v1
#endif
I can go through and change all my stuff and config options to ELF_ABI_v2.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Andrew Morton @ 2020-04-28 23:59 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Michal Hocko, Linus Torvalds, linux-kernel, linux-mm, Mel Gorman,
Kirill A. Shutemov, Christopher Lameter, linuxppc-dev,
Vlastimil Babka
In-Reply-To: <20200428093836.27190-4-srikar@linux.vnet.ibm.com>
On Tue, 28 Apr 2020 15:08:36 +0530 Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> Currently Linux kernel with CONFIG_NUMA on a system with multiple
> possible nodes, marks node 0 as online at boot. However in practice,
> there are systems which have node 0 as memoryless and cpuless.
>
> This can cause numa_balancing to be enabled on systems with only one node
> with memory and CPUs. The existence of this dummy node which is cpuless and
> memoryless node can confuse users/scripts looking at output of lscpu /
> numactl.
>
> By marking, N_ONLINE as NODE_MASK_NONE, lets stop assuming that Node 0 is
> always online.
>
> ...
>
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -116,8 +116,10 @@ EXPORT_SYMBOL(latent_entropy);
> */
> nodemask_t node_states[NR_NODE_STATES] __read_mostly = {
> [N_POSSIBLE] = NODE_MASK_ALL,
> +#ifdef CONFIG_NUMA
> + [N_ONLINE] = NODE_MASK_NONE,
> +#else
> [N_ONLINE] = { { [0] = 1UL } },
> -#ifndef CONFIG_NUMA
> [N_NORMAL_MEMORY] = { { [0] = 1UL } },
> #ifdef CONFIG_HIGHMEM
> [N_HIGH_MEMORY] = { { [0] = 1UL } },
So on all other NUMA machines, when does node 0 get marked online?
This change means that for some time during boot, such machines will
now be running with node 0 marked as offline. What are the
implications of this? Will something break?
^ permalink raw reply
* Re: [PATCH v2] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Segher Boessenkool @ 2020-04-28 23:40 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200428112517.1402927-1-npiggin@gmail.com>
Hi!
On Tue, Apr 28, 2020 at 09:25:17PM +1000, Nicholas Piggin wrote:
> +config BUILD_BIG_ENDIAN_ELF_V2
> + bool "Build big-endian kernel using ELFv2 ABI (EXPERIMENTAL)"
> + depends on PPC64 && CPU_BIG_ENDIAN && EXPERT
> + default n
> + select BUILD_ELF_V2
> + help
> + This builds the kernel image using the ELFv2 ABI, which has a
> + reduced stack overhead and faster function calls. This does not
> + affect the userspace ABIs.
> +
> + ELFv2 is the standard ABI for little-endian, but for big-endian
> + this is an experimental option that is less tested (kernel and
> + toolchain). This requires gcc 4.9 or newer and binutils 2.24 or
> + newer.
Is it clear that this is only for 64-bit? Maybe this text should fit
that in somewhere?
It's not obvious to people who do not already know that ELFv2 is just
the (nick-)name of a particular ABI, not a new kind of ELF (it is just
version 1 ELF in fact), and that ABI is for 64-bit Power only.
Segher
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Fall back to 16 H_SEND_SUB_CRQ_INDIRECT entries with old FW
From: Juliet Kim @ 2020-04-28 22:29 UTC (permalink / raw)
To: Thomas Falcon, netdev; +Cc: linuxppc-dev
In-Reply-To: <8617ba73-8a05-51c4-e52b-164687cecf07@linux.ibm.com>
On 4/28/20 10:35 AM, Thomas Falcon wrote:
> On 4/27/20 12:33 PM, Juliet Kim wrote:
>> The maximum entries for H_SEND_SUB_CRQ_INDIRECT has increased on
>> some platforms from 16 to 128. If Live Partition Mobility is used
>> to migrate a running OS image from a newer source platform to an
>> older target platform, then H_SEND_SUB_CRQ_INDIRECT will fail with
>> H_PARAMETER if 128 entries are queued.
>>
>> Fix this by falling back to 16 entries if H_PARAMETER is returned
>> from the hcall().
>
> Thanks for the submission, but I am having a hard time believing that this is what is happening since the driver does not support sending multiple frames per hypervisor call at this time. Even if it were the case, this approach would omit frame data needed by the VF, so the second attempt may still fail. Are there system logs available that show the driver is attempting to send transmissions with greater than 16 descriptors?
>
> Thanks,
>
> Tom
>
>
I am trying to confirm.
Juliet
>>
>> Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
>> ---
>> drivers/net/ethernet/ibm/ibmvnic.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>> index 4bd33245bad6..b66c2f26a427 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>> @@ -1656,6 +1656,17 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
>> lpar_rc = send_subcrq_indirect(adapter, handle_array[queue_num],
>> (u64)tx_buff->indir_dma,
>> (u64)num_entries);
>> +
>> + /* Old firmware accepts max 16 num_entries */
>> + if (lpar_rc == H_PARAMETER && num_entries > 16) {
>> + tx_crq.v1.n_crq_elem = 16;
>> + tx_buff->num_entries = 16;
>> + lpar_rc = send_subcrq_indirect(adapter,
>> + handle_array[queue_num],
>> + (u64)tx_buff->indir_dma,
>> + 16);
>> + }
>> +
>> dma_unmap_single(dev, tx_buff->indir_dma,
>> sizeof(tx_buff->indir_arr), DMA_TO_DEVICE);
>> } else {
^ permalink raw reply
* [PATCH v4 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Mike Kravetz @ 2020-04-28 20:56 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
Paul Walmsley, Thomas Gleixner, Gerald Schaefer,
Nitesh Narayan Lal, Randy Dunlap, Palmer Dabbelt, Andrew Morton,
David S . Miller, Mike Kravetz
In-Reply-To: <20200428205614.246260-1-mike.kravetz@oracle.com>
Now that architectures provide arch_hugetlb_valid_size(), parsing
of "hugepagesz=" can be done in architecture independent code.
Create a single routine to handle hugepagesz= parsing and remove
all arch specific routines. We can also remove the interface
hugetlb_bad_size() as this is no longer used outside arch independent
code.
This also provides consistent behavior of hugetlbfs command line
options. The hugepagesz= option should only be specified once for
a specific size, but some architectures allow multiple instances.
This appears to be more of an oversight when code was added by some
architectures to set up ALL huge pages sizes.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> [s390]
Acked-by: Will Deacon <will@kernel.org>
---
arch/arm64/mm/hugetlbpage.c | 15 ---------------
arch/powerpc/mm/hugetlbpage.c | 15 ---------------
arch/riscv/mm/hugetlbpage.c | 16 ----------------
arch/s390/mm/hugetlbpage.c | 18 ------------------
arch/sparc/mm/init_64.c | 22 ----------------------
arch/x86/mm/hugetlbpage.c | 16 ----------------
include/linux/hugetlb.h | 1 -
mm/hugetlb.c | 23 +++++++++++++++++------
8 files changed, 17 insertions(+), 109 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 069b96ee2aec..f706b821aba6 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -476,18 +476,3 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
return false;
}
-
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- add_huge_page_size(ps);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
- return 0;
-}
-__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index de54d2a37830..2c3fa0a7787b 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -589,21 +589,6 @@ static int __init add_huge_page_size(unsigned long long size)
return 0;
}
-static int __init hugepage_setup_sz(char *str)
-{
- unsigned long long size;
-
- size = memparse(str, &str);
-
- if (add_huge_page_size(size) != 0) {
- hugetlb_bad_size();
- pr_err("Invalid huge page size specified(%llu)\n", size);
- }
-
- return 1;
-}
-__setup("hugepagesz=", hugepage_setup_sz);
-
static int __init hugetlbpage_init(void)
{
bool configured = false;
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index da1f516bc451..4e5d7e9f0eef 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -22,22 +22,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
- return 0;
-
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index ac25b207624c..242dfc0d462d 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -261,24 +261,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long size;
- char *string = opt;
-
- size = memparse(opt, &opt);
- if (arch_hugetlb_valid_size(size)) {
- hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- pr_err("hugepagesz= specifies an unsupported page size %s\n",
- string);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags)
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 2bfe8e22b706..4618f96fd30f 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -397,28 +397,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
return true;
}
-
-static int __init setup_hugepagesz(char *string)
-{
- unsigned long long hugepage_size;
- int rc = 0;
-
- hugepage_size = memparse(string, &string);
-
- if (!arch_hugetlb_valid_size((unsigned long)hugepage_size)) {
- hugetlb_bad_size();
- pr_err("hugepagesz=%llu not supported by MMU.\n",
- hugepage_size);
- goto out;
- }
-
- add_huge_page_size(hugepage_size);
- rc = 1;
-
-out:
- return rc;
-}
-__setup("hugepagesz=", setup_hugepagesz);
#endif /* CONFIG_HUGETLB_PAGE */
void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 1c4372bfe782..937d640a89e3 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -191,22 +191,6 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
- ps >> 20);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 2eb15f5ab01e..0c13706054ef 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -519,7 +519,6 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
int __init __alloc_bootmem_huge_page(struct hstate *h);
int __init alloc_bootmem_huge_page(struct hstate *h);
-void __init hugetlb_bad_size(void);
void __init hugetlb_add_hstate(unsigned order);
bool __init arch_hugetlb_valid_size(unsigned long size);
struct hstate *size_to_hstate(unsigned long size);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 63ca4241ea87..6a8454bc2917 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3262,12 +3262,6 @@ bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
return size == HPAGE_SIZE;
}
-/* Should be called on processing a hugepagesz=... option */
-void __init hugetlb_bad_size(void)
-{
- parsed_valid_hugepagesz = false;
-}
-
void __init hugetlb_add_hstate(unsigned int order)
{
struct hstate *h;
@@ -3337,6 +3331,23 @@ static int __init hugetlb_nrpages_setup(char *s)
}
__setup("hugepages=", hugetlb_nrpages_setup);
+static int __init hugepagesz_setup(char *s)
+{
+ unsigned long size;
+
+ size = (unsigned long)memparse(s, NULL);
+
+ if (!arch_hugetlb_valid_size(size)) {
+ parsed_valid_hugepagesz = false;
+ pr_err("HugeTLB: unsupported hugepagesz %s\n", s);
+ return 0;
+ }
+
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ return 1;
+}
+__setup("hugepagesz=", hugepagesz_setup);
+
static int __init default_hugepagesz_setup(char *s)
{
unsigned long size;
--
2.25.4
^ permalink raw reply related
* [PATCH v4 3/4] hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
From: Mike Kravetz @ 2020-04-28 20:56 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
Will Deacon, Mina Almasry, Anders Roxell, Jonathan Corbet,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Longpeng,
Albert Ou, Vasily Gorbik, Paul Walmsley, Thomas Gleixner,
Gerald Schaefer, Nitesh Narayan Lal, Randy Dunlap, Palmer Dabbelt,
Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200428205614.246260-1-mike.kravetz@oracle.com>
The routine hugetlb_add_hstate prints a warning if the hstate already
exists. This was originally done as part of kernel command line
parsing. If 'hugepagesz=' was specified more than once, the warning
pr_warn("hugepagesz= specified twice, ignoring\n");
would be printed.
Some architectures want to enable all huge page sizes. They would
call hugetlb_add_hstate for all supported sizes. However, this was
done after command line processing and as a result hstates could have
already been created for some sizes. To make sure no warning were
printed, there would often be code like:
if (!size_to_hstate(size)
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT)
The only time we want to print the warning is as the result of command
line processing. So, remove the warning from hugetlb_add_hstate and
add it to the single arch independent routine processing "hugepagesz=".
After this, calls to size_to_hstate() in arch specific code can be
removed and hugetlb_add_hstate can be called without worrying about
warning messages.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Mina Almasry <almasrymina@google.com>
Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> [s390]
Acked-by: Will Deacon <will@kernel.org>
Tested-by: Anders Roxell <anders.roxell@linaro.org>
---
arch/arm64/mm/hugetlbpage.c | 16 ++++------------
arch/powerpc/mm/hugetlbpage.c | 3 +--
arch/riscv/mm/hugetlbpage.c | 2 +-
arch/sparc/mm/init_64.c | 19 ++++---------------
arch/x86/mm/hugetlbpage.c | 2 +-
mm/hugetlb.c | 9 ++++++---
6 files changed, 17 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index f706b821aba6..14bed8f4674a 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
}
-static void __init add_huge_page_size(unsigned long size)
-{
- if (size_to_hstate(size))
- return;
-
- hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
-}
-
static int __init hugetlbpage_init(void)
{
#ifdef CONFIG_ARM64_4K_PAGES
- add_huge_page_size(PUD_SIZE);
+ hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
#endif
- add_huge_page_size(CONT_PMD_SIZE);
- add_huge_page_size(PMD_SIZE);
- add_huge_page_size(CONT_PTE_SIZE);
+ hugetlb_add_hstate((CONT_PMD_SHIFT + PMD_SHIFT) - PAGE_SHIFT);
+ hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate((CONT_PTE_SHIFT + PAGE_SHIFT) - PAGE_SHIFT);
return 0;
}
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 2c3fa0a7787b..4d5ed1093615 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -584,8 +584,7 @@ static int __init add_huge_page_size(unsigned long long size)
if (!arch_hugetlb_valid_size((unsigned long)size))
return -EINVAL;
- if (!size_to_hstate(size))
- hugetlb_add_hstate(shift - PAGE_SHIFT);
+ hugetlb_add_hstate(shift - PAGE_SHIFT);
return 0;
}
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index 4e5d7e9f0eef..932dadfdca54 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -26,7 +26,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
static __init int gigantic_pages_init(void)
{
/* With CONTIG_ALLOC, we can allocate gigantic pages at runtime */
- if (IS_ENABLED(CONFIG_64BIT) && !size_to_hstate(1UL << PUD_SHIFT))
+ if (IS_ENABLED(CONFIG_64BIT))
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 4618f96fd30f..ae819a16d07a 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -325,23 +325,12 @@ static void __update_mmu_tsb_insert(struct mm_struct *mm, unsigned long tsb_inde
}
#ifdef CONFIG_HUGETLB_PAGE
-static void __init add_huge_page_size(unsigned long size)
-{
- unsigned int order;
-
- if (size_to_hstate(size))
- return;
-
- order = ilog2(size) - PAGE_SHIFT;
- hugetlb_add_hstate(order);
-}
-
static int __init hugetlbpage_init(void)
{
- add_huge_page_size(1UL << HPAGE_64K_SHIFT);
- add_huge_page_size(1UL << HPAGE_SHIFT);
- add_huge_page_size(1UL << HPAGE_256MB_SHIFT);
- add_huge_page_size(1UL << HPAGE_2GB_SHIFT);
+ hugetlb_add_hstate(HPAGE_64K_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_256MB_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_2GB_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 937d640a89e3..cf5781142716 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -195,7 +195,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
static __init int gigantic_pages_init(void)
{
/* With compaction or CMA we can allocate gigantic pages at runtime */
- if (boot_cpu_has(X86_FEATURE_GBPAGES) && !size_to_hstate(1UL << PUD_SHIFT))
+ if (boot_cpu_has(X86_FEATURE_GBPAGES))
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6a8454bc2917..2ae0e506cfc7 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3222,8 +3222,7 @@ static int __init hugetlb_init(void)
}
default_hstate_size = HPAGE_SIZE;
- if (!size_to_hstate(default_hstate_size))
- hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+ hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
}
default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
if (default_hstate_max_huge_pages) {
@@ -3268,7 +3267,6 @@ void __init hugetlb_add_hstate(unsigned int order)
unsigned long i;
if (size_to_hstate(PAGE_SIZE << order)) {
- pr_warn("hugepagesz= specified twice, ignoring\n");
return;
}
BUG_ON(hugetlb_max_hstate >= HUGE_MAX_HSTATE);
@@ -3343,6 +3341,11 @@ static int __init hugepagesz_setup(char *s)
return 0;
}
+ if (size_to_hstate(size)) {
+ pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n", s);
+ return 0;
+ }
+
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
return 1;
}
--
2.25.4
^ permalink raw reply related
* [PATCH v4 4/4] hugetlbfs: clean up command line processing
From: Mike Kravetz @ 2020-04-28 20:56 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
Sandipan Das, Will Deacon, Mina Almasry, Jonathan Corbet,
Christian Borntraeger, Ingo Molnar, Catalin Marinas, Longpeng,
Albert Ou, Vasily Gorbik, Paul Walmsley, Thomas Gleixner,
Gerald Schaefer, Nitesh Narayan Lal, Randy Dunlap, Palmer Dabbelt,
Andrew Morton, David S . Miller, Mike Kravetz
In-Reply-To: <20200428205614.246260-1-mike.kravetz@oracle.com>
With all hugetlb page processing done in a single file clean up code.
- Make code match desired semantics
- Update documentation with semantics
- Make all warnings and errors messages start with 'HugeTLB:'.
- Consistently name command line parsing routines.
- Warn if !hugepages_supported() and command line parameters have
been specified.
- Add comments to code
- Describe some of the subtle interactions
- Describe semantics of command line arguments
This patch also fixes issues with implicitly setting the number of
gigantic huge pages to preallocate. Previously on X86 command line,
hugepages=2 default_hugepagesz=1G
would result in zero 1G pages being preallocated and,
# grep HugePages_Total /proc/meminfo
HugePages_Total: 0
# sysctl -a | grep nr_hugepages
vm.nr_hugepages = 2
vm.nr_hugepages_mempolicy = 2
# cat /proc/sys/vm/nr_hugepages
2
After this patch 2 gigantic pages will be preallocated and all the
proc, sysfs, sysctl and meminfo files will accurately reflect this.
To address the issue with gigantic pages, a small change in behavior
was made to command line processing. Previously the command line,
hugepages=128 default_hugepagesz=2M hugepagesz=2M hugepages=256
would result in the allocation of 256 2M huge pages. The value 128
would be ignored without any warning. After this patch, 128 2M pages
will be allocated and a warning message will be displayed indicating
the value of 256 is ignored. This change in behavior is required
because allocation of implicitly specified gigantic pages must be done
when the default_hugepagesz= is encountered for gigantic pages.
Previously the code waited until later in the boot process (hugetlb_init),
to allocate pages of default size. However the bootmem allocator required
for gigantic allocations is not available at this time.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> [s390]
Acked-by: Will Deacon <will@kernel.org>
Tested-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../admin-guide/kernel-parameters.txt | 40 +++--
Documentation/admin-guide/mm/hugetlbpage.rst | 35 ++++
mm/hugetlb.c | 149 ++++++++++++++----
3 files changed, 179 insertions(+), 45 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7bc83f3d9bdf..cbe657b86d0e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -834,12 +834,15 @@
See also Documentation/networking/decnet.txt.
default_hugepagesz=
- [same as hugepagesz=] The size of the default
- HugeTLB page size. This is the size represented by
- the legacy /proc/ hugepages APIs, used for SHM, and
- default size when mounting hugetlbfs filesystems.
- Defaults to the default architecture's huge page size
- if not specified.
+ [HW] The size of the default HugeTLB page. This is
+ the size represented by the legacy /proc/ hugepages
+ APIs. In addition, this is the default hugetlb size
+ used for shmget(), mmap() and mounting hugetlbfs
+ filesystems. If not specified, defaults to the
+ architecture's default huge page size. Huge page
+ sizes are architecture dependent. See also
+ Documentation/admin-guide/mm/hugetlbpage.rst.
+ Format: size[KMG]
deferred_probe_timeout=
[KNL] Debugging option to set a timeout in seconds for
@@ -1479,13 +1482,24 @@
hugepages using the cma allocator. If enabled, the
boot-time allocation of gigantic hugepages is skipped.
- hugepages= [HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
- hugepagesz= [HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
- On x86-64 and powerpc, this option can be specified
- multiple times interleaved with hugepages= to reserve
- huge pages of different sizes. Valid pages sizes on
- x86-64 are 2M (when the CPU supports "pse") and 1G
- (when the CPU supports the "pdpe1gb" cpuinfo flag).
+ hugepages= [HW] Number of HugeTLB pages to allocate at boot.
+ If this follows hugepagesz (below), it specifies
+ the number of pages of hugepagesz to be allocated.
+ If this is the first HugeTLB parameter on the command
+ line, it specifies the number of pages to allocate for
+ the default huge page size. See also
+ Documentation/admin-guide/mm/hugetlbpage.rst.
+ Format: <integer>
+
+ hugepagesz=
+ [HW] The size of the HugeTLB pages. This is used in
+ conjunction with hugepages (above) to allocate huge
+ pages of a specific size at boot. The pair
+ hugepagesz=X hugepages=Y can be specified once for
+ each supported huge page size. Huge page sizes are
+ architecture dependent. See also
+ Documentation/admin-guide/mm/hugetlbpage.rst.
+ Format: size[KMG]
hung_task_panic=
[KNL] Should the hung task detector generate panics.
diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst b/Documentation/admin-guide/mm/hugetlbpage.rst
index 1cc0bc78d10e..5026e58826e2 100644
--- a/Documentation/admin-guide/mm/hugetlbpage.rst
+++ b/Documentation/admin-guide/mm/hugetlbpage.rst
@@ -100,6 +100,41 @@ with a huge page size selection parameter "hugepagesz=<size>". <size> must
be specified in bytes with optional scale suffix [kKmMgG]. The default huge
page size may be selected with the "default_hugepagesz=<size>" boot parameter.
+Hugetlb boot command line parameter semantics
+hugepagesz - Specify a huge page size. Used in conjunction with hugepages
+ parameter to preallocate a number of huge pages of the specified
+ size. Hence, hugepagesz and hugepages are typically specified in
+ pairs such as:
+ hugepagesz=2M hugepages=512
+ hugepagesz can only be specified once on the command line for a
+ specific huge page size. Valid huge page sizes are architecture
+ dependent.
+hugepages - Specify the number of huge pages to preallocate. This typically
+ follows a valid hugepagesz or default_hugepagesz parameter. However,
+ if hugepages is the first or only hugetlb command line parameter it
+ implicitly specifies the number of huge pages of default size to
+ allocate. If the number of huge pages of default size is implicitly
+ specified, it can not be overwritten by a hugepagesz,hugepages
+ parameter pair for the default size.
+ For example, on an architecture with 2M default huge page size:
+ hugepages=256 hugepagesz=2M hugepages=512
+ will result in 256 2M huge pages being allocated and a warning message
+ indicating that the hugepages=512 parameter is ignored. If a hugepages
+ parameter is preceded by an invalid hugepagesz parameter, it will
+ be ignored.
+default_hugepagesz - Specify the default huge page size. This parameter can
+ only be specified once on the command line. default_hugepagesz can
+ optionally be followed by the hugepages parameter to preallocate a
+ specific number of huge pages of default size. The number of default
+ sized huge pages to preallocate can also be implicitly specified as
+ mentioned in the hugepages section above. Therefore, on an
+ architecture with 2M default huge page size:
+ hugepages=256
+ default_hugepagesz=2M hugepages=256
+ hugepages=256 default_hugepagesz=2M
+ will all result in 256 2M huge pages being allocated. Valid default
+ huge page size is architecture dependent.
+
When multiple huge page sizes are supported, ``/proc/sys/vm/nr_hugepages``
indicates the current number of pre-allocated huge pages of the default size.
Thus, one can use the following command to dynamically allocate/deallocate
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2ae0e506cfc7..8852b0b12270 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -59,8 +59,8 @@ __initdata LIST_HEAD(huge_boot_pages);
/* for command line parsing */
static struct hstate * __initdata parsed_hstate;
static unsigned long __initdata default_hstate_max_huge_pages;
-static unsigned long __initdata default_hstate_size;
static bool __initdata parsed_valid_hugepagesz = true;
+static bool __initdata parsed_default_hugepagesz;
/*
* Protects updates to hugepage_freelists, hugepage_activelist, nr_huge_pages,
@@ -3060,7 +3060,7 @@ static void __init hugetlb_sysfs_init(void)
err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
hstate_kobjs, &hstate_attr_group);
if (err)
- pr_err("Hugetlb: Unable to add hstate %s", h->name);
+ pr_err("HugeTLB: Unable to add hstate %s", h->name);
}
}
@@ -3164,7 +3164,7 @@ static void hugetlb_register_node(struct node *node)
nhs->hstate_kobjs,
&per_node_hstate_attr_group);
if (err) {
- pr_err("Hugetlb: Unable to add hstate %s for node %d\n",
+ pr_err("HugeTLB: Unable to add hstate %s for node %d\n",
h->name, node->dev.id);
hugetlb_unregister_node(node);
break;
@@ -3212,22 +3212,41 @@ static int __init hugetlb_init(void)
{
int i;
- if (!hugepages_supported())
+ if (!hugepages_supported()) {
+ if (hugetlb_max_hstate || default_hstate_max_huge_pages)
+ pr_warn("HugeTLB: huge pages not supported, ignoring associated command-line parameters\n");
return 0;
+ }
- if (!size_to_hstate(default_hstate_size)) {
- if (default_hstate_size != 0) {
- pr_err("HugeTLB: unsupported default_hugepagesz %lu. Reverting to %lu\n",
- default_hstate_size, HPAGE_SIZE);
+ /*
+ * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists. Some
+ * architectures depend on setup being done here.
+ */
+ hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+ if (!parsed_default_hugepagesz) {
+ /*
+ * If we did not parse a default huge page size, set
+ * default_hstate_idx to HPAGE_SIZE hstate. And, if the
+ * number of huge pages for this default size was implicitly
+ * specified, set that here as well.
+ * Note that the implicit setting will overwrite an explicit
+ * setting. A warning will be printed in this case.
+ */
+ default_hstate_idx = hstate_index(size_to_hstate(HPAGE_SIZE));
+ if (default_hstate_max_huge_pages) {
+ if (default_hstate.max_huge_pages) {
+ char buf[32];
+
+ string_get_size(huge_page_size(&default_hstate),
+ 1, STRING_UNITS_2, buf, 32);
+ pr_warn("HugeTLB: Ignoring hugepages=%lu associated with %s page size\n",
+ default_hstate.max_huge_pages, buf);
+ pr_warn("HugeTLB: Using hugepages=%lu for number of default huge pages\n",
+ default_hstate_max_huge_pages);
+ }
+ default_hstate.max_huge_pages =
+ default_hstate_max_huge_pages;
}
-
- default_hstate_size = HPAGE_SIZE;
- hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
- }
- default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
- if (default_hstate_max_huge_pages) {
- if (!default_hstate.max_huge_pages)
- default_hstate.max_huge_pages = default_hstate_max_huge_pages;
}
hugetlb_cma_check();
@@ -3287,20 +3306,29 @@ void __init hugetlb_add_hstate(unsigned int order)
parsed_hstate = h;
}
-static int __init hugetlb_nrpages_setup(char *s)
+/*
+ * hugepages command line processing
+ * hugepages normally follows a valid hugepagsz or default_hugepagsz
+ * specification. If not, ignore the hugepages value. hugepages can also
+ * be the first huge page command line option in which case it implicitly
+ * specifies the number of huge pages for the default size.
+ */
+static int __init hugepages_setup(char *s)
{
unsigned long *mhp;
static unsigned long *last_mhp;
if (!parsed_valid_hugepagesz) {
- pr_warn("hugepages = %s preceded by "
- "an unsupported hugepagesz, ignoring\n", s);
+ pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s);
parsed_valid_hugepagesz = true;
- return 1;
+ return 0;
}
+
/*
- * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter yet,
- * so this hugepages= parameter goes to the "default hstate".
+ * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter
+ * yet, so this hugepages= parameter goes to the "default hstate".
+ * Otherwise, it goes with the previously parsed hugepagesz or
+ * default_hugepagesz.
*/
else if (!hugetlb_max_hstate)
mhp = &default_hstate_max_huge_pages;
@@ -3308,8 +3336,8 @@ static int __init hugetlb_nrpages_setup(char *s)
mhp = &parsed_hstate->max_huge_pages;
if (mhp == last_mhp) {
- pr_warn("hugepages= specified twice without interleaving hugepagesz=, ignoring\n");
- return 1;
+ pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n", s);
+ return 0;
}
if (sscanf(s, "%lu", mhp) <= 0)
@@ -3327,42 +3355,99 @@ static int __init hugetlb_nrpages_setup(char *s)
return 1;
}
-__setup("hugepages=", hugetlb_nrpages_setup);
+__setup("hugepages=", hugepages_setup);
+/*
+ * hugepagesz command line processing
+ * A specific huge page size can only be specified once with hugepagesz.
+ * hugepagesz is followed by hugepages on the command line. The global
+ * variable 'parsed_valid_hugepagesz' is used to determine if prior
+ * hugepagesz argument was valid.
+ */
static int __init hugepagesz_setup(char *s)
{
unsigned long size;
+ struct hstate *h;
+ parsed_valid_hugepagesz = false;
size = (unsigned long)memparse(s, NULL);
if (!arch_hugetlb_valid_size(size)) {
- parsed_valid_hugepagesz = false;
- pr_err("HugeTLB: unsupported hugepagesz %s\n", s);
+ pr_err("HugeTLB: unsupported hugepagesz=%s\n", s);
return 0;
}
- if (size_to_hstate(size)) {
- pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n", s);
- return 0;
+ h = size_to_hstate(size);
+ if (h) {
+ /*
+ * hstate for this size already exists. This is normally
+ * an error, but is allowed if the existing hstate is the
+ * default hstate. More specifically, it is only allowed if
+ * the number of huge pages for the default hstate was not
+ * previously specified.
+ */
+ if (!parsed_default_hugepagesz || h != &default_hstate ||
+ default_hstate.max_huge_pages) {
+ pr_warn("HugeTLB: hugepagesz=%s specified twice, ignoring\n", s);
+ return 0;
+ }
+
+ /*
+ * No need to call hugetlb_add_hstate() as hstate already
+ * exists. But, do set parsed_hstate so that a following
+ * hugepages= parameter will be applied to this hstate.
+ */
+ parsed_hstate = h;
+ parsed_valid_hugepagesz = true;
+ return 1;
}
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ parsed_valid_hugepagesz = true;
return 1;
}
__setup("hugepagesz=", hugepagesz_setup);
+/*
+ * default_hugepagesz command line input
+ * Only one instance of default_hugepagesz allowed on command line.
+ */
static int __init default_hugepagesz_setup(char *s)
{
unsigned long size;
+ parsed_valid_hugepagesz = false;
+ if (parsed_default_hugepagesz) {
+ pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s);
+ return 0;
+ }
+
size = (unsigned long)memparse(s, NULL);
if (!arch_hugetlb_valid_size(size)) {
- pr_err("HugeTLB: unsupported default_hugepagesz %s\n", s);
+ pr_err("HugeTLB: unsupported default_hugepagesz=%s\n", s);
return 0;
}
- default_hstate_size = size;
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ parsed_valid_hugepagesz = true;
+ parsed_default_hugepagesz = true;
+ default_hstate_idx = hstate_index(size_to_hstate(size));
+
+ /*
+ * The number of default huge pages (for this size) could have been
+ * specified as the first hugetlb parameter: hugepages=X. If so,
+ * then default_hstate_max_huge_pages is set. If the default huge
+ * page size is gigantic (>= MAX_ORDER), then the pages must be
+ * allocated here from bootmem allocator.
+ */
+ if (default_hstate_max_huge_pages) {
+ default_hstate.max_huge_pages = default_hstate_max_huge_pages;
+ if (hstate_is_gigantic(&default_hstate))
+ hugetlb_hstate_alloc_pages(&default_hstate);
+ default_hstate_max_huge_pages = 0;
+ }
+
return 1;
}
__setup("default_hugepagesz=", default_hugepagesz_setup);
--
2.25.4
^ permalink raw reply related
* [PATCH v4 0/4] Clean up hugetlb boot command line processing
From: Mike Kravetz @ 2020-04-28 20:56 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Dave Hansen, Heiko Carstens, Peter Xu, Paul Mackerras,
Will Deacon, Mina Almasry, Jonathan Corbet, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, Longpeng, Albert Ou, Vasily Gorbik,
Paul Walmsley, Thomas Gleixner, Nitesh Narayan Lal, Randy Dunlap,
Palmer Dabbelt, Andrew Morton, David S . Miller, Mike Kravetz
v4 -
Fixed huge page order definitions for arm64 (Qian Cai)
Removed hugepages_supported() checks in command line processing as
powerpc does not set hugepages_supported until later in boot (Sandipan)
Added Acks, Reviews and Tested (Will, Gerald, Anders, Sandipan)
v3 -
Used weak attribute method of defining arch_hugetlb_valid_size.
This eliminates changes to arch specific hugetlb.h files (Peter)
Updated documentation (Peter, Randy)
Fixed handling of implicitly specified gigantic page preallocation
in existing code and removed documentation of such. There is now
no difference between handling of gigantic and non-gigantic pages.
(Peter, Nitesh).
This requires the most review as there is a small change to
undocumented behavior. See patch 4 commit message for details.
Added Acks and Reviews (Mina, Peter)
v2 -
Fix build errors with patch 1 (Will)
Change arch_hugetlb_valid_size arg to unsigned long and remove
irrelevant 'extern' keyword (Christophe)
Documentation and other misc changes (Randy, Christophe, Mina)
Do not process command line options if !hugepages_supported()
(Dave, but it sounds like we may want to additional changes to
hugepages_supported() for x86? If that is needed I would prefer
a separate patch.)
Longpeng(Mike) reported a weird message from hugetlb command line processing
and proposed a solution [1]. While the proposed patch does address the
specific issue, there are other related issues in command line processing.
As hugetlbfs evolved, updates to command line processing have been made to
meet immediate needs and not necessarily in a coordinated manner. The result
is that some processing is done in arch specific code, some is done in arch
independent code and coordination is problematic. Semantics can vary between
architectures.
The patch series does the following:
- Define arch specific arch_hugetlb_valid_size routine used to validate
passed huge page sizes.
- Move hugepagesz= command line parsing out of arch specific code and into
an arch independent routine.
- Clean up command line processing to follow desired semantics and
document those semantics.
[1] https://lore.kernel.org/linux-mm/20200305033014.1152-1-longpeng2@huawei.com
Mike Kravetz (4):
hugetlbfs: add arch_hugetlb_valid_size
hugetlbfs: move hugepagesz= parsing to arch independent code
hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
hugetlbfs: clean up command line processing
.../admin-guide/kernel-parameters.txt | 40 ++--
Documentation/admin-guide/mm/hugetlbpage.rst | 35 ++++
arch/arm64/mm/hugetlbpage.c | 30 +--
arch/powerpc/mm/hugetlbpage.c | 30 +--
arch/riscv/mm/hugetlbpage.c | 24 +--
arch/s390/mm/hugetlbpage.c | 24 +--
arch/sparc/mm/init_64.c | 43 +----
arch/x86/mm/hugetlbpage.c | 23 +--
include/linux/hugetlb.h | 2 +-
mm/hugetlb.c | 180 ++++++++++++++----
10 files changed, 260 insertions(+), 171 deletions(-)
--
2.25.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox