* Re: [PATCH v6 07/10] powerpc/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-01-28 21:17 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <3ce8fc61-a61f-5efc-9167-94f5d39b6f1b@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> arch/powerpc/perf/imc-pmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index cb50a9e1fd2d..e837717492e4 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -898,7 +898,7 @@ static int thread_imc_event_init(struct perf_event *event)
> if (event->attr.type != event->pmu->type)
> return -ENOENT;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
>
> /* Sampling not supported */
> @@ -1307,7 +1307,7 @@ static int trace_imc_event_init(struct perf_event *event)
> if (event->attr.type != event->pmu->type)
> return -ENOENT;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
>
> /* Return if this is a couting event */
>
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 06/10] trace/bpf_trace: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-01-28 21:17 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4fdbe164-d83d-a52f-4e8c-fe8bd15c6f8c@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> kernel/trace/bpf_trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index e5ef4ae9edb5..334f1d71ebb1 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1395,7 +1395,7 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
> u32 *ids, prog_cnt, ids_len;
> int ret;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EPERM;
> if (event->attr.type != PERF_TYPE_TRACEPOINT)
> return -EINVAL;
>
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: James Morris @ 2020-01-28 21:16 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <62274abc-6067-9121-9397-c9b626ca5f7a@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> include/linux/capability.h | 4 ++++
> include/uapi/linux/capability.h | 8 +++++++-
> security/selinux/include/classmap.h | 4 ++--
> 3 files changed, 13 insertions(+), 3 deletions(-)
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 10/10] drivers/oprofile: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-01-28 21:18 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <eff5e211-7114-f854-f53f-08491f9dcc26@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
>
> Open access to monitoring for CAP_PERFMON privileged process. Providing
> the access under CAP_PERFMON capability singly, without the rest of
> CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
> makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
> of least privilege: A security design principle that states that a process
> or program be granted only those privileges (e.g., capabilities) necessary
> to accomplish its legitimate function, and only for the time that such
> privileges are actually required)
>
> For backward compatibility reasons access to the monitoring remains open
> for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
> monitoring is discouraged with respect to CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> drivers/oprofile/event_buffer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: James Morris <jamorris@linux.microsoft.com>
>
> diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
> index 12ea4a4ad607..6c9edc8bbc95 100644
> --- a/drivers/oprofile/event_buffer.c
> +++ b/drivers/oprofile/event_buffer.c
> @@ -113,7 +113,7 @@ static int event_buffer_open(struct inode *inode, struct file *file)
> {
> int err = -EPERM;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EPERM;
>
> if (test_and_set_bit_lock(0, &buffer_opened))
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 09/10] drivers/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-01-28 21:18 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <f2877038-da53-f981-4ddb-4e6c1c27c60f@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
>
> Open access to monitoring for CAP_PERFMON privileged process.
> Providing the access under CAP_PERFMON capability singly, without the
> rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
> credentials and makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
> of least privilege: A security design principle that states that a process
> or program be granted only those privileges (e.g., capabilities) necessary
> to accomplish its legitimate function, and only for the time that such
> privileges are actually required)
>
> For backward compatibility reasons access to the monitoring remains open
> for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
> monitoring is discouraged with respect to CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> drivers/perf/arm_spe_pmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Acked-by: James Morris <jamorris@linux.microsoft.com>
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 4e4984a55cd1..5dff81bc3324 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -274,7 +274,7 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
> if (!attr->exclude_kernel)
> reg |= BIT(SYS_PMSCR_EL1_E1SPE_SHIFT);
>
> - if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && capable(CAP_SYS_ADMIN))
> + if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable())
> reg |= BIT(SYS_PMSCR_EL1_CX_SHIFT);
>
> return reg;
> @@ -700,7 +700,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
> return -EOPNOTSUPP;
>
> reg = arm_spe_event_to_pmscr(event);
> - if (!capable(CAP_SYS_ADMIN) &&
> + if (!perfmon_capable() &&
> (reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) |
> BIT(SYS_PMSCR_EL1_CX_SHIFT) |
> BIT(SYS_PMSCR_EL1_PCT_SHIFT))))
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 08/10] parisc/perf: open access for CAP_PERFMON privileged process
From: James Morris @ 2020-01-28 21:17 UTC (permalink / raw)
To: Alexey Budankov
Cc: Song Liu, Peter Zijlstra, Alexei Starovoitov, Lionel Landwerlin,
james.bottomley@hansenpartnership.com, Paul Mackerras, Jiri Olsa,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, Serge Hallyn, Robert Richter, Will Deacon,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
Arnaldo Carvalho de Melo, Stephane Eranian, Namhyung Kim,
linux-arm-kernel, linux-parisc@vger.kernel.org, linux-kernel,
linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <17be72ff-dc52-72ef-fbcc-0e9ec8b61604@linux.intel.com>
On Tue, 28 Jan 2020, Alexey Budankov wrote:
>
> Open access to monitoring for CAP_PERFMON privileged process.
> Providing the access under CAP_PERFMON capability singly, without the
> rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
> credentials and makes operation more secure.
>
> CAP_PERFMON implements the principal of least privilege for performance
> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
> of least privilege: A security design principle that states that a process
> or program be granted only those privileges (e.g., capabilities) necessary
> to accomplish its legitimate function, and only for the time that such
> privileges are actually required)
>
> For backward compatibility reasons access to the monitoring remains open
> for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
> monitoring is discouraged with respect to CAP_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> arch/parisc/kernel/perf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
> index 676683641d00..c4208d027794 100644
> --- a/arch/parisc/kernel/perf.c
> +++ b/arch/parisc/kernel/perf.c
> @@ -300,7 +300,7 @@ static ssize_t perf_write(struct file *file, const char __user *buf,
> else
> return -EFAULT;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
>
> if (count != sizeof(uint32_t))
>
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Segher Boessenkool @ 2020-01-28 20:01 UTC (permalink / raw)
To: Florian Weimer
Cc: libc-alpha, Tulio Magno Quites Machado Filho, linuxppc-dev,
Nicholas Piggin
In-Reply-To: <87o8unbm8u.fsf@oldenburg2.str.redhat.com>
On Tue, Jan 28, 2020 at 05:04:49PM +0100, Florian Weimer wrote:
> * Segher Boessenkool:
>
> >> > I don't think we can save LR in a regular register around the system
> >> > call, explicitly in the inline asm statement, because we still have to
> >> > generate proper unwinding information using CFI directives, something
> >> > that you cannot do from within the asm statement.
> >
> > Why not?
>
> As far as I knowm there isn't a CFI directive that allows us to restore
> the CFI state at the end of the inline assembly. If we say that LR is
> stored in a different register than what the rest of the function uses,
> that would lead to incorrect CFI after the exit of the inline assembler
> fragment.
>
> At least that's what I think. Compilers aren't really my thing.
.cfi_restore? Or .cfi_remember_state / .cfi_restore_state, that is
probably easiest in inline assembler.
> >> > GCC does not model the condition registers,
> >
> > Huh? It does model the condition register, as 8 registers in GCC's
> > internal model (one each for CR0..CR7).
>
> But GCC doesn't expose them as integers to C code, so you can't do much
> without them.
Sure, it doesn't expose any other registers directly, either.
> >> > We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
> >> > provide an ELFv1 port for this given that it's POWER9-specific.
> >
> > We *do* have a 32-bit LE ABI. And ELFv1 is not 32-bit either. Please
> > don't confuse these things :-)
> >
> > The 64-bit LE kernel does not really support 32-bit userland (or BE
> > userland), *that* is what you want to say.
>
> Sorry for the confusion. Is POWER9 running kernels which are not 64-bit
> LE really a thing in practice, though?
Linux only really supports 64-bit LE userland on p9. Anything else is
not supported.
> >> > From the glibc perspective, the major question is how we handle run-time
> >> > selection of the system call instruction sequence.
> >
> > Well, if it is inlined you don't have this problem either! :-)
>
> How so? We would have to put the conditional sequence into all inline
> system calls, of course.
Ah, if you support older systems in your program as well, gotcha. That
is not the usual case (just like people use -mcpu=power9 frequently,
which means the resulting program will not run on any older CPU).
Segher
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-01-28 19:07 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Heiko Carstens, Michal Hocko, linux-mm, Paul Mackerras,
sparclinux, Ingo Molnar, linux-s390, Jason Gunthorpe,
Vlastimil Babka, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Tetsuo Handa, linux-arm-kernel, linux-snps-arc,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Dan Williams,
Mark Brown, Kirill A . Shutemov, Thomas Gleixner, Gerald Schaefer,
Sri Krishna chowdary, Dave Hansen, Greg Kroah-Hartman,
Ard Biesheuvel, linux-mips, Ralf Baechle, linux-kernel,
Paul Burton, Mike Rapoport, Vineet Gupta, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200128174709.GK655507@arrakis.emea.arm.com>
> On Jan 28, 2020, at 12:47 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> The primary goal here is not finding regressions but having clearly
> defined semantics of the page table accessors across architectures. x86
> and arm64 are a good starting point and other architectures will be
> enabled as they are aligned to the same semantics.
This still does not answer the fundamental question. If this test is simply inefficient to find bugs, who wants to spend time to use it regularly? If this is just one off test that may get running once in a few years (when introducing a new arch), how does it justify the ongoing cost to maintain it?
I do agree there could be a need to clearly define this thing but that belongs to documentation rather than testing purpose. It is confusing to mix this with other config options which have somewhat a different purpose, it will then be a waste of time for people who mistakenly enable this for regular automatic testing and never found any bug from it.
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Catalin Marinas @ 2020-01-28 17:47 UTC (permalink / raw)
To: Qian Cai
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Heiko Carstens, Michal Hocko, linux-mm, Paul Mackerras,
sparclinux, Ingo Molnar, linux-s390, Jason Gunthorpe,
Vlastimil Babka, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Tetsuo Handa, linux-arm-kernel, linux-snps-arc,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Dan Williams,
Mark Brown, Kirill A . Shutemov, Thomas Gleixner, Gerald Schaefer,
Sri Krishna chowdary, Dave Hansen, Greg Kroah-Hartman,
Ard Biesheuvel, linux-mips, Ralf Baechle, linux-kernel,
Paul Burton, Mike Rapoport, Vineet Gupta, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <14882A91-17DE-4ABD-ABF2-08E7CCEDF660@lca.pw>
On Mon, Jan 27, 2020 at 09:11:53PM -0500, Qian Cai wrote:
> On Jan 27, 2020, at 8:28 PM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
> > This adds tests which will validate architecture page table helpers and
> > other accessors in their compliance with expected generic MM semantics.
> > This will help various architectures in validating changes to existing
> > page table helpers or addition of new ones.
[...]
> What’s the value of this block of new code? It only supports x86 and
> arm64 which are supposed to be good now. Did those tests ever find any
> regression or this is almost only useful for new architectures which
> only happened once in a few years?
The primary goal here is not finding regressions but having clearly
defined semantics of the page table accessors across architectures. x86
and arm64 are a good starting point and other architectures will be
enabled as they are aligned to the same semantics.
See for example this past discussion:
https://lore.kernel.org/linux-mm/20190628102003.GA56463@arrakis.emea.arm.com/
These tests should act as the 'contract' between the generic mm code and
the architecture port. Without clear semantics, some bugs may be a lot
subtler than a boot failure.
FTR, I fully support this patch (and I should get around to review it
properly; thanks for the reminder ;)).
--
Catalin
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Adhemerval Zanella @ 2020-01-28 17:26 UTC (permalink / raw)
To: linuxppc-dev, npiggin
In-Reply-To: <1580218232.2tezmthp1x.astroid@bobo.none>
On 28/01/2020 11:05, Nicholas Piggin wrote:
> Florian Weimer's on January 28, 2020 11:09 pm:
>> * Nicholas Piggin:
>>
>>> * Proposal is for PPC_FEATURE2_SCV to indicate 'scv 0' support, all other
>>> vectors will return -ENOSYS, and the decision for how to add support for
>>> a new vector deferred until we see the next user.
>>
>> Seems reasonable. We don't have to decide this today.
>>
>>> * Proposal is for scv 0 to provide the standard Linux system call ABI with some
>>> differences:
>>>
>>> - LR is volatile across scv calls. This is necessary for support because the
>>> scv instruction clobbers LR.
>>
>> I think we can express this in the glibc system call assembler wrapper
>> generators. The mcount profiling wrappers already have this property.
>>
>> But I don't think we are so lucky for the inline system calls. GCC
>> recognizes an "lr" clobber with inline asm (even though it is not
>> documented), but it generates rather strange assembler output as a
>> result:
>>
>> long
>> f (long x)
>> {
>> long y;
>> asm ("#" : "=r" (y) : "r" (x) : "lr");
>> return y;
>> }
>>
>> .abiversion 2
>> .section ".text"
>> .align 2
>> .p2align 4,,15
>> .globl f
>> .type f, @function
>> f:
>> .LFB0:
>> .cfi_startproc
>> mflr 0
>> .cfi_register 65, 0
>> #APP
>> # 5 "t.c" 1
>> #
>> # 0 "" 2
>> #NO_APP
>> std 0,16(1)
>> .cfi_offset 65, 16
>> ori 2,2,0
>> ld 0,16(1)
>> mtlr 0
>> .cfi_restore 65
>> blr
>> .long 0
>> .byte 0,0,0,1,0,0,0,0
>> .cfi_endproc
>> .LFE0:
>> .size f,.-f
>>
>>
>> That's with GCC 8.3 at -O2. I don't understand what the ori is about.
>
> ori 2,2,0 is the group terminating nop hint for POWER8 type cores
> which had dispatch grouping rules.
It worth to note that it aims to mitigate a load-hit-store cpu stall
on some powerpc chips.
>
>>
>> I don't think we can save LR in a regular register around the system
>> call, explicitly in the inline asm statement, because we still have to
>> generate proper unwinding information using CFI directives, something
>> that you cannot do from within the asm statement.
>>
>> Supporting this in GCC should not be impossible, but someone who
>> actually knows this stuff needs to look at it.
>
> The generated assembler actually seems okay to me. If we compile
> something like a syscall and with -mcpu=power9:
>
> long
> f (long _r3, long _r4, long _r5, long _r6, long _r7, long _r8, long _r0)
> {
> register long r0 asm ("r0") = _r0;
> register long r3 asm ("r3") = _r3;
> register long r4 asm ("r4") = _r4;
> register long r5 asm ("r5") = _r5;
> register long r6 asm ("r6") = _r6;
> register long r7 asm ("r7") = _r7;
> register long r8 asm ("r8") = _r8;
>
> asm ("# scv" : "=r"(r3) : "r"(r0), "r"(r4), "r"(r5), "r"(r6), "r"(r7), "r"(r8) : "lr", "ctr", "cc", "xer");
>
> return r3;
> }
>
>
> f:
> .LFB0:
> .cfi_startproc
> mflr 0
> std 0,16(1)
> .cfi_offset 65, 16
> mr 0,9
> #APP
> # 12 "a.c" 1
> # scv
> # 0 "" 2
> #NO_APP
> ld 0,16(1)
> mtlr 0
> .cfi_restore 65
> blr
> .long 0
> .byte 0,0,0,1,0,0,0,0
> .cfi_endproc
>
> That gets the LR save/restore right when we're also using r0.
>
>>
>>> - CR1 and CR5-CR7 are volatile. This matches the C ABI and would allow the
>>> system call exit to avoid restoring the CR register.
>>
>> This sounds reasonable, but I don't know what kind of knock-on effects
>> this has. The inline system call wrappers can handle this with minor
>> tweaks.
>
> Okay, good. In the end we would have to check code trace through the
> kernel and libc of course, but I think there's little to no opportunity
> to take advantage of current extra non-volatile cr regs.
>
> mtcr has to write 8 independently renamed registers so it's cracked into
> 2 insns on POWER9 (and likely to always be a bit troublesome). It's not
> much in the scheme of a system call, but while we can tweak the ABI...
We don't really need a mfcr/mfocr to implement the Linux syscall ABI on
powerpc, we can use a 'bns+' plus a neg instead as:
--
#define internal_syscall6(name, err, nr, arg1, arg2, arg3, arg4, arg5, \
arg6) \
({ \
register long int r0 __asm__ ("r0") = (long int) (name); \
register long int r3 __asm__ ("r3") = (long int) (arg1); \
register long int r4 __asm__ ("r4") = (long int) (arg2); \
register long int r5 __asm__ ("r5") = (long int) (arg3); \
register long int r6 __asm__ ("r6") = (long int) (arg4); \
register long int r7 __asm__ ("r7") = (long int) (arg5); \
register long int r8 __asm__ ("r8") = (long int) (arg6); \
__asm__ __volatile__ \
("sc\n\t" \
"bns+ 1f\n\t" \
"neg %1, %1\n\t" \
"1:\n\t" \
: "+r" (r0), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6), \
"+r" (r7), "+r" (r8) \
: \
: "r9", "r10", "r11", "r12", \
"cr0", "memory"); \
r3; \
})
--
And change INTERNAL_SYSCALL_ERROR_P to check for the expected invalid
range (((unsigned long) (val) >= (unsigned long) -4095)) and
INTERNAL_SYSCALL_ERRNO to return a negative value (since the value will
be negated by INTERNAL_SYSCALL_ERROR_P).
The powerpc kernel ABI to use a different constraint to signal error
also requires glibc to reimplement the vDSO symbol call to be arch
specific instead a straight function call (since it might fallbacks
to a syscall).
Even for POWER-specific system call that uses all result bits, either
it should not fail or it would require a arch-specific implementation
to setup the expected error value (since the information would require
another source or a pre-defined value).
In fact I think we make the assumption that INTERNAL_SYSCALL returns
a negative errno value in case or an error and make all the handling
to check for a syscall failure and errno setting generic. This will
required change ia64, mips, nios2, and sparc though.
>
>>
>>> - Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
>>> instruction on the kernel side, and it is currently not implemented well
>>> in glibc, requiring a mfcr (mfocr should be possible and asm goto support
>>> would allow a better implementation). Is it worth continuing this style of
>>> error handling? Or just move to -ve return means error? Using a different
>>> bit would allow the kernel to piggy back the CR return code setting with
>>> a test for the error case exit.
>>
>> GCC does not model the condition registers, so for inline system calls,
>> we have to produce a value anyway that the subsequence C code can check.
>> The assembler syscall wrappers do not need to do this, of course, but
>> I'm not sure which category of interfaces is more important.
>
> Right. asm goto can improve this kind of pattern if it's inlined
> into the C code which tests the result, it can branch using the flags
> to the C error handling label, rather than move flags into GPR, test
> GPR, branch. However...
>
>> But the kernel uses the -errno convention internally, so I think it
>> would make sense to pass this to userspace and not convert back and
>> forth. This would align with what most of the architectures do, and
>> also avoids the GCC oddity.
>
> Yes I would be interested in opinions for this option. It seems like
> matching other architectures is a good idea. Maybe there are some
> reasons not to.
>
>>> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
>>> calls if there was interest in developing an ABI for 32-bit programs.
>>> Marginal benefit in avoiding compat syscall selection.
>>
>> We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
>> provide an ELFv1 port for this given that it's POWER9-specific.
>
> Okay. There's no reason not to enable this for BE, at least for the
> kernel it's no additional work so it probably remains enabled (unless
> there is something really good we could do with the ABI if we exclude
> ELFv1 but I don't see anything).
>
> But if glibc only builds for ELFv2 support that's probably reasonable.
>
>>
>> From the glibc perspective, the major question is how we handle run-time
>> selection of the system call instruction sequence. On i386, we use a
>> function pointer in the TCB to call an instruction sequence in the vDSO.
>> That's problematic from a security perspective. I expect that on
>> POWER9, using a pointer in read-only memory would be equally
>> non-attractive due to a similar lack of PC-relative addressing. We
>> could use the HWCAP bit in the TCB, but that would add another (easy to
>> predict) conditional branch to every system call.
>
> I would have to defer to glibc devs on this. Conditional branch
> should be acceptable I think, scv improves speed as much as several
> mispredicted branches (about 90 cycles).
>
>> I don't think it matters whether both system call variants use the same
>> error convention because we could have different error code extraction
>> code on the two branches.
>
> That's one less difficulty.
We already had to push a similar hack where glibc used to abort transactions
prior syscalls to avoid some side-effects on kernel (commit 56cf2763819d2f).
It was eventually removed from syscall handling by f0458cf4f9ff3d870, where
we only enable TLE if kernel suppors PPC_FEATURE2_HTM_NOSC.
The transaction syscall abort used to read a variable directly from TCB,
so this could be an option. I would expect that we could optimize it where
if glibc is building against a recent kernel and compiler is building
for a ISA 3.0+ cpu we could remove the 'sc' code.
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Vineet Gupta @ 2020-01-28 17:14 UTC (permalink / raw)
To: Qian Cai, Anshuman Khandual
Cc: Mark Rutland, linux-ia64@vger.kernel.org,
linux-sh@vger.kernel.org, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM, Dave Hansen,
Paul Mackerras, sparclinux@vger.kernel.org, Thomas Gleixner,
linux-s390@vger.kernel.org, x86@kernel.org,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, arcml,
linux-arm-kernel@lists.infradead.org, Ingo Molnar, Kees Cook,
Masahiro Yamada, Mark Brown, Kirill A . Shutemov, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, linux-mips@vger.kernel.org, Ralf Baechle,
linux-kernel@vger.kernel.org, Paul Burton, Mike Rapoport,
Martin Schwidefsky, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David S. Miller
In-Reply-To: <016A776F-EFD9-4D2B-A3A9-788008617D95@lca.pw>
On 1/27/20 7:33 PM, Qian Cai wrote:
>
>>> What’s the value of this block of new code? It only supports x86 and arm64
>>> which are supposed to be good now.
>> We have been over the usefulness of this code many times before as the patch is
>> already in it's V12. Currently it is enabled on arm64, x86 (except PAE), arc and
>> ppc32. There are build time or runtime problems with other archs which prevent
> I am not sure if I care too much about arc and ppc32 which are pretty much legacy
> platforms.
You really need to brush up on your definition and knowledge of what "legacy" means.
ARC is actively maintained and used by several customers, some in arch/arc/plat*
and some not in there.
It is present in broadband routers used by major ISP, massively multicore deep
packet inspection system from EZChip, and many more....
Sure you may not care about them, but the maintainers for the platforms do.
It would have been better if you had spent the time and energy in improving the
code over 12 revisions rather than bike shedding.
-Vineet
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-01-28 17:05 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Heiko Carstens, Michal Hocko, Dave Hansen, Paul Mackerras,
sparclinux, Thomas Gleixner, linux-s390, Jason Gunthorpe, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Tetsuo Handa, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Masahiro Yamada, Mark Brown, Kirill A . Shutemov,
Dan Williams, Vlastimil Babka, linux-arm-kernel,
Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <1580174873-18117-1-git-send-email-anshuman.khandual@arm.com>
Le 28/01/2020 à 02:27, Anshuman Khandual a écrit :
> This adds tests which will validate architecture page table helpers and
> other accessors in their compliance with expected generic MM semantics.
> This will help various architectures in validating changes to existing
> page table helpers or addition of new ones.
>
> This test covers basic page table entry transformations including but not
> limited to old, young, dirty, clean, write, write protect etc at various
> level along with populating intermediate entries with next page table page
> and validating them.
>
> Test page table pages are allocated from system memory with required size
> and alignments. The mapped pfns at page table levels are derived from a
> real pfn representing a valid kernel text symbol. This test gets called
> right after page_alloc_init_late().
>
> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
> arm64. Going forward, other architectures too can enable this after fixing
> build or runtime problems (if any) with their page table helpers.
>
> Folks interested in making sure that a given platform's page table helpers
> conform to expected generic MM semantics should enable the above config
> which will just trigger this test during boot. Any non conformity here will
> be reported as an warning which would need to be fixed. This test will help
> catch any changes to the agreed upon semantics expected from generic MM and
> enable platforms to accommodate it thereafter.
>
[...]
>
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> #PPC32
Also tested on PPC64 (under QEMU): book3s/64 64k pages, book3s/64 4k
pages and book3e/64
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
[...]
>
> diff --git a/Documentation/features/debug/debug-vm-pgtable/arch-support.txt b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
> new file mode 100644
> index 000000000000..f3f8111edbe3
> --- /dev/null
> +++ b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
> @@ -0,0 +1,35 @@
> +#
> +# Feature name: debug-vm-pgtable
> +# Kconfig: ARCH_HAS_DEBUG_VM_PGTABLE
> +# description: arch supports pgtable tests for semantics compliance
> +#
> + -----------------------
> + | arch |status|
> + -----------------------
> + | alpha: | TODO |
> + | arc: | ok |
> + | arm: | TODO |
> + | arm64: | ok |
> + | c6x: | TODO |
> + | csky: | TODO |
> + | h8300: | TODO |
> + | hexagon: | TODO |
> + | ia64: | TODO |
> + | m68k: | TODO |
> + | microblaze: | TODO |
> + | mips: | TODO |
> + | nds32: | TODO |
> + | nios2: | TODO |
> + | openrisc: | TODO |
> + | parisc: | TODO |
> + | powerpc/32: | ok |
> + | powerpc/64: | TODO |
You can change the two above lines by
powerpc: ok
> + | riscv: | TODO |
> + | s390: | TODO |
> + | sh: | TODO |
> + | sparc: | TODO |
> + | um: | TODO |
> + | unicore32: | TODO |
> + | x86: | ok |
> + | xtensa: | TODO |
> + -----------------------
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1ec34e16ed65..253dcab0bebc 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -120,6 +120,7 @@ config PPC
> #
> select ARCH_32BIT_OFF_T if PPC32
> select ARCH_HAS_DEBUG_VIRTUAL
> + select ARCH_HAS_DEBUG_VM_PGTABLE if PPC32
Remove the 'if PPC32' as we now know it also work on PPC64.
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 0b6c4042942a..fb0e76d254b3 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -53,6 +53,12 @@ static inline void sync_initial_page_table(void) { }
>
> struct mm_struct;
>
> +#define mm_p4d_folded mm_p4d_folded
> +static inline bool mm_p4d_folded(struct mm_struct *mm)
> +{
> + return !pgtable_l5_enabled();
> +}
> +
For me this should be part of another patch, it is not directly linked
to the tests.
> void set_pte_vaddr_p4d(p4d_t *p4d_page, unsigned long vaddr, pte_t new_pte);
> void set_pte_vaddr_pud(pud_t *pud_page, unsigned long vaddr, pte_t new_pte);
>
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index 798ea36a0549..e0b04787e789 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -1208,6 +1208,12 @@ static inline bool arch_has_pfn_modify_check(void)
> # define PAGE_KERNEL_EXEC PAGE_KERNEL
> #endif
>
> +#ifdef CONFIG_DEBUG_VM_PGTABLE
Not sure it is a good idea to put that in include/asm-generic/pgtable.h
By doing this you are forcing a rebuild of almost all files, whereas
only init/main.o and mm/debug_vm_pgtable.o should be rebuilt when
activating this config option.
> +extern void debug_vm_pgtable(void);
Please don't use the 'extern' keyword, it is useless and not to be used
for functions declaration.
> +#else
> +static inline void debug_vm_pgtable(void) { }
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
>
> #ifndef io_remap_pfn_range
> diff --git a/init/main.c b/init/main.c
> index da1bc0b60a7d..5e59e6ac0780 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1197,6 +1197,7 @@ static noinline void __init kernel_init_freeable(void)
> sched_init_smp();
>
> page_alloc_init_late();
> + debug_vm_pgtable();
Wouldn't it be better to call debug_vm_pgtable() in kernel_init()
between the call to async_synchronise_full() and ftrace_free_init_mem() ?
> /* Initialize page ext after all struct pages are initialized. */
> page_ext_init();
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5ffe144c9794..7cceae923c05 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -653,6 +653,12 @@ config SCHED_STACK_END_CHECK
> data corruption or a sporadic crash at a later stage once the region
> is examined. The runtime overhead introduced is minimal.
>
> +config ARCH_HAS_DEBUG_VM_PGTABLE
> + bool
> + help
> + An architecture should select this when it can successfully
> + build and run DEBUG_VM_PGTABLE.
> +
> config DEBUG_VM
> bool "Debug VM"
> depends on DEBUG_KERNEL
> @@ -688,6 +694,22 @@ config DEBUG_VM_PGFLAGS
>
> If unsure, say N.
>
> +config DEBUG_VM_PGTABLE
> + bool "Debug arch page table for semantics compliance"
> + depends on MMU
> + depends on DEBUG_VM
Does it really need to depend on DEBUG_VM ?
I think we could make it standalone and 'default y if DEBUG_VM' instead.
> + depends on ARCH_HAS_DEBUG_VM_PGTABLE
> + default y
> + help
> + This option provides a debug method which can be used to test
> + architecture page table helper functions on various platforms in
> + verifying if they comply with expected generic MM semantics. This
> + will help architecture code in making sure that any changes or
> + new additions of these helpers still conform to expected
> + semantics of the generic MM.
> +
> + If unsure, say N.
> +
Does it make sense to make it 'default y' and say 'If unsure, say N' ?
> config ARCH_HAS_DEBUG_VIRTUAL
> bool
>
Christophe
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Florian Weimer @ 2020-01-28 16:04 UTC (permalink / raw)
To: Segher Boessenkool
Cc: libc-alpha, Tulio Magno Quites Machado Filho, linuxppc-dev,
Nicholas Piggin
In-Reply-To: <20200128154026.GI22482@gate.crashing.org>
* Segher Boessenkool:
>> > I don't think we can save LR in a regular register around the system
>> > call, explicitly in the inline asm statement, because we still have to
>> > generate proper unwinding information using CFI directives, something
>> > that you cannot do from within the asm statement.
>
> Why not?
As far as I knowm there isn't a CFI directive that allows us to restore
the CFI state at the end of the inline assembly. If we say that LR is
stored in a different register than what the rest of the function uses,
that would lead to incorrect CFI after the exit of the inline assembler
fragment.
At least that's what I think. Compilers aren't really my thing.
>
>> >> - Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
>> >> instruction on the kernel side, and it is currently not implemented well
>> >> in glibc, requiring a mfcr (mfocr should be possible and asm goto support
>> >> would allow a better implementation). Is it worth continuing this style of
>> >> error handling? Or just move to -ve return means error? Using a different
>> >> bit would allow the kernel to piggy back the CR return code setting with
>> >> a test for the error case exit.
>> >
>> > GCC does not model the condition registers,
>
> Huh? It does model the condition register, as 8 registers in GCC's
> internal model (one each for CR0..CR7).
But GCC doesn't expose them as integers to C code, so you can't do much
without them.
>> >> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
>> >> calls if there was interest in developing an ABI for 32-bit programs.
>> >> Marginal benefit in avoiding compat syscall selection.
>> >
>> > We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
>> > provide an ELFv1 port for this given that it's POWER9-specific.
>
> We *do* have a 32-bit LE ABI. And ELFv1 is not 32-bit either. Please
> don't confuse these things :-)
>
> The 64-bit LE kernel does not really support 32-bit userland (or BE
> userland), *that* is what you want to say.
Sorry for the confusion. Is POWER9 running kernels which are not 64-bit
LE really a thing in practice, though?
>> > From the glibc perspective, the major question is how we handle run-time
>> > selection of the system call instruction sequence.
>
> Well, if it is inlined you don't have this problem either! :-)
How so? We would have to put the conditional sequence into all inline
system calls, of course.
Thanks,
Florian
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Florian Weimer @ 2020-01-28 15:58 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linuxppc-dev, Tulio Magno Quites Machado Filho, libc-alpha
In-Reply-To: <1580218232.2tezmthp1x.astroid@bobo.none>
* Nicholas Piggin:
> That gets the LR save/restore right when we're also using r0.
Yes, I agree it looks good. Nice.
>> But the kernel uses the -errno convention internally, so I think it
>> would make sense to pass this to userspace and not convert back and
>> forth. This would align with what most of the architectures do, and
>> also avoids the GCC oddity.
>
> Yes I would be interested in opinions for this option. It seems like
> matching other architectures is a good idea. Maybe there are some
> reasons not to.
If there were a POWER-specific system call that uses all result bits and
doesn't have room for the 4096 error states (or an error number that's
outside that range), that would be a blocker. I can't find such a
system call wrapped in the glibc sources. musl's inline syscalls always
convert the errno state to -errno, so it's not possible to use such a
system call there.
>>> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
>>> calls if there was interest in developing an ABI for 32-bit programs.
>>> Marginal benefit in avoiding compat syscall selection.
>>
>> We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
>> provide an ELFv1 port for this given that it's POWER9-specific.
>
> Okay. There's no reason not to enable this for BE, at least for the
> kernel it's no additional work so it probably remains enabled (unless
> there is something really good we could do with the ABI if we exclude
> ELFv1 but I don't see anything).
>
> But if glibc only builds for ELFv2 support that's probably reasonable.
To be clear, we still support ELFv1 for POWER, but given that this
feature is POWER9 and later, I expect the number of users benefiting
from 32-bit support (or ELFv1 and thus big-endian support) to be quite
small.
Especially if we go the conditional branch route, I would restrict this
to ELFv2 in glibc, at least for default builds.
>> From the glibc perspective, the major question is how we handle run-time
>> selection of the system call instruction sequence. On i386, we use a
>> function pointer in the TCB to call an instruction sequence in the vDSO.
>> That's problematic from a security perspective. I expect that on
>> POWER9, using a pointer in read-only memory would be equally
>> non-attractive due to a similar lack of PC-relative addressing. We
>> could use the HWCAP bit in the TCB, but that would add another (easy to
>> predict) conditional branch to every system call.
>
> I would have to defer to glibc devs on this. Conditional branch
> should be acceptable I think, scv improves speed as much as several
> mispredicted branches (about 90 cycles).
But we'd have to pay for that branch (and likely the LR clobber) on
legacy POWER, and that's something to consider, too.
Is there an additional performance hit if a process uses both the old
and new system call sequence?
Thanks,
Florian
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Segher Boessenkool @ 2020-01-28 15:40 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Florian Weimer, libc-alpha, Tulio Magno Quites Machado Filho,
linuxppc-dev
In-Reply-To: <1580218232.2tezmthp1x.astroid@bobo.none>
On Wed, Jan 29, 2020 at 12:05:40AM +1000, Nicholas Piggin wrote:
> Florian Weimer's on January 28, 2020 11:09 pm:
> > But I don't think we are so lucky for the inline system calls. GCC
> > recognizes an "lr" clobber with inline asm (even though it is not
> > documented), but it generates rather strange assembler output as a
> > result:
> > std 0,16(1)
> > ori 2,2,0
> > ld 0,16(1)
> > That's with GCC 8.3 at -O2. I don't understand what the ori is about.
>
> ori 2,2,0 is the group terminating nop hint for POWER8 type cores
> which had dispatch grouping rules.
Yup. GCC generates that here to force the load into a different
scheduling group than the corresponding store is, because that otherwise
would cause very expensive pipeline flushes. It does that if it knows it
is the same address (like here).
> > I don't think we can save LR in a regular register around the system
> > call, explicitly in the inline asm statement, because we still have to
> > generate proper unwinding information using CFI directives, something
> > that you cannot do from within the asm statement.
Why not?
> >> - Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
> >> instruction on the kernel side, and it is currently not implemented well
> >> in glibc, requiring a mfcr (mfocr should be possible and asm goto support
> >> would allow a better implementation). Is it worth continuing this style of
> >> error handling? Or just move to -ve return means error? Using a different
> >> bit would allow the kernel to piggy back the CR return code setting with
> >> a test for the error case exit.
> >
> > GCC does not model the condition registers,
Huh? It does model the condition register, as 8 registers in GCC's
internal model (one each for CR0..CR7).
There is no way to use CR0 across function calls, with our ABIs: it is
a volatile register.
GCC does not model the SO bits in the CR fields.
If the calling convention would only use registers GCC *does* know
about, we can have a builtin for this, so that you can get better
inlining etc., no need for an assembler wrapper.
> > But the kernel uses the -errno convention internally, so I think it
> > would make sense to pass this to userspace and not convert back and
> > forth. This would align with what most of the architectures do, and
> > also avoids the GCC oddity.
>
> Yes I would be interested in opinions for this option. It seems like
> matching other architectures is a good idea. Maybe there are some
> reasons not to.
Agreed with you both here.
> >> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
> >> calls if there was interest in developing an ABI for 32-bit programs.
> >> Marginal benefit in avoiding compat syscall selection.
> >
> > We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
> > provide an ELFv1 port for this given that it's POWER9-specific.
We *do* have a 32-bit LE ABI. And ELFv1 is not 32-bit either. Please
don't confuse these things :-)
The 64-bit LE kernel does not really support 32-bit userland (or BE
userland), *that* is what you want to say.
> > From the glibc perspective, the major question is how we handle run-time
> > selection of the system call instruction sequence.
Well, if it is inlined you don't have this problem either! :-)
Segher
^ permalink raw reply
* Re: [PASEMI PA6T PPC] Onboard CF card device with new SanDisk High (>8G) CF cards
From: Christian Zigotzky @ 2020-01-28 14:48 UTC (permalink / raw)
To: Rob Herring
Cc: Ulf Hansson, Darren Stevens, R.T.Dickinson,
linux-mmc@vger.kernel.org, Julian Margetson, Christoph Hellwig,
contact@a-eon.com, mad skateman, linuxppc-dev
In-Reply-To: <CAL_Jsq+r9a0BhvO4-VK59+Fh=9Ean=JgKwwxS+bqVz7=OWD5_w@mail.gmail.com>
On 28 January 2020 at 3:16 pm, Rob Herring wrote:
> On Tue, Jan 28, 2020 at 2:01 AM Christian Zigotzky
> <chzigotzky@xenosoft.de> wrote:
>> Hi All,
>>
>> Which mailing list is responsible for the pata_pcmcia driver? We are
>> using new SanDisk High (>8G) CF cards with this driver [1] and we need
>> the following line in the file "drivers/ata/pata_pcmcia.c".
>>
>> + PCMCIA_DEVICE_MANF_CARD(0x00f1, 0x0101), /* SanDisk High
>> (>8G) CFA */
> Run get_maintainers.pl and it will answer that for you:
>
> $ scripts/get_maintainer.pl -f drivers/ata/pata_pcmcia.c
> Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> (maintainer:LIBATA PATA DRIVERS)
> Jens Axboe <axboe@kernel.dk> (maintainer:LIBATA PATA DRIVERS)
> linux-ide@vger.kernel.org (open list:LIBATA PATA DRIVERS)
> linux-kernel@vger.kernel.org (open list)
Thank you!
^ permalink raw reply
* Re: [PATCH] of: Add OF_DMA_DEFAULT_COHERENT & select it on powerpc
From: Rob Herring @ 2020-01-28 14:26 UTC (permalink / raw)
To: Michael Ellerman
Cc: devicetree, ulf.hansson, linux-kernel, linuxppc-dev, robh+dt,
chzigotzky, hch
In-Reply-To: <20200126115247.13402-1-mpe@ellerman.id.au>
On Sun, 26 Jan 2020 22:52:47 +1100, Michael Ellerman wrote:
> There's an OF helper called of_dma_is_coherent(), which checks if a
> device has a "dma-coherent" property to see if the device is coherent
> for DMA.
>
> But on some platforms devices are coherent by default, and on some
> platforms it's not possible to update existing device trees to add the
> "dma-coherent" property.
>
> So add a Kconfig symbol to allow arch code to tell
> of_dma_is_coherent() that devices are coherent by default, regardless
> of the presence of the property.
>
> Select that symbol on powerpc when NOT_COHERENT_CACHE is not set, ie.
> when the system has a coherent cache.
>
> Fixes: 92ea637edea3 ("of: introduce of_dma_is_coherent() helper")
> Cc: stable@vger.kernel.org # v3.16+
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/Kconfig | 1 +
> drivers/of/Kconfig | 4 ++++
> drivers/of/address.c | 6 +++++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
Applied, thanks.
Rob
^ permalink raw reply
* [PATCH 4.9 160/271] perf/ioctl: Add check for the sample_period value
From: Greg Kroah-Hartman @ 2020-01-28 14:05 UTC (permalink / raw)
To: linux-kernel
Cc: Sasha Levin, Ravi Bangoria, maddy, acme, Vince Weaver,
Peter Zijlstra (Intel), Greg Kroah-Hartman, Linus Torvalds,
linuxppc-dev, stable, Arnaldo Carvalho de Melo,
Alexander Shishkin, Stephane Eranian, Thomas Gleixner, Jiri Olsa,
Ingo Molnar
In-Reply-To: <20200128135852.449088278@linuxfoundation.org>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
[ Upstream commit 913a90bc5a3a06b1f04c337320e9aeee2328dd77 ]
perf_event_open() limits the sample_period to 63 bits. See:
0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Make ioctl() consistent with it.
Also on PowerPC, negative sample_period could cause a recursive
PMIs leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: acme@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: maddy@linux.vnet.ibm.com
Cc: mpe@ellerman.id.au
Fixes: 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period to 63 bits")
Link: https://lkml.kernel.org/r/20190604042953.914-1-ravi.bangoria@linux.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5bbf7537a6121..64ace5e9af2a0 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4624,6 +4624,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, &value);
return 0;
--
2.20.1
^ permalink raw reply related
* Re: [PASEMI PA6T PPC] Onboard CF card device with new SanDisk High (>8G) CF cards
From: Rob Herring @ 2020-01-28 14:16 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Ulf Hansson, Darren Stevens, R.T.Dickinson,
linux-mmc@vger.kernel.org, Julian Margetson, Christoph Hellwig,
contact@a-eon.com, mad skateman, linuxppc-dev
In-Reply-To: <688f1943-5b81-6200-d4d9-38d21b69e180@xenosoft.de>
On Tue, Jan 28, 2020 at 2:01 AM Christian Zigotzky
<chzigotzky@xenosoft.de> wrote:
>
> Hi All,
>
> Which mailing list is responsible for the pata_pcmcia driver? We are
> using new SanDisk High (>8G) CF cards with this driver [1] and we need
> the following line in the file "drivers/ata/pata_pcmcia.c".
>
> + PCMCIA_DEVICE_MANF_CARD(0x00f1, 0x0101), /* SanDisk High
> (>8G) CFA */
Run get_maintainers.pl and it will answer that for you:
$ scripts/get_maintainer.pl -f drivers/ata/pata_pcmcia.c
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
(maintainer:LIBATA PATA DRIVERS)
Jens Axboe <axboe@kernel.dk> (maintainer:LIBATA PATA DRIVERS)
linux-ide@vger.kernel.org (open list:LIBATA PATA DRIVERS)
linux-kernel@vger.kernel.org (open list)
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Nicholas Piggin @ 2020-01-28 14:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: linuxppc-dev, Tulio Magno Quites Machado Filho, libc-alpha
In-Reply-To: <87tv4fd8wp.fsf@oldenburg2.str.redhat.com>
Florian Weimer's on January 28, 2020 11:09 pm:
> * Nicholas Piggin:
>
>> * Proposal is for PPC_FEATURE2_SCV to indicate 'scv 0' support, all other
>> vectors will return -ENOSYS, and the decision for how to add support for
>> a new vector deferred until we see the next user.
>
> Seems reasonable. We don't have to decide this today.
>
>> * Proposal is for scv 0 to provide the standard Linux system call ABI with some
>> differences:
>>
>> - LR is volatile across scv calls. This is necessary for support because the
>> scv instruction clobbers LR.
>
> I think we can express this in the glibc system call assembler wrapper
> generators. The mcount profiling wrappers already have this property.
>
> But I don't think we are so lucky for the inline system calls. GCC
> recognizes an "lr" clobber with inline asm (even though it is not
> documented), but it generates rather strange assembler output as a
> result:
>
> long
> f (long x)
> {
> long y;
> asm ("#" : "=r" (y) : "r" (x) : "lr");
> return y;
> }
>
> .abiversion 2
> .section ".text"
> .align 2
> .p2align 4,,15
> .globl f
> .type f, @function
> f:
> .LFB0:
> .cfi_startproc
> mflr 0
> .cfi_register 65, 0
> #APP
> # 5 "t.c" 1
> #
> # 0 "" 2
> #NO_APP
> std 0,16(1)
> .cfi_offset 65, 16
> ori 2,2,0
> ld 0,16(1)
> mtlr 0
> .cfi_restore 65
> blr
> .long 0
> .byte 0,0,0,1,0,0,0,0
> .cfi_endproc
> .LFE0:
> .size f,.-f
>
>
> That's with GCC 8.3 at -O2. I don't understand what the ori is about.
ori 2,2,0 is the group terminating nop hint for POWER8 type cores
which had dispatch grouping rules.
>
> I don't think we can save LR in a regular register around the system
> call, explicitly in the inline asm statement, because we still have to
> generate proper unwinding information using CFI directives, something
> that you cannot do from within the asm statement.
>
> Supporting this in GCC should not be impossible, but someone who
> actually knows this stuff needs to look at it.
The generated assembler actually seems okay to me. If we compile
something like a syscall and with -mcpu=power9:
long
f (long _r3, long _r4, long _r5, long _r6, long _r7, long _r8, long _r0)
{
register long r0 asm ("r0") = _r0;
register long r3 asm ("r3") = _r3;
register long r4 asm ("r4") = _r4;
register long r5 asm ("r5") = _r5;
register long r6 asm ("r6") = _r6;
register long r7 asm ("r7") = _r7;
register long r8 asm ("r8") = _r8;
asm ("# scv" : "=r"(r3) : "r"(r0), "r"(r4), "r"(r5), "r"(r6), "r"(r7), "r"(r8) : "lr", "ctr", "cc", "xer");
return r3;
}
f:
.LFB0:
.cfi_startproc
mflr 0
std 0,16(1)
.cfi_offset 65, 16
mr 0,9
#APP
# 12 "a.c" 1
# scv
# 0 "" 2
#NO_APP
ld 0,16(1)
mtlr 0
.cfi_restore 65
blr
.long 0
.byte 0,0,0,1,0,0,0,0
.cfi_endproc
That gets the LR save/restore right when we're also using r0.
>
>> - CR1 and CR5-CR7 are volatile. This matches the C ABI and would allow the
>> system call exit to avoid restoring the CR register.
>
> This sounds reasonable, but I don't know what kind of knock-on effects
> this has. The inline system call wrappers can handle this with minor
> tweaks.
Okay, good. In the end we would have to check code trace through the
kernel and libc of course, but I think there's little to no opportunity
to take advantage of current extra non-volatile cr regs.
mtcr has to write 8 independently renamed registers so it's cracked into
2 insns on POWER9 (and likely to always be a bit troublesome). It's not
much in the scheme of a system call, but while we can tweak the ABI...
>
>> - Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
>> instruction on the kernel side, and it is currently not implemented well
>> in glibc, requiring a mfcr (mfocr should be possible and asm goto support
>> would allow a better implementation). Is it worth continuing this style of
>> error handling? Or just move to -ve return means error? Using a different
>> bit would allow the kernel to piggy back the CR return code setting with
>> a test for the error case exit.
>
> GCC does not model the condition registers, so for inline system calls,
> we have to produce a value anyway that the subsequence C code can check.
> The assembler syscall wrappers do not need to do this, of course, but
> I'm not sure which category of interfaces is more important.
Right. asm goto can improve this kind of pattern if it's inlined
into the C code which tests the result, it can branch using the flags
to the C error handling label, rather than move flags into GPR, test
GPR, branch. However...
> But the kernel uses the -errno convention internally, so I think it
> would make sense to pass this to userspace and not convert back and
> forth. This would align with what most of the architectures do, and
> also avoids the GCC oddity.
Yes I would be interested in opinions for this option. It seems like
matching other architectures is a good idea. Maybe there are some
reasons not to.
>> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
>> calls if there was interest in developing an ABI for 32-bit programs.
>> Marginal benefit in avoiding compat syscall selection.
>
> We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
> provide an ELFv1 port for this given that it's POWER9-specific.
Okay. There's no reason not to enable this for BE, at least for the
kernel it's no additional work so it probably remains enabled (unless
there is something really good we could do with the ABI if we exclude
ELFv1 but I don't see anything).
But if glibc only builds for ELFv2 support that's probably reasonable.
>
> From the glibc perspective, the major question is how we handle run-time
> selection of the system call instruction sequence. On i386, we use a
> function pointer in the TCB to call an instruction sequence in the vDSO.
> That's problematic from a security perspective. I expect that on
> POWER9, using a pointer in read-only memory would be equally
> non-attractive due to a similar lack of PC-relative addressing. We
> could use the HWCAP bit in the TCB, but that would add another (easy to
> predict) conditional branch to every system call.
I would have to defer to glibc devs on this. Conditional branch
should be acceptable I think, scv improves speed as much as several
mispredicted branches (about 90 cycles).
> I don't think it matters whether both system call variants use the same
> error convention because we could have different error code extraction
> code on the two branches.
That's one less difficulty.
Thanks,
Nick
^ permalink raw reply
* Re: powerpc Linux scv support and scv system call ABI proposal
From: Florian Weimer @ 2020-01-28 13:09 UTC (permalink / raw)
To: Nicholas Piggin
Cc: libc-alpha, Tulio Magno Quites Machado Filho, linuxppc-dev
In-Reply-To: <1580207907.c96c1lh9t0.astroid@bobo.none>
* Nicholas Piggin:
> * Proposal is for PPC_FEATURE2_SCV to indicate 'scv 0' support, all other
> vectors will return -ENOSYS, and the decision for how to add support for
> a new vector deferred until we see the next user.
Seems reasonable. We don't have to decide this today.
> * Proposal is for scv 0 to provide the standard Linux system call ABI with some
> differences:
>
> - LR is volatile across scv calls. This is necessary for support because the
> scv instruction clobbers LR.
I think we can express this in the glibc system call assembler wrapper
generators. The mcount profiling wrappers already have this property.
But I don't think we are so lucky for the inline system calls. GCC
recognizes an "lr" clobber with inline asm (even though it is not
documented), but it generates rather strange assembler output as a
result:
long
f (long x)
{
long y;
asm ("#" : "=r" (y) : "r" (x) : "lr");
return y;
}
.abiversion 2
.section ".text"
.align 2
.p2align 4,,15
.globl f
.type f, @function
f:
.LFB0:
.cfi_startproc
mflr 0
.cfi_register 65, 0
#APP
# 5 "t.c" 1
#
# 0 "" 2
#NO_APP
std 0,16(1)
.cfi_offset 65, 16
ori 2,2,0
ld 0,16(1)
mtlr 0
.cfi_restore 65
blr
.long 0
.byte 0,0,0,1,0,0,0,0
.cfi_endproc
.LFE0:
.size f,.-f
That's with GCC 8.3 at -O2. I don't understand what the ori is about.
I don't think we can save LR in a regular register around the system
call, explicitly in the inline asm statement, because we still have to
generate proper unwinding information using CFI directives, something
that you cannot do from within the asm statement.
Supporting this in GCC should not be impossible, but someone who
actually knows this stuff needs to look at it.
> - CR1 and CR5-CR7 are volatile. This matches the C ABI and would allow the
> system call exit to avoid restoring the CR register.
This sounds reasonable, but I don't know what kind of knock-on effects
this has. The inline system call wrappers can handle this with minor
tweaks.
> - Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
> instruction on the kernel side, and it is currently not implemented well
> in glibc, requiring a mfcr (mfocr should be possible and asm goto support
> would allow a better implementation). Is it worth continuing this style of
> error handling? Or just move to -ve return means error? Using a different
> bit would allow the kernel to piggy back the CR return code setting with
> a test for the error case exit.
GCC does not model the condition registers, so for inline system calls,
we have to produce a value anyway that the subsequence C code can check.
The assembler syscall wrappers do not need to do this, of course, but
I'm not sure which category of interfaces is more important.
But the kernel uses the -errno convention internally, so I think it
would make sense to pass this to userspace and not convert back and
forth. This would align with what most of the architectures do, and
also avoids the GCC oddity.
> - Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
> calls if there was interest in developing an ABI for 32-bit programs.
> Marginal benefit in avoiding compat syscall selection.
We don't have an ELFv2 ABI for 32-bit. I doubt it makes sense to
provide an ELFv1 port for this given that it's POWER9-specific.
From the glibc perspective, the major question is how we handle run-time
selection of the system call instruction sequence. On i386, we use a
function pointer in the TCB to call an instruction sequence in the vDSO.
That's problematic from a security perspective. I expect that on
POWER9, using a pointer in read-only memory would be equally
non-attractive due to a similar lack of PC-relative addressing. We
could use the HWCAP bit in the TCB, but that would add another (easy to
predict) conditional branch to every system call.
I don't think it matters whether both system call variants use the same
error convention because we could have different error code extraction
code on the two branches.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-01-28 12:30 UTC (permalink / raw)
To: Mike Rapoport
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
linux-arm-kernel, Ingo Molnar, Kees Cook, Anshuman Khandual,
Masahiro Yamada, Mark Brown, Kirill A . Shutemov, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
> On Jan 28, 2020, at 7:10 AM, Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> Aren't x86 and arm64 not decent enough?
> Even if this test could be used to detect regressions only on these two
> platforms, the test is valuable.
The question is does it detect regressions good enough? Where is the list of past bugs that it had found?
It is an usual deal for unproven debugging features remain out of tree first and keep gathering unique bugs it found and then justify for a mainline inclusion with enough data.
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Mike Rapoport @ 2020-01-28 12:09 UTC (permalink / raw)
To: Qian Cai
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
linux-arm-kernel, Ingo Molnar, Kees Cook, Anshuman Khandual,
Masahiro Yamada, Mark Brown, Kirill A . Shutemov, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <016A776F-EFD9-4D2B-A3A9-788008617D95@lca.pw>
Hello Qian,
On Mon, Jan 27, 2020 at 10:33:08PM -0500, Qian Cai wrote:
>
> > On Jan 27, 2020, at 10:06 PM, Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> >
> > enablement of this test (for the moment) but then the goal is to integrate all
> > of them going forward. The test not only validates platform's adherence to the
> > expected semantics from generic MM but also helps in keeping it that way during
> > code changes in future as well.
>
> Another option maybe to get some decent arches on board first before merging this
> thing, so it have more changes to catch regressions for developers who might run this.
Aren't x86 and arm64 not decent enough?
Even if this test could be used to detect regressions only on these two
platforms, the test is valuable.
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Mark Brown @ 2020-01-28 11:58 UTC (permalink / raw)
To: Qian Cai
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
linux-arm-kernel, Ingo Molnar, Kees Cook, Anshuman Khandual,
Masahiro Yamada, Mike Rapoport, Kirill A . Shutemov, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Vineet Gupta, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <144F3894-7934-4EC7-A9F9-C6A84CA08C65@lca.pw>
[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]
On Tue, Jan 28, 2020 at 02:12:56AM -0500, Qian Cai wrote:
> > On Jan 28, 2020, at 1:13 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> > ppc32 an indecent / legacy platform ? Are you kidying ?
> > Powerquicc II PRO for instance is fully supported by the
> > manufacturer and widely used in many small networking devices.
> Of course I forgot about embedded devices. The problem is that how
> many developers are actually going to run this debug option on
> embedded devices?
Much fewer if the code isn't upstream than if it is. This isn't
something that every developer is going to enable all the time but that
doesn't mean it's not useful, it's more for people doing work on the
architectures or on memory management (or who suspect they're running
into a relevant problem), and I'm sure some of the automated testing
people will enable it. The more barriers there are in place to getting
the testsuite up and running the less likely it is that any of these
groups will run it regularly.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [FSL P5020 P5040 PPC] Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates
From: Michael Ellerman @ 2020-01-28 11:55 UTC (permalink / raw)
To: Christian Zigotzky, Ulf Hansson
Cc: Darren Stevens, mad skateman, linux-mmc@vger.kernel.org,
Julian Margetson, Christoph Hellwig, Rob Herring,
contact@a-eon.com, R.T.Dickinson, linuxppc-dev
In-Reply-To: <de35b0fc-9e24-2989-58f6-ade0585db1a0@xenosoft.de>
Christian Zigotzky <chzigotzky@xenosoft.de> writes:
> On 24 January 2020 at 12:42 pm, Michael Ellerman wrote:
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>> On Thu, 16 Jan 2020 at 12:18, Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>>>> Hi All,
>>>>
>>>> We still need the attached patch for our onboard SD card interface
>>>> [1,2]. Could you please add this patch to the tree?
>>> No, because according to previous discussion that isn't the correct
>>> solution and more importantly it will break other archs (if I recall
>>> correctly).
>>>
>>> Looks like someone from the ppc community needs to pick up the ball.
>> That's a pretty small community these days :) :/
>>
>> Christian can you test this please? I think I got the polarity of all
>> the tests right, but it's Friday night so maybe I'm wrong :)
>>
>> cheers
> Michael,
>
> Thanks a lot for the new patch! I compiled the RC7 of kernel 5.5 with
> your patch again yesterday and the kernel works without any problems
> with our onboard SD cards. [1]
Thanks for testing.
cheers
^ 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