* [PATCH v3 5/5] powerpc: apm82181: integrate bluestone.dts
From: Christian Lamparter @ 2020-09-05 22:06 UTC (permalink / raw)
To: linuxppc-dev, devicetree; +Cc: Paul Mackerras, Rob Herring, Chris Blake
In-Reply-To: <cover.1599343429.git.chunkeey@gmail.com>
This patch tries to integrate the existing bluestone.dts into the
apm82181.dtsi framework.
The original bluestone.dts produces a peculiar warning message.
> bluestone.dts:120.10-125.4: Warning (i2c_bus_reg):
> /plb/opb/i2c@ef600700/sttm@4C: I2C bus unit address format error, expected "4c"
For now, this has been kept as-is.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
rfc -> v1:
- no changes
v2 -> v3:
- incorporated pcie@d node-name switch
---
arch/powerpc/boot/dts/bluestone.dts | 458 +++++++---------------------
1 file changed, 104 insertions(+), 354 deletions(-)
diff --git a/arch/powerpc/boot/dts/bluestone.dts b/arch/powerpc/boot/dts/bluestone.dts
index aa1ae94cd776..b568fe7ae526 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -8,388 +8,138 @@
/dts-v1/;
+#include "apm82181.dtsi"
+
/ {
- #address-cells = <2>;
- #size-cells = <1>;
model = "apm,bluestone";
compatible = "apm,bluestone";
- dcr-parent = <&{/cpus/cpu@0}>;
aliases {
- ethernet0 = &EMAC0;
serial0 = &UART0;
serial1 = &UART1;
};
+};
- cpus {
- #address-cells = <1>;
- #size-cells = <0>;
-
- cpu@0 {
- device_type = "cpu";
- model = "PowerPC,apm821xx";
- reg = <0x00000000>;
- clock-frequency = <0>; /* Filled in by U-Boot */
- timebase-frequency = <0>; /* Filled in by U-Boot */
- i-cache-line-size = <32>;
- d-cache-line-size = <32>;
- i-cache-size = <32768>;
- d-cache-size = <32768>;
- dcr-controller;
- dcr-access-method = "native";
- next-level-cache = <&L2C0>;
- };
- };
-
- memory {
- device_type = "memory";
- reg = <0x00000000 0x00000000 0x00000000>; /* Filled in by U-Boot */
- };
-
- UIC0: interrupt-controller0 {
- compatible = "ibm,uic";
- interrupt-controller;
- cell-index = <0>;
- dcr-reg = <0x0c0 0x009>;
- #address-cells = <0>;
- #size-cells = <0>;
- #interrupt-cells = <2>;
- };
-
- UIC1: interrupt-controller1 {
- compatible = "ibm,uic";
- interrupt-controller;
- cell-index = <1>;
- dcr-reg = <0x0d0 0x009>;
- #address-cells = <0>;
- #size-cells = <0>;
- #interrupt-cells = <2>;
- interrupts = <0x1e 0x4 0x1f 0x4>; /* cascade */
- interrupt-parent = <&UIC0>;
- };
+&CRYPTO {
+ status = "okay";
+};
- UIC2: interrupt-controller2 {
- compatible = "ibm,uic";
- interrupt-controller;
- cell-index = <2>;
- dcr-reg = <0x0e0 0x009>;
- #address-cells = <0>;
- #size-cells = <0>;
- #interrupt-cells = <2>;
- interrupts = <0xa 0x4 0xb 0x4>; /* cascade */
- interrupt-parent = <&UIC0>;
- };
+&HWRNG {
+ status = "okay";
+};
- UIC3: interrupt-controller3 {
- compatible = "ibm,uic";
- interrupt-controller;
- cell-index = <3>;
- dcr-reg = <0x0f0 0x009>;
- #address-cells = <0>;
- #size-cells = <0>;
- #interrupt-cells = <2>;
- interrupts = <0x10 0x4 0x11 0x4>; /* cascade */
- interrupt-parent = <&UIC0>;
- };
+&NOR {
+ status = "okay";
- OCM: ocm@400040000 {
- compatible = "ibm,ocm";
- status = "okay";
- cell-index = <1>;
- /* configured in U-Boot */
- reg = <4 0x00040000 0x8000>; /* 32K */
- };
+ compatible = "amd,s29gl512n", "cfi-flash";
+ bank-width = <2>;
+ reg = <0x00000000 0x00000000 0x00400000>;
- SDR0: sdr {
- compatible = "ibm,sdr-apm821xx";
- dcr-reg = <0x00e 0x002>;
+ partition@0 {
+ label = "kernel";
+ reg = <0x00000000 0x00180000>;
};
-
- CPR0: cpr {
- compatible = "ibm,cpr-apm821xx";
- dcr-reg = <0x00c 0x002>;
+ partition@180000 {
+ label = "env";
+ reg = <0x00180000 0x00020000>;
};
-
- L2C0: l2c {
- compatible = "ibm,l2-cache-apm82181", "ibm,l2-cache";
- dcr-reg = <0x020 0x008
- 0x030 0x008>;
- cache-line-size = <32>;
- cache-size = <262144>;
- interrupt-parent = <&UIC1>;
- interrupts = <11 1>;
+ partition@1a0000 {
+ label = "u-boot";
+ reg = <0x001a0000 0x00060000>;
};
+};
- plb {
- compatible = "ibm,plb4";
- #address-cells = <2>;
- #size-cells = <1>;
- ranges;
- clock-frequency = <0>; /* Filled in by U-Boot */
+&NAND {
+ status = "okay";
- SDRAM0: sdram {
- compatible = "ibm,sdram-apm821xx";
- dcr-reg = <0x010 0x002>;
+ /* 2Gb Nand Flash */
+ nand {
+ partition@0 {
+ label = "firmware";
+ reg = <0x00000000 0x00C00000>;
};
-
- MAL0: mcmal {
- compatible = "ibm,mcmal2";
- descriptor-memory = "ocm";
- dcr-reg = <0x180 0x062>;
- num-tx-chans = <1>;
- num-rx-chans = <1>;
- #address-cells = <0>;
- #size-cells = <0>;
- interrupt-parent = <&UIC2>;
- interrupts = < /*TXEOB*/ 0x6 0x4
- /*RXEOB*/ 0x7 0x4
- /*SERR*/ 0x3 0x4
- /*TXDE*/ 0x4 0x4
- /*RXDE*/ 0x5 0x4>;
+ partition@c00000 {
+ label = "environment";
+ reg = <0x00C00000 0x00B00000>;
};
+ partition@1700000 {
+ label = "kernel";
+ reg = <0x01700000 0x00E00000>;
+ };
+ partition@2500000 {
+ label = "root";
+ reg = <0x02500000 0x08200000>;
+ };
+ partition@a700000 {
+ label = "device-tree";
+ reg = <0x0A700000 0x00B00000>;
+ };
+ partition@b200000 {
+ label = "config";
+ reg = <0x0B200000 0x00D00000>;
+ };
+ partition@bf00000 {
+ label = "diag";
+ reg = <0x0BF00000 0x00C00000>;
+ };
+ partition@cb00000 {
+ label = "vendor";
+ reg = <0x0CB00000 0x3500000>;
+ };
+ };
+};
- POB0: opb {
- compatible = "ibm,opb";
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0xb0000000 0x00000004 0xb0000000 0x50000000>;
- clock-frequency = <0>; /* Filled in by U-Boot */
-
- EBC0: ebc {
- compatible = "ibm,ebc";
- dcr-reg = <0x012 0x002>;
- #address-cells = <2>;
- #size-cells = <1>;
- clock-frequency = <0>; /* Filled in by U-Boot */
- /* ranges property is supplied by U-Boot */
- ranges = < 0x00000003 0x00000000 0xe0000000 0x8000000>;
- interrupts = <0x6 0x4>;
- interrupt-parent = <&UIC1>;
-
- nor_flash@0,0 {
- compatible = "amd,s29gl512n", "cfi-flash";
- bank-width = <2>;
- reg = <0x00000000 0x00000000 0x00400000>;
- #address-cells = <1>;
- #size-cells = <1>;
- partition@0 {
- label = "kernel";
- reg = <0x00000000 0x00180000>;
- };
- partition@180000 {
- label = "env";
- reg = <0x00180000 0x00020000>;
- };
- partition@1a0000 {
- label = "u-boot";
- reg = <0x001a0000 0x00060000>;
- };
- };
-
- ndfc@1,0 {
- compatible = "ibm,ndfc";
- reg = <0x00000003 0x00000000 0x00002000>;
- ccr = <0x00001000>;
- bank-settings = <0x80002222>;
- #address-cells = <1>;
- #size-cells = <1>;
- /* 2Gb Nand Flash */
- nand {
- #address-cells = <1>;
- #size-cells = <1>;
-
- partition@0 {
- label = "firmware";
- reg = <0x00000000 0x00C00000>;
- };
- partition@c00000 {
- label = "environment";
- reg = <0x00C00000 0x00B00000>;
- };
- partition@1700000 {
- label = "kernel";
- reg = <0x01700000 0x00E00000>;
- };
- partition@2500000 {
- label = "root";
- reg = <0x02500000 0x08200000>;
- };
- partition@a700000 {
- label = "device-tree";
- reg = <0x0A700000 0x00B00000>;
- };
- partition@b200000 {
- label = "config";
- reg = <0x0B200000 0x00D00000>;
- };
- partition@bf00000 {
- label = "diag";
- reg = <0x0BF00000 0x00C00000>;
- };
- partition@cb00000 {
- label = "vendor";
- reg = <0x0CB00000 0x3500000>;
- };
- };
- };
- };
-
- UART0: serial@ef600300 {
- device_type = "serial";
- compatible = "ns16550";
- reg = <0xef600300 0x00000008>;
- virtual-reg = <0xef600300>;
- clock-frequency = <0>; /* Filled in by U-Boot */
- current-speed = <0>; /* Filled in by U-Boot */
- interrupt-parent = <&UIC1>;
- interrupts = <0x1 0x4>;
- };
-
- UART1: serial@ef600400 {
- device_type = "serial";
- compatible = "ns16550";
- reg = <0xef600400 0x00000008>;
- virtual-reg = <0xef600400>;
- clock-frequency = <0>; /* Filled in by U-Boot */
- current-speed = <0>; /* Filled in by U-Boot */
- interrupt-parent = <&UIC0>;
- interrupts = <0x1 0x4>;
- };
-
- IIC0: i2c@ef600700 {
- compatible = "ibm,iic";
- reg = <0xef600700 0x00000014>;
- interrupt-parent = <&UIC0>;
- interrupts = <0x2 0x4>;
- #address-cells = <1>;
- #size-cells = <0>;
- rtc@68 {
- compatible = "st,m41t80";
- reg = <0x68>;
- interrupt-parent = <&UIC0>;
- interrupts = <0x9 0x8>;
- };
- sttm@4C {
- compatible = "adm,adm1032";
- reg = <0x4C>;
- interrupt-parent = <&UIC1>;
- interrupts = <0x1E 0x8>; /* CPU_THERNAL_L */
- };
- };
-
- IIC1: i2c@ef600800 {
- compatible = "ibm,iic";
- reg = <0xef600800 0x00000014>;
- interrupt-parent = <&UIC0>;
- interrupts = <0x3 0x4>;
- };
+&UART0 {
+ status = "okay";
+};
- RGMII0: emac-rgmii@ef601500 {
- compatible = "ibm,rgmii";
- reg = <0xef601500 0x00000008>;
- has-mdio;
- };
+&UART1 {
+ status = "okay";
+};
- TAH0: emac-tah@ef601350 {
- compatible = "ibm,tah";
- reg = <0xef601350 0x00000030>;
- };
+&IIC0 {
+ status = "okay";
+ rtc@68 {
+ compatible = "st,m41t80";
+ reg = <0x68>;
+ interrupt-parent = <&UIC0>;
+ interrupts = <0x9 0x8>;
+ };
+ sttm@4C {
+ compatible = "adm,adm1032";
+ reg = <0x4C>;
+ interrupt-parent = <&UIC1>;
+ interrupts = <0x1E 0x8>; /* CPU_THERNAL_L */
+ };
+};
- EMAC0: ethernet@ef600c00 {
- device_type = "network";
- compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
- interrupt-parent = <&EMAC0>;
- interrupts = <0x0 0x1>;
- #interrupt-cells = <1>;
- #address-cells = <0>;
- #size-cells = <0>;
- interrupt-map = </*Status*/ 0x0 &UIC2 0x10 0x4
- /*Wake*/ 0x1 &UIC2 0x14 0x4>;
- reg = <0xef600c00 0x000000c4>;
- local-mac-address = [000000000000]; /* Filled in by U-Boot */
- mal-device = <&MAL0>;
- mal-tx-channel = <0>;
- mal-rx-channel = <0>;
- cell-index = <0>;
- max-frame-size = <9000>;
- rx-fifo-size = <16384>;
- tx-fifo-size = <2048>;
- phy-mode = "rgmii";
- phy-map = <0x00000000>;
- rgmii-device = <&RGMII0>;
- rgmii-channel = <0>;
- tah-device = <&TAH0>;
- tah-channel = <0>;
- has-inverted-stacr-oc;
- has-new-stacr-staopc;
- };
- };
+&IIC1 {
+ status = "okay";
+};
- PCIE0: pcie@d00000000 {
- device_type = "pci";
- #interrupt-cells = <1>;
- #size-cells = <2>;
- #address-cells = <3>;
- compatible = "ibm,plb-pciex-apm821xx", "ibm,plb-pciex";
- primary;
- port = <0x0>; /* port number */
- reg = <0x0000000d 0x00000000 0x20000000 /* Config space access */
- 0x0000000c 0x08010000 0x00001000>; /* Registers */
- dcr-reg = <0x100 0x020>;
- sdr-base = <0x300>;
+&RGMII0 {
+ status = "okay";
+};
- /* Outbound ranges, one memory and one IO,
- * later cannot be changed
- */
- ranges = <0x02000000 0x00000000 0x80000000 0x0000000e 0x00000000 0x00000000 0x80000000
- 0x02000000 0x00000000 0x00000000 0x0000000f 0x00000000 0x00000000 0x00100000
- 0x01000000 0x00000000 0x00000000 0x0000000f 0x80000000 0x00000000 0x00010000>;
+&TAH0 {
+ status = "okay";
+};
- /* Inbound 2GB range starting at 0 */
- dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x80000000>;
+&MAL0 {
+ status = "okay";
+};
- /* This drives busses 40 to 0x7f */
- bus-range = <0x40 0x7f>;
+&EMAC0 {
+ status = "okay";
+ local-mac-address = [000000000000]; /* Filled in by U-Boot */
+ phy-mode = "rgmii";
+ phy-map = <0x00000000>;
+};
- /* Legacy interrupts (note the weird polarity, the bridge seems
- * to invert PCIe legacy interrupts).
- * We are de-swizzling here because the numbers are actually for
- * port of the root complex virtual P2P bridge. But I want
- * to avoid putting a node for it in the tree, so the numbers
- * below are basically de-swizzled numbers.
- * The real slot is on idsel 0, so the swizzling is 1:1
- */
- interrupt-map-mask = <0x0 0x0 0x0 0x7>;
- interrupt-map = <
- 0x0 0x0 0x0 0x1 &UIC3 0xc 0x4 /* swizzled int A */
- 0x0 0x0 0x0 0x2 &UIC3 0xd 0x4 /* swizzled int B */
- 0x0 0x0 0x0 0x3 &UIC3 0xe 0x4 /* swizzled int C */
- 0x0 0x0 0x0 0x4 &UIC3 0xf 0x4 /* swizzled int D */>;
- };
+&PCIE0 {
+ status = "okay";
+};
- MSI: ppc4xx-msi@C10000000 {
- compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
- reg = < 0xC 0x10000000 0x100
- 0xC 0x10000000 0x100>;
- sdr-base = <0x36C>;
- msi-data = <0x00004440>;
- msi-mask = <0x0000ffe0>;
- interrupts =<0 1 2 3 4 5 6 7>;
- interrupt-parent = <&MSI>;
- #interrupt-cells = <1>;
- #address-cells = <0>;
- #size-cells = <0>;
- msi-available-ranges = <0x0 0x100>;
- interrupt-map = <
- 0 &UIC3 0x18 1
- 1 &UIC3 0x19 1
- 2 &UIC3 0x1A 1
- 3 &UIC3 0x1B 1
- 4 &UIC3 0x1C 1
- 5 &UIC3 0x1D 1
- 6 &UIC3 0x1E 1
- 7 &UIC3 0x1F 1
- >;
- };
- };
+&MSI {
+ status = "okay";
};
--
2.28.0
^ permalink raw reply related
* [PATCH v3 4/5] powerpc: apm82181: add Meraki MR24 AP
From: Christian Lamparter @ 2020-09-05 22:06 UTC (permalink / raw)
To: linuxppc-dev, devicetree; +Cc: Paul Mackerras, Rob Herring, Chris Blake
In-Reply-To: <cover.1599343429.git.chunkeey@gmail.com>
This patch adds the device-tree definitions for Meraki MR24
Accesspoint devices.
Board: MR24 - Meraki MR24 Cloud Managed Access Point
CPU: APM82181 SoC 800 MHz (PLB=200 OPB=100 EBC=100)
Flash size: 32MiB
RAM Size: 128MiB
Wireless: Atheros AR9380 5.0GHz + Atheros AR9380 2.4GHz
EPHY: 1x Gigabit Atheros AR8035
Ready to go images and install instruction can be found @OpenWrt.
Signed-off-by: Chris Blake <chrisrblake93@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
rfc v1 -> v2:
- use new led naming scheme
- space-vs-tab snafu cleanup
- remove led-aliases (openwrt specific)
- overhauled commit message
v2 -> v3:
- added interrupt-properties legacy pci interrupt signalling
to fix wifi
---
arch/powerpc/boot/dts/meraki-mr24.dts | 237 +++++++++++++++++++++
arch/powerpc/platforms/44x/ppc44x_simple.c | 1 +
2 files changed, 238 insertions(+)
create mode 100644 arch/powerpc/boot/dts/meraki-mr24.dts
diff --git a/arch/powerpc/boot/dts/meraki-mr24.dts b/arch/powerpc/boot/dts/meraki-mr24.dts
new file mode 100644
index 000000000000..f91c243e7678
--- /dev/null
+++ b/arch/powerpc/boot/dts/meraki-mr24.dts
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device Tree Source for Meraki MR24 (Ikarem)
+ *
+ * Copyright (C) 2016 Chris Blake <chrisrblake93@gmail.com>
+ *
+ * Based on Cisco Meraki GPL Release r23-20150601 MR24 DTS
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/leds/common.h>
+#include "apm82181.dtsi"
+
+/ {
+ model = "Meraki MR24 Access Point";
+ compatible = "meraki,mr24";
+
+ aliases {
+ serial0 = &UART1;
+ };
+
+ chosen {
+ stdout-path = "/plb/opb/serial@ef600400";
+ };
+};
+
+&CRYPTO {
+ status = "okay";
+};
+
+&HWRNG {
+ status = "okay";
+};
+
+&NAND {
+ status = "okay";
+
+ /* 32 MiB NAND Flash */
+ nand {
+ partition@0 {
+ label = "u-boot";
+ reg = <0x00000000 0x00150000>;
+ read-only;
+ };
+
+ partition@150000 {
+ /*
+ * The u-boot environment size is one NAND
+ * block (16KiB). u-boot allocates four NAND
+ * blocks (64KiB) in order to have spares
+ * around for bad block management
+ */
+ label = "u-boot-env";
+ reg = <0x00150000 0x00010000>;
+ read-only;
+ };
+
+ partition@160000 {
+ /*
+ * redundant u-boot environment.
+ * has to be kept it in sync with the
+ * data in "u-boot-env".
+ */
+ label = "u-boot-env-redundant";
+ reg = <0x00160000 0x00010000>;
+ read-only;
+ };
+
+ partition@170000 {
+ label = "oops";
+ reg = <0x00170000 0x00010000>;
+ };
+
+ partition@180000 {
+ label = "ubi";
+ reg = <0x00180000 0x01e80000>;
+ };
+ };
+};
+
+&UART1 {
+ status = "okay";
+};
+
+&GPIO0 {
+ status = "okay";
+};
+
+&IIC0 {
+ status = "okay";
+ /* Boot ROM is at 0x52-0x53, do not touch */
+ /* Unknown chip at 0x6e, not sure what it is */
+};
+
+&EMAC0 {
+ status = "okay";
+
+ phy-mode = "rgmii-id";
+ phy-map = <0x2>;
+ phy-address = <0x1>;
+ phy-handle = <&phy>;
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy: phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+ };
+};
+
+&POB0 {
+ leds {
+ compatible = "gpio-leds";
+
+ status: power-green {
+ function = LED_FUNCTION_POWER;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 18 GPIO_ACTIVE_LOW>;
+ };
+
+ failsafe: power-amber {
+ function = LED_FUNCTION_FAULT;
+ color = <LED_COLOR_ID_AMBER>;
+ gpios = <&GPIO0 19 GPIO_ACTIVE_LOW>;
+ };
+
+ lan {
+ function = LED_FUNCTION_WAN;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 17 GPIO_ACTIVE_LOW>;
+ };
+
+ /* signal strength indicator */
+ ssi-0 {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 23 GPIO_ACTIVE_LOW>;
+ };
+
+ ssi-1 {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 22 GPIO_ACTIVE_LOW>;
+ };
+
+ ssi-2 {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 21 GPIO_ACTIVE_LOW>;
+ };
+
+ ssi-3 {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&GPIO0 20 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ keys {
+ compatible = "gpio-keys";
+
+ reset {
+ /* Label as per Meraki's "MR24 Installation Guide" */
+ label = "Factory Reset Button";
+ linux,code = <KEY_RESTART>;
+ interrupt-parent = <&UIC1>;
+ interrupts = <0x15 IRQ_TYPE_EDGE_FALLING>;
+ gpios = <&GPIO0 16 GPIO_ACTIVE_LOW>;
+ debounce-interval = <60>;
+ };
+ };
+};
+
+&PCIE0 {
+ status = "okay";
+ /*
+ * relevant lspci topology:
+ *
+ * -+-[0000:40]---00.0-[41-7f]----00.0-[42-45]--+-02.0-[43]----00.0
+ * +-03.0-[44]----00.0
+ *
+ */
+
+ bridge@40,0 {
+ reg = <0x00400000 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ bridge@41,0 {
+ /* IDT PES3T3 PCI Express Switch */
+ compatible = "pci111d,8039";
+ reg = <0x00410000 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ bridge@42,2 {
+ compatible = "pci111d,8039";
+ reg = <0x00421000 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ wifi0: wifi@43,0 {
+ /* Atheros AR9380 2.4GHz */
+ compatible = "pci168c,0030";
+ reg = <0x00430000 0 0 0 0>;
+ interrupts = <3>; /* INTC */
+ };
+ };
+
+ bridge@42,3 {
+ compatible = "pci111d,8039";
+ reg = <0x00421800 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
+
+ wifi1: wifi@44,0 {
+ /* Atheros AR9380 5GHz */
+ compatible = "pci168c,0030";
+ reg = <0x00440000 0 0 0 0>;
+ interrupts = <4>; /* INTD */
+ };
+ };
+ };
+ };
+};
+
+&MSI {
+ status = "okay";
+};
diff --git a/arch/powerpc/platforms/44x/ppc44x_simple.c b/arch/powerpc/platforms/44x/ppc44x_simple.c
index 1122702c804a..7d479928fd48 100644
--- a/arch/powerpc/platforms/44x/ppc44x_simple.c
+++ b/arch/powerpc/platforms/44x/ppc44x_simple.c
@@ -60,6 +60,7 @@ static char *board[] __initdata = {
"amcc,taishan",
"amcc,yosemite",
"mosaixtech,icon",
+ "meraki,mr24",
"wd,mybooklive",
};
--
2.28.0
^ permalink raw reply related
* [PATCH -next] powerpc/eeh: fix compile warning with CONFIG_PROC_FS=n
From: Yang Yingliang @ 2020-09-05 11:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel; +Cc: yangyingliang, oohall
Fix the compile warning:
arch/powerpc/kernel/eeh.c:1639:12: error: 'proc_eeh_show' defined but not used [-Werror=unused-function]
static int proc_eeh_show(struct seq_file *m, void *v)
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/powerpc/kernel/eeh.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 94682382fc8c..420c3c25c6e7 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1636,6 +1636,7 @@ int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
}
EXPORT_SYMBOL_GPL(eeh_pe_inject_err);
+#ifdef CONFIG_PROC_FS
static int proc_eeh_show(struct seq_file *m, void *v)
{
if (!eeh_enabled()) {
@@ -1662,6 +1663,7 @@ static int proc_eeh_show(struct seq_file *m, void *v)
return 0;
}
+#endif
#ifdef CONFIG_DEBUG_FS
static int eeh_enable_dbgfs_set(void *data, u64 val)
--
2.25.1
^ permalink raw reply related
* [PATCH -next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n
From: Yang Yingliang @ 2020-09-05 11:25 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel; +Cc: yangyingliang
Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
powerpc64-linux-gnu-ld: arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference to `mmu_pid_bits'
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++
arch/powerpc/platforms/pseries/lpar.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
index 0ba30b8b935b..a8e292cd88f0 100644
--- a/arch/powerpc/mm/book3s64/mmu_context.c
+++ b/arch/powerpc/mm/book3s64/mmu_context.c
@@ -152,6 +152,7 @@ void hash__setup_new_exec(void)
static int radix__init_new_context(struct mm_struct *mm)
{
+#ifdef CONFIG_PPC_RADIX_MMU
unsigned long rts_field;
int index, max_id;
@@ -177,6 +178,9 @@ static int radix__init_new_context(struct mm_struct *mm)
mm->context.hash_context = NULL;
return index;
+#else
+ return -ENOTSUPP;
+#endif
}
int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index baf24eacd268..e454e218dbba 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -1726,10 +1726,12 @@ void __init hpte_init_pseries(void)
void radix_init_pseries(void)
{
+#ifdef CONFIG_PPC_RADIX_MMU
pr_info("Using radix MMU under hypervisor\n");
pseries_lpar_register_process_table(__pa(process_tb),
0, PRTB_SIZE_SHIFT - 12);
+#endif
}
#ifdef CONFIG_PPC_SMLPAR
--
2.25.1
^ permalink raw reply related
* Re: [PATCH -next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n
From: Christophe Leroy @ 2020-09-06 6:50 UTC (permalink / raw)
To: Yang Yingliang, linuxppc-dev, linux-kernel
In-Reply-To: <20200905112548.3265530-1-yangyingliang@huawei.com>
Le 05/09/2020 à 13:25, Yang Yingliang a écrit :
> Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
> powerpc64-linux-gnu-ld: arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference to `mmu_pid_bits'
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++
In your commit log, you are just mentionning
arch/powerpc/platforms/pseries/lpar.o, which is right.
You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at
all, see below.
> arch/powerpc/platforms/pseries/lpar.c | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c
> index 0ba30b8b935b..a8e292cd88f0 100644
> --- a/arch/powerpc/mm/book3s64/mmu_context.c
> +++ b/arch/powerpc/mm/book3s64/mmu_context.c
> @@ -152,6 +152,7 @@ void hash__setup_new_exec(void)
>
> static int radix__init_new_context(struct mm_struct *mm)
> {
> +#ifdef CONFIG_PPC_RADIX_MMU
This shouldn't be required. radix__init_new_context() is only called
when radix_enabled() returns true.
As it is a static function, when it is not called it gets optimised
away, so you will never get an undefined reference to `mmu_pid_bits` there.
> unsigned long rts_field;
> int index, max_id;
>
> @@ -177,6 +178,9 @@ static int radix__init_new_context(struct mm_struct *mm)
> mm->context.hash_context = NULL;
>
> return index;
> +#else
> + return -ENOTSUPP;
> +#endif
> }
>
> int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index baf24eacd268..e454e218dbba 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -1726,10 +1726,12 @@ void __init hpte_init_pseries(void)
>
> void radix_init_pseries(void)
> {
> +#ifdef CONFIG_PPC_RADIX_MMU
This function is only called from
/arch/powerpc/mm/book3s64/radix_pgtable.c which is only built when
CONFIG_PPC_RADIX_MMU is selected.
So the entire function should be encloded in the #ifdef.
> pr_info("Using radix MMU under hypervisor\n");
>
> pseries_lpar_register_process_table(__pa(process_tb),
> 0, PRTB_SIZE_SHIFT - 12);
> +#endif
> }
>
> #ifdef CONFIG_PPC_SMLPAR
>
Christophe
^ permalink raw reply
* Re: [PATCH -next] powerpc/eeh: fix compile warning with CONFIG_PROC_FS=n
From: Christophe Leroy @ 2020-09-06 7:01 UTC (permalink / raw)
To: Yang Yingliang, linuxppc-dev, linux-kernel, linux-fsdevel,
Alexey Dobriyan
Cc: oohall
In-Reply-To: <20200905111749.3198998-1-yangyingliang@huawei.com>
Le 05/09/2020 à 13:17, Yang Yingliang a écrit :
> Fix the compile warning:
>
> arch/powerpc/kernel/eeh.c:1639:12: error: 'proc_eeh_show' defined but not used [-Werror=unused-function]
> static int proc_eeh_show(struct seq_file *m, void *v)
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> arch/powerpc/kernel/eeh.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 94682382fc8c..420c3c25c6e7 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1636,6 +1636,7 @@ int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
> }
> EXPORT_SYMBOL_GPL(eeh_pe_inject_err);
>
> +#ifdef CONFIG_PROC_FS
I don't like this way of fixing the issue, because you'll get an
unbalanced source code: proc_eeh_show() is apparently referenced all the
time in eeh_init_proc(), but because proc_create_single() is a NULL
macro when CONFIG_PROC_FS is not selected, you get the 'unused function'
error.
I think the right fix should be to rewrite proc_create_single() as a
static inline function that calls proc_create_single_data() when
CONFIG_PROC_FS is selected and just returns NULL otherwise.
> static int proc_eeh_show(struct seq_file *m, void *v)
> {
> if (!eeh_enabled()) {
> @@ -1662,6 +1663,7 @@ static int proc_eeh_show(struct seq_file *m, void *v)
>
> return 0;
> }
> +#endif
>
> #ifdef CONFIG_DEBUG_FS
> static int eeh_enable_dbgfs_set(void *data, u64 val)
>
Christophe
^ permalink raw reply
* Re: [RFC PATCH 12/12] powerpc/64s: power4 nap fixup in C
From: Christophe Leroy @ 2020-09-06 7:32 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200905174335.3161229-13-npiggin@gmail.com>
Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
> There is no need for this to be in asm, use the new intrrupt entry wrapper.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 14 ++++++++
> arch/powerpc/include/asm/processor.h | 1 +
> arch/powerpc/include/asm/thread_info.h | 6 ++++
> arch/powerpc/kernel/exceptions-64s.S | 45 --------------------------
> arch/powerpc/kernel/idle_book3s.S | 4 +++
> 5 files changed, 25 insertions(+), 45 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 3ae3d2f93b61..acfcc7d5779b 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -8,6 +8,16 @@
> #include <asm/ftrace.h>
> #include <asm/runlatch.h>
>
> +static inline void nap_adjust_return(struct pt_regs *regs)
> +{
> +#ifdef CONFIG_PPC_970_NAP
Avoid #ifdef, you can use IS_ENABLED(CONFIG_PPC_970_NAP) in the 'if' below
> + if (test_thread_local_flags(_TLF_NAPPING)) {
> + clear_thread_local_flags(_TLF_NAPPING);
> + regs->nip = (unsigned long)power4_idle_nap_return;
> + }
> +#endif
> +}
> +
> #ifdef CONFIG_PPC_BOOK3S_64
> static inline void interrupt_enter_prepare(struct pt_regs *regs)
> {
> @@ -33,6 +43,8 @@ static inline void interrupt_async_enter_prepare(struct pt_regs *regs)
> if (cpu_has_feature(CPU_FTR_CTRL) &&
> !test_thread_local_flags(_TLF_RUNLATCH))
> __ppc64_runlatch_on();
> +
> + nap_adjust_return(regs);
> }
>
> #else /* CONFIG_PPC_BOOK3S_64 */
> @@ -72,6 +84,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
>
> this_cpu_set_ftrace_enabled(0);
>
> + nap_adjust_return(regs);
> +
> nmi_enter();
> }
>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index ed0d633ab5aa..3da1dba91386 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -424,6 +424,7 @@ extern unsigned long isa300_idle_stop_mayloss(unsigned long psscr_val);
> extern unsigned long isa206_idle_insn_mayloss(unsigned long type);
> #ifdef CONFIG_PPC_970_NAP
> extern void power4_idle_nap(void);
> +extern void power4_idle_nap_return(void);
Please please please, 'extern' keyword is pointless and deprecated for
function prototypes. Don't add new ones.
Also, put it outside the #ifdef, so that you can use IS_ENABLED()
instead of #ifdef when using it.
> #endif
>
> extern unsigned long cpuidle_disable;
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index ca6c97025704..9b15f7edb0cb 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -156,6 +156,12 @@ void arch_setup_new_exec(void);
>
> #ifndef __ASSEMBLY__
>
> +static inline void clear_thread_local_flags(unsigned int flags)
> +{
> + struct thread_info *ti = current_thread_info();
> + ti->local_flags &= ~flags;
> +}
> +
> static inline bool test_thread_local_flags(unsigned int flags)
> {
> struct thread_info *ti = current_thread_info();
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 227bad3a586d..1db6b3438c88 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -692,25 +692,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> ld r1,GPR1(r1)
> .endm
>
> -/*
> - * When the idle code in power4_idle puts the CPU into NAP mode,
> - * it has to do so in a loop, and relies on the external interrupt
> - * and decrementer interrupt entry code to get it out of the loop.
> - * It sets the _TLF_NAPPING bit in current_thread_info()->local_flags
> - * to signal that it is in the loop and needs help to get out.
> - */
> -#ifdef CONFIG_PPC_970_NAP
> -#define FINISH_NAP \
> -BEGIN_FTR_SECTION \
> - ld r11, PACA_THREAD_INFO(r13); \
> - ld r9,TI_LOCAL_FLAGS(r11); \
> - andi. r10,r9,_TLF_NAPPING; \
> - bnel power4_fixup_nap; \
> -END_FTR_SECTION_IFSET(CPU_FTR_CAN_NAP)
> -#else
> -#define FINISH_NAP
> -#endif
> -
> /*
> * There are a few constraints to be concerned with.
> * - Real mode exceptions code/data must be located at their physical location.
> @@ -1250,7 +1231,6 @@ EXC_COMMON_BEGIN(machine_check_common)
> */
> GEN_COMMON machine_check
>
> - FINISH_NAP
> /* Enable MSR_RI when finished with PACA_EXMC */
> li r10,MSR_RI
> mtmsrd r10,1
> @@ -1572,7 +1552,6 @@ EXC_VIRT_BEGIN(hardware_interrupt, 0x4500, 0x100)
> EXC_VIRT_END(hardware_interrupt, 0x4500, 0x100)
> EXC_COMMON_BEGIN(hardware_interrupt_common)
> GEN_COMMON hardware_interrupt
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl do_IRQ
> b interrupt_return
> @@ -1757,7 +1736,6 @@ EXC_VIRT_BEGIN(decrementer, 0x4900, 0x80)
> EXC_VIRT_END(decrementer, 0x4900, 0x80)
> EXC_COMMON_BEGIN(decrementer_common)
> GEN_COMMON decrementer
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl timer_interrupt
> b interrupt_return
> @@ -1842,7 +1820,6 @@ EXC_VIRT_BEGIN(doorbell_super, 0x4a00, 0x100)
> EXC_VIRT_END(doorbell_super, 0x4a00, 0x100)
> EXC_COMMON_BEGIN(doorbell_super_common)
> GEN_COMMON doorbell_super
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> #ifdef CONFIG_PPC_DOORBELL
> bl doorbell_exception
> @@ -2196,7 +2173,6 @@ EXC_COMMON_BEGIN(hmi_exception_early_common)
>
> EXC_COMMON_BEGIN(hmi_exception_common)
> GEN_COMMON hmi_exception
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl handle_hmi_exception
> b interrupt_return
> @@ -2225,7 +2201,6 @@ EXC_VIRT_BEGIN(h_doorbell, 0x4e80, 0x20)
> EXC_VIRT_END(h_doorbell, 0x4e80, 0x20)
> EXC_COMMON_BEGIN(h_doorbell_common)
> GEN_COMMON h_doorbell
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> #ifdef CONFIG_PPC_DOORBELL
> bl doorbell_exception
> @@ -2258,7 +2233,6 @@ EXC_VIRT_BEGIN(h_virt_irq, 0x4ea0, 0x20)
> EXC_VIRT_END(h_virt_irq, 0x4ea0, 0x20)
> EXC_COMMON_BEGIN(h_virt_irq_common)
> GEN_COMMON h_virt_irq
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl do_IRQ
> b interrupt_return
> @@ -2304,7 +2278,6 @@ EXC_VIRT_BEGIN(performance_monitor, 0x4f00, 0x20)
> EXC_VIRT_END(performance_monitor, 0x4f00, 0x20)
> EXC_COMMON_BEGIN(performance_monitor_common)
> GEN_COMMON performance_monitor
> - FINISH_NAP
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl performance_monitor_exception
> b interrupt_return
> @@ -3032,24 +3005,6 @@ USE_FIXED_SECTION(virt_trampolines)
> __end_interrupts:
> DEFINE_FIXED_SYMBOL(__end_interrupts)
>
> -#ifdef CONFIG_PPC_970_NAP
> - /*
> - * Called by exception entry code if _TLF_NAPPING was set, this clears
> - * the NAPPING flag, and redirects the exception exit to
> - * power4_fixup_nap_return.
> - */
> - .globl power4_fixup_nap
> -EXC_COMMON_BEGIN(power4_fixup_nap)
> - andc r9,r9,r10
> - std r9,TI_LOCAL_FLAGS(r11)
> - LOAD_REG_ADDR(r10, power4_idle_nap_return)
> - std r10,_NIP(r1)
> - blr
> -
> -power4_idle_nap_return:
> - blr
> -#endif
> -
> CLOSE_FIXED_SECTION(real_vectors);
> CLOSE_FIXED_SECTION(real_trampolines);
> CLOSE_FIXED_SECTION(virt_vectors);
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 22f249b6f58d..27d2e6a72ec9 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -201,4 +201,8 @@ _GLOBAL(power4_idle_nap)
> mtmsrd r7
> isync
> b 1b
> +
> + .globl power4_idle_nap_return
> +power4_idle_nap_return:
> + blr
Can't this be written in C somewhere ?
> #endif
>
Christophe
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-09-06 21:03 UTC (permalink / raw)
To: Heiner Kallweit
Cc: linux-kernel@vger.kernel.org, Nicholas Piggin,
linux-spi@vger.kernel.org, broonie@kernel.org, paulus@samba.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAFSsGVvRMQoEoBN1hpao_4uM1yF2wwuKPbMkAcwFWyE1X8HDbQ@mail.gmail.com>
(resend as something decided to insert html, some context stripped)
On 5/09/20 5:23 am, Heiner Kallweit wrote:
> On Fri 4. Sep 2020 at 01:58, Chris Packham
> <Chris.Packham@alliedtelesis.co.nz
> <mailto:Chris.Packham@alliedtelesis.co.nz>> wrote:
>
>
<snip>
>
>
> I tried ftrace but I really wasn't sure what I was looking for.
>
> Capturing a "bad" case was pretty tricky. But I think I've
> identified a
>
> fix (I'll send it as a proper patch shortly). The gist is
>
>
>
> diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
>
> index 7e7c92cafdbb..cb120b68c0e2 100644
>
> --- a/drivers/spi/spi-fsl-espi.c
>
> +++ b/drivers/spi/spi-fsl-espi.c
>
> @@ -574,13 +574,14 @@ static void fsl_espi_cpu_irq(struct fsl_espi
>
> *espi, u32 events)
>
> static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
>
> {
>
> struct fsl_espi *espi = context_data;
>
> - u32 events;
>
> + u32 events, mask;
>
>
>
> spin_lock(&espi->lock);
>
>
>
> /* Get interrupt events(tx/rx) */
>
> events = fsl_espi_read_reg(espi, ESPI_SPIE);
>
> - if (!events) {
>
> + mask = fsl_espi_read_reg(espi, ESPI_SPIM);
>
> + if (!(events & mask)) {
>
> spin_unlock(&espi->lock);
>
> return IRQ_NONE;
>
> }
>
>
>
> The SPIE register contains the TXCNT so events is pretty much always
>
> going to have something set. By checking events against what we've
>
> actually requested interrupts for we don't see any spurious events.
>
>
> Usually we shouldn’t receive interrupts we’re not interested in,
> except the interrupt is shared.
My theory is that we get an interrupt to the core for RXT and another
for DON. With the changes to the powerpc interrupt handling and the fact
that fsl_espi_cpu_irq() doesn't actually look at the specific event bits
means that sometimes both events are handled in the processing of the
first interrupt and the second one trips the SPI_DON not set error.
There's an old comment "SPI bus sometimes got lost interrupts..." which
makes me wonder if the interrupt handling change has fixed this original
problem.
I still think the change makes sense regardless because the SPIE
register has some counter fields in it.
> This leads to the question: is the SPI interrupt shared with another
> device on your system?
It's not shared on either the custom board or the T2080RDB.
> Do you see spurious interrupts with the patch under
> /proc/irq/(irq)/spurious?
Yes it looks like it
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 3126
unhandled 0
last_unhandled 0 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 1016
unhandled 0
last_unhandled 4294746100 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 88391
unhandled 0
last_unhandled 4294746100 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 72459
unhandled 2
last_unhandled 4294758632 ms
>
>
>
> I've tested this on the T2080RDB and on our custom hardware and it
> seems
>
> to resolve the problem.
>
>
>
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-09-06 20:59 UTC (permalink / raw)
To: Heiner Kallweit
Cc: linux-kernel@vger.kernel.org, Nicholas Piggin,
linux-spi@vger.kernel.org, broonie@kernel.org, paulus@samba.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAFSsGVvRMQoEoBN1hpao_4uM1yF2wwuKPbMkAcwFWyE1X8HDbQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11616 bytes --]
On 5/09/20 5:23 am, Heiner Kallweit wrote:
On Fri 4. Sep 2020 at 01:58, Chris Packham <Chris.Packham@alliedtelesis.co.nz<mailto:Chris.Packham@alliedtelesis.co.nz>> wrote:
On 1/09/20 6:14 pm, Nicholas Piggin wrote:
> Excerpts from Chris Packham's message of September 1, 2020 11:25 am:
>> On 1/09/20 12:33 am, Heiner Kallweit wrote:
>>> On 30.08.2020 23:59, Chris Packham wrote:
>>>> On 31/08/20 9:41 am, Heiner Kallweit wrote:
>>>>> On 30.08.2020 23:00, Chris Packham wrote:
>>>>>> On 31/08/20 12:30 am, Nicholas Piggin wrote:
>>>>>>> Excerpts from Chris Packham's message of August 28, 2020 8:07 am:
>>>>>> <snip>
>>>>>>
>>>>>>>>>>>> I've also now seen the RX FIFO not empty error on the T2080RDB
>>>>>>>>>>>>
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>>>>>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>>>>>>>>>
>>>>>>>>>>>> With my current workaround of emptying the RX FIFO. It seems
>>>>>>>>>>>> survivable. Interestingly it only ever seems to be 1 extra byte in the
>>>>>>>>>>>> RX FIFO and it seems to be after either a READ_SR or a READ_FSR.
>>>>>>>>>>>>
>>>>>>>>>>>> fsl_espi ffe110000.spi: tx 70
>>>>>>>>>>>> fsl_espi ffe110000.spi: rx 03
>>>>>>>>>>>> fsl_espi ffe110000.spi: Extra RX 00
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>>>>>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>>>>>>>>> fsl_espi ffe110000.spi: tx 05
>>>>>>>>>>>> fsl_espi ffe110000.spi: rx 00
>>>>>>>>>>>> fsl_espi ffe110000.spi: Extra RX 03
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>>>>>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>>>>>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>>>>>>>>> fsl_espi ffe110000.spi: tx 05
>>>>>>>>>>>> fsl_espi ffe110000.spi: rx 00
>>>>>>>>>>>> fsl_espi ffe110000.spi: Extra RX 03
>>>>>>>>>>>>
>>>>>>>>>>>> From all the Micron SPI-NOR datasheets I've got access to it is
>>>>>>>>>>>> possible to continually read the SR/FSR. But I've no idea why it
>>>>>>>>>>>> happens some times and not others.
>>>>>>>>>>> So I think I've got a reproduction and I think I've bisected the problem
>>>>>>>>>>> to commit 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in
>>>>>>>>>>> C"). My day is just finishing now so I haven't applied too much scrutiny
>>>>>>>>>>> to this result. Given the various rabbit holes I've been down on this
>>>>>>>>>>> issue already I'd take this information with a good degree of skepticism.
>>>>>>>>>>>
>>>>>>>>>> OK, so an easy test should be to re-test with a 5.4 kernel.
>>>>>>>>>> It doesn't have yet the change you're referring to, and the fsl-espi driver
>>>>>>>>>> is basically the same as in 5.7 (just two small changes in 5.7).
>>>>>>>>> There's 6cc0c16d82f88 and maybe also other interrupt related patches
>>>>>>>>> around this time that could affect book E, so it's good if that exact
>>>>>>>>> patch is confirmed.
>>>>>>>> My confirmation is basically that I can induce the issue in a 5.4 kernel
>>>>>>>> by cherry-picking 3282a3da25bd. I'm also able to "fix" the issue in
>>>>>>>> 5.9-rc2 by reverting that one commit.
>>>>>>>>
>>>>>>>> I both cases it's not exactly a clean cherry-pick/revert so I also
>>>>>>>> confirmed the bisection result by building at 3282a3da25bd (which sees
>>>>>>>> the issue) and the commit just before (which does not).
>>>>>>> Thanks for testing, that confirms it well.
>>>>>>>
>>>>>>> [snip patch]
>>>>>>>
>>>>>>>> I still saw the issue with this change applied. PPC_IRQ_SOFT_MASK_DEBUG
>>>>>>>> didn't report anything (either with or without the change above).
>>>>>>> Okay, it was a bit of a shot in the dark. I still can't see what
>>>>>>> else has changed.
>>>>>>>
>>>>>>> What would cause this, a lost interrupt? A spurious interrupt? Or
>>>>>>> higher interrupt latency?
>>>>>>>
>>>>>>> I don't think the patch should cause significantly worse latency,
>>>>>>> (it's supposed to be a bit better if anything because it doesn't set
>>>>>>> up the full interrupt frame). But it's possible.
>>>>>> My working theory is that the SPI_DON indication is all about the TX
>>>>>> direction an now that the interrupts are faster we're hitting an error
>>>>>> because there is still RX activity going on. Heiner disagrees with my
>>>>>> interpretation of the SPI_DON indication and the fact that it doesn't
>>>>>> happen every time does throw doubt on it.
>>>>>>
>>>>> It's right that the eSPI spec can be interpreted that SPI_DON refers to
>>>>> TX only. However this wouldn't really make sense, because also for RX
>>>>> we program the frame length, and therefore want to be notified once the
>>>>> full frame was received. Also practical experience shows that SPI_DON
>>>>> is set also after RX-only transfers.
>>>>> Typical SPI NOR use case is that you write read command + start address,
>>>>> followed by a longer read. If the TX-only interpretation would be right,
>>>>> we'd always end up with SPI_DON not being set.
>>>>>
>>>>>> I can't really explain the extra RX byte in the fifo. We know how many
>>>>>> bytes to expect and we pull that many from the fifo so it's not as if
>>>>>> we're missing an interrupt causing us to skip the last byte. I've been
>>>>>> looking for some kind of off-by-one calculation but again if it were
>>>>>> something like that it'd happen all the time.
>>>>>>
>>>>> Maybe it helps to know what value this extra byte in the FIFO has. Is it:
>>>>> - a duplicate of the last read byte
>>>>> - or the next byte (at <end address> + 1)
>>>>> - or a fixed value, e.g. always 0x00 or 0xff
>>>> The values were up thread a bit but I'll repeat them here
>>>>
>>>> fsl_espi ffe110000.spi: tx 70
>>>> fsl_espi ffe110000.spi: rx 03
>>>> fsl_espi ffe110000.spi: Extra RX 00
>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>> fsl_espi ffe110000.spi: tx 05
>>>> fsl_espi ffe110000.spi: rx 00
>>>> fsl_espi ffe110000.spi: Extra RX 03
>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>> fsl_espi ffe110000.spi: tx 05
>>>> fsl_espi ffe110000.spi: rx 00
>>>> fsl_espi ffe110000.spi: Extra RX 03
>>>>
>>>>
>>>> The rx 00 Extra RX 03 is a bit concerning. I've only ever seen them with
>>>> either a READ_SR or a READ_FSR. Never a data read.
>>>>
>>> Just remembered something about SPIE_DON:
>>> Transfers are always full duplex, therefore in case of a read the chip
>>> sends dummy zero's. Having said that in case of a read SPIE_DON means
>>> that the last dummy zero was shifted out.
>>>
>>> READ_SR and READ_FSR are the shortest transfers, 1 byte out and 1 byte in.
>>> So the issue may have a dependency on the length of the transfer.
>>> However I see no good explanation so far. You can try adding a delay of
>>> a few miroseconds between the following to commands in fsl_espi_bufs().
>>>
>>> fsl_espi_write_reg(espi, ESPI_SPIM, mask);
>>>
>>> /* Prevent filling the fifo from getting interrupted */
>>> spin_lock_irq(&espi->lock);
>>>
>>> Maybe enabling interrupts and seeing the SPIE_DON interrupt are too close.
>> I think this might be heading in the right direction. Playing about with
>> a delay does seem to make the two symptoms less likely. Although I have
>> to set it quite high (i.e. msleep(100)) to completely avoid any
>> possibility of seeing either message.
> The patch might replay the interrupt a little bit faster, but it would
> be a few microseconds at most I think (just from improved code).
>
> Would you be able to ftrace the interrupt handler function and see if you
> can see a difference in number or timing of interrupts? I'm at a bit of
> a loss.
I tried ftrace but I really wasn't sure what I was looking for.
Capturing a "bad" case was pretty tricky. But I think I've identified a
fix (I'll send it as a proper patch shortly). The gist is
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7e7c92cafdbb..cb120b68c0e2 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -574,13 +574,14 @@ static void fsl_espi_cpu_irq(struct fsl_espi
*espi, u32 events)
static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
{
struct fsl_espi *espi = context_data;
- u32 events;
+ u32 events, mask;
spin_lock(&espi->lock);
/* Get interrupt events(tx/rx) */
events = fsl_espi_read_reg(espi, ESPI_SPIE);
- if (!events) {
+ mask = fsl_espi_read_reg(espi, ESPI_SPIM);
+ if (!(events & mask)) {
spin_unlock(&espi->lock);
return IRQ_NONE;
}
The SPIE register contains the TXCNT so events is pretty much always
going to have something set. By checking events against what we've
actually requested interrupts for we don't see any spurious events.
Usually we shouldn’t receive interrupts we’re not interested in, except the interrupt is shared.
My theory is that we get an interrupt to the core for RXT and another for DON. With the changes to the powerpc interrupt handling and the fact that fsl_espi_cpu_irq() doesn't actually look at the specific event bits means that sometimes both events are handled in the processing of the first interrupt and the second one trips the SPI_DON not set error.
There's an old comment "SPI bus sometimes got lost interrupts..." which makes me wonder if the interrupt handling change has fixed this original problem.
I still think the change makes sense regardless because the SPIE register has some counter fields in it.
This leads to the question: is the SPI interrupt shared with another device on your system?
It's not shared on either the custom board or the T2080RDB.
Do you see spurious interrupts with the patch under /proc/irq/(irq)/spurious?
Yes it looks like it
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 3126
unhandled 0
last_unhandled 0 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 1016
unhandled 0
last_unhandled 4294746100 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 88391
unhandled 0
last_unhandled 4294746100 ms
[root@linuxbox ~]# /flash/dd_test.sh
[root@linuxbox ~]# cat /proc/irq/53/spurious
count 72459
unhandled 2
last_unhandled 4294758632 ms
I've tested this on the T2080RDB and on our custom hardware and it seems
to resolve the problem.
[-- Attachment #2: Type: text/html, Size: 18247 bytes --]
^ permalink raw reply
* Re: [PATCH v3 4/6] powerpc: Introduce temporary mm
From: Christopher M. Riedl @ 2020-09-07 0:15 UTC (permalink / raw)
To: Jann Horn; +Cc: linuxppc-dev, Kernel Hardening
In-Reply-To: <CAG48ez1W7FcDPAnqQ7TpSnKy--vaQm_f5prsZXRxcybzGg0tpg@mail.gmail.com>
On Thu Aug 27, 2020 at 11:15 AM CDT, Jann Horn wrote:
> On Thu, Aug 27, 2020 at 7:24 AM Christopher M. Riedl <cmr@codefail.de>
> wrote:
> > x86 supports the notion of a temporary mm which restricts access to
> > temporary PTEs to a single CPU. A temporary mm is useful for situations
> > where a CPU needs to perform sensitive operations (such as patching a
> > STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
> > said mappings to other CPUs. A side benefit is that other CPU TLBs do
> > not need to be flushed when the temporary mm is torn down.
> >
> > Mappings in the temporary mm can be set in the userspace portion of the
> > address-space.
> [...]
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> [...]
> > @@ -44,6 +45,70 @@ int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
> > }
> >
> > #ifdef CONFIG_STRICT_KERNEL_RWX
> > +
> > +struct temp_mm {
> > + struct mm_struct *temp;
> > + struct mm_struct *prev;
> > + bool is_kernel_thread;
> > + struct arch_hw_breakpoint brk[HBP_NUM_MAX];
> > +};
> > +
> > +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
> > +{
> > + temp_mm->temp = mm;
> > + temp_mm->prev = NULL;
> > + temp_mm->is_kernel_thread = false;
> > + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
> > +}
> > +
> > +static inline void use_temporary_mm(struct temp_mm *temp_mm)
> > +{
> > + lockdep_assert_irqs_disabled();
> > +
> > + temp_mm->is_kernel_thread = current->mm == NULL;
>
> (That's a somewhat misleading variable name - kernel threads can have
> a non-NULL ->mm, too.)
>
Oh I didn't know that, in that case yes this is not a good name. I am
considering some changes (based on your comments about current->mm
below) which would make this variable superfluous.
> > + if (temp_mm->is_kernel_thread)
> > + temp_mm->prev = current->active_mm;
> > + else
> > + temp_mm->prev = current->mm;
>
> Why the branch? Shouldn't current->active_mm work in both cases?
>
>
Yes you are correct.
> > + /*
> > + * Hash requires a non-NULL current->mm to allocate a userspace address
> > + * when handling a page fault. Does not appear to hurt in Radix either.
> > + */
> > + current->mm = temp_mm->temp;
>
> This looks dangerous to me. There are various places that attempt to
> find all userspace tasks that use a given mm by iterating through all
> tasks on the system and comparing each task's ->mm pointer to
> current's. Things like current_is_single_threaded() as part of various
> security checks, mm_update_next_owner(), zap_threads(), and so on. So
> if this is reachable from userspace task context (which I think it
> is?), I don't think we're allowed to switch out the ->mm pointer here.
>
>
Thanks for pointing this out! I took a step back and looked at this
again in more detail. The only reason for reassigning the ->mm pointer
is that when patching we need to hash the page and allocate an SLB
entry w/ the hash MMU. That codepath includes a check to ensure that
->mm is not NULL. Overwriting ->mm temporarily and restoring it is
pretty crappy in retrospect. I _think_ a better approach is to just call
the hashing and allocate SLB functions from `map_patch` directly - this
both removes the need to overwrite ->mm (since the functions take an mm
parameter) and it avoids taking two exceptions when doing the actual
patching.
This works fine on Power9 and a Power8 at least but needs some testing
on PPC32 before I can send a v4.
> > + switch_mm_irqs_off(NULL, temp_mm->temp, current);
>
> switch_mm_irqs_off() calls switch_mmu_context(), which in the nohash
> implementation increments next->context.active and decrements
> prev->context.active if prev is non-NULL, right? So this would
> increase temp_mm->temp->context.active...
>
> > + if (ppc_breakpoint_available()) {
> > + struct arch_hw_breakpoint null_brk = {0};
> > + int i = 0;
> > +
> > + for (; i < nr_wp_slots(); ++i) {
> > + __get_breakpoint(i, &temp_mm->brk[i]);
> > + if (temp_mm->brk[i].type != 0)
> > + __set_breakpoint(i, &null_brk);
> > + }
> > + }
> > +}
> > +
> > +static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
> > +{
> > + lockdep_assert_irqs_disabled();
> > +
> > + if (temp_mm->is_kernel_thread)
> > + current->mm = NULL;
> > + else
> > + current->mm = temp_mm->prev;
> > + switch_mm_irqs_off(NULL, temp_mm->prev, current);
>
> ... whereas this would increase temp_mm->prev->context.active. As far
> as I can tell, that'll mean that both the original mm and the patching
> mm will have their .active counts permanently too high after
> use_temporary_mm()+unuse_temporary_mm()?
>
Yes you are correct. Hmm, I can't immediately recall why prev=NULL here,
and I can't find anything in the various powerpc
switch_mm_irqs_off/switch_mmu_context implementations that would break
by setting prev=actual previous mm here. I will fix this for v4. Thanks!
> > + if (ppc_breakpoint_available()) {
> > + int i = 0;
> > +
> > + for (; i < nr_wp_slots(); ++i)
> > + if (temp_mm->brk[i].type != 0)
> > + __set_breakpoint(i, &temp_mm->brk[i]);
> > + }
> > +}
^ permalink raw reply
* Re: [PATCH -next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n
From: Yang Yingliang @ 2020-09-07 1:51 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev, linux-kernel
In-Reply-To: <39a976b8-896c-e878-bac7-50bbf3ccbc4f@csgroup.eu>
On 2020/9/6 14:50, Christophe Leroy wrote:
>
>
> Le 05/09/2020 à 13:25, Yang Yingliang a écrit :
>> Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
>> powerpc64-linux-gnu-ld:
>> arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference
>> to `mmu_pid_bits'
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>> arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++
>
> In your commit log, you are just mentionning
> arch/powerpc/platforms/pseries/lpar.o, which is right.
>
> You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at
> all, see below.
>
>> arch/powerpc/platforms/pseries/lpar.c | 2 ++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/mm/book3s64/mmu_context.c
>> b/arch/powerpc/mm/book3s64/mmu_context.c
>> index 0ba30b8b935b..a8e292cd88f0 100644
>> --- a/arch/powerpc/mm/book3s64/mmu_context.c
>> +++ b/arch/powerpc/mm/book3s64/mmu_context.c
>> @@ -152,6 +152,7 @@ void hash__setup_new_exec(void)
>> static int radix__init_new_context(struct mm_struct *mm)
>> {
>> +#ifdef CONFIG_PPC_RADIX_MMU
>
> This shouldn't be required. radix__init_new_context() is only called
> when radix_enabled() returns true.
> As it is a static function, when it is not called it gets optimised
> away, so you will never get an undefined reference to `mmu_pid_bits`
> there.
powerpc64-linux-gnu-ld:
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x0): undefined reference
to `mmu_pid_bits'
powerpc64-linux-gnu-ld:
arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x8): undefined reference
to `mmu_base_pid'
mmu_context.c is always compiled, it uses mmu_pid_bits and mmu_base_pid.
>
>> unsigned long rts_field;
>> int index, max_id;
>> @@ -177,6 +178,9 @@ static int radix__init_new_context(struct
>> mm_struct *mm)
>> mm->context.hash_context = NULL;
>> return index;
>> +#else
>> + return -ENOTSUPP;
>> +#endif
>> }
>> int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c
>> b/arch/powerpc/platforms/pseries/lpar.c
>> index baf24eacd268..e454e218dbba 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -1726,10 +1726,12 @@ void __init hpte_init_pseries(void)
>> void radix_init_pseries(void)
>> {
>> +#ifdef CONFIG_PPC_RADIX_MMU
>
> This function is only called from
> /arch/powerpc/mm/book3s64/radix_pgtable.c which is only built when
> CONFIG_PPC_RADIX_MMU is selected.
>
> So the entire function should be encloded in the #ifdef.
OK, I will send a v2 later.
>
>> pr_info("Using radix MMU under hypervisor\n");
>> pseries_lpar_register_process_table(__pa(process_tb),
>> 0, PRTB_SIZE_SHIFT - 12);
>> +#endif
>> }
>> #ifdef CONFIG_PPC_SMLPAR
>>
>
> Christophe
> .
^ permalink raw reply
* Re: [RFC PATCH 12/12] powerpc/64s: power4 nap fixup in C
From: Nicholas Piggin @ 2020-09-07 4:02 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev
In-Reply-To: <53f5fa9b-03d4-150e-199b-7ffa75d91666@csgroup.eu>
Excerpts from Christophe Leroy's message of September 6, 2020 5:32 pm:
>
>
> Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
>> There is no need for this to be in asm, use the new intrrupt entry wrapper.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/interrupt.h | 14 ++++++++
>> arch/powerpc/include/asm/processor.h | 1 +
>> arch/powerpc/include/asm/thread_info.h | 6 ++++
>> arch/powerpc/kernel/exceptions-64s.S | 45 --------------------------
>> arch/powerpc/kernel/idle_book3s.S | 4 +++
>> 5 files changed, 25 insertions(+), 45 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 3ae3d2f93b61..acfcc7d5779b 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -8,6 +8,16 @@
>> #include <asm/ftrace.h>
>> #include <asm/runlatch.h>
>>
>> +static inline void nap_adjust_return(struct pt_regs *regs)
>> +{
>> +#ifdef CONFIG_PPC_970_NAP
>
> Avoid #ifdef, you can use IS_ENABLED(CONFIG_PPC_970_NAP) in the 'if' below
Yeah I guess.
>> + if (test_thread_local_flags(_TLF_NAPPING)) {
>> + clear_thread_local_flags(_TLF_NAPPING);
>> + regs->nip = (unsigned long)power4_idle_nap_return;
>> + }
>> +#endif
>> +}
>> +
>> #ifdef CONFIG_PPC_BOOK3S_64
>> static inline void interrupt_enter_prepare(struct pt_regs *regs)
>> {
>> @@ -33,6 +43,8 @@ static inline void interrupt_async_enter_prepare(struct pt_regs *regs)
>> if (cpu_has_feature(CPU_FTR_CTRL) &&
>> !test_thread_local_flags(_TLF_RUNLATCH))
>> __ppc64_runlatch_on();
>> +
>> + nap_adjust_return(regs);
>> }
>>
>> #else /* CONFIG_PPC_BOOK3S_64 */
>> @@ -72,6 +84,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
>>
>> this_cpu_set_ftrace_enabled(0);
>>
>> + nap_adjust_return(regs);
>> +
>> nmi_enter();
>> }
>>
>> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>> index ed0d633ab5aa..3da1dba91386 100644
>> --- a/arch/powerpc/include/asm/processor.h
>> +++ b/arch/powerpc/include/asm/processor.h
>> @@ -424,6 +424,7 @@ extern unsigned long isa300_idle_stop_mayloss(unsigned long psscr_val);
>> extern unsigned long isa206_idle_insn_mayloss(unsigned long type);
>> #ifdef CONFIG_PPC_970_NAP
>> extern void power4_idle_nap(void);
>> +extern void power4_idle_nap_return(void);
>
> Please please please, 'extern' keyword is pointless and deprecated for
> function prototypes. Don't add new ones.
>
> Also, put it outside the #ifdef, so that you can use IS_ENABLED()
> instead of #ifdef when using it.
I just copy paste and forget to remove it. I expect someone will do a
"cleanup" patch to get rid of them in one go, I find a random assortment
of extern and not extern to be even uglier :(
>> #endif
>>
>> extern unsigned long cpuidle_disable;
>> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
>> index ca6c97025704..9b15f7edb0cb 100644
>> --- a/arch/powerpc/include/asm/thread_info.h
>> +++ b/arch/powerpc/include/asm/thread_info.h
>> @@ -156,6 +156,12 @@ void arch_setup_new_exec(void);
>>
>> #ifndef __ASSEMBLY__
>>
>> +static inline void clear_thread_local_flags(unsigned int flags)
>> +{
>> + struct thread_info *ti = current_thread_info();
>> + ti->local_flags &= ~flags;
>> +}
>> +
>> static inline bool test_thread_local_flags(unsigned int flags)
>> {
>> struct thread_info *ti = current_thread_info();
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index 227bad3a586d..1db6b3438c88 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -692,25 +692,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>> ld r1,GPR1(r1)
>> .endm
>>
>> -/*
>> - * When the idle code in power4_idle puts the CPU into NAP mode,
>> - * it has to do so in a loop, and relies on the external interrupt
>> - * and decrementer interrupt entry code to get it out of the loop.
>> - * It sets the _TLF_NAPPING bit in current_thread_info()->local_flags
>> - * to signal that it is in the loop and needs help to get out.
>> - */
>> -#ifdef CONFIG_PPC_970_NAP
>> -#define FINISH_NAP \
>> -BEGIN_FTR_SECTION \
>> - ld r11, PACA_THREAD_INFO(r13); \
>> - ld r9,TI_LOCAL_FLAGS(r11); \
>> - andi. r10,r9,_TLF_NAPPING; \
>> - bnel power4_fixup_nap; \
>> -END_FTR_SECTION_IFSET(CPU_FTR_CAN_NAP)
>> -#else
>> -#define FINISH_NAP
>> -#endif
>> -
>> /*
>> * There are a few constraints to be concerned with.
>> * - Real mode exceptions code/data must be located at their physical location.
>> @@ -1250,7 +1231,6 @@ EXC_COMMON_BEGIN(machine_check_common)
>> */
>> GEN_COMMON machine_check
>>
>> - FINISH_NAP
>> /* Enable MSR_RI when finished with PACA_EXMC */
>> li r10,MSR_RI
>> mtmsrd r10,1
>> @@ -1572,7 +1552,6 @@ EXC_VIRT_BEGIN(hardware_interrupt, 0x4500, 0x100)
>> EXC_VIRT_END(hardware_interrupt, 0x4500, 0x100)
>> EXC_COMMON_BEGIN(hardware_interrupt_common)
>> GEN_COMMON hardware_interrupt
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl do_IRQ
>> b interrupt_return
>> @@ -1757,7 +1736,6 @@ EXC_VIRT_BEGIN(decrementer, 0x4900, 0x80)
>> EXC_VIRT_END(decrementer, 0x4900, 0x80)
>> EXC_COMMON_BEGIN(decrementer_common)
>> GEN_COMMON decrementer
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl timer_interrupt
>> b interrupt_return
>> @@ -1842,7 +1820,6 @@ EXC_VIRT_BEGIN(doorbell_super, 0x4a00, 0x100)
>> EXC_VIRT_END(doorbell_super, 0x4a00, 0x100)
>> EXC_COMMON_BEGIN(doorbell_super_common)
>> GEN_COMMON doorbell_super
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> #ifdef CONFIG_PPC_DOORBELL
>> bl doorbell_exception
>> @@ -2196,7 +2173,6 @@ EXC_COMMON_BEGIN(hmi_exception_early_common)
>>
>> EXC_COMMON_BEGIN(hmi_exception_common)
>> GEN_COMMON hmi_exception
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl handle_hmi_exception
>> b interrupt_return
>> @@ -2225,7 +2201,6 @@ EXC_VIRT_BEGIN(h_doorbell, 0x4e80, 0x20)
>> EXC_VIRT_END(h_doorbell, 0x4e80, 0x20)
>> EXC_COMMON_BEGIN(h_doorbell_common)
>> GEN_COMMON h_doorbell
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> #ifdef CONFIG_PPC_DOORBELL
>> bl doorbell_exception
>> @@ -2258,7 +2233,6 @@ EXC_VIRT_BEGIN(h_virt_irq, 0x4ea0, 0x20)
>> EXC_VIRT_END(h_virt_irq, 0x4ea0, 0x20)
>> EXC_COMMON_BEGIN(h_virt_irq_common)
>> GEN_COMMON h_virt_irq
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl do_IRQ
>> b interrupt_return
>> @@ -2304,7 +2278,6 @@ EXC_VIRT_BEGIN(performance_monitor, 0x4f00, 0x20)
>> EXC_VIRT_END(performance_monitor, 0x4f00, 0x20)
>> EXC_COMMON_BEGIN(performance_monitor_common)
>> GEN_COMMON performance_monitor
>> - FINISH_NAP
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl performance_monitor_exception
>> b interrupt_return
>> @@ -3032,24 +3005,6 @@ USE_FIXED_SECTION(virt_trampolines)
>> __end_interrupts:
>> DEFINE_FIXED_SYMBOL(__end_interrupts)
>>
>> -#ifdef CONFIG_PPC_970_NAP
>> - /*
>> - * Called by exception entry code if _TLF_NAPPING was set, this clears
>> - * the NAPPING flag, and redirects the exception exit to
>> - * power4_fixup_nap_return.
>> - */
>> - .globl power4_fixup_nap
>> -EXC_COMMON_BEGIN(power4_fixup_nap)
>> - andc r9,r9,r10
>> - std r9,TI_LOCAL_FLAGS(r11)
>> - LOAD_REG_ADDR(r10, power4_idle_nap_return)
>> - std r10,_NIP(r1)
>> - blr
>> -
>> -power4_idle_nap_return:
>> - blr
>> -#endif
>> -
>> CLOSE_FIXED_SECTION(real_vectors);
>> CLOSE_FIXED_SECTION(real_trampolines);
>> CLOSE_FIXED_SECTION(virt_vectors);
>> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
>> index 22f249b6f58d..27d2e6a72ec9 100644
>> --- a/arch/powerpc/kernel/idle_book3s.S
>> +++ b/arch/powerpc/kernel/idle_book3s.S
>> @@ -201,4 +201,8 @@ _GLOBAL(power4_idle_nap)
>> mtmsrd r7
>> isync
>> b 1b
>> +
>> + .globl power4_idle_nap_return
>> +power4_idle_nap_return:
>> + blr
>
> Can't this be written in C somewhere ?
Yes I think so if you did the entire power4_idle_nap function in C and
used inline asm for the mtmsrd and fixup label (basically the same way
as copy user exceptions return to a fixup location).
You have to return to the same C function of course because you can't
control the stack otherwise. But I don't care too much about avoiding
an extra function call/return here, all the important stuff is in C now.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 12/12] powerpc/64s: power4 nap fixup in C
From: Christophe Leroy @ 2020-09-07 4:48 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1599450777.weoux16jk2.astroid@bobo.none>
Le 07/09/2020 à 06:02, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of September 6, 2020 5:32 pm:
>>
>>
>> Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
>>> There is no need for this to be in asm, use the new intrrupt entry wrapper.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> arch/powerpc/include/asm/interrupt.h | 14 ++++++++
>>> arch/powerpc/include/asm/processor.h | 1 +
>>> arch/powerpc/include/asm/thread_info.h | 6 ++++
>>> arch/powerpc/kernel/exceptions-64s.S | 45 --------------------------
>>> arch/powerpc/kernel/idle_book3s.S | 4 +++
>>> 5 files changed, 25 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>>> index ed0d633ab5aa..3da1dba91386 100644
>>> --- a/arch/powerpc/include/asm/processor.h
>>> +++ b/arch/powerpc/include/asm/processor.h
>>> @@ -424,6 +424,7 @@ extern unsigned long isa300_idle_stop_mayloss(unsigned long psscr_val);
>>> extern unsigned long isa206_idle_insn_mayloss(unsigned long type);
>>> #ifdef CONFIG_PPC_970_NAP
>>> extern void power4_idle_nap(void);
>>> +extern void power4_idle_nap_return(void);
>>
>> Please please please, 'extern' keyword is pointless and deprecated for
>> function prototypes. Don't add new ones.
>>
>> Also, put it outside the #ifdef, so that you can use IS_ENABLED()
>> instead of #ifdef when using it.
>
> I just copy paste and forget to remove it. I expect someone will do a
> "cleanup" patch to get rid of them in one go, I find a random assortment
> of extern and not extern to be even uglier :(
If we don't want to make fixes backporting a huge headache, some
transition with random assortment is the price to pay.
One day, when 'extern' have become the minority, we can get rid of the
few last ones.
But if someone believe it is not such a problem with backporting, I can
provide a cleanup patch now.
Christophe
^ permalink raw reply
* [Bug 209181] New: kernel BUG at arch/powerpc/mm/pgtable.c:304!
From: bugzilla-daemon @ 2020-09-07 5:44 UTC (permalink / raw)
To: linuxppc-dev
https://bugzilla.kernel.org/show_bug.cgi?id=209181
Bug ID: 209181
Summary: kernel BUG at arch/powerpc/mm/pgtable.c:304!
Product: Memory Management
Version: 2.5
Kernel Version: Linux 5.9-rc4
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Page Allocator
Assignee: akpm@linux-foundation.org
Reporter: zlang@redhat.com
CC: linuxppc-dev@lists.ozlabs.org
Regression: No
Description of problem:
The latest upstream mainline kernel always panic on ppc64le machine (P9) as
below:
[ 1.406462] Loading compiled-in X.509 certificates
[ 1.436966] Loaded X.509 cert 'Build time autogenerated kernel key:
834a47793f474746e698c2f3a32aa53ffded35db'
[ 1.437154] zswap: loaded using pool lzo/zbud
[ 1.437509] debug_vm_pgtable: [debug_vm_pgtable ]: Validating
architecture page table helpers
[ 1.437571] ------------[ cut here ]------------
[ 1.437584] WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:185
set_pte_at+0xd8/0x1c0
[ 1.437589] Modules linked in:
[ 1.437596] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4 #1
[ 1.437602] NIP: c00000000009bb28 LR: c00000000152e1c0 CTR:
0000000000000000
[ 1.437608] REGS: c0000001fb6eb7b0 TRAP: 0700 Not tainted (5.9.0-rc4)
[ 1.437613] MSR: 8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 24002824
XER: 0000000a
[ 1.437624] CFAR: c00000000009ba74 IRQMASK: 0
[ 1.437624] GPR00: c00000000152e1c0 c0000001fb6eba40 c000000002138900
c0000000120f4100
[ 1.437624] GPR04: 000b701718150000 c0000000122400a8 05014e0100000080
0000000000000000
[ 1.437624] GPR08: 0000000000000080 07000000000000c0 05000000000000c0
0000000000000001
[ 1.437624] GPR12: 0000000000002000 c000000004050000 0000000000000000
c000000001569d38
[ 1.437624] GPR16: c000000012210000 f0ffffffffffffff c0000001fb52f8a0
c000000002231cb8
[ 1.437624] GPR20: c0000000010d8de0 c000000001000000 c00000001220b2e8
c0000000122f8000
[ 1.437624] GPR24: 0000000000000100 000000000000014e c0000000122f8028
8000000000000105
[ 1.437624] GPR28: 000b701718150000 c0000000122118c0 c0000000120f4100
c0000000122400a8
[ 1.437668] NIP [c00000000009bb28] set_pte_at+0xd8/0x1c0
[ 1.437674] LR [c00000000152e1c0] debug_vm_pgtable+0x8f4/0x1e14
[ 1.437679] Call Trace:
[ 1.437685] [c0000001fb6eba40] [c000000001082f48] _raw_spin_lock+0x88/0x100
(unreliable)
[ 1.437693] [c0000001fb6eba80] [c00000000152dfd4]
debug_vm_pgtable+0x708/0x1e14
[ 1.437700] [c0000001fb6ebb90] [c00000000001208c] do_one_initcall+0xbc/0x5f0
[ 1.437707] [c0000001fb6ebc80] [c0000000014e4d04]
kernel_init_freeable+0x4bc/0x58c
[ 1.437714] [c0000001fb6ebdb0] [c000000000012de8] kernel_init+0x2c/0x164
[ 1.437721] [c0000001fb6ebe20] [c00000000000d5d0]
ret_from_kernel_thread+0x5c/0x6c
[ 1.437726] Instruction dump:
[ 1.437731] 41820068 e8010050 ebc10030 7c0803a6 4bffff8c 4bffff88 3d200700
792907c6
[ 1.437741] 612900c0 7d4a4838 2faa00c0 419eff54 <0fe00000> 4bffff4c 3fe0bfef
63ffffff
[ 1.437751] irq event stamp: 275292
[ 1.437757] hardirqs last enabled at (275291): [<c0000000004ef4d0>]
inc_zone_page_state+0xa0/0xd0
[ 1.437764] hardirqs last disabled at (275292): [<c0000000000096fc>]
program_check_common_virt+0x2bc/0x310
[ 1.437771] softirqs last enabled at (273036): [<c000000000f97044>]
inet6_register_protosw+0x154/0x2a0
[ 1.437778] softirqs last disabled at (273034): [<c000000000f96f34>]
inet6_register_protosw+0x44/0x2a0
[ 1.437784] ---[ end trace 39aeb34808a575d2 ]---
[ 1.437790] ------------[ cut here ]------------
[ 1.437795] kernel BUG at arch/powerpc/mm/pgtable.c:304!
[ 1.437801] Oops: Exception in kernel mode, sig: 5 [#1]
[ 1.437805] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 1.437807] Modules linked in:
[ 1.437811] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W
5.9.0-rc4 #1
[ 1.437815] NIP: c00000000009c1a8 LR: c0000000005f9de0 CTR:
0000000000000000
[ 1.437819] REGS: c0000001fb6eb720 TRAP: 0700 Tainted: G W
(5.9.0-rc4)
[ 1.437822] MSR: 8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 24002828
XER: 0000000a
[ 1.437829] CFAR: c00000000009c148 IRQMASK: 0
[ 1.437829] GPR00: c0000000005f9de0 c0000001fb6eb9b0 c000000002138900
c0000000120f4100
[ 1.437829] GPR04: 000b701718150000 c0000000122400a8 00000000122f8000
0000000000802f12
[ 1.437829] GPR08: 0000000000000000 0000000000000001 0000000000000028
0000000000000001
[ 1.437829] GPR12: 0000000000002000 c000000004050000 0000000000000000
c000000001569d38
[ 1.437829] GPR16: c000000012210000 f0ffffffffffffff c0000001fb52f8a0
c000000002231cb8
[ 1.437829] GPR20: c0000000010d8de0 c000000001000000 c00000001220b2e8
c0000000122f8000
[ 1.437829] GPR24: 0000000000000100 0000000000000008 c000000002231ca8
000000000000000a
[ 1.437829] GPR28: c000000002231cb8 c000000002231cb0 000b701718150000
000000000002dc05
[ 1.437860] NIP [c00000000009c1a8] assert_pte_locked+0x218/0x360
[ 1.437864] LR [c0000000005f9de0] pte_update+0xc0/0x180
[ 1.437867] Call Trace:
[ 1.437870] [c0000001fb6eb9b0] [0000000000000100] 0x100 (unreliable)
[ 1.437875] [c0000001fb6eba20] [c0000000005f9de0] pte_update+0xc0/0x180
[ 1.437879] [c0000001fb6eba80] [c00000000152e1e0]
debug_vm_pgtable+0x914/0x1e14
[ 1.437884] [c0000001fb6ebb90] [c00000000001208c] do_one_initcall+0xbc/0x5f0
[ 1.437888] [c0000001fb6ebc80] [c0000000014e4d04]
kernel_init_freeable+0x4bc/0x58c
[ 1.437893] [c0000001fb6ebdb0] [c000000000012de8] kernel_init+0x2c/0x164
[ 1.437897] [c0000001fb6ebe20] [c00000000000d5d0]
ret_from_kernel_thread+0x5c/0x6c
[ 1.437900] Instruction dump:
[ 1.437903] 7c0803a6 60000000 39400001 7fdffc36 7d4ad830 394affff 7d4a07b4
7d4af838
[ 1.437909] 794a1f24 7d09502a 7d090074 7929d182 <0b090000> 79090022 550ac03e
ebfc0000
[ 1.437916] ---[ end trace 39aeb34808a575d3 ]--
How reproducible:
100% on our ppc64le machines
Steps to Reproduce:
1. git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
2. build and intall the kernel (I'll update the .config file later)
3. boot the kernel
<panic at here>
Additional info:
The HEAD of my test kernel is:
commit f4d51dffc6c01a9e94650d95ce0104964f8ae822
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun Sep 6 17:11:40 2020 -0700
Linux 5.9-rc4
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [Bug 209181] kernel BUG at arch/powerpc/mm/pgtable.c:304!
From: bugzilla-daemon @ 2020-09-07 5:52 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209181-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209181
Christophe Leroy (christophe.leroy@csgroup.eu) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |christophe.leroy@csgroup.eu
--- Comment #1 from Christophe Leroy (christophe.leroy@csgroup.eu) ---
See https://bugzilla.kernel.org/show_bug.cgi?id=209029
Patch at
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200902040122.136414-1-aneesh.kumar@linux.ibm.com/
to deactivate CONFIG_DEBUG_VM_PGTABLE on powerpc until the issue is fixes.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [Bug 209181] kernel BUG at arch/powerpc/mm/pgtable.c:304!
From: bugzilla-daemon @ 2020-09-07 6:18 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209181-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209181
--- Comment #2 from Zorro Lang (zlang@redhat.com) ---
(In reply to Christophe Leroy from comment #1)
> See https://bugzilla.kernel.org/show_bug.cgi?id=209029
>
> Patch at
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200902040122.
> 136414-1-aneesh.kumar@linux.ibm.com/ to deactivate CONFIG_DEBUG_VM_PGTABLE
> on powerpc until the issue is fixes.
Thanks for this info
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [Bug 209181] kernel BUG at arch/powerpc/mm/pgtable.c:304!
From: bugzilla-daemon @ 2020-09-07 6:18 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209181-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209181
Zorro Lang (zlang@redhat.com) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |DUPLICATE
--- Comment #3 from Zorro Lang (zlang@redhat.com) ---
*** This bug has been marked as a duplicate of bug 209029 ***
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [Bug 209029] kernel 5.9-rc2 fails to boot on a PowerMac G5 11,2 - BUG: Kernel NULL pointer dereference on read at 0x00000020
From: bugzilla-daemon @ 2020-09-07 6:18 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209029-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209029
Zorro Lang (zlang@redhat.com) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |zlang@redhat.com
--- Comment #5 from Zorro Lang (zlang@redhat.com) ---
*** Bug 209181 has been marked as a duplicate of this bug. ***
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH] powerepc/book3s64/hash: Align start/end address correctly with bolt mapping
From: Aneesh Kumar K.V @ 2020-09-07 7:25 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
This ensures we don't do a partial mapping of memory. With nvdimm, when
creating namespaces with size not aligned to 16MB, the kernel ends up partially
mapping the pages. This can result in kernel adding multiple hash page table
entries for the same range. A new namespace will result in
create_section_mapping() with start and end overlapping an already existing
bolted hash page table entry.
commit: 6acd7d5ef264 ("libnvdimm/namespace: Enforce memremap_compat_align()")
made sure that we always create namespaces aligned to 16MB. But we can do
better by avoiding mapping pages that are not aligned. This helps to catch
access to these partially mapped pages early.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/hash_utils.c | 12 +++++++++---
arch/powerpc/mm/book3s64/radix_pgtable.c | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index c663e7ba801f..7185bc43b24f 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -260,8 +260,12 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
DBG("htab_bolt_mapping(%lx..%lx -> %lx (%lx,%d,%d)\n",
vstart, vend, pstart, prot, psize, ssize);
- for (vaddr = vstart, paddr = pstart; vaddr < vend;
- vaddr += step, paddr += step) {
+ /* Carefully map only the possible range */
+ vaddr = ALIGN(vstart, step);
+ paddr = ALIGN(pstart, step);
+ vend = ALIGN_DOWN(vend, step);
+
+ for (; vaddr < vend; vaddr += step, paddr += step) {
unsigned long hash, hpteg;
unsigned long vsid = get_kernel_vsid(vaddr, ssize);
unsigned long vpn = hpt_vpn(vaddr, vsid, ssize);
@@ -343,7 +347,9 @@ int htab_remove_mapping(unsigned long vstart, unsigned long vend,
if (!mmu_hash_ops.hpte_removebolted)
return -ENODEV;
- for (vaddr = vstart; vaddr < vend; vaddr += step) {
+ /* Unmap the full range specificied */
+ vaddr = ALIGN_DOWN(vstart, step);
+ for (;vaddr < vend; vaddr += step) {
rc = mmu_hash_ops.hpte_removebolted(vaddr, psize, ssize);
if (rc == -ENOENT) {
ret = -ENOENT;
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index d5f0c10d752a..5c8adeb8c955 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -276,6 +276,7 @@ static int __meminit create_physical_mapping(unsigned long start,
int psize;
start = ALIGN(start, PAGE_SIZE);
+ end = ALIGN_DOWN(end, PAGE_SIZE);
for (addr = start; addr < end; addr += mapping_size) {
unsigned long gap, previous_size;
int rc;
--
2.26.2
^ permalink raw reply related
* Re: [PATCH -next] powerpc/book3s64: fix link error with CONFIG_PPC_RADIX_MMU=n
From: Christophe Leroy @ 2020-09-07 8:11 UTC (permalink / raw)
To: Yang Yingliang, linuxppc-dev, linux-kernel
In-Reply-To: <af37c513-6232-c35c-33e3-f6d8d82c8175@huawei.com>
Le 07/09/2020 à 03:51, Yang Yingliang a écrit :
>
> On 2020/9/6 14:50, Christophe Leroy wrote:
>>
>>
>> Le 05/09/2020 à 13:25, Yang Yingliang a écrit :
>>> Fix link error when CONFIG_PPC_RADIX_MMU is disabled:
>>> powerpc64-linux-gnu-ld:
>>> arch/powerpc/platforms/pseries/lpar.o:(.toc+0x0): undefined reference
>>> to `mmu_pid_bits'
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>>> ---
>>> arch/powerpc/mm/book3s64/mmu_context.c | 4 ++++
>>
>> In your commit log, you are just mentionning
>> arch/powerpc/platforms/pseries/lpar.o, which is right.
>>
>> You shouldn't need to modify arch/powerpc/mm/book3s64/mmu_context.c at
>> all, see below.
>>
>>> arch/powerpc/platforms/pseries/lpar.c | 2 ++
>>> 2 files changed, 6 insertions(+)
>>>
>>> diff --git a/arch/powerpc/mm/book3s64/mmu_context.c
>>> b/arch/powerpc/mm/book3s64/mmu_context.c
>>> index 0ba30b8b935b..a8e292cd88f0 100644
>>> --- a/arch/powerpc/mm/book3s64/mmu_context.c
>>> +++ b/arch/powerpc/mm/book3s64/mmu_context.c
>>> @@ -152,6 +152,7 @@ void hash__setup_new_exec(void)
>>> static int radix__init_new_context(struct mm_struct *mm)
>>> {
>>> +#ifdef CONFIG_PPC_RADIX_MMU
>>
>> This shouldn't be required. radix__init_new_context() is only called
>> when radix_enabled() returns true.
>> As it is a static function, when it is not called it gets optimised
>> away, so you will never get an undefined reference to `mmu_pid_bits`
>> there.
> powerpc64-linux-gnu-ld:
> arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x0): undefined reference
> to `mmu_pid_bits'
> powerpc64-linux-gnu-ld:
> arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x8): undefined reference
> to `mmu_base_pid'
>
>
> mmu_context.c is always compiled, it uses mmu_pid_bits and mmu_base_pid.
Yes, mmu_context.c is always compiled, but as I explained,
radix__init_new_context() is defined as 'static' so it is optimised out
when radix_enabled() returns false because there is no caller in that case.
I just made the test with ppc64_defconfig + CONFIG_PPC_RADIX_MMU=n (GCC 8.1)
The only failure I got was on lpar.c, which I fixed by enclosing the
entire radix_init_pseries() in an #ifdef.
Once this is fixed, the build is OK, without any modification to
mmu_context.c
powerpc64-linux-objdump -x arch/powerpc/mm/book3s64/mmu_context.o shows
only the following objects in the .toc:
RELOCATION RECORDS FOR [.toc]:
OFFSET TYPE VALUE
0000000000000000 R_PPC64_ADDR64 kmalloc_caches
0000000000000008 R_PPC64_ADDR64 vmemmap
0000000000000010 R_PPC64_ADDR64 __pmd_frag_nr
0000000000000018 R_PPC64_ADDR64 __pmd_frag_size_shift
mmu_pid_bits and mmu_base_pid are not part of the undefined objetcs:
0000000000000000 *UND* 0000000000000000 vmemmap
0000000000000000 *UND* 0000000000000000 .mm_iommu_init
0000000000000000 *UND* 0000000000000000 __pmd_frag_nr
0000000000000000 *UND* 0000000000000000 .ida_alloc_range
0000000000000000 *UND* 0000000000000000 .slb_setup_new_exec
0000000000000000 *UND* 0000000000000000 mmu_feature_keys
0000000000000000 *UND* 0000000000000000 .memset
0000000000000000 *UND* 0000000000000000 .memcpy
0000000000000000 *UND* 0000000000000000 .slice_init_new_context_exec
0000000000000000 *UND* 0000000000000000 ._mcount
0000000000000000 *UND* 0000000000000000 .__free_pages
0000000000000000 *UND* 0000000000000000 __pmd_frag_size_shift
0000000000000000 *UND* 0000000000000000 .slice_setup_new_exec
0000000000000000 *UND* 0000000000000000 .ida_free
0000000000000000 *UND* 0000000000000000 .pte_frag_destroy
0000000000000000 *UND* 0000000000000000 .kfree
0000000000000000 *UND* 0000000000000000 .pkey_mm_init
0000000000000000 *UND* 0000000000000000 .kmem_cache_alloc_trace
0000000000000000 *UND* 0000000000000000 .__warn_printk
0000000000000000 *UND* 0000000000000000 _mcount
0000000000000000 *UND* 0000000000000000 kmalloc_caches
Christophe
^ permalink raw reply
* Re: [RFC PATCH 09/12] powerpc: move NMI entry/exit code into wrapper
From: Christophe Leroy @ 2020-09-07 8:25 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200905174335.3161229-10-npiggin@gmail.com>
On 9/5/20 5:43 PM, Nicholas Piggin wrote:
> This moves the common NMI entry and exit code into the interrupt handler
> wrappers.
>
> This changes the behaviour of soft-NMI (watchdog) and HMI interrupts, and
> also MCE interrupts on 64e, by adding missing parts of the NMI entry to
> them.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 26 +++++++++++++++++++
> arch/powerpc/kernel/mce.c | 12 ---------
> arch/powerpc/kernel/traps.c | 38 +++++-----------------------
> arch/powerpc/kernel/watchdog.c | 10 +++-----
> 4 files changed, 37 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 83fe1d64cf23..69eb8a432984 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -31,6 +31,27 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs)
> }
> #endif /* CONFIG_PPC_BOOK3S_64 */
>
> +struct interrupt_nmi_state {
> +#ifdef CONFIG_PPC64
> + u8 ftrace_enabled;
> +#endif
> +};
> +
> +static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct interrupt_nmi_state *state)
> +{
> + this_cpu_set_ftrace_enabled(0);
> +
> + nmi_enter();
> +}
> +
> +static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct interrupt_nmi_state *state)
> +{
> + nmi_exit();
> +
> + this_cpu_set_ftrace_enabled(state->ftrace_enabled);
PPC32 build:
In file included from arch/powerpc/kernel/irq.c:57:0:
./arch/powerpc/include/asm/interrupt.h: In function
‘interrupt_nmi_exit_prepare’:
./arch/powerpc/include/asm/interrupt.h:96:35: error: ‘struct
interrupt_nmi_state’ has no member named ‘ftrace_enabled’
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
^
> +}
> +
> +
> /**
> * DECLARE_INTERRUPT_HANDLER_RAW - Declare raw interrupt handler function
> * @func: Function name of the entry point
> @@ -177,10 +198,15 @@ static __always_inline long ___##func(struct pt_regs *regs); \
> \
> __visible noinstr long func(struct pt_regs *regs) \
> { \
> + struct interrupt_nmi_state state; \
> long ret; \
> \
> + interrupt_nmi_enter_prepare(regs, &state); \
> + \
> ret = ___##func (regs); \
> \
> + interrupt_nmi_exit_prepare(regs, &state); \
> + \
> return ret; \
> } \
> \
Christophe
^ permalink raw reply
* Re: [RFC PATCH 02/12] powerpc: remove arguments from interrupt handler functions
From: Christophe Leroy @ 2020-09-07 9:20 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200905174335.3161229-3-npiggin@gmail.com>
Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
> Make interrupt handlers all just take the pt_regs * argument and load
> DAR/DSISR etc from that. Make those that return a value return long.
I like this, it will likely simplify a bit the VMAP_STACK mess.
Not sure it is that easy. My board is stuck after the start of init.
On the 8xx, on Instruction TLB Error exception, we do
andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
On book3s/32, on ISI exception we do:
andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
On 40x and bookE, on ISI exception we do:
li r5,0 /* Pass zero as arg3 */
And regs->dsisr will just contain nothing
So it means we should at least write back r5 into regs->dsisr from there
? The performance impact should be minimal as we already write _DAR so
the cache line should already be in the cache.
A hacky 'stw r5, _DSISR(r1)' in handle_page_fault() does the trick,
allthough we don't want to do it for both ISI and DSI at the end, so
you'll have to do it in every head_xxx.S
While you are at it, it would probably also make sense to do remove the
address param of bad_page_fault(), there is no point in loading back
regs->dar in handle_page_fault() and machine_check_8xx() and
alignment_exception(), just read regs->dar in bad_page_fault()
The case of do_break() should also be looked at.
Why changing return code from int to long ?
Christophe
>
> This is done to make the function signatures match more closely, which
> will help with a future patch to add wrappers. Explicit arguments could
> be re-added for performance in future but that would require more
> complex wrapper macros.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/asm-prototypes.h | 4 ++--
> arch/powerpc/include/asm/bug.h | 4 ++--
> arch/powerpc/kernel/exceptions-64e.S | 2 --
> arch/powerpc/kernel/exceptions-64s.S | 14 ++------------
> arch/powerpc/mm/book3s64/hash_utils.c | 8 +++++---
> arch/powerpc/mm/book3s64/slb.c | 11 +++++++----
> arch/powerpc/mm/fault.c | 16 +++++++++-------
> 7 files changed, 27 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
> index d714d83bbc7c..2fa0cf6c6011 100644
> --- a/arch/powerpc/include/asm/bug.h
> +++ b/arch/powerpc/include/asm/bug.h
> @@ -111,8 +111,8 @@
> #ifndef __ASSEMBLY__
>
> struct pt_regs;
> -extern int do_page_fault(struct pt_regs *, unsigned long, unsigned long);
> -extern int hash__do_page_fault(struct pt_regs *, unsigned long, unsigned long);
> +extern long do_page_fault(struct pt_regs *);
> +extern long hash__do_page_fault(struct pt_regs *);
no extern
> extern void bad_page_fault(struct pt_regs *, unsigned long, int);
> extern void _exception(int, struct pt_regs *, int, unsigned long);
> extern void _exception_pkey(struct pt_regs *, unsigned long, int);
Christophe
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Joakim Tjernlund @ 2020-09-07 9:53 UTC (permalink / raw)
To: mpe@ellerman.id.au, broonie@kernel.org, paulus@samba.org,
npiggin@gmail.com, Chris.Packham@alliedtelesis.co.nz,
benh@kernel.crashing.org, hkallweit1@gmail.com
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <6054f0ec-d994-105b-6399-6cdb65ddd1b6@alliedtelesis.co.nz>
On Thu, 2020-09-03 at 23:58 +0000, Chris Packham wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On 1/09/20 6:14 pm, Nicholas Piggin wrote:
> > Excerpts from Chris Packham's message of September 1, 2020 11:25 am:
> > > On 1/09/20 12:33 am, Heiner Kallweit wrote:
> > > > On 30.08.2020 23:59, Chris Packham wrote:
> > > > > On 31/08/20 9:41 am, Heiner Kallweit wrote:
> > > > > > On 30.08.2020 23:00, Chris Packham wrote:
> > > > > > > On 31/08/20 12:30 am, Nicholas Piggin wrote:
> > > > > > > > Excerpts from Chris Packham's message of August 28, 2020 8:07 am:
> > > > > > > <snip>
> > > > > > >
> > > > > > > > > > > > > I've also now seen the RX FIFO not empty error on the T2080RDB
> > > > > > > > > > > > >
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> > > > > > > > > > > > >
> > > > > > > > > > > > > With my current workaround of emptying the RX FIFO. It seems
> > > > > > > > > > > > > survivable. Interestingly it only ever seems to be 1 extra byte in the
> > > > > > > > > > > > > RX FIFO and it seems to be after either a READ_SR or a READ_FSR.
> > > > > > > > > > > > >
> > > > > > > > > > > > > fsl_espi ffe110000.spi: tx 70
> > > > > > > > > > > > > fsl_espi ffe110000.spi: rx 03
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Extra RX 00
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> > > > > > > > > > > > > fsl_espi ffe110000.spi: tx 05
> > > > > > > > > > > > > fsl_espi ffe110000.spi: rx 00
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Extra RX 03
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> > > > > > > > > > > > > fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> > > > > > > > > > > > > fsl_espi ffe110000.spi: tx 05
> > > > > > > > > > > > > fsl_espi ffe110000.spi: rx 00
> > > > > > > > > > > > > fsl_espi ffe110000.spi: Extra RX 03
> > > > > > > > > > > > >
> > > > > > > > > > > > > From all the Micron SPI-NOR datasheets I've got access to it is
> > > > > > > > > > > > > possible to continually read the SR/FSR. But I've no idea why it
> > > > > > > > > > > > > happens some times and not others.
> > > > > > > > > > > > So I think I've got a reproduction and I think I've bisected the problem
> > > > > > > > > > > > to commit 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in
> > > > > > > > > > > > C"). My day is just finishing now so I haven't applied too much scrutiny
> > > > > > > > > > > > to this result. Given the various rabbit holes I've been down on this
> > > > > > > > > > > > issue already I'd take this information with a good degree of skepticism.
> > > > > > > > > > > >
> > > > > > > > > > > OK, so an easy test should be to re-test with a 5.4 kernel.
> > > > > > > > > > > It doesn't have yet the change you're referring to, and the fsl-espi driver
> > > > > > > > > > > is basically the same as in 5.7 (just two small changes in 5.7).
> > > > > > > > > > There's 6cc0c16d82f88 and maybe also other interrupt related patches
> > > > > > > > > > around this time that could affect book E, so it's good if that exact
> > > > > > > > > > patch is confirmed.
> > > > > > > > > My confirmation is basically that I can induce the issue in a 5.4 kernel
> > > > > > > > > by cherry-picking 3282a3da25bd. I'm also able to "fix" the issue in
> > > > > > > > > 5.9-rc2 by reverting that one commit.
> > > > > > > > >
> > > > > > > > > I both cases it's not exactly a clean cherry-pick/revert so I also
> > > > > > > > > confirmed the bisection result by building at 3282a3da25bd (which sees
> > > > > > > > > the issue) and the commit just before (which does not).
> > > > > > > > Thanks for testing, that confirms it well.
> > > > > > > >
> > > > > > > > [snip patch]
> > > > > > > >
> > > > > > > > > I still saw the issue with this change applied. PPC_IRQ_SOFT_MASK_DEBUG
> > > > > > > > > didn't report anything (either with or without the change above).
> > > > > > > > Okay, it was a bit of a shot in the dark. I still can't see what
> > > > > > > > else has changed.
> > > > > > > >
> > > > > > > > What would cause this, a lost interrupt? A spurious interrupt? Or
> > > > > > > > higher interrupt latency?
> > > > > > > >
> > > > > > > > I don't think the patch should cause significantly worse latency,
> > > > > > > > (it's supposed to be a bit better if anything because it doesn't set
> > > > > > > > up the full interrupt frame). But it's possible.
> > > > > > > My working theory is that the SPI_DON indication is all about the TX
> > > > > > > direction an now that the interrupts are faster we're hitting an error
> > > > > > > because there is still RX activity going on. Heiner disagrees with my
> > > > > > > interpretation of the SPI_DON indication and the fact that it doesn't
> > > > > > > happen every time does throw doubt on it.
> > > > > > >
> > > > > > It's right that the eSPI spec can be interpreted that SPI_DON refers to
> > > > > > TX only. However this wouldn't really make sense, because also for RX
> > > > > > we program the frame length, and therefore want to be notified once the
> > > > > > full frame was received. Also practical experience shows that SPI_DON
> > > > > > is set also after RX-only transfers.
> > > > > > Typical SPI NOR use case is that you write read command + start address,
> > > > > > followed by a longer read. If the TX-only interpretation would be right,
> > > > > > we'd always end up with SPI_DON not being set.
> > > > > >
> > > > > > > I can't really explain the extra RX byte in the fifo. We know how many
> > > > > > > bytes to expect and we pull that many from the fifo so it's not as if
> > > > > > > we're missing an interrupt causing us to skip the last byte. I've been
> > > > > > > looking for some kind of off-by-one calculation but again if it were
> > > > > > > something like that it'd happen all the time.
> > > > > > >
> > > > > > Maybe it helps to know what value this extra byte in the FIFO has. Is it:
> > > > > > - a duplicate of the last read byte
> > > > > > - or the next byte (at <end address> + 1)
> > > > > > - or a fixed value, e.g. always 0x00 or 0xff
> > > > > The values were up thread a bit but I'll repeat them here
> > > > >
> > > > > fsl_espi ffe110000.spi: tx 70
> > > > > fsl_espi ffe110000.spi: rx 03
> > > > > fsl_espi ffe110000.spi: Extra RX 00
> > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> > > > > fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> > > > > fsl_espi ffe110000.spi: tx 05
> > > > > fsl_espi ffe110000.spi: rx 00
> > > > > fsl_espi ffe110000.spi: Extra RX 03
> > > > > fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> > > > > fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> > > > > fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> > > > > fsl_espi ffe110000.spi: tx 05
> > > > > fsl_espi ffe110000.spi: rx 00
> > > > > fsl_espi ffe110000.spi: Extra RX 03
> > > > >
> > > > >
> > > > > The rx 00 Extra RX 03 is a bit concerning. I've only ever seen them with
> > > > > either a READ_SR or a READ_FSR. Never a data read.
> > > > >
> > > > Just remembered something about SPIE_DON:
> > > > Transfers are always full duplex, therefore in case of a read the chip
> > > > sends dummy zero's. Having said that in case of a read SPIE_DON means
> > > > that the last dummy zero was shifted out.
> > > >
> > > > READ_SR and READ_FSR are the shortest transfers, 1 byte out and 1 byte in.
> > > > So the issue may have a dependency on the length of the transfer.
> > > > However I see no good explanation so far. You can try adding a delay of
> > > > a few miroseconds between the following to commands in fsl_espi_bufs().
> > > >
> > > > fsl_espi_write_reg(espi, ESPI_SPIM, mask);
> > > >
> > > > /* Prevent filling the fifo from getting interrupted */
> > > > spin_lock_irq(&espi->lock);
> > > >
> > > > Maybe enabling interrupts and seeing the SPIE_DON interrupt are too close.
> > > I think this might be heading in the right direction. Playing about with
> > > a delay does seem to make the two symptoms less likely. Although I have
> > > to set it quite high (i.e. msleep(100)) to completely avoid any
> > > possibility of seeing either message.
> > The patch might replay the interrupt a little bit faster, but it would
> > be a few microseconds at most I think (just from improved code).
> >
> > Would you be able to ftrace the interrupt handler function and see if you
> > can see a difference in number or timing of interrupts? I'm at a bit of
> > a loss.
>
> I tried ftrace but I really wasn't sure what I was looking for.
> Capturing a "bad" case was pretty tricky. But I think I've identified a
> fix (I'll send it as a proper patch shortly). The gist is
>
> diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
> index 7e7c92cafdbb..cb120b68c0e2 100644
> --- a/drivers/spi/spi-fsl-espi.c
> +++ b/drivers/spi/spi-fsl-espi.c
> @@ -574,13 +574,14 @@ static void fsl_espi_cpu_irq(struct fsl_espi
> *espi, u32 events)
> static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
> {
> struct fsl_espi *espi = context_data;
> - u32 events;
> + u32 events, mask;
>
> spin_lock(&espi->lock);
>
> /* Get interrupt events(tx/rx) */
> events = fsl_espi_read_reg(espi, ESPI_SPIE);
> - if (!events) {
> + mask = fsl_espi_read_reg(espi, ESPI_SPIM);
> + if (!(events & mask)) {
> spin_unlock(&espi->lock);
> return IRQ_NONE;
> }
>
> The SPIE register contains the TXCNT so events is pretty much always
> going to have something set. By checking events against what we've
> actually requested interrupts for we don't see any spurious events.
>
> I've tested this on the T2080RDB and on our custom hardware and it seems
> to resolve the problem.
>
I looked at the fsl_espi_irq() too and noticed that clearing of the IRQ events
are after processing TX/RX. That looks a bit odd to me.
Jocke
^ permalink raw reply
* [PATCH v2] powerpc/papr_scm: Limit the readability of 'perf_stats' sysfs attribute
From: Vaibhav Jain @ 2020-09-07 11:05 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams, Ira Weiny
The newly introduced 'perf_stats' attribute uses the default access
mode of 0444 letting non-root users access performance stats of an
nvdimm and potentially force the kernel into issuing large number of
expensive HCALLs. Since the information exposed by this attribute
cannot be cached hence its better to ward of access to this attribute
from users who don't need to access these performance statistics.
Hence this patch updates access mode of 'perf_stats' attribute to
be only readable by root users.
Fixes: 2d02bf835e573 ('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Change-log:
v2:
* Instead of checking for perfmon_capable() inside show_perf_stats()
set the attribute as DEVICE_ATTR_ADMIN_RO [ Aneesh ]
* Update patch description
---
arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index f439f0dfea7d1..a88a707a608aa 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -822,7 +822,7 @@ static ssize_t perf_stats_show(struct device *dev,
kfree(stats);
return rc ? rc : seq_buf_used(&s);
}
-DEVICE_ATTR_RO(perf_stats);
+DEVICE_ATTR_ADMIN_RO(perf_stats);
static ssize_t flags_show(struct device *dev,
struct device_attribute *attr, char *buf)
--
2.26.2
^ permalink raw reply related
* Re: [RFC PATCH 02/12] powerpc: remove arguments from interrupt handler functions
From: Christophe Leroy @ 2020-09-07 11:34 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <e34fead9-a356-3ae6-aa33-544380230bd5@csgroup.eu>
On Mon, 2020-09-07 at 11:20 +0200, Christophe Leroy wrote:
>
> Le 05/09/2020 à 19:43, Nicholas Piggin a écrit :
> > Make interrupt handlers all just take the pt_regs * argument and load
> > DAR/DSISR etc from that. Make those that return a value return long.
>
> I like this, it will likely simplify a bit the VMAP_STACK mess.
>
> Not sure it is that easy. My board is stuck after the start of init.
>
>
> On the 8xx, on Instruction TLB Error exception, we do
>
> andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
>
> On book3s/32, on ISI exception we do:
> andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
>
> On 40x and bookE, on ISI exception we do:
> li r5,0 /* Pass zero as arg3 */
>
>
> And regs->dsisr will just contain nothing
>
> So it means we should at least write back r5 into regs->dsisr from there
> ? The performance impact should be minimal as we already write _DAR so
> the cache line should already be in the cache.
>
> A hacky 'stw r5, _DSISR(r1)' in handle_page_fault() does the trick,
> allthough we don't want to do it for both ISI and DSI at the end, so
> you'll have to do it in every head_xxx.S
To get you series build and work, I did the following hacks:
diff --git a/arch/powerpc/include/asm/interrupt.h
b/arch/powerpc/include/asm/interrupt.h
index acfcc7d5779b..c11045d3113a 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -93,7 +93,9 @@ static inline void interrupt_nmi_exit_prepare(struct
pt_regs *regs, struct inter
{
nmi_exit();
+#ifdef CONFIG_PPC64
this_cpu_set_ftrace_enabled(state->ftrace_enabled);
+#endif
#ifdef CONFIG_PPC_BOOK3S_64
/* Check we didn't change the pending interrupt mask. */
diff --git a/arch/powerpc/kernel/entry_32.S
b/arch/powerpc/kernel/entry_32.S
index f4d0af8e1136..66f7adbe1076 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -663,6 +663,7 @@ ppc_swapcontext:
*/
.globl handle_page_fault
handle_page_fault:
+ stw r5,_DSISR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
#ifdef CONFIG_PPC_BOOK3S_32
andis. r0,r5,DSISR_DABRMATCH@h
---
Christophe
^ permalink raw reply related
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