* Re: RFC on writel and writel_relaxed
From: Will Deacon @ 2018-03-27 14:36 UTC (permalink / raw)
To: Sinan Kaya
Cc: Arnd Bergmann, Benjamin Herrenschmidt, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <bba5ebd8-0643-6f09-9367-419b49e53535@codeaurora.org>
On Tue, Mar 27, 2018 at 09:46:51AM -0400, Sinan Kaya wrote:
> On 3/27/2018 7:02 AM, Will Deacon wrote:
> > - See Documentation/DMA-API.txt for more information on consistent memory.
> > + can see it now has ownership. Note that, when using writel(), a prior
> > + wmb() is not needed to guarantee that the cache coherent memory writes
> > + have completed before writing to the MMIO region. The cheaper
> > + writel_relaxed() does not provide this guarantee and must not be used
> > + here.
>
> Can we say the same thing for iowrite32() and iowrite32be(). I also see wmb()
> in front of these.
I don't think so. My reading of memory-barriers.txt says that writeX might
expand to outX, and outX is not ordered with respect to other types of
memory.
Will
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Jason Gunthorpe @ 2018-03-27 14:24 UTC (permalink / raw)
To: Will Deacon
Cc: Arnd Bergmann, Benjamin Herrenschmidt, Sinan Kaya, David Laight,
Oliver, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney
In-Reply-To: <20180327094159.GA29373@arm.com>
On Tue, Mar 27, 2018 at 10:42:00AM +0100, Will Deacon wrote:
> On Tue, Mar 27, 2018 at 09:56:47AM +0200, Arnd Bergmann wrote:
> > On Tue, Mar 27, 2018 at 12:27 AM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > On Tue, Mar 27, 2018 at 09:01:57AM +1100, Benjamin Herrenschmidt wrote:
> > >> On Mon, 2018-03-26 at 17:46 -0400, Sinan Kaya wrote:
> > >
> > > I even see patches adding wmb() based on actual observed memory
> > > corruption during testing on Intel:
> > >
> > > https://patchwork.kernel.org/patch/10177207/
> > >
> > > So you think all of this is unnecessary and writel is totally strongly
> > > ordered, even on multi-socket Intel?
> >
> > This example adds a wmb() between two writes to a coherent DMA
> > area, it is definitely required there. I'm pretty sure I've never seen
> > any bug reports pointing to a missing wmb() between memory
> > and MMIO write accesses, but if you remember seeing them in the
> > list, maybe you can look again for some evidence of something going
> > wrong on x86 without it?
>
> If this is just about ordering accesses to coherent DMA, then using
> dma_wmb() instead will be much better performance on arm/arm64.
dma_wmb() is a NOP on x86, it was tested anyhow and didn't help
this case.. Confusing, but probably not relevant to this discussion.
Jason
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Jason Gunthorpe @ 2018-03-27 14:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Benjamin Herrenschmidt, Sinan Kaya, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Will Deacon,
Paul E. McKenney
In-Reply-To: <CAK8P3a3a+zUNLjqw9CL5rYkXhJ-GMVTVnBU2XUu2f3njZfrqKw@mail.gmail.com>
On Tue, Mar 27, 2018 at 09:56:47AM +0200, Arnd Bergmann wrote:
> On Tue, Mar 27, 2018 at 12:27 AM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Tue, Mar 27, 2018 at 09:01:57AM +1100, Benjamin Herrenschmidt wrote:
> >> On Mon, 2018-03-26 at 17:46 -0400, Sinan Kaya wrote:
> >
> > I even see patches adding wmb() based on actual observed memory
> > corruption during testing on Intel:
> >
> > https://patchwork.kernel.org/patch/10177207/
> >
> > So you think all of this is unnecessary and writel is totally strongly
> > ordered, even on multi-socket Intel?
>
> This example adds a wmb() between two writes to a coherent DMA
> area, it is definitely required there.
Ah! So it is, too many things called 'db' in that driver.. One of the
'db' is also WC BAR memory..
Jason
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Jason Gunthorpe @ 2018-03-27 14:12 UTC (permalink / raw)
To: okaya
Cc: Benjamin Herrenschmidt, Arnd Bergmann, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Will Deacon, Paul E. McKenney
In-Reply-To: <4f27ac42a6953589157f4d7f18051366@codeaurora.org>
On Tue, Mar 27, 2018 at 08:22:55AM -0400, okaya@codeaurora.org wrote:
> On 2018-03-27 07:23, Benjamin Herrenschmidt wrote:
> >On Tue, 2018-03-27 at 11:44 +0200, Arnd Bergmann wrote:
> >>> The interesting thing is that we do seem to have a whole LOT of these
> >>> spurrious wmb before writel all over the tree, I suspect because of
> >>> that incorrect recommendation in memory-barriers.txt.
> >>>
> >>> We should fix that.
> >>
> >>Maybe the problem is just that it's so counter-intuitive that we don't
> >>need that barrier in Linux, when the hardware does need one on some
> >>architectures.
> >>
> >>How about we define a barrier type instruction specifically for this
> >>purpose, something like wmb_before_mmio() and have all architectures
> >>define that to an empty macro?
> >
> >This is exactly what wmb() is about and exactly what Linux rejected
> >back in the day (and in hindsight I agree with him).
> >
> >>That way, having correct code using wmb_before_mmio() will not
> >>trigger an incorrect review comment that leads to extra wmb(). ;-)
> >
> >Ah, you mean have an empty macro that will always be empty on all
> >architectures just to fool people ? :-)
> >
> >Not sure that will fly ... I think we just need to be documenting that
> >stuff better and not have incorrect examples. Also a sweep to remove
> >some useless ones like the one in e1000e would help.
>
> I have been converting wmb+writel to wmb+writel_relaxed. (About 30 patches)
>
> I will have to just remove the wmb and keep writel, then repost.
Okay, but before you do that, can we get a statement how this works
for WC?
Some of these writels are to WC memory, do they need the wmb()?!?
Jason
^ permalink raw reply
* Re: [PATCH v2 01/10] powerpc: Add security feature flags for Spectre/Meltdown
From: Gabriel Paubert @ 2018-03-27 13:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
On Tue, Mar 27, 2018 at 11:01:44PM +1100, Michael Ellerman wrote:
> This commit adds security feature flags to reflect the settings we
> receive from firmware regarding Spectre/Meltdown mitigations.
>
> The feature names reflect the names we are given by firmware on bare
> metal machines. See the hostboot source for details.
>
> Arguably these could be firmware features, but that then requires them
> to be read early in boot so they're available prior to asm feature
> patching, but we don't actually want to use them for patching. We may
> also want to dynamically update them in future, which would be
> incompatible with the way firmware features work (at the moment at
> least). So for now just make them separate flags.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/security_features.h | 65 ++++++++++++++++++++++++++++
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/security.c | 15 +++++++
> 3 files changed, 81 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/security_features.h
> create mode 100644 arch/powerpc/kernel/security.c
>
>
> v2: Rebased on top of LPM changes.
>
> diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
> new file mode 100644
> index 000000000000..db00ad2c72c2
> --- /dev/null
> +++ b/arch/powerpc/include/asm/security_features.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Security related feature bit definitions.
> + *
> + * Copyright 2018, Michael Ellerman, IBM Corporation.
> + */
> +
> +#ifndef _ASM_POWERPC_SECURITY_FEATURES_H
> +#define _ASM_POWERPC_SECURITY_FEATURES_H
> +
> +
> +extern unsigned long powerpc_security_features;
> +
> +static inline void security_ftr_set(unsigned long feature)
> +{
> + powerpc_security_features |= feature;
> +}
> +
> +static inline void security_ftr_clear(unsigned long feature)
> +{
> + powerpc_security_features &= ~feature;
> +}
> +
> +static inline bool security_ftr_enabled(unsigned long feature)
> +{
> + return !!(powerpc_security_features & feature);
> +}
> +
> +
> +// Features indicating support for Spectre/Meltdown mitigations
> +
> +// The L1-D cache can be flushed with ori r30,r30,0
> +#define SEC_FTR_L1D_FLUSH_ORI30 0x0000000000000001ull
> +
> +// The L1-D cache can be flushed with mtspr 882,r0 (aka SPRN_TRIG2)
> +#define SEC_FTR_L1D_FLUSH_TRIG2 0x0000000000000002ull
> +
> +// ori r31,r31,0 acts as a speculation barrier
> +#define SEC_FTR_SPEC_BAR_ORI31 0x0000000000000004ull
> +
> +// Speculation past bctr is disabled
> +#define SEC_FTR_BCCTRL_SERIALISED 0x0000000000000008ull
Nitpicks:
1) bcctr or bcctrL ?
2) seraliaZe seems to be more popular than serialiSe in the kernel
(1769 hits from "grep -ir serializ", 264 with the "s")
Still needs to grep for both in any case, bummer!
Gabriel
> +
> +// Entries in L1-D are private to a SMT thread
> +#define SEC_FTR_L1D_THREAD_PRIV 0x0000000000000010ull
> +
> +// Indirect branch prediction cache disabled
> +#define SEC_FTR_COUNT_CACHE_DISABLED 0x0000000000000020ull
> +
> +
> +// Features indicating need for Spectre/Meltdown mitigations
> +
> +// The L1-D cache should be flushed on MSR[HV] 1->0 transition (hypervisor to guest)
> +#define SEC_FTR_L1D_FLUSH_HV 0x0000000000000040ull
> +
> +// The L1-D cache should be flushed on MSR[PR] 0->1 transition (kernel to userspace)
> +#define SEC_FTR_L1D_FLUSH_PR 0x0000000000000080ull
> +
> +// A speculation barrier should be used for bounds checks (Spectre variant 1)
> +#define SEC_FTR_BNDS_CHK_SPEC_BAR 0x0000000000000100ull
> +
> +// Firmware configuration indicates user favours security over performance
> +#define SEC_FTR_FAVOUR_SECURITY 0x0000000000000200ull
> +
> +#endif /* _ASM_POWERPC_SECURITY_FEATURES_H */
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 1b6bc7fba996..d458c45e5004 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -42,7 +42,7 @@ obj-$(CONFIG_VDSO32) += vdso32/
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_ppc970.o cpu_setup_pa6t.o
> -obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
> +obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o security.o
> obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
> obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
> obj-$(CONFIG_PPC64) += vdso64/
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> new file mode 100644
> index 000000000000..4ccba00d224c
> --- /dev/null
> +++ b/arch/powerpc/kernel/security.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Security related flags and so on.
> +//
> +// Copyright 2018, Michael Ellerman, IBM Corporation.
> +
> +#include <linux/kernel.h>
> +#include <asm/security_features.h>
> +
> +
> +unsigned long powerpc_security_features __read_mostly = \
> + SEC_FTR_L1D_FLUSH_HV | \
> + SEC_FTR_L1D_FLUSH_PR | \
> + SEC_FTR_BNDS_CHK_SPEC_BAR | \
> + SEC_FTR_FAVOUR_SECURITY;
> --
> 2.14.1
>
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Ilya Smith @ 2018-03-27 13:51 UTC (permalink / raw)
To: Michal Hocko
Cc: Matthew Wilcox, rth, ink, mattst88, vgupta, linux, tony.luck,
fenghua.yu, ralf, jejb, Helge Deller, benh, paulus, mpe,
schwidefsky, heiko.carstens, ysato, dalias, davem, tglx, mingo,
hpa, x86, nyc, viro, arnd, gregkh, deepa.kernel, Hugh Dickins,
kstewart, pombredanne, Andrew Morton, steve.capper, punit.agrawal,
aneesh.kumar, npiggin, Kees Cook, bhsharma, riel, nitin.m.gupta,
Kirill A. Shutemov, Dan Williams, Jan Kara, ross.zwisler,
Jerome Glisse, Andrea Arcangeli, Oleg Nesterov, linux-alpha, LKML,
linux-snps-arc, linux-ia64, linux-metag, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <20180327072432.GY5652@dhcp22.suse.cz>
> On 27 Mar 2018, at 10:24, Michal Hocko <mhocko@kernel.org> wrote:
>=20
> On Mon 26-03-18 22:45:31, Ilya Smith wrote:
>>=20
>>> On 26 Mar 2018, at 11:46, Michal Hocko <mhocko@kernel.org> wrote:
>>>=20
>>> On Fri 23-03-18 20:55:49, Ilya Smith wrote:
>>>>=20
>>>>> On 23 Mar 2018, at 15:48, Matthew Wilcox <willy@infradead.org> =
wrote:
>>>>>=20
>>>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
>>>>>> Current implementation doesn't randomize address returned by =
mmap.
>>>>>> All the entropy ends with choosing mmap_base_addr at the process
>>>>>> creation. After that mmap build very predictable layout of =
address
>>>>>> space. It allows to bypass ASLR in many cases. This patch make
>>>>>> randomization of address on any mmap call.
>>>>>=20
>>>>> Why should this be done in the kernel rather than libc? libc is =
perfectly
>>>>> capable of specifying random numbers in the first argument of =
mmap.
>>>> Well, there is following reasons:
>>>> 1. It should be done in any libc implementation, what is not =
possible IMO;
>>>=20
>>> Is this really so helpful?
>>=20
>> Yes, ASLR is one of very important mitigation techniques which are =
really used=20
>> to protect applications. If there is no ASLR, it is very easy to =
exploit=20
>> vulnerable application and compromise the system. We can=E2=80=99t =
just fix all the=20
>> vulnerabilities right now, thats why we have mitigations - techniques =
which are=20
>> makes exploitation more hard or impossible in some cases.
>>=20
>> Thats why it is helpful.
>=20
> I am not questioning ASLR in general. I am asking whether we really =
need
> per mmap ASLR in general. I can imagine that some environments want to
> pay the additional price and other side effects, but considering this
> can be achieved by libc, why to add more code to the kernel?
I believe this is the only one right place for it. Adding these 200+ =
lines of=20
code we give this feature for any user - on desktop, on server, on IoT =
device,=20
on SCADA, etc. But if only glibc will implement =E2=80=98user-mode-aslr=E2=
=80=99 IoT and SCADA=20
devices will never get it.
>>>=20
>>>> 2. User mode is not that layer which should be responsible for =
choosing
>>>> random address or handling entropy;
>>>=20
>>> Why?
>>=20
>> Because of the following reasons:
>> 1. To get random address you should have entropy. These entropy =
shouldn=E2=80=99t be=20
>> exposed to attacker anyhow, the best case is to get it from kernel. =
So this is
>> a syscall.
>=20
> /dev/[u]random is not sufficient?
Using /dev/[u]random makes 3 syscalls - open, read, close. This is a =
performance
issue.
>=20
>> 2. You should have memory map of your process to prevent remapping or =
big
>> fragmentation. Kernel already has this map.
>=20
> /proc/self/maps?
Not any system has /proc and parsing /proc/self/maps is robust so it is =
the=20
performance issue. libc will have to do it on any mmap. And there is a =
possible=20
race here - application may mmap/unmap memory with native syscall during =
other=20
thread reading maps.
>> You will got another one in libc.
>> And any non-libc user of mmap (via syscall, etc) will make hole in =
your map.
>> This one also decrease performance cause you any way call =
syscall_mmap=20
>> which will try to find some address for you in worst case, but after =
you already
>> did some computing on it.
>=20
> I do not understand. a) you should be prepared to pay an additional
> price for an additional security measures and b) how would anybody =
punch
> a hole into your mapping?=20
>=20
I was talking about any code that call mmap directly without libc =
wrapper.
>> 3. The more memory you use in userland for these proposal, the easier =
for
>> attacker to leak it or use in exploitation techniques.
>=20
> This is true in general, isn't it? I fail to see how kernel chosen and
> user chosen ranges would make any difference.
My point here was that libc will have to keep memory representation as a =
tree=20
and this tree increase attack surface. It could be hidden in kernel as =
it is right now.
>=20
>> 4. It is so easy to fix Kernel function and so hard to support memory
>> management from userspace.
>=20
> Well, on the other hand the new layout mode will add a maintenance
> burden on the kernel and will have to be maintained for ever because =
it
> is a user visible ABI.
Thats why I made this patch as RFC and would like to discuss this ABI =
here. I=20
made randomize_va_space parameter to allow disable randomisation per =
whole=20
system. PF_RANDOMIZE flag may disable randomization for concrete process =
(or=20
process groups?). For architecture I=E2=80=99ve made info.random_shift =3D=
0 , so if your=20
arch has small address space you may disable shifting. I also would like =
to add=20
some sysctl to allow process/groups to change this value and allow some=20=
processes to have shifts bigger then another. Lets discuss it, please.
>=20
>>>> 3. Memory fragmentation is unpredictable in this case
>>>>=20
>>>> Off course user mode could use random =E2=80=98hint=E2=80=99 =
address, but kernel may
>>>> discard this address if it is occupied for example and allocate =
just before
>>>> closest vma. So this solution doesn=E2=80=99t give that much =
security like=20
>>>> randomization address inside kernel.
>>>=20
>>> The userspace can use the new MAP_FIXED_NOREPLACE to probe for the
>>> address range atomically and chose a different range on failure.
>>>=20
>>=20
>> This algorithm should track current memory. If he doesn=E2=80=99t he =
may cause
>> infinite loop while trying to choose memory. And each iteration =
increase time
>> needed on allocation new memory, what is not preferred by any libc =
library
>> developer.
>=20
> Well, I am pretty sure userspace can implement proper free ranges
> tracking=E2=80=A6
I think we need to know what libc developers will say on implementing =
ASLR in=20
user-mode. I am pretty sure they will say =E2=80=98nether=E2=80=99 or =
=E2=80=98some-day=E2=80=99. And problem=20
of ASLR will stay forever.
Thanks,
Ilya
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Sinan Kaya @ 2018-03-27 13:46 UTC (permalink / raw)
To: Will Deacon, Arnd Bergmann
Cc: Benjamin Herrenschmidt, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <20180327110258.GF2464@arm.com>
On 3/27/2018 7:02 AM, Will Deacon wrote:
> - See Documentation/DMA-API.txt for more information on consistent memory.
> + can see it now has ownership. Note that, when using writel(), a prior
> + wmb() is not needed to guarantee that the cache coherent memory writes
> + have completed before writing to the MMIO region. The cheaper
> + writel_relaxed() does not provide this guarantee and must not be used
> + here.
Can we say the same thing for iowrite32() and iowrite32be(). I also see wmb()
in front of these.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* RE: RFC on writel and writel_relaxed
From: David Laight @ 2018-03-27 13:20 UTC (permalink / raw)
To: 'Will Deacon', Arnd Bergmann
Cc: Benjamin Herrenschmidt, Jason Gunthorpe, Sinan Kaya, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <20180327110258.GF2464@arm.com>
> Fair enough. I'd rather people used _relaxed by default, but I have to ad=
mit
> that it will probably just result in them getting things wrong...
Certainly requiring the driver writes use explicit barriers should make
them understand when and why they are needed - and then put in the correct =
ones.
The problem I've had is that I have a good idea which barriers are needed
but find that readl/writel seem to contain a lot of extra ones.
Maybe the are required in some places, but the extra synchronising
instructions could easily have measureable performance effects on
hot paths.
Drivers are likely to contain sequences like:
read_io
if (...) return
write_mem
...
write_mem
barrier
write_mem
barrier
write_io
for things like ring updates.
Where the 'mem' might actually be in io space.
In such sequences not all the synchronising instructions are needed.
I'm not at all sure it is easy to get the right set.
David
^ permalink raw reply
* Re: [PATCH v2 10/10] powerpc/64s: Wire up cpu_show_spectre_v2()
From: T T @ 2018-03-27 12:36 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-10-mpe@ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]
unscribed me
On Tuesday, March 27, 2018 05:31:31 AM PDT, Michael Ellerman <mpe@ellerman.id.au> wrote:
Add a definition for cpu_show_spectre_v2() to override the generic
version. This has several permuations, though in practice some may not
occur we cater for any combination.
The most verbose is:
Mitigation: Indirect branch serialisation (kernel only), Indirect
branch cache disabled, ori31 speculation barrier enabled
We don't treat the ori31 speculation barrier as a mitigation on its
own, because it has to be *used* by code in order to be a mitigation
and we don't know if userspace is doing that. So if that's all we see
we say:
Vulnerable, ori31 speculation barrier enabled
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/security.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 0eace3cac818..2cee3dcd231b 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -58,3 +58,36 @@ ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, c
return sprintf(buf, "Vulnerable\n");
}
+
+ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ bool bcs, ccd, ori;
+ struct seq_buf s;
+
+ seq_buf_init(&s, buf, PAGE_SIZE - 1);
+
+ bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
+ ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
+ ori = security_ftr_enabled(SEC_FTR_SPEC_BAR_ORI31);
+
+ if (bcs || ccd) {
+ seq_buf_printf(&s, "Mitigation: ");
+
+ if (bcs)
+ seq_buf_printf(&s, "Indirect branch serialisation (kernel only)");
+
+ if (bcs && ccd)
+ seq_buf_printf(&s, ", ");
+
+ if (ccd)
+ seq_buf_printf(&s, "Indirect branch cache disabled");
+ } else
+ seq_buf_printf(&s, "Vulnerable");
+
+ if (ori)
+ seq_buf_printf(&s, ", ori31 speculation barrier enabled");
+
+ seq_buf_printf(&s, "\n");
+
+ return s.len;
+}
--
2.14.1
[-- Attachment #2: Type: text/html, Size: 4554 bytes --]
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors
From: T T @ 2018-03-27 12:35 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Michael Ellerman
In-Reply-To: <877epxk983.fsf@concordia.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
unscribed me
On Tuesday, March 27, 2018 05:34:30 AM PDT, Michael Ellerman <mpe@ellerman.id.au> wrote:
Nicholas Piggin <npiggin@gmail.com> writes:
> opal_nvram_write currently just assumes success if it encounters an
> error other than OPAL_BUSY or OPAL_BUSY_EVENT. Have it return -EIO
> on other errors instead.
Does that ever happen with current skiboot?
Even if it doesn't I think I'm inclined to tag this for stable.
cheers
> diff --git a/arch/powerpc/platforms/powernv/opal-nvram.c b/arch/powerpc/platforms/powernv/opal-nvram.c
> index 9db4398ded5d..13bf625dc3e8 100644
> --- a/arch/powerpc/platforms/powernv/opal-nvram.c
> +++ b/arch/powerpc/platforms/powernv/opal-nvram.c
> @@ -59,6 +59,8 @@ static ssize_t opal_nvram_write(char *buf, size_t count, loff_t *index)
> if (rc == OPAL_BUSY_EVENT)
> opal_poll_events(NULL);
> }
> + if (rc)
> + return -EIO;
> *index += count;
> return count;
> }
> --
> 2.16.1
[-- Attachment #2: Type: text/html, Size: 2646 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors
From: Nicholas Piggin @ 2018-03-27 12:27 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <877epxk983.fsf@concordia.ellerman.id.au>
On Tue, 27 Mar 2018 23:13:00 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > opal_nvram_write currently just assumes success if it encounters an
> > error other than OPAL_BUSY or OPAL_BUSY_EVENT. Have it return -EIO
> > on other errors instead.
>
> Does that ever happen with current skiboot?
I can now even using the mambo fake flash driver that never returns
failure, because skiboot will return OPAL_INTERNAL_ERROR if we try
to re-enter it. So I hit it when testing sreset-in-opal cases (the
crash path wants to write something to nvram).
Not sure about the skiboot flash layer. Aside from programming
errors, it looks like perhaps ECC and BMC failure or unresponsive
could cause errors to come back here.
> Even if it doesn't I think I'm inclined to tag this for stable.
It's turning some relatively minor types of errors into a system
hang, so it seems like it could go in stable. I've hit the -EIO case
in these basic tests and it hasn't had obvious bugs.
Thanks,
Nick
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: okaya @ 2018-03-27 12:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Will Deacon, Paul E. McKenney
In-Reply-To: <1522149801.7364.49.camel@kernel.crashing.org>
On 2018-03-27 07:23, Benjamin Herrenschmidt wrote:
> On Tue, 2018-03-27 at 11:44 +0200, Arnd Bergmann wrote:
>> > The interesting thing is that we do seem to have a whole LOT of these
>> > spurrious wmb before writel all over the tree, I suspect because of
>> > that incorrect recommendation in memory-barriers.txt.
>> >
>> > We should fix that.
>>
>> Maybe the problem is just that it's so counter-intuitive that we don't
>> need that barrier in Linux, when the hardware does need one on some
>> architectures.
>>
>> How about we define a barrier type instruction specifically for this
>> purpose, something like wmb_before_mmio() and have all architectures
>> define that to an empty macro?
>
> This is exactly what wmb() is about and exactly what Linux rejected
> back in the day (and in hindsight I agree with him).
>
>> That way, having correct code using wmb_before_mmio() will not
>> trigger an incorrect review comment that leads to extra wmb(). ;-)
>
> Ah, you mean have an empty macro that will always be empty on all
> architectures just to fool people ? :-)
>
> Not sure that will fly ... I think we just need to be documenting that
> stuff better and not have incorrect examples. Also a sweep to remove
> some useless ones like the one in e1000e would help.
I have been converting wmb+writel to wmb+writel_relaxed. (About 30
patches)
I will have to just remove the wmb and keep writel, then repost.
Some of these got applied. It will cause some churn for the maintainers.
>
> Cheers,
> Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors
From: Michael Ellerman @ 2018-03-27 12:13 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180326150233.23089-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> opal_nvram_write currently just assumes success if it encounters an
> error other than OPAL_BUSY or OPAL_BUSY_EVENT. Have it return -EIO
> on other errors instead.
Does that ever happen with current skiboot?
Even if it doesn't I think I'm inclined to tag this for stable.
cheers
> diff --git a/arch/powerpc/platforms/powernv/opal-nvram.c b/arch/powerpc/platforms/powernv/opal-nvram.c
> index 9db4398ded5d..13bf625dc3e8 100644
> --- a/arch/powerpc/platforms/powernv/opal-nvram.c
> +++ b/arch/powerpc/platforms/powernv/opal-nvram.c
> @@ -59,6 +59,8 @@ static ssize_t opal_nvram_write(char *buf, size_t count, loff_t *index)
> if (rc == OPAL_BUSY_EVENT)
> opal_poll_events(NULL);
> }
> + if (rc)
> + return -EIO;
> *index += count;
> return count;
> }
> --
> 2.16.1
^ permalink raw reply
* [PATCH v2 10/10] powerpc/64s: Wire up cpu_show_spectre_v2()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Add a definition for cpu_show_spectre_v2() to override the generic
version. This has several permuations, though in practice some may not
occur we cater for any combination.
The most verbose is:
Mitigation: Indirect branch serialisation (kernel only), Indirect
branch cache disabled, ori31 speculation barrier enabled
We don't treat the ori31 speculation barrier as a mitigation on its
own, because it has to be *used* by code in order to be a mitigation
and we don't know if userspace is doing that. So if that's all we see
we say:
Vulnerable, ori31 speculation barrier enabled
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/security.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 0eace3cac818..2cee3dcd231b 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -58,3 +58,36 @@ ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, c
return sprintf(buf, "Vulnerable\n");
}
+
+ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ bool bcs, ccd, ori;
+ struct seq_buf s;
+
+ seq_buf_init(&s, buf, PAGE_SIZE - 1);
+
+ bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
+ ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
+ ori = security_ftr_enabled(SEC_FTR_SPEC_BAR_ORI31);
+
+ if (bcs || ccd) {
+ seq_buf_printf(&s, "Mitigation: ");
+
+ if (bcs)
+ seq_buf_printf(&s, "Indirect branch serialisation (kernel only)");
+
+ if (bcs && ccd)
+ seq_buf_printf(&s, ", ");
+
+ if (ccd)
+ seq_buf_printf(&s, "Indirect branch cache disabled");
+ } else
+ seq_buf_printf(&s, "Vulnerable");
+
+ if (ori)
+ seq_buf_printf(&s, ", ori31 speculation barrier enabled");
+
+ seq_buf_printf(&s, "\n");
+
+ return s.len;
+}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 09/10] powerpc/64s: Wire up cpu_show_spectre_v1()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Add a definition for cpu_show_spectre_v1() to override the generic
version. Currently this just prints "Not affected" or "Vulnerable"
based on the firmware flag.
Although the kernel does have array_index_nospec() in a few places, we
haven't yet audited all the powerpc code to see where it's necessary,
so for now we don't list that as a mitigation.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/security.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 865db6f8bcca..0eace3cac818 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -50,3 +50,11 @@ ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, cha
return sprintf(buf, "Vulnerable\n");
}
+
+ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ if (!security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR))
+ return sprintf(buf, "Not affected\n");
+
+ return sprintf(buf, "Vulnerable\n");
+}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 08/10] powerpc/pseries: Use the security flags in pseries_setup_rfi_flush()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Now that we have the security flags we can simplify the code in
pseries_setup_rfi_flush() because the security flags have pessimistic
defaults.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/pseries/setup.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index fb84c1df6ed7..1f122359cd8f 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -501,30 +501,27 @@ void pseries_setup_rfi_flush(void)
bool enable;
long rc;
- /* Enable by default */
- enable = true;
- types = L1D_FLUSH_FALLBACK;
-
rc = plpar_get_cpu_characteristics(&result);
- if (rc == H_SUCCESS) {
+ if (rc == H_SUCCESS)
init_cpu_char_feature_flags(&result);
- if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
- types |= L1D_FLUSH_MTTRIG;
- if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
- types |= L1D_FLUSH_ORI;
-
- if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
- (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
- enable = false;
- }
-
/*
* We're the guest so this doesn't apply to us, clear it to simplify
* handling of it elsewhere.
*/
security_ftr_clear(SEC_FTR_L1D_FLUSH_HV);
+ types = L1D_FLUSH_FALLBACK;
+
+ if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_TRIG2))
+ types |= L1D_FLUSH_MTTRIG;
+
+ if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_ORI30))
+ types |= L1D_FLUSH_ORI;
+
+ enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
+ security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR);
+
setup_rfi_flush(types, enable);
}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 07/10] powerpc/powernv: Use the security flags in pnv_setup_rfi_flush()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Now that we have the security flags we can significantly simplify the
code in pnv_setup_rfi_flush(), because we can use the flags instead of
checking device tree properties and because the security flags have
pessimistic defaults.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/powernv/setup.c | 41 +++++++++-------------------------
1 file changed, 10 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 5f242b1bab01..a90e995e5cc1 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -66,7 +66,7 @@ static void init_fw_feat_flags(struct device_node *np)
if (fw_feature_is("enabled", "fw-bcctrl-serialized", np))
security_ftr_set(SEC_FTR_BCCTRL_SERIALISED);
- if (fw_feature_is("enabled", "inst-spec-barrier-ori31,31,0", np))
+ if (fw_feature_is("enabled", "inst-l1d-flush-ori30,30,0", np))
security_ftr_set(SEC_FTR_L1D_FLUSH_ORI30);
if (fw_feature_is("enabled", "inst-l1d-flush-trig2", np))
@@ -99,11 +99,10 @@ static void pnv_setup_rfi_flush(void)
{
struct device_node *np, *fw_features;
enum l1d_flush_type type;
- int enable;
+ bool enable;
/* Default to fallback in case fw-features are not available */
type = L1D_FLUSH_FALLBACK;
- enable = 1;
np = of_find_node_by_name(NULL, "ibm,opal");
fw_features = of_get_child_by_name(np, "fw-features");
@@ -111,40 +110,20 @@ static void pnv_setup_rfi_flush(void)
if (fw_features) {
init_fw_feat_flags(fw_features);
+ of_node_put(fw_features);
- np = of_get_child_by_name(fw_features, "inst-l1d-flush-trig2");
- if (np && of_property_read_bool(np, "enabled"))
+ if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_TRIG2))
type = L1D_FLUSH_MTTRIG;
- of_node_put(np);
-
- np = of_get_child_by_name(fw_features, "inst-l1d-flush-ori30,30,0");
- if (np && of_property_read_bool(np, "enabled"))
+ if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_ORI30))
type = L1D_FLUSH_ORI;
-
- of_node_put(np);
-
- /* Enable unless firmware says NOT to */
- enable = 2;
- np = of_get_child_by_name(fw_features, "needs-l1d-flush-msr-hv-1-to-0");
- if (np && of_property_read_bool(np, "disabled"))
- enable--;
-
- of_node_put(np);
-
- np = of_get_child_by_name(fw_features, "needs-l1d-flush-msr-pr-0-to-1");
- if (np && of_property_read_bool(np, "disabled"))
- enable--;
-
- np = of_get_child_by_name(fw_features, "speculation-policy-favor-security");
- if (np && of_property_read_bool(np, "disabled"))
- enable = 0;
-
- of_node_put(np);
- of_node_put(fw_features);
}
- setup_rfi_flush(type, enable > 0);
+ enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
+ (security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR) || \
+ security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
+
+ setup_rfi_flush(type, enable);
}
static void __init pnv_setup_arch(void)
--
2.14.1
^ permalink raw reply related
* [PATCH v2 06/10] powerpc/64s: Enhance the information in cpu_show_meltdown()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Now that we have the security feature flags we can make the
information displayed in the "meltdown" file more informative.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/security_features.h | 1 +
arch/powerpc/kernel/security.c | 30 ++++++++++++++++++++++++++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index db00ad2c72c2..400a9050e035 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -10,6 +10,7 @@
extern unsigned long powerpc_security_features;
+extern bool rfi_flush;
static inline void security_ftr_set(unsigned long feature)
{
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 564e7f182a16..865db6f8bcca 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -6,6 +6,7 @@
#include <linux/kernel.h>
#include <linux/device.h>
+#include <linux/seq_buf.h>
#include <asm/security_features.h>
@@ -19,8 +20,33 @@ unsigned long powerpc_security_features __read_mostly = \
ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
{
- if (rfi_flush)
- return sprintf(buf, "Mitigation: RFI Flush\n");
+ bool thread_priv;
+
+ thread_priv = security_ftr_enabled(SEC_FTR_L1D_THREAD_PRIV);
+
+ if (rfi_flush || thread_priv) {
+ struct seq_buf s;
+ seq_buf_init(&s, buf, PAGE_SIZE - 1);
+
+ seq_buf_printf(&s, "Mitigation: ");
+
+ if (rfi_flush)
+ seq_buf_printf(&s, "RFI Flush");
+
+ if (rfi_flush && thread_priv)
+ seq_buf_printf(&s, ", ");
+
+ if (thread_priv)
+ seq_buf_printf(&s, "L1D private per thread");
+
+ seq_buf_printf(&s, "\n");
+
+ return s.len;
+ }
+
+ if (!security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV) &&
+ !security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR))
+ return sprintf(buf, "Not affected\n");
return sprintf(buf, "Vulnerable\n");
}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 05/10] powerpc/64s: Move cpu_show_meltdown()
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
This landed in setup_64.c for no good reason other than we had nowhere
else to put it. Now that we have a security-related file, that is a
better place for it so move it.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/security.c | 11 +++++++++++
arch/powerpc/kernel/setup_64.c | 8 --------
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 4ccba00d224c..564e7f182a16 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -5,6 +5,8 @@
// Copyright 2018, Michael Ellerman, IBM Corporation.
#include <linux/kernel.h>
+#include <linux/device.h>
+
#include <asm/security_features.h>
@@ -13,3 +15,12 @@ unsigned long powerpc_security_features __read_mostly = \
SEC_FTR_L1D_FLUSH_PR | \
SEC_FTR_BNDS_CHK_SPEC_BAR | \
SEC_FTR_FAVOUR_SECURITY;
+
+
+ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ if (rfi_flush)
+ return sprintf(buf, "Mitigation: RFI Flush\n");
+
+ return sprintf(buf, "Vulnerable\n");
+}
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 4ec4a27b36a9..7f7621668613 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -934,12 +934,4 @@ static __init int rfi_flush_debugfs_init(void)
}
device_initcall(rfi_flush_debugfs_init);
#endif
-
-ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
-{
- if (rfi_flush)
- return sprintf(buf, "Mitigation: RFI Flush\n");
-
- return sprintf(buf, "Vulnerable\n");
-}
#endif /* CONFIG_PPC_BOOK3S_64 */
--
2.14.1
^ permalink raw reply related
* [PATCH v2 04/10] powerpc/powernv: Set or clear security feature flags
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Now that we have feature flags for security related things, set or
clear them based on what we see in the device tree provided by
firmware.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/powernv/setup.c | 56 ++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 092715b9674b..5f242b1bab01 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -38,9 +38,63 @@
#include <asm/smp.h>
#include <asm/tm.h>
#include <asm/setup.h>
+#include <asm/security_features.h>
#include "powernv.h"
+
+static bool fw_feature_is(const char *state, const char *name,
+ struct device_node *fw_features)
+{
+ struct device_node *np;
+ bool rc = false;
+
+ np = of_get_child_by_name(fw_features, name);
+ if (np) {
+ rc = of_property_read_bool(np, state);
+ of_node_put(np);
+ }
+
+ return rc;
+}
+
+static void init_fw_feat_flags(struct device_node *np)
+{
+ if (fw_feature_is("enabled", "inst-spec-barrier-ori31,31,0", np))
+ security_ftr_set(SEC_FTR_SPEC_BAR_ORI31);
+
+ if (fw_feature_is("enabled", "fw-bcctrl-serialized", np))
+ security_ftr_set(SEC_FTR_BCCTRL_SERIALISED);
+
+ if (fw_feature_is("enabled", "inst-spec-barrier-ori31,31,0", np))
+ security_ftr_set(SEC_FTR_L1D_FLUSH_ORI30);
+
+ if (fw_feature_is("enabled", "inst-l1d-flush-trig2", np))
+ security_ftr_set(SEC_FTR_L1D_FLUSH_TRIG2);
+
+ if (fw_feature_is("enabled", "fw-l1d-thread-split", np))
+ security_ftr_set(SEC_FTR_L1D_THREAD_PRIV);
+
+ if (fw_feature_is("enabled", "fw-count-cache-disabled", np))
+ security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+
+ /*
+ * The features below are enabled by default, so we instead look to see
+ * if firmware has *disabled* them, and clear them if so.
+ */
+ if (fw_feature_is("disabled", "speculation-policy-favor-security", np))
+ security_ftr_clear(SEC_FTR_FAVOUR_SECURITY);
+
+ if (fw_feature_is("disabled", "needs-l1d-flush-msr-pr-0-to-1", np))
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_PR);
+
+ if (fw_feature_is("disabled", "needs-l1d-flush-msr-hv-1-to-0", np))
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_HV);
+
+ if (fw_feature_is("disabled", "needs-spec-barrier-for-bound-checks", np))
+ security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
+}
+
static void pnv_setup_rfi_flush(void)
{
struct device_node *np, *fw_features;
@@ -56,6 +110,8 @@ static void pnv_setup_rfi_flush(void)
of_node_put(np);
if (fw_features) {
+ init_fw_feat_flags(fw_features);
+
np = of_get_child_by_name(fw_features, "inst-l1d-flush-trig2");
if (np && of_property_read_bool(np, "enabled"))
type = L1D_FLUSH_MTTRIG;
--
2.14.1
^ permalink raw reply related
* [PATCH v2 03/10] powerpc/pseries: Set or clear security feature flags
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Now that we have feature flags for security related things, set or
clear them based on what we receive from the hypercall.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/pseries/setup.c | 43 ++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index f34f9081ec60..fb84c1df6ed7 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -68,6 +68,7 @@
#include <asm/plpar_wrappers.h>
#include <asm/kexec.h>
#include <asm/isa-bridge.h>
+#include <asm/security_features.h>
#include "pseries.h"
@@ -459,6 +460,40 @@ static void __init find_and_init_phbs(void)
of_pci_check_probe_only();
}
+static void init_cpu_char_feature_flags(struct h_cpu_char_result *result)
+{
+ if (result->character & H_CPU_CHAR_SPEC_BAR_ORI31)
+ security_ftr_set(SEC_FTR_SPEC_BAR_ORI31);
+
+ if (result->character & H_CPU_CHAR_BCCTRL_SERIALISED)
+ security_ftr_set(SEC_FTR_BCCTRL_SERIALISED);
+
+ if (result->character & H_CPU_CHAR_L1D_FLUSH_ORI30)
+ security_ftr_set(SEC_FTR_L1D_FLUSH_ORI30);
+
+ if (result->character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
+ security_ftr_set(SEC_FTR_L1D_FLUSH_TRIG2);
+
+ if (result->character & H_CPU_CHAR_L1D_THREAD_PRIV)
+ security_ftr_set(SEC_FTR_L1D_THREAD_PRIV);
+
+ if (result->character & H_CPU_CHAR_COUNT_CACHE_DISABLED)
+ security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+
+ /*
+ * The features below are enabled by default, so we instead look to see
+ * if firmware has *disabled* them, and clear them if so.
+ */
+ if (!(result->character & H_CPU_BEHAV_FAVOUR_SECURITY))
+ security_ftr_clear(SEC_FTR_FAVOUR_SECURITY);
+
+ if (!(result->character & H_CPU_BEHAV_L1D_FLUSH_PR))
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_PR);
+
+ if (!(result->character & H_CPU_BEHAV_BNDS_CHK_SPEC_BAR))
+ security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
+}
+
void pseries_setup_rfi_flush(void)
{
struct h_cpu_char_result result;
@@ -472,6 +507,8 @@ void pseries_setup_rfi_flush(void)
rc = plpar_get_cpu_characteristics(&result);
if (rc == H_SUCCESS) {
+ init_cpu_char_feature_flags(&result);
+
if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
types |= L1D_FLUSH_MTTRIG;
if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
@@ -482,6 +519,12 @@ void pseries_setup_rfi_flush(void)
enable = false;
}
+ /*
+ * We're the guest so this doesn't apply to us, clear it to simplify
+ * handling of it elsewhere.
+ */
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_HV);
+
setup_rfi_flush(types, enable);
}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 02/10] powerpc/pseries: Add new H_GET_CPU_CHARACTERISTICS flags
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
In-Reply-To: <20180327120153.31612-1-mpe@ellerman.id.au>
Add some additional values which have been defined for the
H_GET_CPU_CHARACTERISTICS hypercall.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/hvcall.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index eca3f9c68907..5a740feb7bd7 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -337,6 +337,9 @@
#define H_CPU_CHAR_L1D_FLUSH_ORI30 (1ull << 61) // IBM bit 2
#define H_CPU_CHAR_L1D_FLUSH_TRIG2 (1ull << 60) // IBM bit 3
#define H_CPU_CHAR_L1D_THREAD_PRIV (1ull << 59) // IBM bit 4
+#define H_CPU_CHAR_BRANCH_HINTS_HONORED (1ull << 58) // IBM bit 5
+#define H_CPU_CHAR_THREAD_RECONFIG_CTRL (1ull << 57) // IBM bit 6
+#define H_CPU_CHAR_COUNT_CACHE_DISABLED (1ull << 56) // IBM bit 7
#define H_CPU_BEHAV_FAVOUR_SECURITY (1ull << 63) // IBM bit 0
#define H_CPU_BEHAV_L1D_FLUSH_PR (1ull << 62) // IBM bit 1
--
2.14.1
^ permalink raw reply related
* [PATCH v2 01/10] powerpc: Add security feature flags for Spectre/Meltdown
From: Michael Ellerman @ 2018-03-27 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mauricfo
This commit adds security feature flags to reflect the settings we
receive from firmware regarding Spectre/Meltdown mitigations.
The feature names reflect the names we are given by firmware on bare
metal machines. See the hostboot source for details.
Arguably these could be firmware features, but that then requires them
to be read early in boot so they're available prior to asm feature
patching, but we don't actually want to use them for patching. We may
also want to dynamically update them in future, which would be
incompatible with the way firmware features work (at the moment at
least). So for now just make them separate flags.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/security_features.h | 65 ++++++++++++++++++++++++++++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/security.c | 15 +++++++
3 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/security_features.h
create mode 100644 arch/powerpc/kernel/security.c
v2: Rebased on top of LPM changes.
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
new file mode 100644
index 000000000000..db00ad2c72c2
--- /dev/null
+++ b/arch/powerpc/include/asm/security_features.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Security related feature bit definitions.
+ *
+ * Copyright 2018, Michael Ellerman, IBM Corporation.
+ */
+
+#ifndef _ASM_POWERPC_SECURITY_FEATURES_H
+#define _ASM_POWERPC_SECURITY_FEATURES_H
+
+
+extern unsigned long powerpc_security_features;
+
+static inline void security_ftr_set(unsigned long feature)
+{
+ powerpc_security_features |= feature;
+}
+
+static inline void security_ftr_clear(unsigned long feature)
+{
+ powerpc_security_features &= ~feature;
+}
+
+static inline bool security_ftr_enabled(unsigned long feature)
+{
+ return !!(powerpc_security_features & feature);
+}
+
+
+// Features indicating support for Spectre/Meltdown mitigations
+
+// The L1-D cache can be flushed with ori r30,r30,0
+#define SEC_FTR_L1D_FLUSH_ORI30 0x0000000000000001ull
+
+// The L1-D cache can be flushed with mtspr 882,r0 (aka SPRN_TRIG2)
+#define SEC_FTR_L1D_FLUSH_TRIG2 0x0000000000000002ull
+
+// ori r31,r31,0 acts as a speculation barrier
+#define SEC_FTR_SPEC_BAR_ORI31 0x0000000000000004ull
+
+// Speculation past bctr is disabled
+#define SEC_FTR_BCCTRL_SERIALISED 0x0000000000000008ull
+
+// Entries in L1-D are private to a SMT thread
+#define SEC_FTR_L1D_THREAD_PRIV 0x0000000000000010ull
+
+// Indirect branch prediction cache disabled
+#define SEC_FTR_COUNT_CACHE_DISABLED 0x0000000000000020ull
+
+
+// Features indicating need for Spectre/Meltdown mitigations
+
+// The L1-D cache should be flushed on MSR[HV] 1->0 transition (hypervisor to guest)
+#define SEC_FTR_L1D_FLUSH_HV 0x0000000000000040ull
+
+// The L1-D cache should be flushed on MSR[PR] 0->1 transition (kernel to userspace)
+#define SEC_FTR_L1D_FLUSH_PR 0x0000000000000080ull
+
+// A speculation barrier should be used for bounds checks (Spectre variant 1)
+#define SEC_FTR_BNDS_CHK_SPEC_BAR 0x0000000000000100ull
+
+// Firmware configuration indicates user favours security over performance
+#define SEC_FTR_FAVOUR_SECURITY 0x0000000000000200ull
+
+#endif /* _ASM_POWERPC_SECURITY_FEATURES_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1b6bc7fba996..d458c45e5004 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -42,7 +42,7 @@ obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_ppc970.o cpu_setup_pa6t.o
-obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
+obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o security.o
obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
obj-$(CONFIG_PPC64) += vdso64/
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
new file mode 100644
index 000000000000..4ccba00d224c
--- /dev/null
+++ b/arch/powerpc/kernel/security.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Security related flags and so on.
+//
+// Copyright 2018, Michael Ellerman, IBM Corporation.
+
+#include <linux/kernel.h>
+#include <asm/security_features.h>
+
+
+unsigned long powerpc_security_features __read_mostly = \
+ SEC_FTR_L1D_FLUSH_HV | \
+ SEC_FTR_L1D_FLUSH_PR | \
+ SEC_FTR_BNDS_CHK_SPEC_BAR | \
+ SEC_FTR_FAVOUR_SECURITY;
--
2.14.1
^ permalink raw reply related
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 11:21 UTC (permalink / raw)
To: Will Deacon
Cc: Arnd Bergmann, Jason Gunthorpe, Sinan Kaya, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
peterz, mingo, corbet
In-Reply-To: <20180327095745.GB29373@arm.com>
On Tue, 2018-03-27 at 10:57 +0100, Will Deacon wrote:
> >
> > The interesting thing is that we do seem to have a whole LOT of these
> > spurrious wmb before writel all over the tree, I suspect because of
> > that incorrect recommendation in memory-barriers.txt.
> >
> > We should fix that.
>
> Patch below. Thoughts?
Looks good, we should probably also have a clearer (explicit)
definition of that ordering in the driver-api doco.
Now, to remove all those useless wmb's and find what other bugs they
were papering over ... ;-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
From: Joe Perches @ 2018-03-27 11:48 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel,
patches
In-Reply-To: <5ec8703b-2979-7a1d-fbc3-bd528d7a5e29@roeck-us.net>
On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
> On 03/27/2018 12:35 AM, Joe Perches wrote:
> > On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> > > Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
> >
> > fyi: It's marked Maintained in MAINTAINERS
> >
>
> I did not say that it was not maintained, and I am aware of the information
> in the MAINTAINERS file, thanks.
As am I.
I do agree that hwmon is obscure. Almost everything about
anything is obscure to those that don't fully understand it.
I am unaware though of any use of "obsolete" in drivers/hwmon/
or in Documentation/ about hwmon or the wiki/
In what sense do you mean hwmon is obsolete?
And the cocci script?
Anything later than:
https://systeme.lip6.fr/pipermail/cocci/2016-December/003887.html
https://github.com/groeck/coccinelle-patches/tree/master/hwmon
?
btw: I think the perl scripts I posted for DEV_ATTR_<RW|RO|WO>
transforms are simpler and less prone to tool versioning
issues.
https://lkml.org/lkml/2017/12/22/844
It'd be fairly simple to add SENSOR_ support.
^ 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