* Re: [RFC 1/5] seq_file: introduce seq_open_data helper
From: Andreas Dilger @ 2018-03-01 23:44 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Jonathan Corbet, Alexander Viro, Tony Luck, linux-ia64,
Michael Ellerman, linuxppc-dev, linux-doc, linux-kernel,
linux-fsdevel
In-Reply-To: <20180301233724.20440-1-linux@rasmusvillemoes.dk>
[-- Attachment #1: Type: text/plain, Size: 4152 bytes --]
On Mar 1, 2018, at 4:37 PM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
> There are quite a few callers of seq_open that could be simplified by
> setting the ->private member via the seq_open call instead of fetching
> file->private_data afterwards.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> I've just included a few examples of possible users of this helper,
> there are many more similar cases. As a bonus, the first two fix
> potential NULL derefs (if one believes that seq_open can actually
> fail).
>
> seq_open_private would have been a better name, but that one is
> already taken...
>
> Documentation/filesystems/seq_file.txt | 9 +++++----
> fs/seq_file.c | 9 ++++++++-
> include/linux/seq_file.h | 1 +
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
> index 9de4303201e1..68571b8275d8 100644
> --- a/Documentation/filesystems/seq_file.txt
> +++ b/Documentation/filesystems/seq_file.txt
> @@ -234,10 +234,11 @@ Here, the call to seq_open() takes the seq_operations structure we created
> before, and gets set up to iterate through the virtual file.
>
> On a successful open, seq_open() stores the struct seq_file pointer in
> -file->private_data. If you have an application where the same iterator can
> -be used for more than one file, you can store an arbitrary pointer in the
> -private field of the seq_file structure; that value can then be retrieved
> -by the iterator functions.
> +file->private_data. If you have an application where the same iterator
> +can be used for more than one file, you can store an arbitrary pointer
> +in the private field of the seq_file structure; that value can then be
> +retrieved by the iterator functions. Using the wrapper seq_open_data()
> +allows you to set the initial value for that field.
>
> There is also a wrapper function to seq_open() called seq_open_private(). It
> kmallocs a zero filled block of memory and stores a pointer to it in the
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index eea09f6d8830..f2145cb6e23d 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -45,7 +45,7 @@ static void *seq_buf_alloc(unsigned long size)
> * Note: seq_open() will allocate a struct seq_file and store its
> * pointer in @file->private_data. This pointer should not be modified.
> */
> -int seq_open(struct file *file, const struct seq_operations *op)
> +int seq_open_data(struct file *file, const struct seq_operations *op, void *data)
> {
> struct seq_file *p;
>
> @@ -59,6 +59,7 @@ int seq_open(struct file *file, const struct seq_operations *op)
>
> mutex_init(&p->lock);
> p->op = op;
> + p->private = data;
>
> // No refcounting: the lifetime of 'p' is constrained
> // to the lifetime of the file.
> @@ -85,6 +86,12 @@ int seq_open(struct file *file, const struct seq_operations *op)
> }
> EXPORT_SYMBOL(seq_open);
>
> +int seq_open(struct file *file, const struct seq_operations *op)
> +{
> + return seq_open_data(file, op, NULL);
> +}
> +EXPORT_SYMBOL(seq_open_data);
This is a bit confusing. You export "seq_open" after seq_open_data(),
and export "seq_open_data" here after seq_open(). Not strictly a bug,
but could become one in the future.
Cheers, Andreas
> +
> static int traverse(struct seq_file *m, loff_t offset)
> {
> loff_t pos = 0, index;
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index ab437dd2e3b9..f5ff376fa62b 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -107,6 +107,7 @@ void seq_pad(struct seq_file *m, char c);
>
> char *mangle_path(char *s, const char *p, const char *esc);
> int seq_open(struct file *, const struct seq_operations *);
> +int seq_open_data(struct file *, const struct seq_operations *, void *);
> ssize_t seq_read(struct file *, char __user *, size_t, loff_t *);
> loff_t seq_lseek(struct file *, loff_t, int);
> int seq_release(struct inode *, struct file *);
> --
> 2.15.1
>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 1/1] powerpc/pseries: fix EEH recovery of IOV devices
From: Bjorn Helgaas @ 2018-03-02 0:43 UTC (permalink / raw)
To: linuxppc-dev, linux-pci, mpe, bhelgaas, bryantly
In-Reply-To: <4bd50291799b920b42f8a12f5635c40ccae1302e.1519258921.git.sam.bobroff@au1.ibm.com>
On Thu, Feb 22, 2018 at 11:23:35AM +1100, Sam Bobroff wrote:
> Currently EEH recovery will fail on pSeries platforms for
> passed-through physical devices that support IOV, when CONFIG_PCI_IOV
> is set and the hypervisor doesn't provide a device tree node
> "ibm,open-sriov-vf-bar-info" for the device. (Found on an IOV capable
> device using the ipr driver.)
>
> EEH recovery fails in pci_enable_resources() at the check on
> r->parent, because r->flags is set and r->parent is not. This state
> is due to sriov_init() setting the start, end and flags members of the
> IOV BARs but the parent not being set later in
> pseries_pci_fixup_iov_resources(), because the
> "ibm,open-sriov-vf-bar-info" property is missing, causing it to bail
> out early.
>
> Correct this by zeroing the IOV resources in the bailout path, so that
> they are not seen by pci_enable_resources().
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> Hi,
>
> This is a fix to allow EEH recovery to succeed in a specific
> situation, which I've tried to explain in the commit message. But
> while I'm fairly sure of the situation leading up to the problem,
> I'm not sure how it should be fixed, which is why I've posted this
> as an RFC.
>
> Just zeroing out the flags is sufficient in my tests but it seemed
> cleaner to also clear start and end, but other changes would work as
> well: just clearing the flags, or just removing IORESOURCE_IO and
> IORESOURCE_MEM or even adding some new flag and also adjusting the
> test. Perhaps there's something else. I'm not sure what
> implications these choices have for the generic PCI code so I would
> appreciate feedback on it.
>
> Note that this problem doesn't seem to have been introduced by
> pseries_pci_fixup_iov_resources() (which was added recently): recovery
> already failed (in this way) before it's introduction.
>
> arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 372d7ada1a0c..019836ffe53d 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -618,13 +618,24 @@ static void pseries_pci_fixup_iov_resources(struct pci_dev *pdev)
> {
> const int *indexes;
> struct device_node *dn = pci_device_to_OF_node(pdev);
> + int i;
> + struct resource *r;
>
> if (!pdev->is_physfn || pdev->is_added)
> return;
> /*Firmware must support open sriov otherwise dont configure*/
> indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> - if (!indexes)
> + if (!indexes) {
> + /* Hide the BARs we can't configure, otherwise other
> + * code may see r->flags != 0 and r->parent == 0
> + * and raise an error. */
> + pci_warn(pdev, "No hypervisor support for SRIOV on this device, IOV BARs disabled.\n");
> + for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> + r = &pdev->resource[PCI_IOV_RESOURCES + i];
> + r->start = r->end = r->flags = 0;
> + }
> return;
Just to make sure I'm understanding the situation, "pdev" here is a PF
(not a VF), right? And if "ibm,open-sriov-vf-bar-info" existed,
of_pci_parse_iov_addrs() would fill in the pdev resources
corresponding to VF BAR0, VF BAR1, etc, in the PF's SR-IOV Capability?
The VF BARs are only used by the VFs, so I guess it should be safe to
enable the PF even if the VF BARs are not assigned, so I think this
patch is OK as far as it goes.
But I think you would also want to do something to prevent the VFs
from being enabled, i.e., we never want to set PCI_SRIOV_CTRL_VFE and
PCI_SRIOV_CTRL_MSE on this PF. That would cause VFs to decode memory
space that we have not configured and don't know about. I don't know
off the top of my head what the mechanism for that would be or if we
even have one.
> + }
> /* Assign the addresses from device tree*/
> of_pci_parse_iov_addrs(pdev, indexes);
> }
> --
> 2.16.1.74.g9b0b1f47b
>
^ permalink raw reply
* [PATCH v2] selftests/powerpc: Skip the subpage_prot tests if the syscall is unavailable
From: Michael Ellerman @ 2018-03-02 2:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
The subpage_prot syscall is only functional when the system is using
the Hash MMU. Since commit 5b2b80714796 ("powerpc/mm: Invalidate
subpage_prot() system call on radix platforms") it returns ENOENT when
the Radix MMU is active. Currently this just makes the test fail.
Additionally the syscall is not available if the kernel is built with
4K pages, or if CONFIG_PPC_SUBPAGE_PROT=n, in which case it returns
ENOSYS because the syscall is missing entirely.
So check explicitly for ENOENT and ENOSYS and skip if we see either of
those.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/mm/subpage_prot.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/powerpc/mm/subpage_prot.c b/tools/testing/selftests/powerpc/mm/subpage_prot.c
index 35ade7406dcd..3ae77ba93208 100644
--- a/tools/testing/selftests/powerpc/mm/subpage_prot.c
+++ b/tools/testing/selftests/powerpc/mm/subpage_prot.c
@@ -135,6 +135,16 @@ static int run_test(void *addr, unsigned long size)
return 0;
}
+static int syscall_available(void)
+{
+ int rc;
+
+ errno = 0;
+ rc = syscall(__NR_subpage_prot, 0, 0, 0);
+
+ return rc == 0 || (errno != ENOENT && errno != ENOSYS);
+}
+
int test_anon(void)
{
unsigned long align;
@@ -145,6 +155,8 @@ int test_anon(void)
void *mallocblock;
unsigned long mallocsize;
+ SKIP_IF(!syscall_available());
+
if (getpagesize() != 0x10000) {
fprintf(stderr, "Kernel page size must be 64K!\n");
return 1;
@@ -180,6 +192,8 @@ int test_file(void)
off_t filesize;
int fd;
+ SKIP_IF(!syscall_available());
+
fd = open(file_name, O_RDWR);
if (fd == -1) {
perror("failed to open file");
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v4] watchdog: add SPDX identifiers for watchdog subsystem
From: Joel Stanley @ 2018-03-02 3:39 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wim Van Sebroeck, Guenter Roeck, Nicolas Ferre, Alexandre Belloni,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Eric Anholt, Stefan Wahren,
Support Opensource, Baruch Siach, William Breathitt Gray,
Jimmy Vance, Keguang Zhang, Joachim Eastwood, Tomas Winkler,
Johannes Thumshirn, Andreas Werner, Carlo Caione, Kevin Hilman,
Matthias Brugger, Wan ZongShun, Michal Simek, Vladimir Zapolskiy,
Sylvain Lemieux, Kukjin Kim, Krzysztof Kozlowski, Zwane Mwaikambo,
Jim Cromie, Barry Song, Patrice Chotard, Maxime Ripard,
Chen-Yu Tsai, Marc Gonzalez, Mans Rullgard, Thierry Reding,
Jonathan Hunter, Masahiro Yamada, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Jun Nie, Baoyou Xie, Shawn Guo,
LINUXWATCHDOG, Linux Kernel Mailing List, Linux ARM,
linux-rpi-kernel, adi-buildroot-devel, linux-mips, linux-amlogic,
linux-mediatek, linux-samsung-soc, linux-tegra, linuxppc-dev,
patches
In-Reply-To: <20180228150209.2525-1-marcus.folkesson@gmail.com>
On Thu, Mar 1, 2018 at 1:31 AM, Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
> license
>
For this one:
> drivers/watchdog/aspeed_wdt.c | 5 +---
Acked-by: Joel Stanley <joel@jms.id.au>
Cheers,
Joel
^ permalink raw reply
* Re: Hotplug + Reboot is crashing HPT guest with HPT resizing enabled
From: David Gibson @ 2018-03-02 2:21 UTC (permalink / raw)
To: Bharata B Rao; +Cc: linuxppc-dev
In-Reply-To: <CAGZKiBoy5kZxijbGg0VaRr_HbgPyoHS-UnkXyhHt15sCpcT4ow@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]
On Fri, Feb 23, 2018 at 03:02:40PM +0530, Bharata B Rao wrote:
> Hi,
>
> Rebooting a hash guest after hotplugging memory to it is crashing the
> guest. This is seen only when HPT resizing is enabled. I see guest crashing
> at multiple places, but this location is fairly commonly seen:
>
> kernel BUG at mm/slub.c:3912!
>
> Testing with latest guest kernel and ppc-for-2.12 branch of QEMU.
Ugh. We had several bugs along these lines, but I thought I'd fixed
them. I wonder what this one is.
> A bit of debugging shows me that when memory is added, the guest kernel
> tries to resize HPT to a htab_shift value lesser than the value with which
> the guest has booted. For eg. a 8GB guest boots with htab_shift of 26. When
> 1G is hot-added,
> arch/powerpc/mm/hash_utils_64.c:resize_hpt_for_hotplug() ends up assigning
> 24 to target_hpt_shift. This looks suspicious as we are increasing the
> memory, but kernel is asking for shrinking the HPT size.
So the shrink-HPT-on-add-memory is actually expected and should be
harmless. It occurs because qemu estimates HPT size on the
traditional HPT == RAM size / 64 formular, which was devised with 4k
pages in mind. The kernel on the other hand, knows it is using 64k
pages and so estimates a smaller HPT size. Hot plugging memory always
prompts the guest to re-estimate the required HPT size, but if the
added memory is small enough, that size can still be smaller than
qemu's initial guess.
> HPT resizing
> requests fail though, but next reboot crashes the guest.
As noted the shrink is expected, so we need to debug the crash
separately. Do you have 9478956794c11239b7c1c3ef9ce95c883bb839a3 in
your tree?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v4.1 backport 00/15] powerpc stable backports for v4.1
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
Hi Sasha,
This is a backport to v4.1 of the RFI flush series that went upstream recently.
There's also a few other commits I noticed had not made it to v4.1.
cheers
^ permalink raw reply
* [PATCH v4.1 backport 01/15] powerpc/pseries: Add H_GET_CPU_CHARACTERISTICS flags & wrapper
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Michael Neuling <mikey@neuling.org>
commit 191eccb1580939fb0d47deb405b82a85b0379070 upstream.
A new hypervisor call has been defined to communicate various
characteristics of the CPU to guests. Add definitions for the hcall
number, flags and a wrapper function.
Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
[Balbir fixed conflicts in backport]
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/hvcall.h | 17 +++++++++++++++++
arch/powerpc/include/asm/plpar_wrappers.h | 14 ++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 85bc8c0d257b..51adbde09845 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -239,6 +239,7 @@
#define H_GET_HCA_INFO 0x1B8
#define H_GET_PERF_COUNT 0x1BC
#define H_MANAGE_TRACE 0x1C0
+#define H_GET_CPU_CHARACTERISTICS 0x1C8
#define H_FREE_LOGICAL_LAN_BUFFER 0x1D4
#define H_QUERY_INT_STATE 0x1E4
#define H_POLL_PENDING 0x1D8
@@ -285,6 +286,17 @@
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
+/* H_GET_CPU_CHARACTERISTICS return values */
+#define H_CPU_CHAR_SPEC_BAR_ORI31 (1ull << 63) // IBM bit 0
+#define H_CPU_CHAR_BCCTRL_SERIALISED (1ull << 62) // IBM bit 1
+#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_BEHAV_FAVOUR_SECURITY (1ull << 63) // IBM bit 0
+#define H_CPU_BEHAV_L1D_FLUSH_PR (1ull << 62) // IBM bit 1
+#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ull << 61) // IBM bit 2
+
#ifndef __ASSEMBLY__
/**
@@ -423,6 +435,11 @@ extern long pseries_big_endian_exceptions(void);
#endif /* CONFIG_PPC_PSERIES */
+struct h_cpu_char_result {
+ u64 character;
+ u64 behaviour;
+};
+
#endif /* __ASSEMBLY__ */
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_HVCALL_H */
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index 67859edbf8fd..6e05cb397a5c 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -323,4 +323,18 @@ static inline long plapr_set_watchpoint0(unsigned long dawr0, unsigned long dawr
return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR, dawr0, dawrx0);
}
+static inline long plpar_get_cpu_characteristics(struct h_cpu_char_result *p)
+{
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
+ long rc;
+
+ rc = plpar_hcall(H_GET_CPU_CHARACTERISTICS, retbuf);
+ if (rc == H_SUCCESS) {
+ p->character = retbuf[0];
+ p->behaviour = retbuf[1];
+ }
+
+ return rc;
+}
+
#endif /* _ASM_POWERPC_PLPAR_WRAPPERS_H */
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 03/15] powerpc/64s: Simple RFI macro conversions
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit 222f20f140623ef6033491d0103ee0875fe87d35 upstream.
This commit does simple conversions of rfi/rfid to the new macros that
include the expected destination context. By simple we mean cases
where there is a single well known destination context, and it's
simply a matter of substituting the instruction for the appropriate
macro.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
[Balbir fixed issues with backporting to stable]
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/exception-64s.h | 2 +-
arch/powerpc/kernel/entry_64.S | 11 ++++++++---
arch/powerpc/kernel/exceptions-64s.S | 18 +++++++++---------
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 7 +++----
arch/powerpc/kvm/book3s_rmhandlers.S | 7 +++++--
arch/powerpc/kvm/book3s_segment.S | 4 ++--
6 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index c8c8a81e3976..6dfb6e928f81 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -220,7 +220,7 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
mtspr SPRN_##h##SRR0,r12; \
mfspr r12,SPRN_##h##SRR1; /* and SRR1 */ \
mtspr SPRN_##h##SRR1,r10; \
- h##rfid; \
+ h##RFI_TO_KERNEL; \
b . /* prevent speculative execution */
#define EXCEPTION_PROLOG_PSERIES_1(label, h) \
__EXCEPTION_PROLOG_PSERIES_1(label, h)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index de276553cc79..e3bb07c78cee 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -34,6 +34,11 @@
#include <asm/ftrace.h>
#include <asm/hw_irq.h>
#include <asm/context_tracking.h>
+#ifdef CONFIG_PPC_BOOK3S
+#include <asm/exception-64s.h>
+#else
+#include <asm/exception-64e.h>
+#endif
/*
* System calls.
@@ -1030,7 +1035,7 @@ _GLOBAL(enter_rtas)
mtspr SPRN_SRR0,r5
mtspr SPRN_SRR1,r6
- rfid
+ RFI_TO_KERNEL
b . /* prevent speculative execution */
rtas_return_loc:
@@ -1055,7 +1060,7 @@ rtas_return_loc:
mtspr SPRN_SRR0,r3
mtspr SPRN_SRR1,r4
- rfid
+ RFI_TO_KERNEL
b . /* prevent speculative execution */
.align 3
@@ -1126,7 +1131,7 @@ _GLOBAL(enter_prom)
LOAD_REG_IMMEDIATE(r12, MSR_SF | MSR_ISF | MSR_LE)
andc r11,r11,r12
mtsrr1 r11
- rfid
+ RFI_TO_KERNEL
#endif /* CONFIG_PPC_BOOK3E */
1: /* Return from OF */
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 7662bfae0493..591efc03f80d 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -46,7 +46,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE) \
mtspr SPRN_SRR0,r10 ; \
ld r10,PACAKMSR(r13) ; \
mtspr SPRN_SRR1,r10 ; \
- rfid ; \
+ RFI_TO_KERNEL ; \
b . ; /* prevent speculative execution */
#define SYSCALL_PSERIES_3 \
@@ -54,7 +54,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE) \
1: mfspr r12,SPRN_SRR1 ; \
xori r12,r12,MSR_LE ; \
mtspr SPRN_SRR1,r12 ; \
- rfid ; /* return to userspace */ \
+ RFI_TO_USER ; /* return to userspace */ \
b . ; /* prevent speculative execution */
#if defined(CONFIG_RELOCATABLE)
@@ -508,7 +508,7 @@ BEGIN_FTR_SECTION
LOAD_HANDLER(r12, machine_check_handle_early)
1: mtspr SPRN_SRR0,r12
mtspr SPRN_SRR1,r11
- rfid
+ RFI_TO_KERNEL
b . /* prevent speculative execution */
2:
/* Stack overflow. Stay on emergency stack and panic.
@@ -602,7 +602,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
ld r11,PACA_EXGEN+EX_R11(r13)
ld r12,PACA_EXGEN+EX_R12(r13)
ld r13,PACA_EXGEN+EX_R13(r13)
- HRFID
+ HRFI_TO_UNKNOWN
b .
#endif
@@ -667,7 +667,7 @@ masked_##_H##interrupt: \
ld r10,PACA_EXGEN+EX_R10(r13); \
ld r11,PACA_EXGEN+EX_R11(r13); \
GET_SCRATCH0(r13); \
- ##_H##rfid; \
+ ##_H##RFI_TO_KERNEL; \
b .
MASKED_INTERRUPT()
@@ -757,7 +757,7 @@ kvmppc_skip_interrupt:
addi r13, r13, 4
mtspr SPRN_SRR0, r13
GET_SCRATCH0(r13)
- rfid
+ RFI_TO_KERNEL
b .
kvmppc_skip_Hinterrupt:
@@ -769,7 +769,7 @@ kvmppc_skip_Hinterrupt:
addi r13, r13, 4
mtspr SPRN_HSRR0, r13
GET_SCRATCH0(r13)
- hrfid
+ HRFI_TO_KERNEL
b .
#endif
@@ -1447,7 +1447,7 @@ machine_check_handle_early:
li r3,MSR_ME
andc r10,r10,r3 /* Turn off MSR_ME */
mtspr SPRN_SRR1,r10
- rfid
+ RFI_TO_KERNEL
b .
2:
/*
@@ -1465,7 +1465,7 @@ machine_check_handle_early:
*/
bl machine_check_queue_event
MACHINE_CHECK_HANDLER_WINDUP
- rfid
+ RFI_TO_USER_OR_KERNEL
9:
/* Deliver the machine check to host kernel in V mode. */
MACHINE_CHECK_HANDLER_WINDUP
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index a3018f109cd3..a7bd4100f158 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -64,7 +64,7 @@ _GLOBAL_TOC(kvmppc_hv_entry_trampoline)
mtmsrd r0,1 /* clear RI in MSR */
mtsrr0 r5
mtsrr1 r6
- RFI
+ RFI_TO_KERNEL
kvmppc_call_hv_entry:
ld r4, HSTATE_KVM_VCPU(r13)
@@ -164,7 +164,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mtsrr0 r8
mtsrr1 r7
beq cr1, 13f /* machine check */
- RFI
+ RFI_TO_KERNEL
/* On POWER7, we have external interrupts set to use HSRR0/1 */
11: mtspr SPRN_HSRR0, r8
@@ -877,8 +877,7 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ld r0, VCPU_GPR(R0)(r4)
ld r4, VCPU_GPR(R4)(r4)
-
- hrfid
+ HRFI_TO_GUEST
b .
secondary_too_late:
diff --git a/arch/powerpc/kvm/book3s_rmhandlers.S b/arch/powerpc/kvm/book3s_rmhandlers.S
index 16c4d88ba27d..a328f99a887c 100644
--- a/arch/powerpc/kvm/book3s_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_rmhandlers.S
@@ -46,6 +46,9 @@
#define FUNC(name) name
+#define RFI_TO_KERNEL RFI
+#define RFI_TO_GUEST RFI
+
.macro INTERRUPT_TRAMPOLINE intno
.global kvmppc_trampoline_\intno
@@ -141,7 +144,7 @@ kvmppc_handler_skip_ins:
GET_SCRATCH0(r13)
/* And get back into the code */
- RFI
+ RFI_TO_KERNEL
#endif
/*
@@ -164,6 +167,6 @@ _GLOBAL_TOC(kvmppc_entry_trampoline)
ori r5, r5, MSR_EE
mtsrr0 r7
mtsrr1 r6
- RFI
+ RFI_TO_KERNEL
#include "book3s_segment.S"
diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
index acee37cde840..af3a91c00b46 100644
--- a/arch/powerpc/kvm/book3s_segment.S
+++ b/arch/powerpc/kvm/book3s_segment.S
@@ -156,7 +156,7 @@ no_dcbz32_on:
PPC_LL r9, SVCPU_R9(r3)
PPC_LL r3, (SVCPU_R3)(r3)
- RFI
+ RFI_TO_GUEST
kvmppc_handler_trampoline_enter_end:
@@ -389,5 +389,5 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
cmpwi r12, BOOK3S_INTERRUPT_DOORBELL
beqa BOOK3S_INTERRUPT_DOORBELL
- RFI
+ RFI_TO_KERNEL
kvmppc_handler_trampoline_exit_end:
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 02/15] powerpc/64: Add macros for annotating the destination of rfid/hrfid
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit 50e51c13b3822d14ff6df4279423e4b7b2269bc3 upstream.
The rfid/hrfid ((Hypervisor) Return From Interrupt) instruction is
used for switching from the kernel to userspace, and from the
hypervisor to the guest kernel. However it can and is also used for
other transitions, eg. from real mode kernel code to virtual mode
kernel code, and it's not always clear from the code what the
destination context is.
To make it clearer when reading the code, add macros which encode the
expected destination context.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/exception-64e.h | 6 ++++++
arch/powerpc/include/asm/exception-64s.h | 29 +++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/arch/powerpc/include/asm/exception-64e.h b/arch/powerpc/include/asm/exception-64e.h
index a8b52b61043f..bd8958445336 100644
--- a/arch/powerpc/include/asm/exception-64e.h
+++ b/arch/powerpc/include/asm/exception-64e.h
@@ -208,5 +208,11 @@ exc_##label##_book3e:
ori r3,r3,interrupt_base_book3e@l; \
mtspr SPRN_IVOR##vector_number,r3;
+#define RFI_TO_KERNEL \
+ rfi
+
+#define RFI_TO_USER \
+ rfi
+
#endif /* _ASM_POWERPC_EXCEPTION_64E_H */
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 77f52b26dad6..c8c8a81e3976 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -50,6 +50,35 @@
#define EX_PPR 88 /* SMT thread status register (priority) */
#define EX_CTR 96
+/* Macros for annotating the expected destination of (h)rfid */
+
+#define RFI_TO_KERNEL \
+ rfid
+
+#define RFI_TO_USER \
+ rfid
+
+#define RFI_TO_USER_OR_KERNEL \
+ rfid
+
+#define RFI_TO_GUEST \
+ rfid
+
+#define HRFI_TO_KERNEL \
+ hrfid
+
+#define HRFI_TO_USER \
+ hrfid
+
+#define HRFI_TO_USER_OR_KERNEL \
+ hrfid
+
+#define HRFI_TO_GUEST \
+ hrfid
+
+#define HRFI_TO_UNKNOWN \
+ hrfid
+
#ifdef CONFIG_RELOCATABLE
#define __EXCEPTION_RELON_PROLOG_PSERIES_1(label, h) \
ld r12,PACAKBASE(r13); /* get high part of &label */ \
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 04/15] powerpc/64: Convert fast_exception_return to use RFI_TO_USER/KERNEL
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit a08f828cf47e6c605af21d2cdec68f84e799c318 upstream.
Similar to the syscall return path, in fast_exception_return we may be
returning to user or kernel context. We already have a test for that,
because we conditionally restore r13. So use that existing test and
branch, and bifurcate the return based on that.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kernel/entry_64.S | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index e3bb07c78cee..f7421248cbd8 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -845,7 +845,7 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ACCOUNT_CPU_USER_EXIT(r2, r4)
REST_GPR(13, r1)
-1:
+
mtspr SPRN_SRR1,r3
ld r2,_CCR(r1)
@@ -858,8 +858,22 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ld r3,GPR3(r1)
ld r4,GPR4(r1)
ld r1,GPR1(r1)
+ RFI_TO_USER
+ b . /* prevent speculative execution */
+
+1: mtspr SPRN_SRR1,r3
- rfid
+ ld r2,_CCR(r1)
+ mtcrf 0xFF,r2
+ ld r2,_NIP(r1)
+ mtspr SPRN_SRR0,r2
+
+ ld r0,GPR0(r1)
+ ld r2,GPR2(r1)
+ ld r3,GPR3(r1)
+ ld r4,GPR4(r1)
+ ld r1,GPR1(r1)
+ RFI_TO_KERNEL
b . /* prevent speculative execution */
#endif /* CONFIG_PPC_BOOK3E */
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 08/15] powerpc/64s: Support disabling RFI flush with no_rfi_flush and nopti
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
commit bc9c9304a45480797e13a8e1df96ffcf44fb62fe upstream.
Because there may be some performance overhead of the RFI flush, add
kernel command line options to disable it.
We add a sensibly named 'no_rfi_flush' option, but we also hijack the
x86 option 'nopti'. The RFI flush is not the same as KPTI, but if we
see 'nopti' we can guess that the user is trying to avoid any overhead
of Meltdown mitigations, and it means we don't have to educate every
one about a different command line option.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kernel/setup_64.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e76bcacaf150..a3993ac21b50 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -811,8 +811,29 @@ early_initcall(disable_hardlockup_detector);
#ifdef CONFIG_PPC_BOOK3S_64
static enum l1d_flush_type enabled_flush_types;
static void *l1d_flush_fallback_area;
+static bool no_rfi_flush;
bool rfi_flush;
+static int __init handle_no_rfi_flush(char *p)
+{
+ pr_info("rfi-flush: disabled on command line.");
+ no_rfi_flush = true;
+ return 0;
+}
+early_param("no_rfi_flush", handle_no_rfi_flush);
+
+/*
+ * The RFI flush is not KPTI, but because users will see doco that says to use
+ * nopti we hijack that option here to also disable the RFI flush.
+ */
+static int __init handle_no_pti(char *p)
+{
+ pr_info("rfi-flush: disabling due to 'nopti' on command line.\n");
+ handle_no_rfi_flush(NULL);
+ return 0;
+}
+early_param("nopti", handle_no_pti);
+
static void do_nothing(void *unused)
{
/*
@@ -883,7 +904,8 @@ void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
enabled_flush_types = types;
- rfi_flush_enable(enable);
+ if (!no_rfi_flush)
+ rfi_flush_enable(enable);
}
#endif /* CONFIG_PPC_BOOK3S_64 */
#endif
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 05/15] powerpc/64: Convert the syscall exit path to use RFI_TO_USER/KERNEL
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit b8e90cb7bc04a509e821e82ab6ed7a8ef11ba333 upstream.
In the syscall exit path we may be returning to user or kernel
context. We already have a test for that, because we conditionally
restore r13. So use that existing test and branch, and bifurcate the
return based on that.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kernel/entry_64.S | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index f7421248cbd8..36a8bf3d053b 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -223,13 +223,23 @@ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
ACCOUNT_CPU_USER_EXIT(r11, r12)
HMT_MEDIUM_LOW_HAS_PPR
ld r13,GPR13(r1) /* only restore r13 if returning to usermode */
+ ld r2,GPR2(r1)
+ ld r1,GPR1(r1)
+ mtlr r4
+ mtcr r5
+ mtspr SPRN_SRR0,r7
+ mtspr SPRN_SRR1,r8
+ RFI_TO_USER
+ b . /* prevent speculative execution */
+
+ /* exit to kernel */
1: ld r2,GPR2(r1)
ld r1,GPR1(r1)
mtlr r4
mtcr r5
mtspr SPRN_SRR0,r7
mtspr SPRN_SRR1,r8
- RFI
+ RFI_TO_KERNEL
b . /* prevent speculative execution */
syscall_error:
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 07/15] powerpc/64s: Add support for RFI flush of L1-D cache
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
commit aa8a5e0062ac940f7659394f4817c948dc8c0667 upstream.
On some CPUs we can prevent the Meltdown vulnerability by flushing the
L1-D cache on exit from kernel to user mode, and from hypervisor to
guest.
This is known to be the case on at least Power7, Power8 and Power9. At
this time we do not know the status of the vulnerability on other CPUs
such as the 970 (Apple G5), pasemi CPUs (AmigaOne X1000) or Freescale
CPUs. As more information comes to light we can enable this, or other
mechanisms on those CPUs.
The vulnerability occurs when the load of an architecturally
inaccessible memory region (eg. userspace load of kernel memory) is
speculatively executed to the point where its result can influence the
address of a subsequent speculatively executed load.
In order for that to happen, the first load must hit in the L1,
because before the load is sent to the L2 the permission check is
performed. Therefore if no kernel addresses hit in the L1 the
vulnerability can not occur. We can ensure that is the case by
flushing the L1 whenever we return to userspace. Similarly for
hypervisor vs guest.
In order to flush the L1-D cache on exit, we add a section of nops at
each (h)rfi location that returns to a lower privileged context, and
patch that with some sequence. Newer firmwares are able to advertise
to us that there is a special nop instruction that flushes the L1-D.
If we do not see that advertised, we fall back to doing a displacement
flush in software.
For guest kernels we support migration between some CPU versions, and
different CPUs may use different flush instructions. So that we are
prepared to migrate to a machine with a different flush instruction
activated, we may have to patch more than one flush instruction at
boot if the hypervisor tells us to.
In the end this patch is mostly the work of Nicholas Piggin and
Michael Ellerman. However a cast of thousands contributed to analysis
of the issue, earlier versions of the patch, back ports testing etc.
Many thanks to all of them.
Tested-by: Jon Masters <jcm@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
[Balbir - back ported to stable with changes]
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/exception-64s.h | 40 +++++++++++---
arch/powerpc/include/asm/feature-fixups.h | 15 ++++++
arch/powerpc/include/asm/paca.h | 10 ++++
arch/powerpc/include/asm/setup.h | 13 +++++
arch/powerpc/kernel/asm-offsets.c | 4 ++
arch/powerpc/kernel/exceptions-64s.S | 86 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/setup_64.c | 79 ++++++++++++++++++++++++++++
arch/powerpc/kernel/vmlinux.lds.S | 9 ++++
arch/powerpc/lib/feature-fixups.c | 42 +++++++++++++++
9 files changed, 290 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 6dfb6e928f81..9bddbec441b8 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -50,34 +50,58 @@
#define EX_PPR 88 /* SMT thread status register (priority) */
#define EX_CTR 96
-/* Macros for annotating the expected destination of (h)rfid */
+/*
+ * Macros for annotating the expected destination of (h)rfid
+ *
+ * The nop instructions allow us to insert one or more instructions to flush the
+ * L1-D cache when returning to userspace or a guest.
+ */
+#define RFI_FLUSH_SLOT \
+ RFI_FLUSH_FIXUP_SECTION; \
+ nop; \
+ nop; \
+ nop
#define RFI_TO_KERNEL \
rfid
#define RFI_TO_USER \
- rfid
+ RFI_FLUSH_SLOT; \
+ rfid; \
+ b rfi_flush_fallback
#define RFI_TO_USER_OR_KERNEL \
- rfid
+ RFI_FLUSH_SLOT; \
+ rfid; \
+ b rfi_flush_fallback
#define RFI_TO_GUEST \
- rfid
+ RFI_FLUSH_SLOT; \
+ rfid; \
+ b rfi_flush_fallback
#define HRFI_TO_KERNEL \
hrfid
#define HRFI_TO_USER \
- hrfid
+ RFI_FLUSH_SLOT; \
+ hrfid; \
+ b hrfi_flush_fallback
#define HRFI_TO_USER_OR_KERNEL \
- hrfid
+ RFI_FLUSH_SLOT; \
+ hrfid; \
+ b hrfi_flush_fallback
#define HRFI_TO_GUEST \
- hrfid
+ RFI_FLUSH_SLOT; \
+ hrfid; \
+ b hrfi_flush_fallback
#define HRFI_TO_UNKNOWN \
- hrfid
+ RFI_FLUSH_SLOT; \
+ hrfid; \
+ b hrfi_flush_fallback
#ifdef CONFIG_RELOCATABLE
#define __EXCEPTION_RELON_PROLOG_PSERIES_1(label, h) \
diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index 9a67a38bf7b9..7068bafbb2d6 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -184,4 +184,19 @@ label##3: \
FTR_ENTRY_OFFSET label##1b-label##3b; \
.popsection;
+#define RFI_FLUSH_FIXUP_SECTION \
+951: \
+ .pushsection __rfi_flush_fixup,"a"; \
+ .align 2; \
+952: \
+ FTR_ENTRY_OFFSET 951b-952b; \
+ .popsection;
+
+
+#ifndef __ASSEMBLY__
+
+extern long __start___rfi_flush_fixup, __stop___rfi_flush_fixup;
+
+#endif
+
#endif /* __ASM_POWERPC_FEATURE_FIXUPS_H */
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 70bd4381f8e6..45e2aefece16 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -192,6 +192,16 @@ struct paca_struct {
#endif
struct kvmppc_host_state kvm_hstate;
#endif
+#ifdef CONFIG_PPC_BOOK3S_64
+ /*
+ * rfi fallback flush must be in its own cacheline to prevent
+ * other paca data leaking into the L1d
+ */
+ u64 exrfi[13] __aligned(0x80);
+ void *rfi_flush_fallback_area;
+ u64 l1d_flush_congruence;
+ u64 l1d_flush_sets;
+#endif
};
extern struct paca_struct *paca;
diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index e9d384cbd021..7916b56f2e60 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -26,6 +26,19 @@ void initmem_init(void);
void setup_panic(void);
#define ARCH_PANIC_TIMEOUT 180
+void rfi_flush_enable(bool enable);
+
+/* These are bit flags */
+enum l1d_flush_type {
+ L1D_FLUSH_NONE = 0x1,
+ L1D_FLUSH_FALLBACK = 0x2,
+ L1D_FLUSH_ORI = 0x4,
+ L1D_FLUSH_MTTRIG = 0x8,
+};
+
+void __init setup_rfi_flush(enum l1d_flush_type, bool enable);
+void do_rfi_flush_fixups(enum l1d_flush_type types);
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_POWERPC_SETUP_H */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index d8d332e65078..69761d8ad871 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -244,6 +244,10 @@ int main(void)
#ifdef CONFIG_PPC_BOOK3S_64
DEFINE(PACAMCEMERGSP, offsetof(struct paca_struct, mc_emergency_sp));
DEFINE(PACA_IN_MCE, offsetof(struct paca_struct, in_mce));
+ DEFINE(PACA_RFI_FLUSH_FALLBACK_AREA, offsetof(struct paca_struct, rfi_flush_fallback_area));
+ DEFINE(PACA_EXRFI, offsetof(struct paca_struct, exrfi));
+ DEFINE(PACA_L1D_FLUSH_CONGRUENCE, offsetof(struct paca_struct, l1d_flush_congruence));
+ DEFINE(PACA_L1D_FLUSH_SETS, offsetof(struct paca_struct, l1d_flush_sets));
#endif
DEFINE(PACAHWCPUID, offsetof(struct paca_struct, hw_cpu_id));
DEFINE(PACAKEXECSTATE, offsetof(struct paca_struct, kexec_state));
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index c7dfd67c14da..9c102be8cd97 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1572,6 +1572,92 @@ power4_fixup_nap:
blr
#endif
+ .globl rfi_flush_fallback
+rfi_flush_fallback:
+ SET_SCRATCH0(r13);
+ GET_PACA(r13);
+ std r9,PACA_EXRFI+EX_R9(r13)
+ std r10,PACA_EXRFI+EX_R10(r13)
+ std r11,PACA_EXRFI+EX_R11(r13)
+ std r12,PACA_EXRFI+EX_R12(r13)
+ std r8,PACA_EXRFI+EX_R13(r13)
+ mfctr r9
+ ld r10,PACA_RFI_FLUSH_FALLBACK_AREA(r13)
+ ld r11,PACA_L1D_FLUSH_SETS(r13)
+ ld r12,PACA_L1D_FLUSH_CONGRUENCE(r13)
+ /*
+ * The load adresses are at staggered offsets within cachelines,
+ * which suits some pipelines better (on others it should not
+ * hurt).
+ */
+ addi r12,r12,8
+ mtctr r11
+ DCBT_STOP_ALL_STREAM_IDS(r11) /* Stop prefetch streams */
+
+ /* order ld/st prior to dcbt stop all streams with flushing */
+ sync
+1: li r8,0
+ .rept 8 /* 8-way set associative */
+ ldx r11,r10,r8
+ add r8,r8,r12
+ xor r11,r11,r11 // Ensure r11 is 0 even if fallback area is not
+ add r8,r8,r11 // Add 0, this creates a dependency on the ldx
+ .endr
+ addi r10,r10,128 /* 128 byte cache line */
+ bdnz 1b
+
+ mtctr r9
+ ld r9,PACA_EXRFI+EX_R9(r13)
+ ld r10,PACA_EXRFI+EX_R10(r13)
+ ld r11,PACA_EXRFI+EX_R11(r13)
+ ld r12,PACA_EXRFI+EX_R12(r13)
+ ld r8,PACA_EXRFI+EX_R13(r13)
+ GET_SCRATCH0(r13);
+ rfid
+
+ .globl hrfi_flush_fallback
+hrfi_flush_fallback:
+ SET_SCRATCH0(r13);
+ GET_PACA(r13);
+ std r9,PACA_EXRFI+EX_R9(r13)
+ std r10,PACA_EXRFI+EX_R10(r13)
+ std r11,PACA_EXRFI+EX_R11(r13)
+ std r12,PACA_EXRFI+EX_R12(r13)
+ std r8,PACA_EXRFI+EX_R13(r13)
+ mfctr r9
+ ld r10,PACA_RFI_FLUSH_FALLBACK_AREA(r13)
+ ld r11,PACA_L1D_FLUSH_SETS(r13)
+ ld r12,PACA_L1D_FLUSH_CONGRUENCE(r13)
+ /*
+ * The load adresses are at staggered offsets within cachelines,
+ * which suits some pipelines better (on others it should not
+ * hurt).
+ */
+ addi r12,r12,8
+ mtctr r11
+ DCBT_STOP_ALL_STREAM_IDS(r11) /* Stop prefetch streams */
+
+ /* order ld/st prior to dcbt stop all streams with flushing */
+ sync
+1: li r8,0
+ .rept 8 /* 8-way set associative */
+ ldx r11,r10,r8
+ add r8,r8,r12
+ xor r11,r11,r11 // Ensure r11 is 0 even if fallback area is not
+ add r8,r8,r11 // Add 0, this creates a dependency on the ldx
+ .endr
+ addi r10,r10,128 /* 128 byte cache line */
+ bdnz 1b
+
+ mtctr r9
+ ld r9,PACA_EXRFI+EX_R9(r13)
+ ld r10,PACA_EXRFI+EX_R10(r13)
+ ld r11,PACA_EXRFI+EX_R11(r13)
+ ld r12,PACA_EXRFI+EX_R12(r13)
+ ld r8,PACA_EXRFI+EX_R13(r13)
+ GET_SCRATCH0(r13);
+ hrfid
+
/*
* Hash table stuff
*/
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 8161d66830a2..e76bcacaf150 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -807,4 +807,83 @@ static int __init disable_hardlockup_detector(void)
return 0;
}
early_initcall(disable_hardlockup_detector);
+
+#ifdef CONFIG_PPC_BOOK3S_64
+static enum l1d_flush_type enabled_flush_types;
+static void *l1d_flush_fallback_area;
+bool rfi_flush;
+
+static void do_nothing(void *unused)
+{
+ /*
+ * We don't need to do the flush explicitly, just enter+exit kernel is
+ * sufficient, the RFI exit handlers will do the right thing.
+ */
+}
+
+void rfi_flush_enable(bool enable)
+{
+ if (rfi_flush == enable)
+ return;
+
+ if (enable) {
+ do_rfi_flush_fixups(enabled_flush_types);
+ on_each_cpu(do_nothing, NULL, 1);
+ } else
+ do_rfi_flush_fixups(L1D_FLUSH_NONE);
+
+ rfi_flush = enable;
+}
+
+static void init_fallback_flush(void)
+{
+ u64 l1d_size, limit;
+ int cpu;
+
+ l1d_size = ppc64_caches.dsize;
+ limit = min(safe_stack_limit(), ppc64_rma_size);
+
+ /*
+ * Align to L1d size, and size it at 2x L1d size, to catch possible
+ * hardware prefetch runoff. We don't have a recipe for load patterns to
+ * reliably avoid the prefetcher.
+ */
+ l1d_flush_fallback_area = __va(memblock_alloc_base(l1d_size * 2, l1d_size, limit));
+ memset(l1d_flush_fallback_area, 0, l1d_size * 2);
+
+ for_each_possible_cpu(cpu) {
+ /*
+ * The fallback flush is currently coded for 8-way
+ * associativity. Different associativity is possible, but it
+ * will be treated as 8-way and may not evict the lines as
+ * effectively.
+ *
+ * 128 byte lines are mandatory.
+ */
+ u64 c = l1d_size / 8;
+
+ paca[cpu].rfi_flush_fallback_area = l1d_flush_fallback_area;
+ paca[cpu].l1d_flush_congruence = c;
+ paca[cpu].l1d_flush_sets = c / 128;
+ }
+}
+
+void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
+{
+ if (types & L1D_FLUSH_FALLBACK) {
+ pr_info("rfi-flush: Using fallback displacement flush\n");
+ init_fallback_flush();
+ }
+
+ if (types & L1D_FLUSH_ORI)
+ pr_info("rfi-flush: Using ori type flush\n");
+
+ if (types & L1D_FLUSH_MTTRIG)
+ pr_info("rfi-flush: Using mttrig type flush\n");
+
+ enabled_flush_types = types;
+
+ rfi_flush_enable(enable);
+}
+#endif /* CONFIG_PPC_BOOK3S_64 */
#endif
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 1db685104ffc..b542a80477c8 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -72,6 +72,15 @@ SECTIONS
/* Read-only data */
RODATA
+#ifdef CONFIG_PPC64
+ . = ALIGN(8);
+ __rfi_flush_fixup : AT(ADDR(__rfi_flush_fixup) - LOAD_OFFSET) {
+ __start___rfi_flush_fixup = .;
+ *(__rfi_flush_fixup)
+ __stop___rfi_flush_fixup = .;
+ }
+#endif
+
EXCEPTION_TABLE(0)
NOTES :kernel :notes
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 7ce3870d7ddd..a18d648d31a6 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -20,6 +20,7 @@
#include <asm/code-patching.h>
#include <asm/page.h>
#include <asm/sections.h>
+#include <asm/setup.h>
struct fixup_entry {
@@ -113,6 +114,47 @@ void do_feature_fixups(unsigned long value, void *fixup_start, void *fixup_end)
}
}
+#ifdef CONFIG_PPC_BOOK3S_64
+void do_rfi_flush_fixups(enum l1d_flush_type types)
+{
+ unsigned int instrs[3], *dest;
+ long *start, *end;
+ int i;
+
+ start = PTRRELOC(&__start___rfi_flush_fixup),
+ end = PTRRELOC(&__stop___rfi_flush_fixup);
+
+ instrs[0] = 0x60000000; /* nop */
+ instrs[1] = 0x60000000; /* nop */
+ instrs[2] = 0x60000000; /* nop */
+
+ if (types & L1D_FLUSH_FALLBACK)
+ /* b .+16 to fallback flush */
+ instrs[0] = 0x48000010;
+
+ i = 0;
+ if (types & L1D_FLUSH_ORI) {
+ instrs[i++] = 0x63ff0000; /* ori 31,31,0 speculation barrier */
+ instrs[i++] = 0x63de0000; /* ori 30,30,0 L1d flush*/
+ }
+
+ if (types & L1D_FLUSH_MTTRIG)
+ instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+
+ for (i = 0; start < end; start++, i++) {
+ dest = (void *)start + *start;
+
+ pr_devel("patching dest %lx\n", (unsigned long)dest);
+
+ patch_instruction(dest, instrs[0]);
+ patch_instruction(dest + 1, instrs[1]);
+ patch_instruction(dest + 2, instrs[2]);
+ }
+
+ printk(KERN_DEBUG "rfi-flush: patched %d locations\n", i);
+}
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
void do_lwsync_fixups(unsigned long value, void *fixup_start, void *fixup_end)
{
long *start, *end;
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 06/15] powerpc/64s: Convert slb_miss_common to use RFI_TO_USER/KERNEL
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit c7305645eb0c1621351cfc104038831ae87c0053 upstream.
In the SLB miss handler we may be returning to user or kernel. We need
to add a check early on and save the result in the cr4 register, and
then we bifurcate the return path based on that.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[mpe: Backport to 4.4 based on patch from Balbir]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kernel/exceptions-64s.S | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 591efc03f80d..c7dfd67c14da 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1511,6 +1511,8 @@ slb_miss_realmode:
andi. r10,r12,MSR_RI /* check for unrecoverable exception */
beq- 2f
+ andi. r10,r12,MSR_PR /* check for user mode (PR != 0) */
+ bne 1f
.machine push
.machine "power4"
@@ -1524,7 +1526,23 @@ slb_miss_realmode:
ld r11,PACA_EXSLB+EX_R11(r13)
ld r12,PACA_EXSLB+EX_R12(r13)
ld r13,PACA_EXSLB+EX_R13(r13)
- rfid
+ RFI_TO_KERNEL
+ b . /* prevent speculative execution */
+
+1:
+.machine push
+.machine "power4"
+ mtcrf 0x80,r9
+ mtcrf 0x01,r9 /* slb_allocate uses cr0 and cr7 */
+.machine pop
+
+ RESTORE_PPR_PACA(PACA_EXSLB, r9)
+ ld r9,PACA_EXSLB+EX_R9(r13)
+ ld r10,PACA_EXSLB+EX_R10(r13)
+ ld r11,PACA_EXSLB+EX_R11(r13)
+ ld r12,PACA_EXSLB+EX_R12(r13)
+ ld r13,PACA_EXSLB+EX_R13(r13)
+ RFI_TO_USER
b . /* prevent speculative execution */
2: mfspr r11,SPRN_SRR0
@@ -1533,7 +1551,7 @@ slb_miss_realmode:
mtspr SPRN_SRR0,r10
ld r10,PACAKMSR(r13)
mtspr SPRN_SRR1,r10
- rfid
+ RFI_TO_KERNEL
b .
unrecov_slb:
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 09/15] powerpc/64s: Improve RFI L1-D cache flush fallback
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Nicholas Piggin <npiggin@gmail.com>
commit bfcd89a1d61a4a6bca3319e2ab70d7c745baf823 upstream.
The fallback RFI flush is used when firmware does not provide a way
to flush the cache. It's a "displacement flush" that evicts useful
data by displacing it with an uninteresting buffer.
The flush has to take care to work with implementation specific cache
replacment policies, so the recipe has been in flux. The initial
slow but conservative approach is to touch all lines of a congruence
class, with dependencies between each load. It has since been
determined that a linear pattern of loads without dependencies is
sufficient, and is significantly faster.
Measuring the speed of a null syscall with RFI fallback flush enabled
gives the relative improvement:
P8 - 1.83x
P9 - 1.75x
The flush also becomes simpler and more adaptable to different cache
geometries.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/paca.h | 3 +-
arch/powerpc/kernel/asm-offsets.c | 3 +-
arch/powerpc/kernel/exceptions-64s.S | 76 +++++++++++++++++-------------------
arch/powerpc/kernel/setup_64.c | 13 +-----
4 files changed, 39 insertions(+), 56 deletions(-)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 45e2aefece16..08e5df3395fa 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -199,8 +199,7 @@ struct paca_struct {
*/
u64 exrfi[13] __aligned(0x80);
void *rfi_flush_fallback_area;
- u64 l1d_flush_congruence;
- u64 l1d_flush_sets;
+ u64 l1d_flush_size;
#endif
};
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 69761d8ad871..23fe603a98d3 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -246,8 +246,7 @@ int main(void)
DEFINE(PACA_IN_MCE, offsetof(struct paca_struct, in_mce));
DEFINE(PACA_RFI_FLUSH_FALLBACK_AREA, offsetof(struct paca_struct, rfi_flush_fallback_area));
DEFINE(PACA_EXRFI, offsetof(struct paca_struct, exrfi));
- DEFINE(PACA_L1D_FLUSH_CONGRUENCE, offsetof(struct paca_struct, l1d_flush_congruence));
- DEFINE(PACA_L1D_FLUSH_SETS, offsetof(struct paca_struct, l1d_flush_sets));
+ DEFINE(PACA_L1D_FLUSH_SIZE, offsetof(struct paca_struct, l1d_flush_size));
#endif
DEFINE(PACAHWCPUID, offsetof(struct paca_struct, hw_cpu_id));
DEFINE(PACAKEXECSTATE, offsetof(struct paca_struct, kexec_state));
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 9c102be8cd97..3b8991df5101 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1579,39 +1579,37 @@ rfi_flush_fallback:
std r9,PACA_EXRFI+EX_R9(r13)
std r10,PACA_EXRFI+EX_R10(r13)
std r11,PACA_EXRFI+EX_R11(r13)
- std r12,PACA_EXRFI+EX_R12(r13)
- std r8,PACA_EXRFI+EX_R13(r13)
mfctr r9
ld r10,PACA_RFI_FLUSH_FALLBACK_AREA(r13)
- ld r11,PACA_L1D_FLUSH_SETS(r13)
- ld r12,PACA_L1D_FLUSH_CONGRUENCE(r13)
- /*
- * The load adresses are at staggered offsets within cachelines,
- * which suits some pipelines better (on others it should not
- * hurt).
- */
- addi r12,r12,8
+ ld r11,PACA_L1D_FLUSH_SIZE(r13)
+ srdi r11,r11,(7 + 3) /* 128 byte lines, unrolled 8x */
mtctr r11
DCBT_STOP_ALL_STREAM_IDS(r11) /* Stop prefetch streams */
/* order ld/st prior to dcbt stop all streams with flushing */
sync
-1: li r8,0
- .rept 8 /* 8-way set associative */
- ldx r11,r10,r8
- add r8,r8,r12
- xor r11,r11,r11 // Ensure r11 is 0 even if fallback area is not
- add r8,r8,r11 // Add 0, this creates a dependency on the ldx
- .endr
- addi r10,r10,128 /* 128 byte cache line */
+
+ /*
+ * The load adresses are at staggered offsets within cachelines,
+ * which suits some pipelines better (on others it should not
+ * hurt).
+ */
+1:
+ ld r11,(0x80 + 8)*0(r10)
+ ld r11,(0x80 + 8)*1(r10)
+ ld r11,(0x80 + 8)*2(r10)
+ ld r11,(0x80 + 8)*3(r10)
+ ld r11,(0x80 + 8)*4(r10)
+ ld r11,(0x80 + 8)*5(r10)
+ ld r11,(0x80 + 8)*6(r10)
+ ld r11,(0x80 + 8)*7(r10)
+ addi r10,r10,0x80*8
bdnz 1b
mtctr r9
ld r9,PACA_EXRFI+EX_R9(r13)
ld r10,PACA_EXRFI+EX_R10(r13)
ld r11,PACA_EXRFI+EX_R11(r13)
- ld r12,PACA_EXRFI+EX_R12(r13)
- ld r8,PACA_EXRFI+EX_R13(r13)
GET_SCRATCH0(r13);
rfid
@@ -1622,39 +1620,37 @@ hrfi_flush_fallback:
std r9,PACA_EXRFI+EX_R9(r13)
std r10,PACA_EXRFI+EX_R10(r13)
std r11,PACA_EXRFI+EX_R11(r13)
- std r12,PACA_EXRFI+EX_R12(r13)
- std r8,PACA_EXRFI+EX_R13(r13)
mfctr r9
ld r10,PACA_RFI_FLUSH_FALLBACK_AREA(r13)
- ld r11,PACA_L1D_FLUSH_SETS(r13)
- ld r12,PACA_L1D_FLUSH_CONGRUENCE(r13)
- /*
- * The load adresses are at staggered offsets within cachelines,
- * which suits some pipelines better (on others it should not
- * hurt).
- */
- addi r12,r12,8
+ ld r11,PACA_L1D_FLUSH_SIZE(r13)
+ srdi r11,r11,(7 + 3) /* 128 byte lines, unrolled 8x */
mtctr r11
DCBT_STOP_ALL_STREAM_IDS(r11) /* Stop prefetch streams */
/* order ld/st prior to dcbt stop all streams with flushing */
sync
-1: li r8,0
- .rept 8 /* 8-way set associative */
- ldx r11,r10,r8
- add r8,r8,r12
- xor r11,r11,r11 // Ensure r11 is 0 even if fallback area is not
- add r8,r8,r11 // Add 0, this creates a dependency on the ldx
- .endr
- addi r10,r10,128 /* 128 byte cache line */
+
+ /*
+ * The load adresses are at staggered offsets within cachelines,
+ * which suits some pipelines better (on others it should not
+ * hurt).
+ */
+1:
+ ld r11,(0x80 + 8)*0(r10)
+ ld r11,(0x80 + 8)*1(r10)
+ ld r11,(0x80 + 8)*2(r10)
+ ld r11,(0x80 + 8)*3(r10)
+ ld r11,(0x80 + 8)*4(r10)
+ ld r11,(0x80 + 8)*5(r10)
+ ld r11,(0x80 + 8)*6(r10)
+ ld r11,(0x80 + 8)*7(r10)
+ addi r10,r10,0x80*8
bdnz 1b
mtctr r9
ld r9,PACA_EXRFI+EX_R9(r13)
ld r10,PACA_EXRFI+EX_R10(r13)
ld r11,PACA_EXRFI+EX_R11(r13)
- ld r12,PACA_EXRFI+EX_R12(r13)
- ld r8,PACA_EXRFI+EX_R13(r13)
GET_SCRATCH0(r13);
hrfid
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index a3993ac21b50..55d8da909db5 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -873,19 +873,8 @@ static void init_fallback_flush(void)
memset(l1d_flush_fallback_area, 0, l1d_size * 2);
for_each_possible_cpu(cpu) {
- /*
- * The fallback flush is currently coded for 8-way
- * associativity. Different associativity is possible, but it
- * will be treated as 8-way and may not evict the lines as
- * effectively.
- *
- * 128 byte lines are mandatory.
- */
- u64 c = l1d_size / 8;
-
paca[cpu].rfi_flush_fallback_area = l1d_flush_fallback_area;
- paca[cpu].l1d_flush_congruence = c;
- paca[cpu].l1d_flush_sets = c / 128;
+ paca[cpu].l1d_flush_size = l1d_size;
}
}
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 10/15] powerpc/pseries: Query hypervisor for RFI flush settings
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Michael Neuling <mikey@neuling.org>
commit 8989d56878a7735dfdb234707a2fee6faf631085 upstream.
A new hypervisor call is available which tells the guest settings
related to the RFI flush. Use it to query the appropriate flush
instruction(s), and whether the flush is required.
Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/platforms/pseries/setup.c | 37 +++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index e6e8b241d717..b4867b4d5a1e 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -507,6 +507,39 @@ static void __init find_and_init_phbs(void)
}
}
+static void pseries_setup_rfi_flush(void)
+{
+ struct h_cpu_char_result result;
+ enum l1d_flush_type types;
+ bool enable;
+ long rc;
+
+ /* Enable by default */
+ enable = true;
+
+ rc = plpar_get_cpu_characteristics(&result);
+ if (rc == H_SUCCESS) {
+ types = L1D_FLUSH_NONE;
+
+ 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;
+
+ /* Use fallback if nothing set in hcall */
+ if (types == L1D_FLUSH_NONE)
+ types = L1D_FLUSH_FALLBACK;
+
+ if (!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR))
+ enable = false;
+ } else {
+ /* Default to fallback if case hcall is not available */
+ types = L1D_FLUSH_FALLBACK;
+ }
+
+ setup_rfi_flush(types, enable);
+}
+
static void __init pSeries_setup_arch(void)
{
set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
@@ -523,7 +556,9 @@ static void __init pSeries_setup_arch(void)
fwnmi_init();
- /* By default, only probe PCI (can be overriden by rtas_pci) */
+ pseries_setup_rfi_flush();
+
+ /* By default, only probe PCI (can be overridden by rtas_pci) */
pci_add_flags(PCI_PROBE_ONLY);
/* Find and initialize PCI host bridges */
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 11/15] powerpc/powernv: Check device-tree for RFI flush settings
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Oliver O'Halloran <oohall@gmail.com>
commit 6e032b350cd1fdb830f18f8320ef0e13b4e24094 upstream.
New device-tree properties are available which tell the hypervisor
settings related to the RFI flush. Use them to determine the
appropriate flush instruction to use, and whether the flush is
required.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/platforms/powernv/setup.c | 49 ++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 20974478f8d0..e3c98361e6e4 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -38,14 +38,63 @@
#include <asm/cputhreads.h>
#include <asm/cpuidle.h>
#include <asm/code-patching.h>
+#include <asm/setup.h>
#include "powernv.h"
#include "subcore.h"
+static void pnv_setup_rfi_flush(void)
+{
+ struct device_node *np, *fw_features;
+ enum l1d_flush_type type;
+ int 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");
+ of_node_put(np);
+
+ if (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;
+
+ 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"))
+ 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--;
+
+ of_node_put(np);
+ of_node_put(fw_features);
+ }
+
+ setup_rfi_flush(type, enable > 0);
+}
+
static void __init pnv_setup_arch(void)
{
set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
+ pnv_setup_rfi_flush();
+
/* Initialize SMP */
pnv_smp_init();
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 12/15] powerpc/64s: Allow control of RFI flush via debugfs
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
commit 236003e6b5443c45c18e613d2b0d776a9f87540e upstream.
Expose the state of the RFI flush (enabled/disabled) via debugfs, and
allow it to be enabled/disabled at runtime.
eg: $ cat /sys/kernel/debug/powerpc/rfi_flush
1
$ echo 0 > /sys/kernel/debug/powerpc/rfi_flush
$ cat /sys/kernel/debug/powerpc/rfi_flush
0
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kernel/setup_64.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 55d8da909db5..ec01a98a5978 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -38,6 +38,7 @@
#include <linux/hugetlb.h>
#include <linux/memory.h>
#include <linux/nmi.h>
+#include <linux/debugfs.h>
#include <asm/io.h>
#include <asm/kdump.h>
@@ -896,5 +897,34 @@ void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
if (!no_rfi_flush)
rfi_flush_enable(enable);
}
+
+#ifdef CONFIG_DEBUG_FS
+static int rfi_flush_set(void *data, u64 val)
+{
+ if (val == 1)
+ rfi_flush_enable(true);
+ else if (val == 0)
+ rfi_flush_enable(false);
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+static int rfi_flush_get(void *data, u64 *val)
+{
+ *val = rfi_flush ? 1 : 0;
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_rfi_flush, rfi_flush_get, rfi_flush_set, "%llu\n");
+
+static __init int rfi_flush_debugfs_init(void)
+{
+ debugfs_create_file("rfi_flush", 0600, powerpc_debugfs_root, NULL, &fops_rfi_flush);
+ return 0;
+}
+device_initcall(rfi_flush_debugfs_init);
+#endif
#endif /* CONFIG_PPC_BOOK3S_64 */
#endif
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 13/15] powerpc/pseries: include linux/types.h in asm/hvcall.h
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Michal Suchanek <msuchanek@suse.de>
commit 1b689a95ce7427075f9ac9fb4aea1af530742b7f upstream.
Commit 6e032b350cd1 ("powerpc/powernv: Check device-tree for RFI flush
settings") uses u64 in asm/hvcall.h without including linux/types.h
This breaks hvcall.h users that do not include the header themselves.
Fixes: 6e032b350cd1 ("powerpc/powernv: Check device-tree for RFI flush settings")
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/hvcall.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 51adbde09845..449bbb87c257 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -298,6 +298,7 @@
#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ull << 61) // IBM bit 2
#ifndef __ASSEMBLY__
+#include <linux/types.h>
/**
* plpar_hcall_norets: - Make a pseries hypervisor call with no return arguments
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 14/15] powerpc/ptrace: Fix out of bounds array access warning
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Khem Raj <raj.khem@gmail.com>
commit 1e407ee3b21f981140491d5b8a36422979ca246f upstream.
gcc-6 correctly warns about a out of bounds access
arch/powerpc/kernel/ptrace.c:407:24: warning: index 32 denotes an offset greater than size of 'u64[32][1] {aka long long unsigned int[32][1]}' [-Warray-bounds]
offsetof(struct thread_fp_state, fpr[32][0]));
^
check the end of array instead of beginning of next element to fix this
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index f21897b42057..93f200f14e19 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -376,7 +376,7 @@ static int fpr_get(struct task_struct *target, const struct user_regset *regset,
#else
BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
- offsetof(struct thread_fp_state, fpr[32][0]));
+ offsetof(struct thread_fp_state, fpr[32]));
return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.fp_state, 0, -1);
@@ -404,7 +404,7 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
return 0;
#else
BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
- offsetof(struct thread_fp_state, fpr[32][0]));
+ offsetof(struct thread_fp_state, fpr[32]));
return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
&target->thread.fp_state, 0, -1);
--
2.14.1
^ permalink raw reply related
* [PATCH v4.1 backport 15/15] powerpc/vdso64: Use double word compare on pointers
From: Michael Ellerman @ 2018-03-02 4:45 UTC (permalink / raw)
To: Alexander.Levin; +Cc: stable, linuxppc-dev, npiggin
In-Reply-To: <20180302044549.14930-1-mpe@ellerman.id.au>
From: Anton Blanchard <anton@samba.org>
commit 5045ea37377ce8cca6890d32b127ad6770e6dce5 upstream.
__kernel_get_syscall_map() and __kernel_clock_getres() use cmpli to
check if the passed in pointer is non zero. cmpli maps to a 32 bit
compare on binutils, so we ignore the top 32 bits.
A simple test case can be created by passing in a bogus pointer with
the bottom 32 bits clear. Using a clk_id that is handled by the VDSO,
then one that is handled by the kernel shows the problem:
printf("%d\n", clock_getres(CLOCK_REALTIME, (void *)0x100000000));
printf("%d\n", clock_getres(CLOCK_BOOTTIME, (void *)0x100000000));
And we get:
0
-1
The bigger issue is if we pass a valid pointer with the bottom 32 bits
clear, in this case we will return success but won't write any data
to the pointer.
I stumbled across this issue because the LLVM integrated assembler
doesn't accept cmpli with 3 arguments. Fix this by converting them to
cmpldi.
Fixes: a7f290dad32e ("[PATCH] powerpc: Merge vdso's and add vdso support to 32 bits kernel")
Cc: stable@vger.kernel.org # v2.6.15+
Signed-off-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/vdso64/datapage.S | 2 +-
arch/powerpc/kernel/vdso64/gettimeofday.S | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vdso64/datapage.S
index 79796de11737..3263ee23170d 100644
--- a/arch/powerpc/kernel/vdso64/datapage.S
+++ b/arch/powerpc/kernel/vdso64/datapage.S
@@ -57,7 +57,7 @@ V_FUNCTION_BEGIN(__kernel_get_syscall_map)
bl V_LOCAL_FUNC(__get_datapage)
mtlr r12
addi r3,r3,CFG_SYSCALL_MAP64
- cmpli cr0,r4,0
+ cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
li r0,__NR_syscalls
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index a76b4af37ef2..382021324883 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -145,7 +145,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
bne cr0,99f
li r3,0
- cmpli cr0,r4,0
+ cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
lis r5,CLOCK_REALTIME_RES@h
--
2.14.1
^ permalink raw reply related
* [PATCH v3] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Alistair Popple @ 2018-03-02 5:18 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Balbir Singh, Mark Hairgrove, mpe, Alistair Popple
When sending TLB invalidates to the NPU we need to send extra flushes due
to a hardware issue. The original implementation would lock the all the
ATSD MMIO registers sequentially before unlocking and relocking each of
them sequentially to do the extra flush.
This introduced a deadlock as it is possible for one thread to hold one
ATSD register whilst waiting for another register to be freed while the
other thread is holding that register waiting for the one in the first
thread to be freed.
For example if there are two threads and two ATSD registers:
Thread A Thread B
Acquire 1
Acquire 2
Release 1 Acquire 1
Wait 1 Wait 2
Both threads will be stuck waiting to acquire a register resulting in an
RCU stall warning or soft lockup.
This patch solves the deadlock by refactoring the code to ensure registers
are not released between flushes and to ensure all registers are either
acquired or released together and in order.
Fixes: bbd5ff50afff ("powerpc/powernv/npu-dma: Add explicit flush when sending an ATSD")
Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
v2:
- Added memory barriers around ATSD register aquisition/release
- Added compiler barriers around npdev[][] assignment
v3:
- Added comments to describe required locking
---
arch/powerpc/platforms/powernv/npu-dma.c | 228 +++++++++++++++++++------------
1 file changed, 139 insertions(+), 89 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index 0a253b64ac5f..0dec96eb3358 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -410,6 +410,11 @@ struct npu_context {
void *priv;
};
+struct mmio_atsd_reg {
+ struct npu *npu;
+ int reg;
+};
+
/*
* Find a free MMIO ATSD register and mark it in use. Return -ENOSPC
* if none are available.
@@ -419,7 +424,7 @@ static int get_mmio_atsd_reg(struct npu *npu)
int i;
for (i = 0; i < npu->mmio_atsd_count; i++) {
- if (!test_and_set_bit(i, &npu->mmio_atsd_usage))
+ if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage))
return i;
}
@@ -428,86 +433,90 @@ static int get_mmio_atsd_reg(struct npu *npu)
static void put_mmio_atsd_reg(struct npu *npu, int reg)
{
- clear_bit(reg, &npu->mmio_atsd_usage);
+ clear_bit_unlock(reg, &npu->mmio_atsd_usage);
}
/* MMIO ATSD register offsets */
#define XTS_ATSD_AVA 1
#define XTS_ATSD_STAT 2
-static int mmio_launch_invalidate(struct npu *npu, unsigned long launch,
- unsigned long va)
+static void mmio_launch_invalidate(struct mmio_atsd_reg *mmio_atsd_reg,
+ unsigned long launch, unsigned long va)
{
- int mmio_atsd_reg;
-
- do {
- mmio_atsd_reg = get_mmio_atsd_reg(npu);
- cpu_relax();
- } while (mmio_atsd_reg < 0);
+ struct npu *npu = mmio_atsd_reg->npu;
+ int reg = mmio_atsd_reg->reg;
__raw_writeq(cpu_to_be64(va),
- npu->mmio_atsd_regs[mmio_atsd_reg] + XTS_ATSD_AVA);
+ npu->mmio_atsd_regs[reg] + XTS_ATSD_AVA);
eieio();
- __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[mmio_atsd_reg]);
-
- return mmio_atsd_reg;
+ __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[reg]);
}
-static int mmio_invalidate_pid(struct npu *npu, unsigned long pid, bool flush)
+static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
+ unsigned long pid, bool flush)
{
+ int i;
unsigned long launch;
- /* IS set to invalidate matching PID */
- launch = PPC_BIT(12);
+ for (i = 0; i <= max_npu2_index; i++) {
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
+
+ /* IS set to invalidate matching PID */
+ launch = PPC_BIT(12);
- /* PRS set to process-scoped */
- launch |= PPC_BIT(13);
+ /* PRS set to process-scoped */
+ launch |= PPC_BIT(13);
- /* AP */
- launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
+ /* AP */
+ launch |= (u64)
+ mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
+ /* PID */
+ launch |= pid << PPC_BITLSHIFT(38);
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+ /* No flush */
+ launch |= !flush << PPC_BITLSHIFT(39);
- /* Invalidating the entire process doesn't use a va */
- return mmio_launch_invalidate(npu, launch, 0);
+ /* Invalidating the entire process doesn't use a va */
+ mmio_launch_invalidate(&mmio_atsd_reg[i], launch, 0);
+ }
}
-static int mmio_invalidate_va(struct npu *npu, unsigned long va,
- unsigned long pid, bool flush)
+static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
+ unsigned long va, unsigned long pid, bool flush)
{
+ int i;
unsigned long launch;
- /* IS set to invalidate target VA */
- launch = 0;
+ for (i = 0; i <= max_npu2_index; i++) {
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
- /* PRS set to process scoped */
- launch |= PPC_BIT(13);
+ /* IS set to invalidate target VA */
+ launch = 0;
- /* AP */
- launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
+ /* PRS set to process scoped */
+ launch |= PPC_BIT(13);
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
+ /* AP */
+ launch |= (u64)
+ mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+ /* PID */
+ launch |= pid << PPC_BITLSHIFT(38);
- return mmio_launch_invalidate(npu, launch, va);
+ /* No flush */
+ launch |= !flush << PPC_BITLSHIFT(39);
+
+ mmio_launch_invalidate(&mmio_atsd_reg[i], launch, va);
+ }
}
#define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
-struct mmio_atsd_reg {
- struct npu *npu;
- int reg;
-};
-
static void mmio_invalidate_wait(
- struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS], bool flush)
+ struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
{
struct npu *npu;
int i, reg;
@@ -522,16 +531,65 @@ static void mmio_invalidate_wait(
reg = mmio_atsd_reg[i].reg;
while (__raw_readq(npu->mmio_atsd_regs[reg] + XTS_ATSD_STAT))
cpu_relax();
+ }
+}
- put_mmio_atsd_reg(npu, reg);
+/*
+ * Acquires all the address translation shootdown (ATSD) registers required to
+ * launch an ATSD on all links this npu_context is active on.
+ */
+static void acquire_atsd_reg(struct npu_context *npu_context,
+ struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
+{
+ int i, j;
+ struct npu *npu;
+ struct pci_dev *npdev;
+ struct pnv_phb *nphb;
- /*
- * The GPU requires two flush ATSDs to ensure all entries have
- * been flushed. We use PID 0 as it will never be used for a
- * process on the GPU.
+ for (i = 0; i <= max_npu2_index; i++) {
+ mmio_atsd_reg[i].reg = -1;
+ for (j = 0; j < NV_MAX_LINKS; j++) {
+ /* There are no ordering requirements with respect to
+ * the setup of struct npu_context, but to ensure
+ * consistent behaviour we need to ensure npdev[][] is
+ * only read once.
+ */
+ npdev = READ_ONCE(npu_context->npdev[i][j]);
+ if (!npdev)
+ continue;
+
+ nphb = pci_bus_to_host(npdev->bus)->private_data;
+ npu = &nphb->npu;
+ mmio_atsd_reg[i].npu = npu;
+ mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
+ while (mmio_atsd_reg[i].reg < 0) {
+ mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
+ cpu_relax();
+ }
+ break;
+ }
+ }
+}
+
+/*
+ * Release previously acquired ATSD registers. To avoid deadlocks the registers
+ * must be released in the same order they were acquired above in
+ * acquire_atsd_reg.
+ */
+static void release_atsd_reg(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
+{
+ int i;
+
+ for (i = 0; i <= max_npu2_index; i++) {
+ /* We can't rely on npu_context->npdev[][] being the same here
+ * as when acquire_atsd_reg() was called, hence we use the
+ * values stored in mmio_atsd_reg during the acquire phase
+ * rather than re-reading npdev[][]
*/
- if (flush)
- mmio_invalidate_pid(npu, 0, true);
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
+
+ put_mmio_atsd_reg(mmio_atsd_reg[i].npu, mmio_atsd_reg[i].reg);
}
}
@@ -542,10 +600,6 @@ static void mmio_invalidate_wait(
static void mmio_invalidate(struct npu_context *npu_context, int va,
unsigned long address, bool flush)
{
- int i, j;
- struct npu *npu;
- struct pnv_phb *nphb;
- struct pci_dev *npdev;
struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
unsigned long pid = npu_context->mm->context.id;
@@ -561,37 +615,25 @@ static void mmio_invalidate(struct npu_context *npu_context, int va,
* Loop over all the NPUs this process is active on and launch
* an invalidate.
*/
- for (i = 0; i <= max_npu2_index; i++) {
- mmio_atsd_reg[i].reg = -1;
- for (j = 0; j < NV_MAX_LINKS; j++) {
- npdev = npu_context->npdev[i][j];
- if (!npdev)
- continue;
-
- nphb = pci_bus_to_host(npdev->bus)->private_data;
- npu = &nphb->npu;
- mmio_atsd_reg[i].npu = npu;
-
- if (va)
- mmio_atsd_reg[i].reg =
- mmio_invalidate_va(npu, address, pid,
- flush);
- else
- mmio_atsd_reg[i].reg =
- mmio_invalidate_pid(npu, pid, flush);
-
- /*
- * The NPU hardware forwards the shootdown to all GPUs
- * so we only have to launch one shootdown per NPU.
- */
- break;
- }
+ acquire_atsd_reg(npu_context, mmio_atsd_reg);
+ if (va)
+ mmio_invalidate_va(mmio_atsd_reg, address, pid, flush);
+ else
+ mmio_invalidate_pid(mmio_atsd_reg, pid, flush);
+
+ mmio_invalidate_wait(mmio_atsd_reg);
+ if (flush) {
+ /*
+ * The GPU requires two flush ATSDs to ensure all entries have
+ * been flushed. We use PID 0 as it will never be used for a
+ * process on the GPU.
+ */
+ mmio_invalidate_pid(mmio_atsd_reg, 0, true);
+ mmio_invalidate_wait(mmio_atsd_reg);
+ mmio_invalidate_pid(mmio_atsd_reg, 0, true);
+ mmio_invalidate_wait(mmio_atsd_reg);
}
-
- mmio_invalidate_wait(mmio_atsd_reg, flush);
- if (flush)
- /* Wait for the flush to complete */
- mmio_invalidate_wait(mmio_atsd_reg, false);
+ release_atsd_reg(mmio_atsd_reg);
}
static void pnv_npu2_mn_release(struct mmu_notifier *mn,
@@ -726,7 +768,15 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
&nvlink_index)))
return ERR_PTR(-ENODEV);
- npu_context->npdev[npu->index][nvlink_index] = npdev;
+
+ /* npdev is a pci_dev pointer setup by the PCI code. We assign it to
+ * npdev[][] to indicate to the mmu notifiers that an invalidation
+ * should also be sent over this nvlink. The notifiers don't use any
+ * other fields in npu_context, so we just need to ensure that when they
+ * deference npu_context->npdev[][] it is either a valid pointer or
+ * NULL.
+ */
+ WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
if (!nphb->npu.nmmu_flush) {
/*
@@ -778,7 +828,7 @@ void pnv_npu2_destroy_context(struct npu_context *npu_context,
if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
&nvlink_index)))
return;
- npu_context->npdev[npu->index][nvlink_index] = NULL;
+ WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], NULL);
opal_npu_destroy_context(nphb->opal_id, npu_context->mm->context.id,
PCI_DEVID(gpdev->bus->number, gpdev->devfn));
kref_put(&npu_context->kref, pnv_npu2_release_context);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Masahiro Yamada @ 2018-03-02 5:50 UTC (permalink / raw)
To: Ulf Magnusson
Cc: Arnd Bergmann, Rich Felker, Kernel Hardening, X86 ML,
Paul Mackerras, H. Peter Anvin, sparclinux, Sam Ravnborg,
Yoshinori Sato, Jonathan Corbet, Richard Weinberger,
Linux-sh list, Ingo Molnar, Emese Revfy, Kees Cook, uml-devel,
Linux Kbuild mailing list, Peter Oberparleiter, Jeff Dike,
linuxppc-dev, user-mode-linux-user, Thomas Gleixner, Michal Marek,
Greg Kroah-Hartman, Randy Dunlap, open list:DOCUMENTATION,
Linux Kernel Mailing List, Linus Torvalds, David S. Miller
In-Reply-To: <20180221213946.w4v6ywy7fbiy6oyc@huvuddator>
2018-02-22 6:39 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
>> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
>> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
>> > <yamada.masahiro@socionext.com> wrote:
>> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
>> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>> >>> <yamada.masahiro@socionext.com> wrote:
>> >>>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> >>
>> >> Let me clarify my concern.
>> >>
>> >> When we test the compiler flag, is there a case
>> >> where a particular flag depends on -m{32,64} ?
>> >>
>> >> For example, is there a compiler that supports -fstack-protector
>> >> for 64bit mode, but unsupports it for 32bit mode?
>> >>
>> >> $(cc-option -m32) -> y
>> >> $(cc-option -m64) -> y
>> >> $(cc-option -fstack-protector) -> y
>> >> $(cc-option -m32 -fstack-protector) -> n
>> >> $(cc-option -m64 -fstack-protector) -> y
>> >>
>> >> I guess this is unlikely to happen,
>> >> but I am not whether it is zero possibility.
>> >>
>> >> If this could happen,
>> >> $(cc-option ) must be evaluated together with
>> >> correct bi-arch option (either -m32 or -m64).
>> >>
>> >>
>> >> Currently, -m32/-m64 is specified in Makefile,
>> >> but we are moving compiler tests to Kconfig
>> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
>> >
>> > I don't think it can happen for this particular combination (stack protector
>> > and word size), but I'm sure we'll eventually run into options that
>> > need to be tested in combination. For the current CFLAGS_KERNEL
>> > setting, we definitely have the case of needing the variables to be
>> > evaluated in a specific order.
>> >
>>
>>
>>
>>
>> I was thinking of how we can handle complex cases
>> in the current approach.
>>
>>
>>
>> (Case 1)
>>
>> Compiler flag -foo and -bar interacts, so
>> we also need to check the combination of the two.
>>
>>
>> config CC_HAS_FOO
>> def_bool $(cc-option -foo)
>>
>> config CC_HAS_BAR
>> def_bool $(cc-option -bar)
>>
>> config CC_HAS_FOO_WITH_BAR
>> def_bool $(cc-option -foo -bar)
>>
>>
>>
>> (Case 2)
>> Compiler flag -foo is sensitive to word-size.
>> So, we need to test this option together with -m32/-m64.
>> User can toggle CONFIG_64BIT, like i386/x86_64.
>>
>>
>> config CC_NEEDS_M64
>> def_bool $(cc-option -m64) && 64BIT
>>
>> config CC_NEEDS_M32
>> def_bool $(cc-option -m32) && !64BIT
>>
>> config CC_HAS_FOO
>> bool
>> default $(cc-option -m64 -foo) if CC_NEEDS_M64
>> default $(cc-option -m32 -foo) if CC_NEEDS_M32
>> default $(cc-option -foo)
>>
>>
>>
>> (Case 3)
>> Compiler flag -foo is sensitive to endian-ness.
>>
>>
>> config CC_NEEDS_BIG_ENDIAN
>> def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
>>
>> config CC_NEEDS_LITTLE_ENDIAN
>> def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
>>
>> config CC_HAS_FOO
>> bool
>> default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
>> default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
>> default $(cc-option -foo)
>>
>>
>>
>>
>> Hmm, I think I can implement those somehow.
>> But, I hope we do not have many instances like this...
>>
>>
>> If you know more naive cases, please share your knowledge.
>>
>> Thanks!
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
> Would get pretty bad if a test needs to consider multiple symbols.
> Exponential explosion there...
>
>
> I thought some more about the implementation of dynamic (post-parsing)
> functions to see how bad it would get with the current implementation.
>
> Some background on how things work now:
>
> 1. All expression operands in Kconfig are symbols.
>
> 2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
> you symbols with those strings as names and S_UNKNOWN type (because
> they act like references to undefined symbols).
>
> 3. For "foo-$(fn foo)", you also get a symbol with that string as its
> name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
>
> 4. Symbols with S_UNKNOWN type get their name as their string value,
> and the tristate value n.
>
> So, if you do string expansion on the names of symbols with S_UNKNOWN
> type in sym_calc_value(), you're almost there with the current
> implementation, except for the tristate case.
>
> Maybe you could set the tristate value of S_UNKNOWN symbols depending on
> the string value you end up with. Things are getting pretty confusing at
> that point.
>
> Could have something like S_DYNAMIC as well. More Kconfig complexity...
>
> Then there's other complications:
>
> 1. SYMBOL_CONST is no longer constant.
>
> 2. Dependency loop detection needs to consider symbol references
> within strings.
>
> 3. Dependency loop detection relies on static knowledge of what
> symbols a symbol depends on. That might get messy for certain
> expansions, though it might be things you wouldn't do in practice.
>
> 4. Symbols still need to be properly invalidated. It looks like at
> least menuconfig just does a dumb invalidate-everything whenever
> the value of a symbol is changed though, so it might not require
> extra work. (Bit messier in Kconfiglib, which does minimal
> invalidation to keep scripts fast, but just need to extract a few
> extra deps there.)
>
>
> It looks like dynamic functions could get quite messy, but might be
> doable if absolutely required. There's probably more devils in the
> details though.
>
> I don't think the static function model precludes switching models later
> btw, when people have more experience.
I really want to start with the static function model
and see if we need the dynamic function implementation.
Here is an idea for the migration path in case
we need to do that in the future.
Currently, every time user input is given,
sym_clear_all_valid() is called.
It is not efficient to blindly re-evaluate expensive $(shell ...)
So, have a list of symbols the function depends on
in its arguments.
For example,
config CC_HAS_SANE_STACKPROTECTOR
def_bool $(shell $srctree/scripts/gcc-has-stack-protector.sh
$CC $(1), CFLAGS_BASE)
Here the first argument
$srctree/scripts/gcc-x86-has-stack-protector.sh $CC $(1)
is the shell command.
$(1), $(2), ... will be replaced with the values of symbols (or expressions)
that follow when running the shell command.
The second argument
CFLAGS_BASE
is the dependency symbol (or expression).
CFLAGS_BASE can be dynamically changed like
config CFLAGS_BASE
string
default "-m64" if 64BIT
default "-m32"
When and only when CFLAGS_BASE is updated, the function should be re-calculated.
(This will require efforts to minimize the amount of re-evaluation.)
cc-option will be implemented like follows:
macro cc-option $(shell $CC -Werror $$(1) $(1) -c -x c /dev/null -o
/dev/null, CFLAGS_BASE)
Please notice the difference between $$(1) and $(1).
$(1) is immediately expanded by cc-option macro.
$$(1) is escaped since we want to expand it by $(shell ...), not by
$(cc-option ...)
For example,
$(cc-option -fstack-protector)
will be expanded to
$(shell gcc -Werror $(1) -fstack-protector -c -x c /dev/null -o
/dev/null, CFLAGS_BASE)
Since macros are just textual shorthand, so this expansion happens
during the parse phase.
Then, the evaluation phase does the following every time CFLAGS_BASE is updated.
gcc -Werror [value of CFLAGS_BASE] -fstack-protector -c -x c /dev/null
-o /dev/null
This is a new form of expression, so it will be managed in AST tree
with a flag E_SHELL (or E_FUNC) etc.
Not implemented at all. Just a rough sketch.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: Hotplug + Reboot is crashing HPT guest with HPT resizing enabled
From: Bharata B Rao @ 2018-03-02 8:12 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20180302022144.GG13135@umbus.fritz.box>
[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]
On Fri, Mar 2, 2018 at 7:51 AM, David Gibson <david@gibson.dropbear.id.au>
wrote:
> On Fri, Feb 23, 2018 at 03:02:40PM +0530, Bharata B Rao wrote:
> > Hi,
> >
> > Rebooting a hash guest after hotplugging memory to it is crashing the
> > guest. This is seen only when HPT resizing is enabled. I see guest
> crashing
> > at multiple places, but this location is fairly commonly seen:
> >
> > kernel BUG at mm/slub.c:3912!
> >
> > Testing with latest guest kernel and ppc-for-2.12 branch of QEMU.
>
> Ugh. We had several bugs along these lines, but I thought I'd fixed
> them. I wonder what this one is.
>
> > A bit of debugging shows me that when memory is added, the guest kernel
> > tries to resize HPT to a htab_shift value lesser than the value with
> which
> > the guest has booted. For eg. a 8GB guest boots with htab_shift of 26.
> When
> > 1G is hot-added,
> > arch/powerpc/mm/hash_utils_64.c:resize_hpt_for_hotplug() ends up
> assigning
> > 24 to target_hpt_shift. This looks suspicious as we are increasing the
> > memory, but kernel is asking for shrinking the HPT size.
>
> So the shrink-HPT-on-add-memory is actually expected and should be
> harmless. It occurs because qemu estimates HPT size on the
> traditional HPT == RAM size / 64 formular, which was devised with 4k
> pages in mind. The kernel on the other hand, knows it is using 64k
> pages and so estimates a smaller HPT size. Hot plugging memory always
> prompts the guest to re-estimate the required HPT size, but if the
> added memory is small enough, that size can still be smaller than
> qemu's initial guess.
>
Thanks for the clarification.
>
> > HPT resizing
> > requests fail though, but next reboot crashes the guest.
>
> As noted the shrink is expected, so we need to debug the crash
> separately. Do you have 9478956794c11239b7c1c3ef9ce95c883bb839a3 in
> your tree?
>
I do now and I no longer see the crash that I was observing last week.
Regards,
Bharata.
[-- Attachment #2: Type: text/html, Size: 2698 bytes --]
^ permalink raw reply
* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Ulf Magnusson @ 2018-03-02 9:03 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Arnd Bergmann, Rich Felker, Kernel Hardening, X86 ML,
Paul Mackerras, H. Peter Anvin, sparclinux, Sam Ravnborg,
Yoshinori Sato, Jonathan Corbet, Richard Weinberger,
Linux-sh list, Ingo Molnar, Emese Revfy, Kees Cook, uml-devel,
Linux Kbuild mailing list, Peter Oberparleiter, Jeff Dike,
linuxppc-dev, user-mode-linux-user, Thomas Gleixner, Michal Marek,
Greg Kroah-Hartman, Randy Dunlap, open list:DOCUMENTATION,
Linux Kernel Mailing List, Linus Torvalds, David S. Miller
In-Reply-To: <CAK7LNARd-9KNRTHT3jrNganqLisheAWbDQMGi_ujp+1GpYzSOg@mail.gmail.com>
On Fri, Mar 02, 2018 at 02:50:39PM +0900, Masahiro Yamada wrote:
> 2018-02-22 6:39 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> > On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> >> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> >> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> >> > <yamada.masahiro@socionext.com> wrote:
> >> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> >> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> >> >>> <yamada.masahiro@socionext.com> wrote:
> >> >>>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> >> >>
> >> >> Let me clarify my concern.
> >> >>
> >> >> When we test the compiler flag, is there a case
> >> >> where a particular flag depends on -m{32,64} ?
> >> >>
> >> >> For example, is there a compiler that supports -fstack-protector
> >> >> for 64bit mode, but unsupports it for 32bit mode?
> >> >>
> >> >> $(cc-option -m32) -> y
> >> >> $(cc-option -m64) -> y
> >> >> $(cc-option -fstack-protector) -> y
> >> >> $(cc-option -m32 -fstack-protector) -> n
> >> >> $(cc-option -m64 -fstack-protector) -> y
> >> >>
> >> >> I guess this is unlikely to happen,
> >> >> but I am not whether it is zero possibility.
> >> >>
> >> >> If this could happen,
> >> >> $(cc-option ) must be evaluated together with
> >> >> correct bi-arch option (either -m32 or -m64).
> >> >>
> >> >>
> >> >> Currently, -m32/-m64 is specified in Makefile,
> >> >> but we are moving compiler tests to Kconfig
> >> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> >> >
> >> > I don't think it can happen for this particular combination (stack protector
> >> > and word size), but I'm sure we'll eventually run into options that
> >> > need to be tested in combination. For the current CFLAGS_KERNEL
> >> > setting, we definitely have the case of needing the variables to be
> >> > evaluated in a specific order.
> >> >
> >>
> >>
> >>
> >>
> >> I was thinking of how we can handle complex cases
> >> in the current approach.
> >>
> >>
> >>
> >> (Case 1)
> >>
> >> Compiler flag -foo and -bar interacts, so
> >> we also need to check the combination of the two.
> >>
> >>
> >> config CC_HAS_FOO
> >> def_bool $(cc-option -foo)
> >>
> >> config CC_HAS_BAR
> >> def_bool $(cc-option -bar)
> >>
> >> config CC_HAS_FOO_WITH_BAR
> >> def_bool $(cc-option -foo -bar)
> >>
> >>
> >>
> >> (Case 2)
> >> Compiler flag -foo is sensitive to word-size.
> >> So, we need to test this option together with -m32/-m64.
> >> User can toggle CONFIG_64BIT, like i386/x86_64.
> >>
> >>
> >> config CC_NEEDS_M64
> >> def_bool $(cc-option -m64) && 64BIT
> >>
> >> config CC_NEEDS_M32
> >> def_bool $(cc-option -m32) && !64BIT
> >>
> >> config CC_HAS_FOO
> >> bool
> >> default $(cc-option -m64 -foo) if CC_NEEDS_M64
> >> default $(cc-option -m32 -foo) if CC_NEEDS_M32
> >> default $(cc-option -foo)
> >>
> >>
> >>
> >> (Case 3)
> >> Compiler flag -foo is sensitive to endian-ness.
> >>
> >>
> >> config CC_NEEDS_BIG_ENDIAN
> >> def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
> >>
> >> config CC_NEEDS_LITTLE_ENDIAN
> >> def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
> >>
> >> config CC_HAS_FOO
> >> bool
> >> default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> >> default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
> >> default $(cc-option -foo)
> >>
> >>
> >>
> >>
> >> Hmm, I think I can implement those somehow.
> >> But, I hope we do not have many instances like this...
> >>
> >>
> >> If you know more naive cases, please share your knowledge.
> >>
> >> Thanks!
> >>
> >>
> >> --
> >> Best Regards
> >> Masahiro Yamada
> >
> > Would get pretty bad if a test needs to consider multiple symbols.
> > Exponential explosion there...
> >
> >
> > I thought some more about the implementation of dynamic (post-parsing)
> > functions to see how bad it would get with the current implementation.
> >
> > Some background on how things work now:
> >
> > 1. All expression operands in Kconfig are symbols.
> >
> > 2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
> > you symbols with those strings as names and S_UNKNOWN type (because
> > they act like references to undefined symbols).
> >
> > 3. For "foo-$(fn foo)", you also get a symbol with that string as its
> > name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
> >
> > 4. Symbols with S_UNKNOWN type get their name as their string value,
> > and the tristate value n.
> >
> > So, if you do string expansion on the names of symbols with S_UNKNOWN
> > type in sym_calc_value(), you're almost there with the current
> > implementation, except for the tristate case.
> >
> > Maybe you could set the tristate value of S_UNKNOWN symbols depending on
> > the string value you end up with. Things are getting pretty confusing at
> > that point.
> >
> > Could have something like S_DYNAMIC as well. More Kconfig complexity...
> >
> > Then there's other complications:
> >
> > 1. SYMBOL_CONST is no longer constant.
> >
> > 2. Dependency loop detection needs to consider symbol references
> > within strings.
> >
> > 3. Dependency loop detection relies on static knowledge of what
> > symbols a symbol depends on. That might get messy for certain
> > expansions, though it might be things you wouldn't do in practice.
> >
> > 4. Symbols still need to be properly invalidated. It looks like at
> > least menuconfig just does a dumb invalidate-everything whenever
> > the value of a symbol is changed though, so it might not require
> > extra work. (Bit messier in Kconfiglib, which does minimal
> > invalidation to keep scripts fast, but just need to extract a few
> > extra deps there.)
> >
> >
> > It looks like dynamic functions could get quite messy, but might be
> > doable if absolutely required. There's probably more devils in the
> > details though.
> >
> > I don't think the static function model precludes switching models later
> > btw, when people have more experience.
>
>
>
> I really want to start with the static function model
> and see if we need the dynamic function implementation.
Yeah, let's start with static functions, IMO.
Either we'll learn that they're powerful enough in practice, and save
ourselves some work, or we'll gain experience for later. Converting from
static to dynamic functions should be painless, if needed.
My plan would be something like:
1. Implement static functions
2. Convert as many simple cases over to them as possible
3. See how bad the bad cases get. If they get really bad, then decide
what to do next (extend Kconfig, handle them in the Makefiles,
etc.)
>
> Here is an idea for the migration path in case
> we need to do that in the future.
>
>
>
> Currently, every time user input is given,
> sym_clear_all_valid() is called.
>
> It is not efficient to blindly re-evaluate expensive $(shell ...)
I think menuconfig only reevalutes the symbols in the menu that's
currently shown in the interface (along with their dependencies).
Maybe that'd be bad enough though.
>
>
> So, have a list of symbols the function depends on
> in its arguments.
>
> For example,
>
> config CC_HAS_SANE_STACKPROTECTOR
> def_bool $(shell $srctree/scripts/gcc-has-stack-protector.sh
> $CC $(1), CFLAGS_BASE)
>
>
> Here the first argument
> $srctree/scripts/gcc-x86-has-stack-protector.sh $CC $(1)
>
> is the shell command.
> $(1), $(2), ... will be replaced with the values of symbols (or expressions)
> that follow when running the shell command.
>
>
> The second argument
> CFLAGS_BASE
> is the dependency symbol (or expression).
>
>
> CFLAGS_BASE can be dynamically changed like
>
> config CFLAGS_BASE
> string
> default "-m64" if 64BIT
> default "-m32"
>
>
> When and only when CFLAGS_BASE is updated, the function should be re-calculated.
> (This will require efforts to minimize the amount of re-evaluation.)
>
>
>
>
> cc-option will be implemented like follows:
>
> macro cc-option $(shell $CC -Werror $$(1) $(1) -c -x c /dev/null -o
> /dev/null, CFLAGS_BASE)
>
>
>
> Please notice the difference between $$(1) and $(1).
>
> $(1) is immediately expanded by cc-option macro.
>
> $$(1) is escaped since we want to expand it by $(shell ...), not by
> $(cc-option ...)
>
>
>
> For example,
>
> $(cc-option -fstack-protector)
>
> will be expanded to
>
> $(shell gcc -Werror $(1) -fstack-protector -c -x c /dev/null -o
> /dev/null, CFLAGS_BASE)
>
> Since macros are just textual shorthand, so this expansion happens
> during the parse phase.
>
>
>
> Then, the evaluation phase does the following every time CFLAGS_BASE is updated.
>
> gcc -Werror [value of CFLAGS_BASE] -fstack-protector -c -x c /dev/null
> -o /dev/null
>
>
> This is a new form of expression, so it will be managed in AST tree
> with a flag E_SHELL (or E_FUNC) etc.
>
>
> Not implemented at all. Just a rough sketch.
A simpler syntax like
$(shell $CC -Werror {CFLAGS_BASE} -c -x c /dev/null -o /dev/null)
might work as well. Then you wouldn't have to any double escaping, and
having both $(1) and the value for it appear in the same place seems a
bit redundant.
I wonder if dependency management might get messy...
Anyway, let's just go with static functions...
Cheers,
Ulf
^ 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