* [PATCH V2 12/13] ARM: dts: Add minimal Raspberry Pi 4 support
From: Stefan Wahren @ 2019-08-13 16:20 UTC (permalink / raw)
To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
Wolfram Sang, Rob Herring, Mark Rutland, Michael Turquette,
Stephen Boyd
Cc: devicetree, Stefan Wahren, bcm-kernel-feedback-list,
linux-rpi-kernel, linux-clk, linux-arm-kernel, linux-i2c
In-Reply-To: <1565713248-4906-1-git-send-email-wahrenst@gmx.net>
This adds minimal support for the new Raspberry Pi 4 without the
fancy stuff like GENET, PCIe, xHCI, 40 bit DMA and V3D. The RPi 4 is
available in 3 different variants (1, 2 and 4 GB RAM), so leave the memory
size to zero and let the bootloader take care of it. The DWC2 is still
usable as peripheral via the USB-C port.
Other differences to the Raspberry Pi 3:
- additional GIC 400 Interrupt controller
- new thermal IP and HWRNG
- additional MMC interface (emmc2)
- additional UART, I2C, SPI and PWM interfaces
- clock stretching bug in I2C IP has been fixed
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Eric Anholt <eric@anholt.net>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 121 ++++
arch/arm/boot/dts/bcm2711.dtsi | 662 ++++++++++++++++++++++
arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi | 7 +
arch/arm/boot/dts/bcm283x.dtsi | 4 +-
5 files changed, 793 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/boot/dts/bcm2711-rpi-4-b.dts
create mode 100644 arch/arm/boot/dts/bcm2711.dtsi
create mode 100644 arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9159fa2..031ca32 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -83,6 +83,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
bcm2837-rpi-3-b.dtb \
bcm2837-rpi-3-b-plus.dtb \
bcm2837-rpi-cm3-io3.dtb \
+ bcm2711-rpi-4-b.dtb \
bcm2835-rpi-zero.dtb \
bcm2835-rpi-zero-w.dtb
dtb-$(CONFIG_ARCH_BCM_5301X) += \
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
new file mode 100644
index 0000000..3825273
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2711.dtsi"
+#include "bcm2835-rpi.dtsi"
+#include "bcm283x-rpi-usb-peripheral.dtsi"
+
+/ {
+ compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
+ model = "Raspberry Pi 4 Model B";
+
+ chosen {
+ /* 8250 auxiliary UART instead of pl011 */
+ stdout-path = "serial1:115200n8";
+ };
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0 0 0x00000000>;
+ };
+
+ leds {
+ act {
+ gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
+ };
+
+ pwr {
+ label = "PWR";
+ gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ wifi_pwrseq: wifi-pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ reset-gpios = <&expgpio 1 GPIO_ACTIVE_LOW>;
+ };
+
+ sd_io_1v8_reg: sd_io_1v8_reg {
+ compatible = "regulator-gpio";
+ regulator-name = "vdd-sd-io";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-settling-time-us = <5000>;
+ gpios = <&expgpio 4 GPIO_ACTIVE_HIGH>;
+ states = <1800000 0x1
+ 3300000 0x0>;
+ status = "okay";
+ };
+};
+
+&firmware {
+ expgpio: gpio {
+ compatible = "raspberrypi,firmware-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-line-names = "BT_ON",
+ "WL_ON",
+ "PWR_LED_OFF",
+ "GLOBAL_RESET",
+ "VDD_SD_IO_SEL",
+ "CAM_GPIO",
+ "",
+ "";
+ status = "okay";
+ };
+};
+
+&pwm1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm1_0_gpio40 &pwm1_1_gpio41>;
+ status = "okay";
+};
+
+/* SDHCI is used to control the SDIO for wireless */
+&sdhci {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&emmc_gpio34>;
+ bus-width = <4>;
+ non-removable;
+ mmc-pwrseq = <&wifi_pwrseq>;
+ status = "okay";
+
+ brcmf: wifi@1 {
+ reg = <1>;
+ compatible = "brcm,bcm4329-fmac";
+ };
+};
+
+/* EMMC2 is used to drive the SD card */
+&emmc2 {
+ vqmmc-supply = <&sd_io_1v8_reg>;
+ broken-cd;
+ status = "okay";
+};
+
+/* uart0 communicates with the BT module */
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_ctsrts_gpio30 &uart0_gpio32 &gpclk2_gpio43>;
+ status = "okay";
+
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ max-speed = <2000000>;
+ shutdown-gpios = <&expgpio 0 GPIO_ACTIVE_HIGH>;
+ };
+};
+
+/* uart1 is mapped to the pin header */
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_gpio14>;
+ status = "okay";
+};
+
+&vchiq {
+ interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
new file mode 100644
index 0000000..023ff27
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -0,0 +1,662 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "bcm283x.dtsi"
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/soc/bcm2835-pm.h>
+
+/ {
+ compatible = "brcm,bcm2711";
+
+ #address-cells = <2>;
+ #size-cells = <1>;
+
+ interrupt-parent = <&gicv2>;
+
+ soc {
+ ranges = <0x7e000000 0x0 0xfe000000 0x01800000>,
+ <0x7c000000 0x0 0xfc000000 0x02000000>,
+ <0x40000000 0x0 0xff800000 0x00800000>;
+ /* Emulate a contiguous 30-bit address range for DMA */
+ dma-ranges = <0xc0000000 0x0 0x00000000 0x3c000000>;
+
+ local_intc: local_intc@40000000 {
+ compatible = "brcm,bcm2836-l1-intc";
+ reg = <0x40000000 0x100>;
+ };
+
+ gicv2: gic400@40041000 {
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ compatible = "arm,gic-400";
+ reg = <0x40041000 0x1000>,
+ <0x40042000 0x2000>,
+ <0x40044000 0x2000>,
+ <0x40046000 0x2000>;
+ interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_HIGH)>;
+ };
+
+ dma: dma@7e007000 {
+ compatible = "brcm,bcm2835-dma";
+ reg = <0x7e007000 0xb00>;
+ interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
+ /* DMA lite 7 - 10 */
+ <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "dma0",
+ "dma1",
+ "dma2",
+ "dma3",
+ "dma4",
+ "dma5",
+ "dma6",
+ "dma7",
+ "dma8",
+ "dma9",
+ "dma10";
+ #dma-cells = <1>;
+ brcm,dma-channel-mask = <0x07f5>;
+ };
+
+ pm: watchdog@7e100000 {
+ compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+ #power-domain-cells = <1>;
+ #reset-cells = <1>;
+ reg = <0x7e100000 0x114>,
+ <0x7e00a000 0x24>,
+ <0x7ec11000 0x20>;
+ clocks = <&clocks BCM2835_CLOCK_V3D>,
+ <&clocks BCM2835_CLOCK_PERI_IMAGE>,
+ <&clocks BCM2835_CLOCK_H264>,
+ <&clocks BCM2835_CLOCK_ISP>;
+ clock-names = "v3d", "peri_image", "h264", "isp";
+ system-power-controller;
+ };
+
+ rng@7e104000 {
+ interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+
+ /* RNG is incompatible to brcm,bcm2835-rng */
+ status = "disabled";
+ };
+
+ uart2: serial@7e201400 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0x7e201400 0x200>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_UART>,
+ <&clocks BCM2835_CLOCK_VPU>;
+ clock-names = "uartclk", "apb_pclk";
+ arm,primecell-periphid = <0x00241011>;
+ status = "disabled";
+ };
+
+ uart3: serial@7e201600 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0x7e201600 0x200>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_UART>,
+ <&clocks BCM2835_CLOCK_VPU>;
+ clock-names = "uartclk", "apb_pclk";
+ arm,primecell-periphid = <0x00241011>;
+ status = "disabled";
+ };
+
+ uart4: serial@7e201800 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0x7e201800 0x200>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_UART>,
+ <&clocks BCM2835_CLOCK_VPU>;
+ clock-names = "uartclk", "apb_pclk";
+ arm,primecell-periphid = <0x00241011>;
+ status = "disabled";
+ };
+
+ uart5: serial@7e201a00 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0x7e201a00 0x200>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_UART>,
+ <&clocks BCM2835_CLOCK_VPU>;
+ clock-names = "uartclk", "apb_pclk";
+ arm,primecell-periphid = <0x00241011>;
+ status = "disabled";
+ };
+
+ spi@7e204000 {
+ reg = <0x7e204000 0x0200>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ spi3: spi@7e204600 {
+ compatible = "brcm,bcm2835-spi";
+ reg = <0x7e204600 0x0200>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi4: spi@7e204800 {
+ compatible = "brcm,bcm2835-spi";
+ reg = <0x7e204800 0x0200>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi5: spi@7e204a00 {
+ compatible = "brcm,bcm2835-spi";
+ reg = <0x7e204a00 0x0200>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi6: spi@7e204c00 {
+ compatible = "brcm,bcm2835-spi";
+ reg = <0x7e204c00 0x0200>;
+ interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@7e205600 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ reg = <0x7e205600 0x200>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c4: i2c@7e205800 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ reg = <0x7e205800 0x200>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c5: i2c@7e205a00 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ reg = <0x7e205a00 0x200>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c6: i2c@7e205c00 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ reg = <0x7e205c00 0x200>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2835_CLOCK_VPU>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ pwm1: pwm@7e20c800 {
+ compatible = "brcm,bcm2835-pwm";
+ reg = <0x7e20c800 0x28>;
+ clocks = <&clocks BCM2835_CLOCK_PWM>;
+ assigned-clocks = <&clocks BCM2835_CLOCK_PWM>;
+ assigned-clock-rates = <10000000>;
+ #pwm-cells = <2>;
+ status = "disabled";
+ };
+
+ emmc2: emmc2@7e340000 {
+ compatible = "brcm,bcm2711-emmc2";
+ reg = <0x7e340000 0x100>;
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clocks BCM2711_CLOCK_EMMC2>;
+ status = "disabled";
+ };
+
+ hvs@7e400000 {
+ interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
+ arm-pmu {
+ compatible = "arm,cortex-a72-pmu", "arm,armv8-pmuv3";
+ interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>;
+ /* This only applies to the ARMv7 stub */
+ arm,cpu-registers-not-fw-configured;
+
+ /* The ARM cores doesn't enter deep enough states */
+ always-on;
+ };
+
+ cpus: cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ enable-method = "brcm,bcm2836-smp"; // for ARM 32-bit
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x000000d8>;
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <1>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x000000e0>;
+ };
+
+ cpu2: cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <2>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x000000e8>;
+ };
+
+ cpu3: cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <3>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x000000f0>;
+ };
+ };
+};
+
+&clk_osc {
+ clock-frequency = <54000000>;
+};
+
+&clocks {
+ compatible = "brcm,bcm2711-cprman";
+};
+
+&cpu_thermal {
+ coefficients = <(-487) 410040>;
+};
+
+&dsi0 {
+ interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&dsi1 {
+ interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&gpio {
+ compatible = "brcm,bcm2711-gpio";
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+
+ gpclk0_gpio49: gpclk0_gpio49 {
+ brcm,pins = <49>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ gpclk1_gpio50: gpclk1_gpio50 {
+ brcm,pins = <50>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ gpclk2_gpio51: gpclk2_gpio51 {
+ brcm,pins = <51>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+
+ i2c0_gpio46: i2c0_gpio46 {
+ brcm,pins = <46 47>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ i2c1_gpio46: i2c1_gpio46 {
+ brcm,pins = <46 47>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ };
+ i2c3_gpio2: i2c3_gpio2 {
+ brcm,pins = <2 3>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c3_gpio4: i2c3_gpio4 {
+ brcm,pins = <4 5>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c4_gpio6: i2c4_gpio6 {
+ brcm,pins = <6 7>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c4_gpio8: i2c4_gpio8 {
+ brcm,pins = <8 9>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c5_gpio10: i2c5_gpio10 {
+ brcm,pins = <10 11>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c5_gpio12: i2c5_gpio12 {
+ brcm,pins = <12 13>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c6_gpio0: i2c6_gpio0 {
+ brcm,pins = <0 1>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c6_gpio22: i2c6_gpio22 {
+ brcm,pins = <22 23>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ i2c_slave_gpio8: i2c_slave_gpio8 {
+ brcm,pins = <8 9 10 11>;
+ brcm,function = <BCM2835_FSEL_ALT3>;
+ };
+
+ jtag_gpio48: jtag_gpio48 {
+ brcm,pins = <48 49 50 51 52 53>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ };
+
+ mii_gpio28: mii_gpio28 {
+ brcm,pins = <28 29 30 31>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ };
+ mii_gpio36: mii_gpio36 {
+ brcm,pins = <36 37 38 39>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+
+ pcm_gpio50: pcm_gpio50 {
+ brcm,pins = <50 51 52 53>;
+ brcm,function = <BCM2835_FSEL_ALT2>;
+ };
+
+ pwm0_0_gpio12: pwm0_0_gpio12 {
+ brcm,pins = <12>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ pwm0_0_gpio18: pwm0_0_gpio18 {
+ brcm,pins = <18>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ pwm1_0_gpio40: pwm1_0_gpio40 {
+ brcm,pins = <40>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ pwm0_1_gpio13: pwm0_1_gpio13 {
+ brcm,pins = <13>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ pwm0_1_gpio19: pwm0_1_gpio19 {
+ brcm,pins = <19>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ pwm1_1_gpio41: pwm1_1_gpio41 {
+ brcm,pins = <41>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ pwm0_1_gpio45: pwm0_1_gpio45 {
+ brcm,pins = <45>;
+ brcm,function = <BCM2835_FSEL_ALT0>;
+ };
+ pwm0_0_gpio52: pwm0_0_gpio52 {
+ brcm,pins = <52>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ pwm0_1_gpio53: pwm0_1_gpio53 {
+ brcm,pins = <53>;
+ brcm,function = <BCM2835_FSEL_ALT1>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+
+ /* The following group consists of:
+ * RGMII_START_STOP
+ * RGMII_RX_OK
+ */
+ rgmii_gpio35: rgmii_gpio35 {
+ brcm,pins = <35 36>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ };
+ rgmii_irq_gpio34: rgmii_irq_gpio34 {
+ brcm,pins = <34>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ rgmii_irq_gpio39: rgmii_irq_gpio39 {
+ brcm,pins = <39>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ };
+ rgmii_mdio_gpio28: rgmii_mdio_gpio28 {
+ brcm,pins = <28 29>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ rgmii_mdio_gpio37: rgmii_mdio_gpio37 {
+ brcm,pins = <37 38>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ };
+
+ spi0_gpio46: spi0_gpio46 {
+ brcm,pins = <46 47 48 49>;
+ brcm,function = <BCM2835_FSEL_ALT2>;
+ };
+ spi2_gpio46: spi2_gpio46 {
+ brcm,pins = <46 47 48 49 50>;
+ brcm,function = <BCM2835_FSEL_ALT5>;
+ };
+ spi3_gpio0: spi3_gpio0 {
+ brcm,pins = <0 1 2 3>;
+ brcm,function = <BCM2835_FSEL_ALT3>;
+ };
+ spi4_gpio4: spi4_gpio4 {
+ brcm,pins = <4 5 6 7>;
+ brcm,function = <BCM2835_FSEL_ALT3>;
+ };
+ spi5_gpio12: spi5_gpio12 {
+ brcm,pins = <12 13 14 15>;
+ brcm,function = <BCM2835_FSEL_ALT3>;
+ };
+ spi6_gpio18: spi6_gpio18 {
+ brcm,pins = <18 19 20 21>;
+ brcm,function = <BCM2835_FSEL_ALT3>;
+ };
+
+ uart2_gpio0: uart2_gpio0 {
+ pin-tx {
+ brcm,pins = <0>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ pin-rx {
+ brcm,pins = <1>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ };
+ uart2_ctsrts_gpio2: uart2_ctsrts_gpio2 {
+ pin-cts {
+ brcm,pins = <2>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ pin-rts {
+ brcm,pins = <3>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ };
+ uart3_gpio4: uart3_gpio4 {
+ pin-tx {
+ brcm,pins = <4>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ pin-rx {
+ brcm,pins = <5>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ };
+ uart3_ctsrts_gpio6: uart3_ctsrts_gpio6 {
+ pin-cts {
+ brcm,pins = <6>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ pin-rts {
+ brcm,pins = <7>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ };
+ uart4_gpio8: uart4_gpio8 {
+ pin-tx {
+ brcm,pins = <8>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ pin-rx {
+ brcm,pins = <9>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ };
+ uart4_ctsrts_gpio10: uart4_ctsrts_gpio10 {
+ pin-cts {
+ brcm,pins = <10>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ pin-rts {
+ brcm,pins = <11>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ };
+ uart5_gpio12: uart5_gpio12 {
+ pin-tx {
+ brcm,pins = <12>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ pin-rx {
+ brcm,pins = <13>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ };
+ uart5_ctsrts_gpio14: uart5_ctsrts_gpio14 {
+ pin-cts {
+ brcm,pins = <14>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_UP>;
+ };
+ pin-rts {
+ brcm,pins = <15>;
+ brcm,function = <BCM2835_FSEL_ALT4>;
+ brcm,pull = <BCM2835_PUD_OFF>;
+ };
+ };
+};
+
+&i2c0 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2c1 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2c2 {
+ compatible = "brcm,bcm2711-i2c", "brcm,bcm2835-i2c";
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&mailbox {
+ interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&sdhci {
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&sdhost {
+ interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&spi1 {
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&spi2 {
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&system_timer {
+ interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&txp {
+ interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart0 {
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart1 {
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&usb {
+ interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&vec {
+ interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
new file mode 100644
index 0000000..0ff0e9e
--- /dev/null
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+&usb {
+ dr_mode = "peripheral";
+ g-rx-fifo-size = <256>;
+ g-np-tx-fifo-size = <32>;
+ g-tx-fifo-size = <256 256 512 512 512 768 768>;
+};
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index cbc9422..5655ae4 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -53,7 +53,7 @@
#address-cells = <1>;
#size-cells = <1>;
- timer@7e003000 {
+ system_timer: timer@7e003000 {
compatible = "brcm,bcm2835-system-timer";
reg = <0x7e003000 0x1000>;
interrupts = <1 0>, <1 1>, <1 2>, <1 3>;
@@ -64,7 +64,7 @@
clock-frequency = <1000000>;
};
- txp@7e004000 {
+ txp: txp@7e004000 {
compatible = "brcm,bcm2835-txp";
reg = <0x7e004000 0x20>;
interrupts = <1 11>;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 00/11] Add arm64/signal initial kselftest support
From: Dave Martin @ 2019-08-13 16:22 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-1-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:49PM +0100, Cristian Marussi wrote:
> Hi
>
> this patchset aims to add the initial arch-specific arm64 support to
> kselftest starting with signals-related test-cases.
> A common internal test-case layout is proposed which then it is anyway
> wired-up to the toplevel kselftest Makefile, so that it should be possible
> at the end to run it on an arm64 target in the usual way with KSFT.
The tests look like a reasonable base overall and something that we can
extend later as needed.
There are various minor things that need attention -- see my comments on
the individual patches. Apart for some things that can be factored out,
I don't think any of it involves redesign.
A few general comments:
* Please wrap all commit messages to <= 75 chars, and follow the other
guidelines about commit messages in
Documentation/process/submitting-patches.rst).
* Remember to run scripts/checkpatch.pl on your patches. Currently
various issues are reported: they should mostly be trivial to fix.
checkpatch does report some false positives, but most of the warnings
I see look relevant.
* If you like, you can add an Author: line alongside the copyright
notice in new files that you create. (You'll see this elsewhere in
the kernel if you grep.)
One general stylistic issue (IMHO):
* Try to avoid inventing names for things that have no established
name (for example "magic0" to mean "magic number 0").
The risk is that the reader wastes time grepping for the definition,
when really the text should be read at face value. It's best to use
all caps just for #define names, abbreviations, and other things
that are customarily capitalised (like "CPU" etc.). Other words
containing underscores may resemble variable / function names, and
may cause confusion of there is no actual variable or function with
that name.
I don't think it's worth heavily reworking the patches for this, but
it's something to bear in mind.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 01/11] kselftest: arm64: introduce new boilerplate code
From: Dave Martin @ 2019-08-13 16:23 UTC (permalink / raw)
To: Cristian Marussi
Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest, dave.martin
In-Reply-To: <20190802170300.20662-2-cristian.marussi@arm.com>
^ Regarding the subject line, "boilerplate code" sounds a bit vague.
Could we say something like "Add skeleton Makefile"?
On Fri, Aug 02, 2019 at 06:02:50PM +0100, Cristian Marussi wrote:
> Added a new arm64-specific empty subsystem amongst TARGETS of KSFT build
> framework; once populated with testcases, it will be possible to build
> and invoke the new KSFT TARGETS=arm64 related tests from the toplevel
> Makefile in the usual ways.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> Reviewed the build instructions reported in the README, to be more
> agnostic regarding user/device etc..
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/arm64/Makefile | 51 ++++++++++++++++++++++++++
> tools/testing/selftests/arm64/README | 43 ++++++++++++++++++++++
> 3 files changed, 95 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/Makefile
> create mode 100644 tools/testing/selftests/arm64/README
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 25b43a8c2b15..1722dae9381a 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> TARGETS = android
> +TARGETS += arm64
> TARGETS += bpf
> TARGETS += breakpoints
> TARGETS += capabilities
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> new file mode 100644
> index 000000000000..03a0d4f71218
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
> +
> +# When ARCH not overridden for crosscompiling, lookup machine
> +ARCH ?= $(shell uname -m)
> +ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
> +
> +ifeq ("x$(ARCH)", "xarm64")
> +SUBDIRS :=
> +else
> +SUBDIRS :=
> +endif
> +
> +CFLAGS := -Wall -O2 -g
> +
> +export CC
> +export CFLAGS
> +
> +all:
> + @for DIR in $(SUBDIRS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + mkdir -p $$BUILD_TARGET; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +install: all
> + @for DIR in $(SUBDIRS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +run_tests: all
> + @for DIR in $(SUBDIRS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +# Avoid any output on non arm64 on emit_tests
> +emit_tests: all
> + @for DIR in $(SUBDIRS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +clean:
> + @for DIR in $(SUBDIRS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +.PHONY: all clean install run_tests emit_tests
> diff --git a/tools/testing/selftests/arm64/README b/tools/testing/selftests/arm64/README
> new file mode 100644
> index 000000000000..dee3306071cc
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/README
> @@ -0,0 +1,43 @@
> +KSelfTest ARM64
> +===============
> +
> +- These tests are arm64 specific and so not built or run but just skipped
> + completely when env-variable ARCH is found to be different than 'arm64'
> + and `uname -m` reports other than 'aarch64'.
> +
> +- Holding true the above, ARM64 KSFT tests can be run:
> +
> + + as standalone (example for signal tests)
> +
> + $ make -C tools/testing/selftest/arm64/signal \
> + INSTALL_PATH=<your-installation-path> install
> +
> + and then launching on the target device inside the installed path:
> +
> + $ <your-installed-path>/test_arm64_signals.sh [-k | -v]
> +
> + + within the KSelfTest framework using standard Linux top-level-makefile
> + targets:
> +
> + $ make TARGETS=arm64 kselftest-clean
> + $ make TARGETS=arm64 kselftest
> +
> + Further details on building and running KFST can be found in:
> + Documentation/dev-tools/kselftest.rst
The next two paragraphs aren't relevant yet. Can we split them out of
this patch and add them alongside the relevant code / Makefile changes?
> +
> +- Tests can depend on some arch-specific definitions which can be found in a
> + standard Kernel Headers installation in $(top_srcdir)/usr/include.
> + Such Kernel Headers are automatically installed (via make headers_install)
> + by KSFT framework itself in a dedicated directory when tests are launched
> + via KSFT itself; when running standalone, instead, a Warning is issued
> + if such headers cannot be found somewhere (we try to guess a few standard
> + locations anyway)
> +
> +- Some of these tests may be related to possibly not implemented ARMv8
> + features: depending on their implementation status on the effective HW
> + we'll expect different results. The tests' harness will take care to check
> + at run-time if the required features are supported and will act accordingly.
> + Moreover, in order to avoid any kind of compile-time dependency on the
> + toolchain (possibly due to the above mentioned not-implemented features),
> + we make strictly use of direct 'S3_ sysreg' raw-encoding while checking for
> + those features and/or lookin up sysregs.
This last paragraph is only relevant for people adding new tests. It
probably makes sense to start "When adding new tests, try to avoid
unnecessary toolchain dependencies where possible. [...]"
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 02/11] kselftest: arm64: adds first test and common utils
From: Dave Martin @ 2019-08-13 16:24 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-3-cristian.marussi@arm.com>
For the subject line, maybe name the test being added (same as for the
other patches).
On Fri, Aug 02, 2019 at 06:02:51PM +0100, Cristian Marussi wrote:
> Added some arm64/signal specific boilerplate and utility code to help
> further testcase development.
>
> A simple testcase and related helpers are also introduced in this commit:
> mangle_pstate_invalid_compat_toggle is a simple mangle testcase which
> messes with the ucontext_t from within the sig_handler, trying to toggle
"signal handler"?
> PSTATE state bits to switch the system between 32bit/64bit execution state.
> Expects SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> A few fixes:
> - test_arm64_signals.sh runner script generation has been reviewed in order to
> be safe against the .gitignore
> - using kselftest.h officially provided defines for tests' return values
> - removed SAFE_WRITE()/dump_uc()
> - looking for si_code==SEGV_ACCERR on SEGV test cases to better understand if
> the sigfault had been directly triggered by Kernel
> ---
> tools/testing/selftests/arm64/Makefile | 2 +-
> .../testing/selftests/arm64/signal/.gitignore | 6 +
> tools/testing/selftests/arm64/signal/Makefile | 88 ++++++
> tools/testing/selftests/arm64/signal/README | 59 ++++
> .../arm64/signal/test_arm64_signals.src_shell | 55 ++++
> .../selftests/arm64/signal/test_signals.c | 26 ++
> .../selftests/arm64/signal/test_signals.h | 137 +++++++++
> .../arm64/signal/test_signals_utils.c | 261 ++++++++++++++++++
> .../arm64/signal/test_signals_utils.h | 13 +
> .../arm64/signal/testcases/.gitignore | 1 +
> .../mangle_pstate_invalid_compat_toggle.c | 25 ++
> .../arm64/signal/testcases/testcases.c | 150 ++++++++++
> .../arm64/signal/testcases/testcases.h | 83 ++++++
> 13 files changed, 905 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/arm64/signal/.gitignore
> create mode 100644 tools/testing/selftests/arm64/signal/Makefile
> create mode 100644 tools/testing/selftests/arm64/signal/README
> create mode 100755 tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
> create mode 100644 tools/testing/selftests/arm64/signal/test_signals.c
> create mode 100644 tools/testing/selftests/arm64/signal/test_signals.h
> create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.c
> create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.h
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/.gitignore
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.h
>
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index 03a0d4f71218..af59dc74e0dc 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -6,7 +6,7 @@ ARCH ?= $(shell uname -m)
> ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
>
> ifeq ("x$(ARCH)", "xarm64")
> -SUBDIRS :=
> +SUBDIRS := signal
> else
> SUBDIRS :=
> endif
> diff --git a/tools/testing/selftests/arm64/signal/.gitignore b/tools/testing/selftests/arm64/signal/.gitignore
> new file mode 100644
> index 000000000000..434f65c15f03
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/.gitignore
> @@ -0,0 +1,6 @@
> +# Helper script's internal testcases list (TPROGS) is regenerated
> +# each time by Makefile on standalone (non KSFT driven) runs.
> +# Committing such list creates a dependency between testcases
> +# patches such that they are no more easily revertable. Just ignore.
> +test_arm64_signals.src_shell
> +test_arm64_signals.sh
> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
> new file mode 100644
> index 000000000000..8c8d08be4b0d
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/Makefile
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
> +
> +# Supports also standalone invokation out of KSFT-tree
> +# Compile standalone and run on your device with:
> +#
> +# $ make -C tools/testing/selftests/arm64/signal INSTALL_PATH=<your-dir> install
I'm wondering whether supporting stand-alone invocation is actually
worth it. Maybe this just adds complexity for little benefit.
Although it's useful for debugging and development, it doesn't look like
other tests in kselftest support standalone invocation -- did I miss
some?
> +#
> +# Run standalone on device with:
> +#
> +# $ <your-device-instdir>/test_arm64_signals.sh [-k|-v]
> +#
> +# If INSTALL_PATH= is NOT provided it will default to ./install
> +
> +# A proper top_srcdir is needed both by KSFT(lib.mk)
> +# and standalone builds
> +top_srcdir = ../../../../..
> +
> +CFLAGS += -std=gnu99 -I. -I$(top_srcdir)/tools/testing/selftests/
> +SRCS := $(filter-out testcases/testcases.c,$(wildcard testcases/*.c))
> +PROGS := $(patsubst %.c,%,$(SRCS))
> +
> +# Guessing as best as we can where the Kernel headers
> +# could have been installed depending on ENV config and
> +# type of invocation.
> +ifeq ($(KBUILD_OUTPUT),)
> +khdr_dir = $(top_srcdir)/usr/include
> +else
> +ifeq (0,$(MAKELEVEL))
> +khdr_dir = $(KBUILD_OUTPUT)/usr/include
> +else
> +# the KSFT preferred location when KBUILD_OUTPUT is set
> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> +endif
> +endif
When is KBUILD_OUTPUT set / not set?
> +
> +CFLAGS += -I$(khdr_dir)
Do we rely on any non-UAPI headers? If not, the default should probably
be to rely on the system headers (or toolchain default headers) -- i.e.,
add no -I option at all.
I'm wondering why none of the other kselftests need this header search
logic.
> +
> +# Standalone run
> +ifeq (0,$(MAKELEVEL))
> +CC := $(CROSS_COMPILE)gcc
> +RUNNER_SRC = test_arm64_signals.src_shell
> +RUNNER = test_arm64_signals.sh
> +INSTALL_PATH ?= install/
> +
> +all: $(RUNNER)
> +
> +$(RUNNER): $(PROGS)
$(RUNNER_SRC) should also be in the dependencies here.
> + cp $(RUNNER_SRC) $(RUNNER)
> + sed -i -e 's#PROGS=.*#PROGS="$(PROGS)"#' $@
Or just a single command: sed -e '...' <$< >$@
> +
> +install: all
> + mkdir -p $(INSTALL_PATH)/testcases
> + cp $(PROGS) $(INSTALL_PATH)/testcases
> + cp $(RUNNER) $(INSTALL_PATH)/
> +
> +.PHONY clean:
> + rm -f $(PROGS)
> +# KSFT run
> +else
> +# Generated binaries to be installed by top KSFT script
> +TEST_GEN_PROGS := $(notdir $(PROGS))
> +
> +# Get Kernel headers installed and use them.
> +KSFT_KHDR_INSTALL := 1
> +
> +# This include mk will also mangle the TEST_GEN_PROGS list
> +# to account for any OUTPUT target-dirs optionally provided
> +# by the toplevel makefile
> +include ../../lib.mk
> +
> +$(TEST_GEN_PROGS): $(PROGS)
> + cp $(PROGS) $(OUTPUT)/
> +
> +clean:
> + $(CLEAN)
> + rm -f $(PROGS)
> +endif
> +
> +# Common test-unit targets to build common-layout test-cases executables
> +# Needs secondary expansion to properly include the testcase c-file in pre-reqs
> +.SECONDEXPANSION:
> +$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> + @if [ ! -d $(khdr_dir) ]; then \
> + echo -n "\n!!! WARNING: $(khdr_dir) NOT FOUND."; \
> + echo "===> Are you sure Kernel Headers have been installed properly ?\n"; \
> + fi
> + $(CC) $(CFLAGS) $^ -o $@
> diff --git a/tools/testing/selftests/arm64/signal/README b/tools/testing/selftests/arm64/signal/README
> new file mode 100644
> index 000000000000..53f005f7910a
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/README
> @@ -0,0 +1,59 @@
> +KSelfTest arm64/signal/
> +=======================
> +
> +Signals Tests
> ++++++++++++++
> +
> +- Tests are built around a common main compilation unit: such shared main
> + enforces a standard sequence of operations needed to perform a single
> + signal-test (setup/trigger/run/result/cleanup)
> +
> +- The above mentioned ops are configurable on a test-by-test basis: each test
> + is described (and configured) using the descriptor signals.h::struct tdescr
> +
> +- Each signal testcase is compiled into its own executable: a separate
> + executable is used for each test since many tests complete successfully
> + by receiving some kind of fatal signal from the Kernel, so it's safer
> + to run each test unit in its own standalone process, so as to start each
> + test from a clean slate.
> +
> +- New tests can be simply defined in testcases/ dir providing a proper struct
> + tdescr overriding all the defaults we wish to change (as of now providing a
> + custom run method is mandatory though)
> +
> +- Signals' test-cases hereafter defined belong currently to two
> + principal families:
> +
> + - 'mangle_' tests: a real signal (SIGUSR1) is raised and used as a trigger
> + and then the test case code messes-up with the sigframe ucontext_t from
> + inside the sighandler itself.
"messes-up" makes it sound a bit like the test case code itself goes
wrong.
Maybe just say something like "the test case code modifies the signal
frame from inside the signal handler itself."
> +
> + - 'fake_sigreturn_' tests: a brand new custom artificial sigframe structure
> + is placed on the stack and a sigreturn syscall is called to simulate a
> + real signal return. This kind of tests does not use a trigger usually and
> + they are just fired using some simple included assembly trampoline code.
> +
> + - Most of these tests are successfully passing if the process gets killed by
> + some fatal signal: usually SIGSEGV or SIGBUS. Since while writing this
> + kind of tests it is extremely easy in fact to end-up injecting other
> + unrelated SEGV bugs in the testcases, it becomes extremely tricky to
> + be really sure that the tests are really addressing what they are meant
> + to address and they are not instead falling apart due to unplanned bugs
> + in the test code.
> + In order to alleviate the misery of the life of such test-developer, a few
> + helpers are provided:
> +
> + - a couple of ASSERT_BAD/GOOD_CONTEXT() macros to easily parse a ucontext_t
> + and verify if it is indeed GOOD or BAD (depending on what we were
> + expecting), using the same logic/perspective as in the arm64 Kernel signals
> + routines.
> +
> + - a sanity mechanism to be used in 'fake_sigreturn_'-alike tests: enabled by
> + default it takes care to verify that the test-execution had at least
> + successfully progressed up to the stage of triggering the fake sigreturn
> + call.
> +
> + In both cases test results are expected in terms of:
> + - some fatal signal sent by the Kernel to the test process
> + or
> + - analyzing some final regs state
> diff --git a/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell b/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
> new file mode 100755
> index 000000000000..163e941e2997
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
Unusual filename?
In the non-standalone case, is this run directly with TPROGS set in the
environment instead of modifying the script? (I haven't understood all
the logic yet.)
If so, it is a shell script, and should just be called
test_arm64_signals.sh
Otherwise, it's a non-executable template for a shell script, so should
have 0644 permissions and could be called test_arm64_signals.sh.in or
test_arm64_signals.sh.template, say.
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
> +
> +ret=0
> +keep_on_fail=0
> +err_out="2> /dev/null"
> +
> +usage() {
> + echo "Usage: `basename $0` [-v] [-k]"
> + exit 1
> +}
> +
> +# avoiding getopt to avoid compatibility issues on targets
> +# with limited resources
> +while [ $# -gt 0 ]
> +do
> + case $1 in
> + "-k")
> + keep_on_fail=1
> + ;;
> + "-v")
> + err_out=
> + ;;
> + *)
> + usage
> + ;;
> + esac
> + shift
> +done
> +
> +TPROGS=
> +
> +tot=$(echo $TPROGS | wc -w)
> +
> +# Tests are expected in testcases/ subdir inside the installation path
> +workdir="`dirname $0 2>/dev/null`"
> +[ -n $workdir ] && cd $workdir
> +
> +passed=0
> +run=0
> +for test in $TPROGS
> +do
> + run=$((run + 1))
> + eval ./$test $err_out
> + if [ $? != 0 ]; then
> + [ $keep_on_fail = 0 ] && echo "===>>> FAILED:: $test <<<===" && ret=1 && break
> + else
> + passed=$((passed + 1))
> + fi
> +done
> +
> +echo "==>> PASSED: $passed/$run on $tot available tests."
> +
> +exit $ret
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.c b/tools/testing/selftests/arm64/signal/test_signals.c
> new file mode 100644
> index 000000000000..3447d7011aec
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
We should probably have a brief comment to say what this is.
For example:
/*
* Generic test wrapper for arm64 signal tests
* Each test provides its own tde to link with this wrapper.
*/
> +
> +#include <kselftest.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +
> +struct tdescr *current;
> +extern struct tdescr tde;
> +
> +int main(int argc, char *argv[])
> +{
> + current = &tde;
> +
> + ksft_print_msg("%s :: %s - SIG_TRIG:%d SIG_OK:%d -- current:%p\n",
> + current->name, current->descr, current->sig_trig,
> + current->sig_ok, current);
Does the user need all this?
It's sufficient to print the test name, a one-line description and
results. If something goes wrong, we can print a bit more detail.
Maybe just do something like
#ifdef DEBUG
#define debug_printf(format, ...) ksft_print_msg(format, ## __VA_ARGS__)
#else
#define debug_printf(format, ...) ((void)0)
#endif
(Unless kselftest already has something like this, in which case you
could just use that.)
> + if (test_setup(current)) {
> + if (test_run(current))
> + test_result(current);
> + test_cleanup(current);
> + }
> +
> + return current->pass ? KSFT_PASS : KSFT_FAIL;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
> new file mode 100644
> index 000000000000..85db3ac44b32
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals.h
> @@ -0,0 +1,137 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_H__
> +#define __TEST_SIGNALS_H__
> +
> +#include <assert.h>
> +#include <stdbool.h>
> +#include <signal.h>
> +#include <ucontext.h>
> +#include <stdint.h>
Does anything in this header use <assert.h> or <stdint.h>?
> +
> +/*
> + * Using ARCH specific and sanitized Kernel headers installed by KSFT
> + * framework since we asked for it by setting flag KSFT_KHDR_INSTALL
> + * in our Makefile.
> + */
> +#include <asm/ptrace.h>
> +#include <asm/hwcap.h>
> +
> +/* pasted from include/linux/stringify.h */
> +#define __stringify_1(x...) #x
> +#define __stringify(x...) __stringify_1(x)
> +
> +/*
I think we can delete this entire comment.
The macro name is fairly self-explanatory anyway. Although the
rationale is interesting, our approach to reading system registers
here is just the same as elsewhere in the kernel.
> + * Reads a sysreg using the, possibly provided, S3_ encoding in order to
> + * avoid inject any dependency on the used toolchain regarding possibly
> + * still unsupported ARMv8 extensions.
> + *
> + * Using a standard mnemonic here to indicate the specific sysreg (like SSBS)
> + * would introduce a compile-time dependency on possibly unsupported ARMv8
> + * Extensions: you could end-up failing to build the test depending on the
> + * available toolchain.
> + * This is undesirable since some tests, even if specifically targeted at some
> + * ARMv8 Extensions, can be plausibly run even on hardware lacking the above
> + * optional ARM features. (SSBS bit preservation is an example: Kernel handles
> + * it transparently not caring at all about the effective set of supported
> + * features).
> + * On the other side we will expect to observe different behaviours if the
> + * feature is supported or not: usually getting a SIGILL when trying to use
> + * unsupported features. For this reason we have anyway in place some
> + * preliminary run-time checks about the cpu effectively supported features.
> + *
> + * This helper macro is meant to be used for regs readable at EL0, BUT some
> + * EL1 sysregs are indeed readable too through MRS emulation Kernel-mechanism
> + * if the required reg is included in the supported encoding space:
> + *
> + * Documentation/arm64/cpu-feature-regsiters.txt
> + *
> + * "The infrastructure emulates only the following system register space:
> + * Op0=3, Op1=0, CRn=0, CRm=0,4,5,6,7
> + */
> +#define get_regval(regname, out) \
> + asm volatile("mrs %0, " __stringify(regname) : "=r" (out) :: "memory")
> +
> +/* Regs encoding and masks naming copied in from sysreg.h */
> +#define SYS_ID_AA64MMFR1_EL1 S3_0_C0_C7_1 /* MRS Emulated */
> +#define SYS_ID_AA64MMFR2_EL1 S3_0_C0_C7_2 /* MRS Emulated */
These ID regs are part of armv8.0-a, so we don't need to use the magic
syntax.
> +#define ID_AA64MMFR1_PAN_SHIFT 20
> +#define ID_AA64MMFR2_UAO_SHIFT 4
> +
> +/* Local Helpers */
Can these names indicate the sysreg they should be used with, e.g.
#define ID_AA64MMFR1_EL1_PAN_SUPPORTED(val) ...
#define ID_AA64MMFR2_EL1_UAO_SUPPORTED(val) ...
> +#define IS_PAN_SUPPORTED(val) \
> + (!!((val) & (0xfUL << ID_AA64MMFR1_PAN_SHIFT)))
> +#define IS_UAO_SUPPORTED(val) \
> + (!!((val) & (0xfUL << ID_AA64MMFR2_UAO_SHIFT)))
> +
> +#define S3_MRS_SSBS_SYSREG S3_3_C4_C2_6 /* EL0 supported */
Maybe just SSBS_SYSREG.
Sysreg encodings are always for use with MRS/MSR anyway, and "S3" is
really part of the definition rather than part of the name.
> +
> +/*
> + * Feature flags used in tdescr.feats_required to specify
> + * any feature by the test
> + */
> +enum {
> + FSSBS_BIT,
> + FPAN_BIT,
> + FUAO_BIT,
> + FMAX_END
> +};
> +
> +#define FEAT_SSBS (1UL << FSSBS_BIT)
> +#define FEAT_PAN (1UL << FPAN_BIT)
> +#define FEAT_UAO (1UL << FUAO_BIT)
> +
> +/*
> + * A descriptor used to describe and configure a test case.
> + * Fields with a non-trivial meaning are described inline in the following.
> + */
> +struct tdescr {
> + /* KEEP THIS FIELD FIRST for easier lookup from assembly */
> + void *token;
> + /* when disabled token based sanity checking is skipped in handler */
> + bool sanity_disabled;
> + /* just a name for the test-case; manadatory field */
> + char *name;
> + char *descr;
> + unsigned long feats_required;
> + /* bitmask of effectively supported feats: populated at run-time */
> + unsigned long feats_supported;
> + bool feats_ok;
Is feats_ok used?
> + bool initialized;
> + unsigned int minsigstksz;
> + /* signum used as a test trigger. Zero if no trigger-signal is used */
> + int sig_trig;
> + /*
> + * signum considered as a successful test completion.
> + * Zero when no signal is expected on success
> + */
> + int sig_ok;
> + /* signum expected on unsupported CPU features. */
> + int sig_unsupp;
> + /* a timeout in second for test completion */
> + unsigned int timeout;
> + bool triggered;
> + bool pass;
> + /* optional sa_flags for the installed handler */
> + int sa_flags;
> + ucontext_t saved_uc;
> +
> + /* a setup function to be called before test starts */
> + int (*setup)(struct tdescr *td);
> + void (*cleanup)(struct tdescr *td);
Add a comment to say what cleanup() is?
> +
> + /* an optional function to be used as a trigger for test starting */
> + int (*trigger)(struct tdescr *td);
> + /*
> + * the actual test-core: invoked differently depending on the
> + * presence of the trigger function above; this is mandatory
> + */
> + int (*run)(struct tdescr *td, siginfo_t *si, ucontext_t *uc);
> +
> + /* an optional function for custom results' processing */
> + void (*check_result)(struct tdescr *td);
> +
> + void *priv;
> +};
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> new file mode 100644
> index 000000000000..ac0055f6340b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -0,0 +1,261 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <sys/auxv.h>
> +#include <linux/auxvec.h>
> +#include <ucontext.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +#include "testcases/testcases.h"
> +
> +extern struct tdescr *current;
> +
> +static char *feats_store[FMAX_END] = {
> + "SSBS",
> + "PAN",
> + "UAO"
> +};
> +
> +#define MAX_FEATS_SZ 128
> +static inline char *feats_to_string(unsigned long feats)
> +{
> + static char feats_string[MAX_FEATS_SZ];
> +
> + for (int i = 0; i < FMAX_END && feats_store[i][0]; i++) {
> + if (feats & 1UL << i)
> + snprintf(feats_string, MAX_FEATS_SZ - 1, "%s %s ",
> + feats_string, feats_store[i]);
> + }
> +
> + return feats_string;
> +}
> +
> +static void unblock_signal(int signum)
> +{
> + sigset_t sset;
> +
> + sigemptyset(&sset);
> + sigaddset(&sset, signum);
> + sigprocmask(SIG_UNBLOCK, &sset, NULL);
> +}
> +
> +static void default_result(struct tdescr *td, bool force_exit)
> +{
> + if (td->pass)
> + fprintf(stderr, "==>> completed. PASS(1)\n");
> + else
> + fprintf(stdout, "==>> completed. FAIL(0)\n");
> + if (force_exit)
> + exit(td->pass ? EXIT_SUCCESS : EXIT_FAILURE);
> +}
> +
> +static inline bool are_feats_ok(struct tdescr *td)
> +{
> + return td ? td->feats_required == td->feats_supported : 0;
Should this be something like
(td->feats_required & td->feats_supported) == td->feats_required ?
Otherwise additional supported features that our test doesn't care about
will cause this check to fail.
Do we really need to check td?
assert(foo); followed by dereferincing foo is usually a bit pointless
because you'd get a SIGSEGV anyway.
However, since the tests generate deliberate SIGSEGVs too this could
be confusing -- in which case, having an explicit assert() here does
no harm.
> +}
> +
> +static void default_handler(int signum, siginfo_t *si, void *uc)
> +{
> + if (current->sig_trig && signum == current->sig_trig) {
> + fprintf(stderr, "Handling SIG_TRIG\n");
> + current->triggered = 1;
> + /* ->run was asserted NON-NULL in test_setup() already */
> + current->run(current, si, uc);
> + } else if (signum == SIGILL && !current->initialized) {
> + /*
> + * A SIGILL here while still not initialized means we failed
> + * even to asses the existence of features during init
> + */
> + fprintf(stdout,
> + "Got SIGILL test_init. Marking ALL features UNSUPPORTED.\n");
> + current->feats_supported = 0;
> + } else if (current->sig_ok && signum == current->sig_ok) {
> + /* it's a bug in the test code when this assert fail */
Why? Is this because sig_ok is considered acceptable only as an effect
of the test -- i.e., we shouldn't see it if the test hasn't been
triggered yet?
> + assert(!current->sig_trig || current->triggered);
> + fprintf(stderr,
> + "SIG_OK -- SP:%p si_addr@:0x%p si_code:%d token@:0x%p offset:%ld\n",
> + ((ucontext_t *)uc)->uc_mcontext.sp,
> + si->si_addr, si->si_code, current->token,
> + current->token - si->si_addr);
> + /*
> + * fake_sigreturn tests, which have sanity_enabled=1, set, at
> + * the very last time, the token field to the SP address used
> + * to place the fake sigframe: so token==0 means we never made
> + * it to the end, segfaulting well-before, and the test is
> + * possibly broken.
> + */
> + if (!current->sanity_disabled && !current->token) {
> + fprintf(stdout,
> + "current->token ZEROED...test is probably broken!\n");
> + assert(0);
In case someone builds with -DNDEBUG, should we add abort()?
> + }
> + /*
> + * Trying to narrow down the SEGV to the ones generated by
> + * Kernel itself via arm64_notify_segfault()
> + */
> + if (current->sig_ok == SIGSEGV && si->si_code != SEGV_ACCERR) {
> + fprintf(stdout,
> + "si_code != SEGV_ACCERR...test is probably broken!\n");
> + assert(0);
> + }
I'm not sure whether si_code is really ABI here, though I'm not sure
what else we can do to diagnose the signal more accurately.
Maybe add a comment to say that this might need to change if this
aspect of the kernel ABI evolves.
> + fprintf(stderr, "Handling SIG_OK\n");
> + current->pass = 1;
> + /*
> + * Some tests can lead to SEGV loops: in such a case we want
> + * to terminate immediately exiting straight away
> + */
> + default_result(current, 1);
> + } else {
> + if (signum == current->sig_unsupp && !are_feats_ok(current)) {
> + fprintf(stderr, "-- RX SIG_UNSUPP on unsupported feature...OK\n");
> + current->pass = 1;
> + } else if (signum == SIGALRM && current->timeout) {
> + fprintf(stderr, "-- Timeout !\n");
> + } else {
> + fprintf(stderr,
> + "-- RX UNEXPECTED SIGNAL: %d\n", signum);
> + }
> + default_result(current, 1);
> + }
> +}
> +
> +static int default_setup(struct tdescr *td)
> +{
> + struct sigaction sa;
> +
> + sa.sa_sigaction = default_handler;
> + sa.sa_flags = SA_SIGINFO;
Add SA_RESTART?
I'm not sure whether this affects these tests, but the libc stdio
functions don't like being interrupted by signals. SA_RESTART should
hide most issues of this sort.
> + if (td->sa_flags)
> + sa.sa_flags |= td->sa_flags;
Do we need the if() here? If td->sa_flags == 0, the assignment is
harmless anyway.
> + sigemptyset(&sa.sa_mask);
> + /* uncatchable signals naturally skipped ... */
> + for (int sig = 1; sig < 32; sig++)
> + sigaction(sig, &sa, NULL);
> + /*
> + * RT Signals default disposition is Term but they cannot be
> + * generated by the Kernel in response to our tests; so just catch
> + * them all and report them as UNEXPECTED signals.
> + */
> + for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++)
> + sigaction(sig, &sa, NULL);
> +
> + /* just in case...unblock explicitly all we need */
> + if (td->sig_trig)
> + unblock_signal(td->sig_trig);
> + if (td->sig_ok)
> + unblock_signal(td->sig_ok);
> + if (td->sig_unsupp)
> + unblock_signal(td->sig_unsupp);
> +
> + if (td->timeout) {
> + unblock_signal(SIGALRM);
> + alarm(td->timeout);
> + }
> + fprintf(stderr, "Registered handlers for all signals.\n");
> +
> + return 1;
> +}
> +
> +static inline int default_trigger(struct tdescr *td)
> +{
> + return !raise(td->sig_trig);
> +}
> +
> +static int test_init(struct tdescr *td)
> +{
> + td->minsigstksz = getauxval(AT_MINSIGSTKSZ);
> + if (!td->minsigstksz)
> + td->minsigstksz = MINSIGSTKSZ;
> + fprintf(stderr, "Detected MINSTKSIGSZ:%d\n", td->minsigstksz);
> +
> + if (td->feats_required) {
> + bool feats_ok = false;
> + td->feats_supported = 0;
> + /*
> + * Checking for CPU required features using both the
> + * auxval and the arm64 MRS Emulation to read sysregs.
> + */
> + if (getauxval(AT_HWCAP) & HWCAP_CPUID) {
> + uint64_t val = 0;
> +
Would it be simpler just to query all these features unconditionally?
We just need to check that all the features the test needs are present.
If other features are present, we can happily ignore them, but
discovering them is harmless.
> + if (td->feats_required & FEAT_SSBS) {
> + /* Uses HWCAP to check capability */
> + if (getauxval(AT_HWCAP) & HWCAP_SSBS)
> + td->feats_supported |= FEAT_SSBS;
> + }
> + if (td->feats_required & FEAT_PAN) {
> + /* Uses MRS emulation to check capability */
> + get_regval(SYS_ID_AA64MMFR1_EL1, val);
> + if (IS_PAN_SUPPORTED(val))
> + td->feats_supported |= FEAT_PAN;
> + }
> + if (td->feats_required & FEAT_UAO) {
> + /* Uses MRS emulation to check capability */
> + get_regval(SYS_ID_AA64MMFR2_EL1 , val);
> + if (IS_UAO_SUPPORTED(val))
> + td->feats_supported |= FEAT_UAO;
> + }
> + } else {
> + fprintf(stderr,
> + "HWCAP_CPUID NOT available. Mark ALL feats UNSUPPORTED.\n");
> + }
> + feats_ok = are_feats_ok(td);
> + fprintf(stderr,
> + "Required Features: [%s] %ssupported\n",
> + feats_ok ? feats_to_string(td->feats_supported) :
> + feats_to_string(td->feats_required ^ td->feats_supported),
> + !feats_ok ? "NOT " : "");
> + }
> +
> + td->initialized = 1;
> + return 1;
> +}
> +
> +int test_setup(struct tdescr *td)
> +{
> + /* assert core invariants symptom of a rotten testcase */
> + assert(current);
> + assert(td);
> + assert(td->name);
> + assert(td->run);
> +
> + if (!test_init(td))
> + return 0;
> +
> + if (td->setup)
> + return td->setup(td);
> + else
> + return default_setup(td);
> +}
> +
> +int test_run(struct tdescr *td)
> +{
> + if (td->sig_trig) {
> + if (td->trigger)
> + return td->trigger(td);
> + else
> + return default_trigger(td);
> + } else {
> + return td->run(td, NULL, NULL);
> + }
> +}
> +
> +void test_result(struct tdescr *td)
> +{
> + if (td->check_result)
> + td->check_result(td);
> + default_result(td, 0);
> +}
> +
> +void test_cleanup(struct tdescr *td)
> +{
> + if (td->cleanup)
> + td->cleanup(td);
> +}
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> new file mode 100644
> index 000000000000..8658d1a7d4b9
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_UTILS_H__
> +#define __TEST_SIGNALS_UTILS_H__
> +
> +#include "test_signals.h"
> +
> +int test_setup(struct tdescr *td);
> +void test_cleanup(struct tdescr *td);
> +int test_run(struct tdescr *td);
> +void test_result(struct tdescr *td);
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> new file mode 100644
> index 000000000000..8651272e3cfc
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -0,0 +1 @@
> +mangle_pstate_invalid_compat_toggle
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> new file mode 100644
> index 000000000000..971193e7501b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
Each testcase should have a comment explaining what it is trying to
test, and how.
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /* This config should trigger a SIGSEGV by Kernel */
> + uc->uc_mcontext.pstate ^= PSR_MODE32_BIT;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_PSTATE_INVALID_STATE_TOGGLE",
> + .descr = "Mangling uc_mcontext with INVALID STATE_TOGGLE",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_pstate_run,
> +};
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> new file mode 100644
> index 000000000000..a59785092e1f
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> @@ -0,0 +1,150 @@
> +#include "testcases.h"
> +
> +struct _aarch64_ctx *get_header(struct _aarch64_ctx *head, uint32_t magic,
> + size_t resv_sz, size_t *offset)
> +{
> + size_t offs = 0;
> + struct _aarch64_ctx *found = NULL;
> +
> + if (!head || resv_sz < HDR_SZ)
> + return found;
> +
> + do {
> + if (head->magic == magic) {
> + found = head;
> + break;
> + }
> + offs += head->size;
> + head = GET_RESV_NEXT_HEAD(head);
Are offs and head tracking the same thing here?
Maybe it would be cleaner to have GET_RESV_NEXT_HEAD() do the bounds
checking itself.
> + } while (offs < resv_sz - HDR_SZ);
> +
> + if (offset)
> + *offset = offs;
> +
> + return found;
> +}
> +
> +bool validate_extra_context(struct extra_context *extra, char **err)
> +{
> + struct _aarch64_ctx *term;
> +
> + if (!extra || !err)
> + return false;
> +
> + fprintf(stderr, "Validating EXTRA...\n");
> + term = GET_RESV_NEXT_HEAD(extra);
> + if (!term || term->magic || term->size) {
> + *err = "UN-Terminated EXTRA context";
This sounds like the extra context doesn't contain a terminator, which
isn't what we're checking here. Maybe say "terminator missing after
extra context", or similar.
> + return false;
> + }
> + if (extra->datap & 0x0fUL)
> + *err = "Extra DATAP misaligned";
> + else if (extra->size & 0x0fUL)
> + *err = "Extra SIZE misaligned";
> + else if (extra->datap != (uint64_t)term + sizeof(*term))
> + *err = "Extra DATAP misplaced (not contiguos)";
> + if (*err)
> + return false;
> +
> + return true;
> +}
> +
> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
> +{
> + bool terminated = false;
> + size_t offs = 0;
> + int flags = 0;
> + struct extra_context *extra = NULL;
> + struct _aarch64_ctx *head =
> + (struct _aarch64_ctx *)uc->uc_mcontext.__reserved;
> +
> + if (!err)
> + return false;
> + /* Walk till the end terminator verifying __reserved contents */
> + while (head && !terminated && offs < resv_sz) {
> + if ((uint64_t)head & 0x0fUL) {
> + *err = "Misaligned HEAD";
> + return false;
> + }
> +
> + switch (head->magic) {
> + case 0:
> + if (head->size)
> + *err = "Bad size for MAGIC0";
Or "terminator". We don't have an actual symbolic name for magic number
0. (Arguably it would have been nice to have a name, but we managed
without.)
> + else
> + terminated = true;
> + break;
> + case FPSIMD_MAGIC:
> + if (flags & FPSIMD_CTX)
> + *err = "Multiple FPSIMD_MAGIC";
> + else if (head->size !=
> + sizeof(struct fpsimd_context))
> + *err = "Bad size for fpsimd_context";
> + flags |= FPSIMD_CTX;
> + break;
> + case ESR_MAGIC:
> + if (head->size != sizeof(struct esr_context))
> + fprintf(stderr,
> + "Bad size for esr_context is not an error...just ignore.\n");
Why isn't this an error? Should the kernel ever write an esr_context
with a different size?
> + break;
> + case SVE_MAGIC:
> + if (flags & SVE_CTX)
> + *err = "Multiple SVE_MAGIC";
> + else if (head->size !=
> + sizeof(struct sve_context))
> + *err = "Bad size for sve_context";
> + flags |= SVE_CTX;
> + break;
> + case EXTRA_MAGIC:
> + if (flags & EXTRA_CTX)
> + *err = "Multiple EXTRA_MAGIC";
> + else if (head->size !=
> + sizeof(struct extra_context))
> + *err = "Bad size for extra_context";
> + flags |= EXTRA_CTX;
> + extra = (struct extra_context *)head;
> + break;
> + case KSFT_BAD_MAGIC:
> + /*
> + * This is a BAD magic header defined
> + * artificially by a testcase and surely
> + * unknown to the Kernel parse_user_sigframe().
> + * It MUST cause a Kernel induced SEGV
> + */
> + *err = "BAD MAGIC !";
> + break;
> + default:
> + /*
> + * A still unknown Magic: potentially freshly added
> + * to the Kernel code and still unknown to the
> + * tests.
> + */
> + fprintf(stdout,
> + "SKIP Unknown MAGIC: 0x%X - Is KSFT arm64/signal up to date ?\n",
> + head->magic);
> + break;
> + }
> +
> + if (*err)
> + return false;
> +
> + offs += head->size;
Can this addition cause offs to become > resv_sz? If so, the next
comparison will go wrong.
> + if (resv_sz - offs < sizeof(*head)) {
> + *err = "HEAD Overrun";
> + return false;
> + }
> +
> + if (flags & EXTRA_CTX)
> + if (!validate_extra_context(extra, err))
> + return false;
Can we validate the contents of the extra context too?
Ideally we can use the same code to check __reserved[] and the extra
context.
> +
> + head = GET_RESV_NEXT_HEAD(head);
> + }
> +
> + if (terminated && !(flags & FPSIMD_CTX)) {
> + *err = "Missing FPSIMD";
> + return false;
> + }
> +
> + return true;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.h b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> new file mode 100644
> index 000000000000..624717c71b1d
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> @@ -0,0 +1,83 @@
> +#ifndef __TESTCASES_H__
> +#define __TESTCASES_H__
> +
Pedantically, we should have <stddef.h> for NULL.
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <unistd.h>
Is <unistd.h> used now that SAFE_WRITE() is gone?
> +#include <ucontext.h>
> +#include <assert.h>
> +
> +/* Architecture specific sigframe definitions */
> +#include <asm/sigcontext.h>
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 03/11] kselftest: arm64: mangle_pstate_invalid_daif_bits
From: Dave Martin @ 2019-08-13 16:24 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-4-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:52PM +0100, Cristian Marussi wrote:
> Added a simple mangle testcase which messes with the ucontext_t
Strange past tense? How about "Add"?
> from within the sig_handler, trying to set PSTATE DAIF bits to an
"signal handler"?
> invalid value (masking everything). Expects SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 1 +
> .../mangle_pstate_invalid_daif_bits.c | 28 +++++++++++++++++++
> 2 files changed, 29 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 8651272e3cfc..8a0a29f0cc2a 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -1 +1,2 @@
> mangle_pstate_invalid_compat_toggle
> +mangle_pstate_invalid_daif_bits
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
> new file mode 100644
> index 000000000000..af899d4bb655
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /*
> + * This config should trigger a SIGSEGV by Kernel when it checks
> + * the sigframe consistency in valid_user_regs() routine.
> + */
> + uc->uc_mcontext.pstate |= PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT;
> +
> + return 1;
> +}
Hmmm, there was a lot of common framework code, but it seems like a good
investment if adding a new test is as simple as this :)
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 04/11] kselftest: arm64: mangle_pstate_invalid_mode_el
From: Dave Martin @ 2019-08-13 16:24 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-5-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:53PM +0100, Cristian Marussi wrote:
> Added 3 simple mangle testcases that mess with the ucontext_t
Add
> from within the sig_handler, trying to toggle PSTATE mode bits to
signal handler
> trick the system into switching to EL1/EL2/EL3. Expects SIGSEGV
> on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 3 ++
> .../mangle_pstate_invalid_mode_el1.c | 29 +++++++++++++++++++
> .../mangle_pstate_invalid_mode_el2.c | 29 +++++++++++++++++++
> .../mangle_pstate_invalid_mode_el3.c | 29 +++++++++++++++++++
> 4 files changed, 90 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 8a0a29f0cc2a..226bb179b673 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -1,2 +1,5 @@
> mangle_pstate_invalid_compat_toggle
> mangle_pstate_invalid_daif_bits
> +mangle_pstate_invalid_mode_el1
> +mangle_pstate_invalid_mode_el2
> +mangle_pstate_invalid_mode_el3
What about having
!*.[ch]
mangle_*
rather than having to update .gitignore to list every test executable?
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c
> new file mode 100644
> index 000000000000..07aed7624383
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1.c
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /*
> + * This config should trigger a SIGSEGV by Kernel
> + * when checking valid_user_regs()
> + */
> + uc->uc_mcontext.pstate &= ~PSR_MODE_MASK;
> + uc->uc_mcontext.pstate |= PSR_MODE_EL1t;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_PSTATE_INVALID_MODE_EL1t",
> + .descr = "Mangling uc_mcontext with INVALID MODE EL1t",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_pstate_run,
> +};
These tests seem identical except for the EL number.
Can we macro-ise them?
mangle_pstate_invalid_mode_el1.c could become
--8<--
#include "mangle_pstate_invalid_mode.h"
DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(1)
-->8--
(for example).
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 05/11] kselftest: arm64: mangle_pstate_ssbs_regs
From: Dave Martin @ 2019-08-13 16:25 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-6-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:54PM +0100, Cristian Marussi wrote:
> Added a simple mangle testcase which messes with the ucontext_t
Add
> from within the sig_handler, trying to toggle PSTATE SSBS bit.
signal handler
> Expect SIGILL if SSBS feature unsupported or that the value set in
> PSTATE.SSBS is preserved on test PASS.
The test doesn't set PSTATE.SSBS directly.
Maybe something like: "Expect SIGILL if the SSBS feature is unsupported.
Otherwise, expect sigreturn to set PSTATE.SSBS from the corresponding
bit in pstate in the signal frame."
> This commit also introduces a new common utility function:
> get_current_context() which can be used to grab a ucontext without
> the help of libc, and detect if such ucontext has been actively used
> to jump back into it.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../selftests/arm64/signal/test_signals.h | 4 +
> .../arm64/signal/test_signals_utils.c | 93 +++++++++++++++++++
> .../arm64/signal/test_signals_utils.h | 2 +
> .../arm64/signal/testcases/.gitignore | 1 +
> .../testcases/mangle_pstate_ssbs_regs.c | 56 +++++++++++
> 5 files changed, 156 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
>
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
> index 85db3ac44b32..37bed0590226 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals.h
> @@ -116,6 +116,10 @@ struct tdescr {
> /* optional sa_flags for the installed handler */
> int sa_flags;
> ucontext_t saved_uc;
> + /* used by get_current_ctx() */
> + size_t live_sz;
> + ucontext_t *live_uc;
> + volatile bool live_uc_valid;
>
> /* a setup function to be called before test starts */
> int (*setup)(struct tdescr *td);
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> index ac0055f6340b..faf55ba99d58 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.c
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -11,12 +11,16 @@
> #include <linux/auxvec.h>
> #include <ucontext.h>
>
> +#include <asm/unistd.h>
> +
> #include "test_signals.h"
> #include "test_signals_utils.h"
> #include "testcases/testcases.h"
>
> extern struct tdescr *current;
>
> +static int sig_copyctx = SIGUSR2;
> +
> static char *feats_store[FMAX_END] = {
> "SSBS",
> "PAN",
> @@ -37,6 +41,85 @@ static inline char *feats_to_string(unsigned long feats)
> return feats_string;
> }
>
> +/*
> + * Obtaining a valid and full-blown ucontext_t from userspace is tricky:
> + * libc getcontext does() not save all the regs and messes with some of
> + * them (pstate value in particular is not reliable).
> + * Here we use a service signal to grab the ucontext_t from inside a
> + * dedicated signal handler, since there, it is populated by Kernel
> + * itself in setup_sigframe(). The grabbed context is then stored and
> + * made available in td->live_uc.
> + *
> + * Anyway this function really serves a dual purpose:
> + *
> + * 1. grab a valid sigcontext into td->live_uc for result analysis: in
> + * such case it returns 1.
> + *
> + * 2. detect if somehow a previously grabbed live_uc context has been
> + * used actively with a sigreturn: in such a case the execution would have
> + * magically resumed in the middle of the function itself (seen_already==1):
> + * in such a case return 0, since in fact we have not just simply grabbed
> + * the context.
> + *
> + * This latter case is useful to detect when a fake_sigreturn test-case has
> + * unexpectedly survived without hittig a SEGV.
> + */
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc)
> +{
> + static volatile sig_atomic_t seen_already;
> +
> + if (!td || !dest_uc) {
> + fprintf(stdout, "Signal-based Context dumping NOT available\n");
Should this ever happen inless there is a test bug?
Maybe this should just be an assert.
> + return 0;
> + }
> +
> + /* it's a genuine invokation..reinit */
> + seen_already = 0;
> + td->live_uc_valid = 0;
> + td->live_sz = sizeof(*dest_uc);
> + memset(dest_uc, 0x00, td->live_sz);
Eventually we will need to examine the signal frame to determine its
size, but for now this is fine.
It will start to matter for SVE.
> + td->live_uc = dest_uc;
> + /*
> + * Grab ucontext_t triggering a signal...
> + * ASM equivalent of raise(sig_copyctx);
> + *
> + * Note that:
> + * - live_uc_valid is declared volatile in struct tdescr
> + * since it will be changed inside the sig_copyctx handler.
> + * - the kill() syscall invocation returns only after any possible
> + * registered sig_handler for the invoked signal has returned,
sig_handler looks like the name of some function of variable, but I
can't find it. Did I miss something?
> + * so that live_uc_valid flag is surely up to date when this
> + * function return it.
> + * - the additional 'memory' clobber is there to avoid possible
> + * compiler's assumption on the content pointed by dest_uc, which
> + * is changed inside the handler, but not referenced here anyway.
> + */
> + asm volatile ("mov x8, %0\n\t"
> + "svc #0\n\t"
> + "mov x1, %1\n\t"
> + "mov x8, %2\n\t"
> + "svc #0"
> + :
> + : "i" (__NR_getpid),
> + "r" (sig_copyctx),
> + "i" (__NR_kill)
> + : "x1","x8","x0","memory");
> + /*
> + * If we get here with seen_already==1 it implies the td->live_uc
> + * context has been used to get back here....this probably means
> + * a test has failed to cause a SEGV...anyway the live_uc has not
> + * just been acquired...so return 0
> + */
> + if (seen_already) {
> + fprintf(stdout,
> + "Successful sigreturn detected: live_uc is stale !\n");
> + return 0;
> + }
> + seen_already = 1;
> +
> + return td->live_uc_valid;
> +}
> +
> static void unblock_signal(int signum)
> {
> sigset_t sset;
> @@ -112,6 +195,12 @@ static void default_handler(int signum, siginfo_t *si, void *uc)
> * to terminate immediately exiting straight away
> */
> default_result(current, 1);
> + } else if (signum == sig_copyctx && current->live_uc) {
> + memcpy(current->live_uc, uc, current->live_sz);
> + ASSERT_GOOD_CONTEXT(current->live_uc);
> + current->live_uc_valid = 1;
> + fprintf(stderr,
> + "GOOD CONTEXT grabbed from sig_copyctx handler\n");
> } else {
> if (signum == current->sig_unsupp && !are_feats_ok(current)) {
> fprintf(stderr, "-- RX SIG_UNSUPP on unsupported feature...OK\n");
> @@ -214,6 +303,10 @@ static int test_init(struct tdescr *td)
> !feats_ok ? "NOT " : "");
> }
>
> + if (td->sig_trig == sig_copyctx)
> + sig_copyctx = SIGUSR1;
What's this for? What if we have the same signal for sig_trig and
sig_copyctx?
> + unblock_signal(sig_copyctx);
> +
> td->initialized = 1;
> return 1;
> }
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> index 8658d1a7d4b9..ce35be8ebc8e 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -10,4 +10,6 @@ int test_setup(struct tdescr *td);
> void test_cleanup(struct tdescr *td);
> int test_run(struct tdescr *td);
> void test_result(struct tdescr *td);
> +
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc);
> #endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 226bb179b673..a48a118b1a1a 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -3,3 +3,4 @@ mangle_pstate_invalid_daif_bits
> mangle_pstate_invalid_mode_el1
> mangle_pstate_invalid_mode_el2
> mangle_pstate_invalid_mode_el3
> +mangle_pstate_ssbs_regs
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> new file mode 100644
> index 000000000000..a399d9aa40d5
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_ssbs_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /* set bit value */
Should we clear SSBS in the test setup (using MSR), to make sure that
sigreturn really succeeds in _changing_ the bit to 1?
> + uc->uc_mcontext.pstate |= PSR_SSBS_BIT;
> + fprintf(stderr, "SSBS set to 1 -- PSTATE: 0x%016lX\n",
> + uc->uc_mcontext.pstate);
> + /* Save after mangling...it should be preserved */
> + td->saved_uc = *uc;
> +
> + return 1;
> +}
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Dave Martin @ 2019-08-13 16:25 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-7-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which builds a ucontext_t
Add
> with a bad magic header and place it onto the stack.
> Expects a SIGSEGV on test PASS.
> This commit also introduces a common utility assembly function to
> invoke a sigreturn using a fake provided sigframe.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> tools/testing/selftests/arm64/signal/Makefile | 2 +-
> .../testing/selftests/arm64/signal/signals.S | 64 +++++++++++++++++++
> .../arm64/signal/test_signals_utils.h | 1 +
> .../arm64/signal/testcases/.gitignore | 1 +
> .../testcases/fake_sigreturn_bad_magic.c | 63 ++++++++++++++++++
> 5 files changed, 130 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/arm64/signal/signals.S
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
>
> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
> index 8c8d08be4b0d..b3dcf315b5a4 100644
> --- a/tools/testing/selftests/arm64/signal/Makefile
> +++ b/tools/testing/selftests/arm64/signal/Makefile
> @@ -80,7 +80,7 @@ endif
> # Common test-unit targets to build common-layout test-cases executables
> # Needs secondary expansion to properly include the testcase c-file in pre-reqs
> .SECONDEXPANSION:
> -$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> +$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c signals.S $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> @if [ ! -d $(khdr_dir) ]; then \
> echo -n "\n!!! WARNING: $(khdr_dir) NOT FOUND."; \
> echo "===> Are you sure Kernel Headers have been installed properly ?\n"; \
> diff --git a/tools/testing/selftests/arm64/signal/signals.S b/tools/testing/selftests/arm64/signal/signals.S
> new file mode 100644
> index 000000000000..6262b877400b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/signals.S
> @@ -0,0 +1,64 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (C) 2019 ARM Limited
> + */
> +
> +#include <asm/unistd.h>
> +
> +.section ".rodata", "a"
The section name is not usually quoted in .section (though I guess it
works).
> +call_fmt:
> + .asciz "Calling sigreturn with fake sigframe sized:%zd at calculated SP @%08lX\n"
> +
> +.text
> +
> +.globl fake_sigreturn
> +
> +/* fake_sigreturn x0:&sigframe, x1:sigframe_size, x2:alignment_SP */
> +fake_sigreturn:
> + mov x20, x0
> + mov x21, x1
> + mov x22, x2
> + mov x23, sp
> +
> + /* create space on the stack for fake sigframe..."x22"-aligned */
> + mov x0, #0
Why to we move 0 into x0? We clobber x0 again in the next instruction:
> + add x0, x21, x22
> + sub x22, x22, #1
> + bic x0, x0, x22
> + sub x23, x23, x0
Can you explain the logic here? I'm not sure I understand what this is
trying to do exactly.
I notice you further modify this in patch 11 -- see my comments there,
where I also suggest an alternative way of specifying a misaligned frame
that might be simpler.
> +
> + ldr x0, =call_fmt
> + mov x1, x21
> + mov x2, x23
> + bl printf
> +
> + mov sp, x23
> +
> + /* now fill it with the provided content... */
> + mov x0, sp
> + mov x1, x20
> + mov x2, x21
> + bl memcpy
> +
> + /*
> + * Here saving a last minute SP to current->token acts as a marker:
> + * if we got here, we are successfully faking a sigreturn; in other
> + * words we are sure no bad fatal signal has been raised till now
> + * for unrelated reasons, so we should consider the possibl observed
typo: possible (or possibly?)
> + * fatal signal like SEGV coming from Kernel restore_sigframe() and
> + * triggered as expected from our test-case.
> + * For simplicity this assumes that current field 'token' is laid out
> + * as first in struct tdescr
> + */
> + ldr x0, current
> + str x23, [x0]
> + /* SP is already pointing back to the just built fake sigframe here */
> + mov x8, #__NR_rt_sigreturn
> + svc #0
> +
> + /*
> + * Above sigreturn should not return...looping here leads to a timeout
> + * and ensure proper and clean test failure, instead of jumping around
> + * on a potentially corrupted stack.
> + */
> + b .
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> index ce35be8ebc8e..2a71da7e6695 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -12,4 +12,5 @@ int test_run(struct tdescr *td);
> void test_result(struct tdescr *td);
>
> bool get_current_context(struct tdescr *td, ucontext_t *dest_uc);
> +int fake_sigreturn(void *sigframe, size_t sz, int alignment);
> #endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index a48a118b1a1a..0ea6fdc3765c 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -4,3 +4,4 @@ mangle_pstate_invalid_mode_el1
> mangle_pstate_invalid_mode_el2
> mangle_pstate_invalid_mode_el3
> mangle_pstate_ssbs_regs
> +fake_sigreturn_bad_magic
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
> new file mode 100644
> index 000000000000..b4c063e02a7a
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
We should probably also include <signal.h> here, since we're using
siginfo_t etc.
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, offset;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /*
> + * find the terminator, preserving existing headers
> + * and verify amount of spare room in __reserved area.
> + */
> + head = get_terminator(shead, resv_sz, &offset);
> + /*
> + * try stripping extra_context header when low on space:
> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
> + * and the other for the usual terminator.
> + */
> + if (head && resv_sz - offset < HDR_SZ * 2) {
Can we factor out this logic for finding space in the signal frame?
We do pretty much the same thing in all the fake_sigreturn tests...
> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> + resv_sz - offset);
> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> + }
> + /* just give up and timeout if still not enough space */
Do we actually time out? I don't see where we actually wait, so doesn't
test_run() just fail immediately?
The same applies to all the other fake_sigreturn tests too.
> + if (head && resv_sz - offset >= HDR_SZ) {
Should this be HDR_SZ * 2 again? We need space for the face header and
space to write a terminator after it.
> + fprintf(stderr, "Mangling template header. Spare space:%zd\n",
> + resv_sz - offset);
> + /*
> + * use a well known NON existent bad magic...something
> + * we should pretty sure won't be ever defined in Kernel
> + */
> + head->magic = KSFT_BAD_MAGIC;
> + head->size = HDR_SZ;
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 16);
> + }
> +
> + return 1;
> +}
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 07/11] kselftest: arm64: fake_sigreturn_bad_size_for_magic0
From: Dave Martin @ 2019-08-13 16:25 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-8-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:56PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which builds a ucontext_t
Add
> with a badly sized magic0 header and place it onto the stack.
I usually call a record with magic number 0 a "terminator record".
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 1 +
> .../fake_sigreturn_bad_size_for_magic0.c | 57 +++++++++++++++++++
> 2 files changed, 58 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 0ea6fdc3765c..cf2a73599818 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -5,3 +5,4 @@ mangle_pstate_invalid_mode_el2
> mangle_pstate_invalid_mode_el3
> mangle_pstate_ssbs_regs
> fake_sigreturn_bad_magic
> +fake_sigreturn_bad_size_for_magic0
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
> new file mode 100644
> index 000000000000..2f53c4740c85
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
#include <signal.h> ?
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +#define MIN_SZ_ALIGN 16
> +
> +static int fake_sigreturn_bad_size_for_magic0_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, offset;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /*
> + * find the terminator, preserving existing headers
> + * and verify amount of spare room in __reserved area.
> + */
> + head = get_terminator(shead, resv_sz, &offset);
> + /*
> + * try stripping extra_context header when low on space:
> + * we need at least HDR_SZ + 16 space for the bad sized terminator.
> + */
> + if (head && resv_sz - offset < HDR_SZ + MIN_SZ_ALIGN) {
> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> + resv_sz - offset);
> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> + }
> + /* just give up and timeout if still not enough space */
> + if (head && resv_sz - offset >= HDR_SZ + MIN_SZ_ALIGN) {
> + head->magic = 0;
> + head->size = MIN_SZ_ALIGN;
This is different from the amount of space we tested for
(HDR_SZ + MIN_SZ_ALIGN) earlier.
I'm not sure it matters which we use, but we should be consistent.
I suggest sticking with HDR_SZ, unless there's something I've missed.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 08/11] kselftest: arm64: fake_sigreturn_missing_fpsimd
From: Dave Martin @ 2019-08-13 16:26 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-9-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:57PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which builds a ucontext_t
Add
> without the required fpsimd_context and place it onto the stack.
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 1 +
> .../testcases/fake_sigreturn_missing_fpsimd.c | 44 +++++++++++++++++++
> 2 files changed, 45 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index cf2a73599818..17d1c5e73319 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -6,3 +6,4 @@ mangle_pstate_invalid_mode_el3
> mangle_pstate_ssbs_regs
> fake_sigreturn_bad_magic
> fake_sigreturn_bad_size_for_magic0
> +fake_sigreturn_missing_fpsimd
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
> new file mode 100644
> index 000000000000..b8dd57ce6844
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
signal.h?
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_missing_fpsimd_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, offset;
> + struct _aarch64_ctx *head = GET_SF_RESV_HEAD(sf);
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + head = get_header(head, FPSIMD_MAGIC, resv_sz, &offset);
> + /* just give up and timeout if still not enough space */
> + if (head && resv_sz - offset >= HDR_SZ) {
> + fprintf(stderr, "Mangling template header. Spare space:%zd\n",
> + resv_sz - offset);
> + /* Just overwrite fpsmid_context */
> + write_terminator_record(head);
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 16);
> + }
> +
> + return 1;
> +}
[...]
Seems reasonable otherwise.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd
From: Dave Martin @ 2019-08-13 16:26 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-10-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:58PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which builds a ucontext_t
Add
> with an anomalous additional fpsimd_context and place it onto the stack.
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 1 +
> .../fake_sigreturn_duplicated_fpsimd.c | 62 +++++++++++++++++++
> 2 files changed, 63 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 17d1c5e73319..94f9baaf638c 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -7,3 +7,4 @@ mangle_pstate_ssbs_regs
> fake_sigreturn_bad_magic
> fake_sigreturn_bad_size_for_magic0
> fake_sigreturn_missing_fpsimd
> +fake_sigreturn_duplicated_fpsimd
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> new file mode 100644
> index 000000000000..09af7a0f8776
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
signal.h?
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, offset;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /*
> + * find the terminator, preserving existing headers
> + * and verify amount of spare room in __reserved area.
> + */
> + head = get_terminator(shead, resv_sz, &offset);
> + /*
> + * try stripping extra_context header when low on space:
> + * we need at least space for one additional fpsimd_context
> + */
> + if (head && resv_sz - offset < sizeof(struct fpsimd_context)) {
> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> + resv_sz - offset);
> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> + }
> +
> + /* just give up and timeout if still not enough space */
> + if (head &&
> + resv_sz - offset >= sizeof(struct fpsimd_context) + HDR_SZ) {
> + fprintf(stderr, "Mangling template header. Spare space:%zd\n",
> + resv_sz - offset);
> + /* Add a spurios fpsimd_context */
> + head->magic = FPSIMD_MAGIC;
> + head->size = sizeof(struct fpsimd_context);
> + /* and terminate */
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 16);
> + }
> +
> + return 1;
> +}
[...]
Otherwise looks ok.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 10/11] kselftest: arm64: fake_sigreturn_bad_size
From: Dave Martin @ 2019-08-13 16:26 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-11-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:02:59PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which builds a ucontext_t
Add
> with a badly sized header that causes a overrun in the __reserved
> area and place it onto the stack. Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> .../arm64/signal/testcases/.gitignore | 1 +
> .../testcases/fake_sigreturn_bad_size.c | 85 +++++++++++++++++++
> 2 files changed, 86 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> index 94f9baaf638c..3408e0f5ba98 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/.gitignore
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -8,3 +8,4 @@ fake_sigreturn_bad_magic
> fake_sigreturn_bad_size_for_magic0
> fake_sigreturn_missing_fpsimd
> fake_sigreturn_duplicated_fpsimd
> +fake_sigreturn_bad_size
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
> new file mode 100644
> index 000000000000..1467fb534d8b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
signal.h?
> +#include <stdio.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +#define MIN_SZ_ALIGN 16
> +
> +static int fake_sigreturn_bad_size_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, need_sz, offset;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /*
> + * find the terminator, preserving existing headers
> + * and verify amount of spare room in __reserved area.
> + */
> + head = get_terminator(shead, resv_sz, &offset);
> + /*
> + * try stripping extra_context header when low on space:
> + * we need at least for the bad sized esr_context.
double space
> + */
> + need_sz = HDR_SZ + sizeof(struct esr_context);
> + if (head && resv_sz - offset < need_sz) {
> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> + resv_sz - offset);
> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> + }
> + /* just give up and timeout if still not enough space */
> + if (head && resv_sz - offset >= need_sz) {
> + fprintf(stderr, "Mangling template header. Spare space:%zd\n",
> + resv_sz - offset);
> + /*
> + * Use an esr_context to build a fake header with a
> + * size greater then the free __reserved area minus HDR_SZ;
> + * using ESR_MAGIC here since it is not checked for size nor
> + * is limited to one instance.
> + *
> + * At first inject an additional normal esr_context
> + */
> + head->magic = ESR_MAGIC;
> + head->size = sizeof(struct esr_context);
> + /* and terminate properly */
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> + ASSERT_GOOD_CONTEXT(&sf.uc);
> +
> + /*
> + * now mess with fake esr_context size: leaving less space than
> + * neededwhile keeping size value 16-aligned
> + *
> + * It must trigger a SEGV from Kernel on:
> + *
> + * resv_sz - offset < sizeof(*head)
> + */
> + /* at first set the maximum good 16-aligned size */
> + head->size = (resv_sz - offset - need_sz + MIN_SZ_ALIGN) & ~0xfUL;
> + /* plus a bit more of 16-aligned sized stuff */
> + head->size += MIN_SZ_ALIGN;
> + /* and terminate properly */
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 16);
> + }
> +
> + return 1;
> +}
[...]
Otherwise looks reasonable.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 11/11] kselftest: arm64: fake_sigreturn_misaligned_sp
From: Dave Martin @ 2019-08-13 16:27 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <20190802170300.20662-12-cristian.marussi@arm.com>
On Fri, Aug 02, 2019 at 06:03:00PM +0100, Cristian Marussi wrote:
> Added a simple fake_sigreturn testcase which places a valid
Add
> sigframe on a non-16 bytes aligned SP.
> fake_sigretrun() helper function has been patched accordingly
> to support placing a sigframe on a non-16 bytes aligned address.
> Expects a SIGSEGV on test PASS.
>
> Adds also a test TODO lists holding some further test ideas.
Adds -> Also add
lists -> list
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> Re-added this text after fixing the forced misaglinment procedure in
> fake_sigreturn() itself: require a ZERO alignment and you'll get
> your sigframe placed on a misaligned SP (2-bytes off the 16-align)
> ---
> .../testing/selftests/arm64/signal/signals.S | 21 +++++++++----
> .../arm64/signal/testcases/TODO.readme | 8 +++++
> .../testcases/fake_sigreturn_misaligned_sp.c | 30 +++++++++++++++++++
> 3 files changed, 53 insertions(+), 6 deletions(-)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/TODO.readme
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned_sp.c
>
> diff --git a/tools/testing/selftests/arm64/signal/signals.S b/tools/testing/selftests/arm64/signal/signals.S
> index 6262b877400b..2099871176ed 100644
> --- a/tools/testing/selftests/arm64/signal/signals.S
> +++ b/tools/testing/selftests/arm64/signal/signals.S
> @@ -13,19 +13,28 @@ call_fmt:
>
> .globl fake_sigreturn
>
> -/* fake_sigreturn x0:&sigframe, x1:sigframe_size, x2:alignment_SP */
> +/* fake_sigreturn x0:&sigframe, x1:sigframe_sz, x2:align */
> fake_sigreturn:
> - mov x20, x0
> - mov x21, x1
> - mov x22, x2
> - mov x23, sp
>
> - /* create space on the stack for fake sigframe..."x22"-aligned */
> + /* Save args and decide which aligment to enforce */
> + mov x23, sp
> + mov x20, x0
> + mov x21, x1
> + /* x22 and x24 used for forcing alignment or misalignment */
> + mov x22, x2
> + mov x24, #0
> + cbnz x22, 1f
> + mov x22, #16
> + mov x24, #2
> +
> +1: /* create space on the stack for fake sigframe..."x22"-aligned */
> mov x0, #0
> add x0, x21, x22
> sub x22, x22, #1
> bic x0, x0, x22
> sub x23, x23, x0
> + /* force misaligned by x24 bytes if required alignment was zero */
> + add x23, x23, x24
>
> ldr x0, =call_fmt
> mov x1, x21
Would it be simpler for the third argument to specify a number of bytes
to subtract from SP after allocating 16-byte aligned storage to
accommodate sigframe_sz?
Then 0 gives an aligned frame, 1 gives a frame misaligned by 1 byte,
etc.
Also if all this is a fix to the original fake_sigreturn, can we merge
it into the original patch instead?
> diff --git a/tools/testing/selftests/arm64/signal/testcases/TODO.readme b/tools/testing/selftests/arm64/signal/testcases/TODO.readme
> new file mode 100644
> index 000000000000..5c949492e7ab
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/TODO.readme
> @@ -0,0 +1,8 @@
> +Some more possible ideas for signals tests:
> +
> +- fake_sigreturn_unmapped_sp
> +- fake_sigreturn_kernelspace_sp
> +- fake_sigreturn_sve_bad_extra_context
> +- mangle_sve_invalid_extra_context
> +- mangle_pstate_invalid_el for H modes (+ macroization ?)
> +- fake_sigreturn_overflow_reserved
This seems a reasonable list, but it occurs to me that it will tend to
go out of sync as tests get added. So maybe just put this list in the
cover letter instead of including it in the patch.
We should probably have a one-line description of each proposed test,
since the names are a bit cryptic.
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned_sp.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned_sp.c
> new file mode 100644
> index 000000000000..3ee8c500c7d1
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned_sp.c
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
signal.h?
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_misaligned_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + /* Forcing sigframe on misaligned (=!16) SP */
> + fake_sigreturn(&sf, sizeof(sf), 0);
> +
> + return 1;
> +}
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 11/11] kselftest: arm64: fake_sigreturn_misaligned_sp
From: Dave Martin @ 2019-08-13 16:28 UTC (permalink / raw)
To: Cristian Marussi; +Cc: linux-arm-kernel, linux-kselftest
In-Reply-To: <8811be0d-efb3-b6da-9f6b-acaeb3edce7d@arm.com>
On Wed, Aug 07, 2019 at 05:04:13PM +0100, Cristian Marussi wrote:
> On 02/08/2019 18:03, Cristian Marussi wrote:
> > Added a simple fake_sigreturn testcase which places a valid
> > sigframe on a non-16 bytes aligned SP.
> > fake_sigretrun() helper function has been patched accordingly
> > to support placing a sigframe on a non-16 bytes aligned address.
> > Expects a SIGSEGV on test PASS.
> >
> > Adds also a test TODO lists holding some further test ideas.
> >
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > Re-added this text after fixing the forced misaglinment procedure in
> > fake_sigreturn() itself: require a ZERO alignment and you'll get
> > your sigframe placed on a misaligned SP (2-bytes off the 16-align)
> > ---
> > .../testing/selftests/arm64/signal/signals.S | 21 +++++++++----
> > .../arm64/signal/testcases/TODO.readme | 8 +++++
> > .../testcases/fake_sigreturn_misaligned_sp.c | 30 +++++++++++++++++++
> > 3 files changed, 53 insertions(+), 6 deletions(-)
> > create mode 100644 tools/testing/selftests/arm64/signal/testcases/TODO.readme
> > create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_misaligned_sp.c
> >
>
> When this test was re-added in V3, the related .gitignore was missed.
> It will go in V4
Ack, or otherwise try switching to using wildcards in .gitignore as
suggested in my reply to patch 4.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V16 00/13] PCI: tegra: Add Tegra194 PCIe support
From: Lorenzo Pieralisi @ 2019-08-13 16:33 UTC (permalink / raw)
To: Vidya Sagar
Cc: mark.rutland, devicetree, mperttunen, mmaddireddy, linux-pci,
catalin.marinas, will.deacon, linux-kernel, kthota, kishon,
linux-tegra, robh+dt, thierry.reding, gustavo.pimentel,
jingoohan1, bhelgaas, digetx, jonathanh, linux-arm-kernel,
sagar.tv
In-Reply-To: <20190813113627.27251-1-vidyas@nvidia.com>
On Tue, Aug 13, 2019 at 05:06:14PM +0530, Vidya Sagar wrote:
> Tegra194 has six PCIe controllers based on Synopsys DesignWare core.
> There are two Universal PHY (UPHY) blocks with each supporting 12(HSIO:
> Hisg Speed IO) and 8(NVHS: NVIDIA High Speed) lanes respectively.
> Controllers:0~4 use UPHY lanes from HSIO brick whereas Controller:5 uses
> UPHY lanes from NVHS brick. Lane mapping in HSIO UPHY brick to each PCIe
> controller (0~4) is controlled in XBAR module by BPMP-FW. Since PCIe
> core has PIPE interface, a glue module called PIPE-to-UPHY (P2U) is used
> to connect each UPHY lane (applicable to both HSIO and NVHS UPHY bricks)
> to PCIe controller
> This patch series
> - Adds support for P2U PHY driver
> - Adds support for PCIe host controller
> - Adds device tree nodes each PCIe controllers
> - Enables nodes applicable to p2972-0000 platform
> - Adds helper APIs in Designware core driver to get capability regs offset
> - Adds defines for new feature registers of PCIe spec revision 4
> - Makes changes in DesignWare core driver to get Tegra194 PCIe working
>
> Testing done on P2972-0000 platform
> - Able to get PCIe link up with on-board Marvel eSATA controller
> - Able to get PCIe link up with NVMe cards connected to M.2 Key-M slot
> - Able to do data transfers with both SATA drives and NVMe cards
> - Able to perform suspend-resume sequence
>
> Note
> - Enabling x8 slot on P2972-0000 platform requires pinmux driver for Tegra194.
> It is being worked on currently and hence Controller:5 (i.e. x8 slot) is
> disabled in this patch series. A future patch series would enable this.
> - This series is based on top of the following series
> Jisheng's patches to add support to .remove() in Designware sub-system
> https://patchwork.kernel.org/project/linux-pci/list/?series=98559
> (Update: Jisheng's patches are now accepted and applied for v5.2)
> My patches made on top of Jisheng's patches to export various symbols
> http://patchwork.ozlabs.org/project/linux-pci/list/?series=115671
> (Update: My above patch series is accepted and applied for v5.3)
> Another patch of mine to enable BPMP-FW resume in noirq phase
> http://patchwork.ozlabs.org/patch/1140973/
> (This is already accepted)
>
> V16:
> * Added empty lines (cosmetic changes) where required in pcie-tegra194.c file
> to address Lorenzo's review comments.
>
> V15:
> * Refactored pcie-tegra194.c code to call only tegra_bpmp_transfer() API
> in both .probe() path and .resume_noirq() path.
>
> V14:
> * Addressed Lorenzo's review comments in pcie-tegra194.c file (Patch 13/13)
> * Added a new patch to export dw_pcie_wait_for_link() API
>
> V13:
> * Addressed Bjorn's review comments for adding Gen-4 specific defines to pci_regs.h header file
>
> V12:
> * Modified the commit message of patch-3 in this series to address review
> comments from Lorenzo
>
> V11:
> * Removed device-tree patches from the series as they are applied to relevant
> Tegra specific trees by Thierry Reding.
> * Included older Tegra chips to extend quirk that disables MSI interrupt being
> used for Tegra PCIe root ports.
> * Addressed review comments in P2U driver file.
>
> V10:
> * Used _relaxed() versions of readl() & writel()
>
> V9:
> * Made the drivers dependent on ARCH_TEGRA_194_SOC directly
> * Addressed review comments from Dmitry
>
> V8:
> * Changed P2U driver file name from pcie-p2u-tegra194.c to phy-tegra194-p2u.c
> * Addressed review comments from Thierry and Rob
>
> V7:
> * Took care of review comments from Rob
> * Added a quirk to disable MSI for root ports
> * Removed using pcie_pme_disable_msi() API in host controller driver
>
> V6:
> * Removed patch that exports pcie_bus_config symbol
> * Took care of review comments from Thierry and Rob
>
> V5:
> * Removed redundant APIs in pcie-designware-ep.c file after moving them
> to pcie-designware.c file based on Bjorn's review comments
>
> V4:
> * Rebased on top of linux-next top of the tree
> * Addressed Gustavo's comments and added his Ack for some of the changes.
>
> V3:
> * Addressed review comments from Thierry
>
> V2:
> * Addressed review comments from Bjorn, Thierry, Jonathan, Rob & Kishon
> * Added more patches in v2 series
>
> Vidya Sagar (13):
> PCI: Add #defines for some of PCIe spec r4.0 features
> PCI: Disable MSI for Tegra root ports
> PCI: dwc: Perform dbi regs write lock towards the end
> PCI: dwc: Move config space capability search API
> PCI: dwc: Add ext config space capability search API
> PCI: dwc: Export dw_pcie_wait_for_link() API
> dt-bindings: PCI: designware: Add binding for CDM register check
> PCI: dwc: Add support to enable CDM register check
> dt-bindings: Add PCIe supports-clkreq property
> dt-bindings: PCI: tegra: Add device tree support for Tegra194
> dt-bindings: PHY: P2U: Add Tegra194 P2U block
> phy: tegra: Add PCIe PIPE2UPHY support
> PCI: tegra: Add Tegra194 PCIe support
>
> .../bindings/pci/designware-pcie.txt | 5 +
> .../bindings/pci/nvidia,tegra194-pcie.txt | 155 ++
> Documentation/devicetree/bindings/pci/pci.txt | 5 +
> .../bindings/phy/phy-tegra194-p2u.txt | 28 +
> drivers/pci/controller/dwc/Kconfig | 10 +
> drivers/pci/controller/dwc/Makefile | 1 +
> .../pci/controller/dwc/pcie-designware-ep.c | 37 +-
> .../pci/controller/dwc/pcie-designware-host.c | 14 +-
> drivers/pci/controller/dwc/pcie-designware.c | 88 +
> drivers/pci/controller/dwc/pcie-designware.h | 12 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 1631 +++++++++++++++++
> drivers/pci/quirks.c | 53 +
> drivers/phy/tegra/Kconfig | 7 +
> drivers/phy/tegra/Makefile | 1 +
> drivers/phy/tegra/phy-tegra194-p2u.c | 120 ++
> include/uapi/linux/pci_regs.h | 14 +-
> 16 files changed, 2139 insertions(+), 42 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
> create mode 100644 Documentation/devicetree/bindings/phy/phy-tegra194-p2u.txt
> create mode 100644 drivers/pci/controller/dwc/pcie-tegra194.c
> create mode 100644 drivers/phy/tegra/phy-tegra194-p2u.c
>
I have applied it to pci/tegra, subject to kbuild test validation,
for v5.4.
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
From: Mathieu Poirier @ 2019-08-13 16:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Suzuki K Poulose, Peter Zijlstra, Alexey Budankov, Igor Lubashev,
Linux Kernel Mailing List, James Morris, Alexander Shishkin,
Ingo Molnar, Namhyung Kim, Jiri Olsa, linux-arm-kernel
In-Reply-To: <20190813132349.GB12299@kernel.org>
Hi Arnaldo,
On Tue, 13 Aug 2019 at 07:23, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Mon, Aug 12, 2019 at 03:42:17PM -0600, Mathieu Poirier escreveu:
> > On Mon, 12 Aug 2019 at 14:29, Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Mon, Aug 12, 2019 at 05:27:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > > > > > @@ -281,7 +283,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > > > > > .events = POLLIN,
> > > > > > };
> > > > > >
> > > > > > - if (geteuid() != 0) {
> > > > > > + if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> > > > > > pr_err("ftrace only works for root!\n");
> > > > >
> > > > > I guess we should update the error message too?
> > > > >
> > > >
> > > > I.e. I applied this as a follow up patch:
> > > >
> > > > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > > > index 01a5bb58eb04..ba8b65c2f9dc 100644
> > > > --- a/tools/perf/builtin-ftrace.c
> > > > +++ b/tools/perf/builtin-ftrace.c
> > > > @@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > > > };
> > > >
> > > > if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> > > > - pr_err("ftrace only works for root!\n");
> > > > + pr_err("ftrace only works for %s!\n",
> > > > +#ifdef HAVE_LIBCAP_SUPPORT
> > > > + "users with the SYS_ADMIN capability"
> > > > +#else
> > > > + "root"
> > > > +#endif
> > >
> > > );
> > >
> > > :-)
> > >
> > > > return -1;
> > > > }
> > > >
> > >
> > > I've pushed the whole set to my tmp.perf/cap branch, please chec
> >
> > Please hold on before moving further - I'm getting a segmentation
> > fault on ARM64 that I'm still trying to figure out.
>
> This is just sitting in my tmp branch, and in my local perf/core branch,
> so that I can test it with the containers, etc.
>
> Is this related to the following fix?
That is the first thing I thought about but no, it has nothing to do
with it. Patch 3/4 is where the problem shows up. The code in the
patch is fine, it is the repercussion it has on other part that needs
to be investigated.
Right now I see that kmap->ref_reloc_sym is NULL here [1] when tracing
with anything else than the 'u' option. I am currently investigating
the problem.
Igor, please see if you can reproduce on QEMU or an ARM64 based platform.
[1] https://elixir.bootlin.com/linux/v5.3-rc4/source/tools/perf/util/event.c#L945
>
> commit 3e70008a6021fffd2cd1614734603ea970773060
> Author: Leo Yan <leo.yan@linaro.org>
> Date: Fri Aug 9 18:47:52 2019 +0800
>
> perf trace: Fix segmentation fault when access syscall info on arm64
>
> 'perf trace' reports the segmentation fault as below on Arm64:
>
> # perf trace -e string -e augmented_raw_syscalls.c
> LLVM: dumping tools/perf/examples/bpf/augmented_raw_syscalls.o
> perf: Segmentation fault
> Obtained 12 stack frames.
> perf(sighandler_dump_stack+0x47) [0xaaaaac96ac87]
> linux-vdso.so.1(+0x5b7) [0xffffadbeb5b7]
> /lib/aarch64-linux-gnu/libc.so.6(strlen+0x10) [0xfffface7d5d0]
> /lib/aarch64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ac7) [0xfffface49f97]
> /lib/aarch64-linux-gnu/libc.so.6(__vsnprintf_chk+0xc7) [0xffffacedfbe7]
> perf(scnprintf+0x97) [0xaaaaac9ca3ff]
> perf(+0x997bb) [0xaaaaac8e37bb]
> perf(cmd_trace+0x28e7) [0xaaaaac8ec09f]
> perf(+0xd4a13) [0xaaaaac91ea13]
> perf(main+0x62f) [0xaaaaac8a147f]
> /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe3) [0xfffface22d23]
> perf(+0x57723) [0xaaaaac8a1723]
> Segmentation fault
>
> This issue is introduced by commit 30a910d7d3e0 ("perf trace:
> Preallocate the syscall table"), it allocates trace->syscalls.table[]
> array and the element count is 'trace->sctbl->syscalls.nr_entries'; but
> on Arm64, the system call number is not continuously used; e.g. the
> syscall maximum id is 436 but the real entries is only 281.
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/8] Fix issues with 52-bit kernel virtual addressing
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
Hi all,
This patch series addresses some issues with 52-bit kernel VAs reported
by Qian Cai and Geert. It's all confined to asm/memory.h and I got a bit
carried away cleaning that thing up so the patches get more worthless
as you go through the series. Still, I'd like to queue this on top of
the 52-bit VA stuff currently sitting in -next.
Although Geert and Steve tested my initial hacks, I dropped the tags
because I've split things up and could've easily broken things again.
Cheers,
Will
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
--->8
Will Deacon (8):
arm64: memory: Fix virt_addr_valid() using __is_lm_address()
arm64: memory: Ensure address tag is masked in conversion macros
arm64: memory: Rewrite default page_to_virt()/virt_to_page()
arm64: memory: Simplify virt_to_page() implementation
arm64: memory: Simplify _VA_START and _PAGE_OFFSET definitions
arm64: memory: Implement __tag_set() as common function
arm64: memory: Add comments to end of non-trivial #ifdef blocks
arm64: memory: Cosmetic cleanups
arch/arm64/include/asm/memory.h | 89 ++++++++++++++++++++---------------------
1 file changed, 44 insertions(+), 45 deletions(-)
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/8] arm64: memory: Fix virt_addr_valid() using __is_lm_address()
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
virt_addr_valid() is intended to test whether or not the passed address
is a valid linear map address. Unfortunately, it relies on
_virt_addr_is_linear() which is broken because it assumes the linear
map is at the top of the address space, which it no longer is.
Reimplement virt_addr_valid() using __is_lm_address() and remove
_virt_addr_is_linear() entirely. At the same time, ensure we evaluate
the macro parameter only once and move it within the __ASSEMBLY__ block.
Reported-by: Qian Cai <cai@lca.pw>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 14c127c957c1 ("arm64: mm: Flip kernel VA space")
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index afaf512c0e1b..442ab861cab8 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -244,9 +244,9 @@ static inline const void *__tag_set(const void *addr, u8 tag)
/*
* The linear kernel range starts in the middle of the virtual adddress
* space. Testing the top bit for the start of the region is a
- * sufficient check.
+ * sufficient check and avoids having to worry about the tag.
*/
-#define __is_lm_address(addr) (!((addr) & BIT(vabits_actual - 1)))
+#define __is_lm_address(addr) (!(((u64)addr) & BIT(vabits_actual - 1)))
#define __lm_to_phys(addr) (((addr) + physvirt_offset))
#define __kimg_to_phys(addr) ((addr) - kimage_voffset)
@@ -326,13 +326,13 @@ static inline void *phys_to_virt(phys_addr_t x)
#define virt_to_page(vaddr) ((struct page *)((__virt_to_pgoff(vaddr)) + VMEMMAP_START))
#endif
-#endif
-#define _virt_addr_is_linear(kaddr) \
- (__tag_reset((u64)(kaddr)) >= PAGE_OFFSET)
+#define virt_addr_valid(addr) ({ \
+ __typeof__(addr) __addr = addr; \
+ __is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr)); \
+})
-#define virt_addr_valid(kaddr) \
- (_virt_addr_is_linear(kaddr) && pfn_valid(virt_to_pfn(kaddr)))
+#endif
/*
* Given that the GIC architecture permits ITS implementations that can only be
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/8] arm64: memory: Ensure address tag is masked in conversion macros
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
When converting a linear virtual address to a physical address, pfn or
struct page *, we must make sure that the tag bits are masked before the
calculation otherwise we end up with corrupt pointers when running with
CONFIG_KASAN_SW_TAGS=y:
| Unable to handle kernel paging request at virtual address 0037fe0007580d08
| [0037fe0007580d08] address between user and kernel address ranges
Mask out the tag in __virt_to_phys_nodebug() and virt_to_page().
Reported-by: Qian Cai <cai@lca.pw>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 9cb1c5ddd2c4 ("arm64: mm: Remove bit-masking optimisations for PAGE_OFFSET and VMEMMAP_START")
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 442ab861cab8..47b4dc73b8bf 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -252,7 +252,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)
#define __kimg_to_phys(addr) ((addr) - kimage_voffset)
#define __virt_to_phys_nodebug(x) ({ \
- phys_addr_t __x = (phys_addr_t)(x); \
+ phys_addr_t __x = (phys_addr_t)(__tag_reset(x)); \
__is_lm_address(__x) ? __lm_to_phys(__x) : \
__kimg_to_phys(__x); \
})
@@ -324,7 +324,8 @@ static inline void *phys_to_virt(phys_addr_t x)
((void *)__addr_tag); \
})
-#define virt_to_page(vaddr) ((struct page *)((__virt_to_pgoff(vaddr)) + VMEMMAP_START))
+#define virt_to_page(vaddr) \
+ ((struct page *)((__virt_to_pgoff(__tag_reset(vaddr))) + VMEMMAP_START))
#endif
#define virt_addr_valid(addr) ({ \
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 4/8] arm64: memory: Simplify virt_to_page() implementation
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
Build virt_to_page() on top of virt_to_pfn() so we can avoid the need
for explicit shifting.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 77074b3a1025..56be462c69ce 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -311,7 +311,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#define ARCH_PFN_OFFSET ((unsigned long)PHYS_PFN_OFFSET)
#if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
-#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
+#define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
#else
#define page_to_virt(x) ({ \
__typeof__(x) __page = x; \
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/8] arm64: memory: Rewrite default page_to_virt()/virt_to_page()
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
The default implementations of page_to_virt() and virt_to_page() are
fairly confusing to read and the former evaluates its 'page' parameter
twice in the macro
Rewrite them so that the computation is expressed as 'base + index' in
both cases and the parameter is always evaluated exactly once.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 47b4dc73b8bf..77074b3a1025 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -313,19 +313,18 @@ static inline void *phys_to_virt(phys_addr_t x)
#if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
#else
-#define __virt_to_pgoff(kaddr) (((u64)(kaddr) - PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
-#define __page_to_voff(kaddr) (((u64)(kaddr) - VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
-
-#define page_to_virt(page) ({ \
- unsigned long __addr = \
- ((__page_to_voff(page)) + PAGE_OFFSET); \
- const void *__addr_tag = \
- __tag_set((void *)__addr, page_kasan_tag(page)); \
- ((void *)__addr_tag); \
+#define page_to_virt(x) ({ \
+ __typeof__(x) __page = x; \
+ u64 __idx = ((u64)__page - VMEMMAP_START) / sizeof(struct page);\
+ u64 __addr = PAGE_OFFSET + (__idx * PAGE_SIZE); \
+ (void *)__tag_set((const void *)__addr, page_kasan_tag(__page));\
})
-#define virt_to_page(vaddr) \
- ((struct page *)((__virt_to_pgoff(__tag_reset(vaddr))) + VMEMMAP_START))
+#define virt_to_page(x) ({ \
+ u64 __idx = (__tag_reset((u64)x) - PAGE_OFFSET) / PAGE_SIZE; \
+ u64 __addr = VMEMMAP_START + (__idx * sizeof(struct page)); \
+ (struct page *)__addr; \
+})
#endif
#define virt_addr_valid(addr) ({ \
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 5/8] arm64: memory: Simplify _VA_START and _PAGE_OFFSET definitions
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
Rather than subtracting from -1 and then adding 1, we can simply
subtract from 0.
Cc: Steve Capper <steve.capper@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 56be462c69ce..5552c8cba1e2 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -44,8 +44,7 @@
* VA_START - the first kernel virtual address.
*/
#define VA_BITS (CONFIG_ARM64_VA_BITS)
-#define _PAGE_OFFSET(va) (UL(0xffffffffffffffff) - \
- (UL(1) << (va)) + 1)
+#define _PAGE_OFFSET(va) (-(UL(1) << (va)))
#define PAGE_OFFSET (_PAGE_OFFSET(VA_BITS))
#define KIMAGE_VADDR (MODULES_END)
#define BPF_JIT_REGION_START (KASAN_SHADOW_END)
@@ -63,8 +62,7 @@
#else
#define VA_BITS_MIN (VA_BITS)
#endif
-#define _VA_START(va) (UL(0xffffffffffffffff) - \
- (UL(1) << ((va) - 1)) + 1)
+#define _VA_START(va) (-(UL(1) << ((va) - 1)))
#define KERNEL_START _text
#define KERNEL_END _end
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 6/8] arm64: memory: Implement __tag_set() as common function
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
There's no need for __tag_set() to be a complicated macro when
CONFIG_KASAN_SW_TAGS=y and a simple static inline otherwise. Rewrite
the thing as a common static inline function.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 5552c8cba1e2..e902132b808c 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -218,20 +218,20 @@ static inline unsigned long kaslr_offset(void)
#ifdef CONFIG_KASAN_SW_TAGS
#define __tag_shifted(tag) ((u64)(tag) << 56)
-#define __tag_set(addr, tag) (__typeof__(addr))( \
- ((u64)(addr) & ~__tag_shifted(0xff)) | __tag_shifted(tag))
#define __tag_reset(addr) untagged_addr(addr)
#define __tag_get(addr) (__u8)((u64)(addr) >> 56)
#else
-static inline const void *__tag_set(const void *addr, u8 tag)
-{
- return addr;
-}
-
+#define __tag_shifted(tag) 0UL
#define __tag_reset(addr) (addr)
#define __tag_get(addr) 0
#endif
+static inline const void *__tag_set(const void *addr, u8 tag)
+{
+ u64 __addr = (u64)addr & ~__tag_shifted(0xff);
+ return (const void *)(__addr | __tag_shifted(tag));
+}
+
/*
* Physical vs virtual RAM address space conversion. These are
* private definitions which should NOT be used outside memory.h
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 7/8] arm64: memory: Add comments to end of non-trivial #ifdef blocks
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
Commenting the #endif of a multi-statement #ifdef block with the
condition which guards it is useful and can save having to scroll back
through the file to figure out which set of Kconfig options apply to
a particular piece of code.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index e902132b808c..d31e4b6e349f 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -57,11 +57,13 @@
#define PCI_IO_END (VMEMMAP_START - SZ_2M)
#define PCI_IO_START (PCI_IO_END - PCI_IO_SIZE)
#define FIXADDR_TOP (PCI_IO_START - SZ_2M)
+
#if VA_BITS > 48
#define VA_BITS_MIN (48)
#else
#define VA_BITS_MIN (VA_BITS)
#endif
+
#define _VA_START(va) (-(UL(1) << ((va) - 1)))
#define KERNEL_START _text
@@ -86,7 +88,7 @@
#else
#define KASAN_THREAD_SHIFT 0
#define KASAN_SHADOW_END (_VA_START(VA_BITS_MIN))
-#endif
+#endif /* CONFIG_KASAN */
#define MIN_THREAD_SHIFT (14 + KASAN_THREAD_SHIFT)
@@ -224,7 +226,7 @@ static inline unsigned long kaslr_offset(void)
#define __tag_shifted(tag) 0UL
#define __tag_reset(addr) (addr)
#define __tag_get(addr) 0
-#endif
+#endif /* CONFIG_KASAN_SW_TAGS */
static inline const void *__tag_set(const void *addr, u8 tag)
{
@@ -263,7 +265,7 @@ extern phys_addr_t __phys_addr_symbol(unsigned long x);
#else
#define __virt_to_phys(x) __virt_to_phys_nodebug(x)
#define __phys_addr_symbol(x) __pa_symbol_nodebug(x)
-#endif
+#endif /* CONFIG_DEBUG_VIRTUAL */
#define __phys_to_virt(x) ((unsigned long)((x) - physvirt_offset))
#define __phys_to_kimg(x) ((unsigned long)((x) + kimage_voffset))
@@ -323,14 +325,14 @@ static inline void *phys_to_virt(phys_addr_t x)
u64 __addr = VMEMMAP_START + (__idx * sizeof(struct page)); \
(struct page *)__addr; \
})
-#endif
+#endif /* !CONFIG_SPARSEMEM_VMEMMAP || CONFIG_DEBUG_VIRTUAL */
#define virt_addr_valid(addr) ({ \
__typeof__(addr) __addr = addr; \
__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr)); \
})
-#endif
+#endif /* !ASSEMBLY */
/*
* Given that the GIC architecture permits ITS implementations that can only be
@@ -345,4 +347,4 @@ static inline void *phys_to_virt(phys_addr_t x)
#include <asm-generic/memory_model.h>
-#endif
+#endif /* __ASM_MEMORY_H */
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 8/8] arm64: memory: Cosmetic cleanups
From: Will Deacon @ 2019-08-13 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Steve Capper, Catalin Marinas, Qian Cai,
Andrey Konovalov, Geert Uytterhoeven, Will Deacon
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
Cleanup memory.h so that the indentation is consistent, remove pointless
line-wrapping and use consistent parameter names for different versions
of the same macro.
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/memory.h | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index d31e4b6e349f..69f4cecb7241 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -12,10 +12,10 @@
#include <linux/compiler.h>
#include <linux/const.h>
+#include <linux/sizes.h>
#include <linux/types.h>
#include <asm/bug.h>
#include <asm/page-def.h>
-#include <linux/sizes.h>
/*
* Size of the PCI I/O space. This must remain a power of two so that
@@ -66,8 +66,8 @@
#define _VA_START(va) (-(UL(1) << ((va) - 1)))
-#define KERNEL_START _text
-#define KERNEL_END _end
+#define KERNEL_START _text
+#define KERNEL_END _end
#ifdef CONFIG_ARM64_VA_BITS_52
#define MAX_USER_VA_BITS 52
@@ -132,14 +132,14 @@
* 16 KB granule: 128 level 3 entries, with contiguous bit
* 64 KB granule: 32 level 3 entries, with contiguous bit
*/
-#define SEGMENT_ALIGN SZ_2M
+#define SEGMENT_ALIGN SZ_2M
#else
/*
* 4 KB granule: 16 level 3 entries, with contiguous bit
* 16 KB granule: 4 level 3 entries, without contiguous bit
* 64 KB granule: 1 level 3 entry
*/
-#define SEGMENT_ALIGN SZ_64K
+#define SEGMENT_ALIGN SZ_64K
#endif
/*
@@ -253,8 +253,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)
#define __virt_to_phys_nodebug(x) ({ \
phys_addr_t __x = (phys_addr_t)(__tag_reset(x)); \
- __is_lm_address(__x) ? __lm_to_phys(__x) : \
- __kimg_to_phys(__x); \
+ __is_lm_address(__x) ? __lm_to_phys(__x) : __kimg_to_phys(__x); \
})
#define __pa_symbol_nodebug(x) __kimg_to_phys((phys_addr_t)(x))
@@ -301,17 +300,17 @@ static inline void *phys_to_virt(phys_addr_t x)
#define __pa_nodebug(x) __virt_to_phys_nodebug((unsigned long)(x))
#define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
#define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
-#define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned long)(x)))
-#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
+#define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys((unsigned long)(x)))
+#define sym_to_pfn(x) __phys_to_pfn(__pa_symbol(x))
/*
- * virt_to_page(k) convert a _valid_ virtual address to struct page *
- * virt_addr_valid(k) indicates whether a virtual address is valid
+ * virt_to_page(x) convert a _valid_ virtual address to struct page *
+ * virt_addr_valid(x) indicates whether a virtual address is valid
*/
#define ARCH_PFN_OFFSET ((unsigned long)PHYS_PFN_OFFSET)
#if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
-#define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
+#define virt_to_page(x) pfn_to_page(virt_to_pfn(x))
#else
#define page_to_virt(x) ({ \
__typeof__(x) __page = x; \
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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