Re: [RFC PATCH 00/16] pkeys-based page table hardening
On 06/12/2024 20:14, Jann Horn wrote: > On Fri, Dec 6, 2024 at 11:13 AM Kevin Brodsky wrote: >> [...] >> >> Page tables were chosen as they are a popular (and critical) target for >> attacks, but there are of course many others - this is only a starting >> point (see section "Further use-cases"). It has become more and more >> common for accesses to such target data to be mediated by a hypervisor >> in vendor kernels; the hope is that kpkeys can provide much of that >> protection in a simpler manner. No benchmarking has been performed at >> this stage, but the runtime overhead should also be lower (though likely >> not negligible). > Yeah, it isn't great that vendor kernels contain such invasive changes... > > I guess one difference between this approach and a hypervisor-based > approach is that a hypervisor that uses a second layer of page tables > can also prevent access through aliasing mappings, while pkeys only > prevent access through a specific mapping? (Like if an attacker > managed to add a page that is mapped into userspace to a page > allocator freelist, allocate this page as a page table, and use the > userspace mapping to write into this page table. But I guess whether > that is an issue depends on the threat model.) Yes, that's correct. If an attacker is able to modify page tables then kpkeys are easily defeated. (kpkeys_hardened_pgtables does mitigate precisely that, though.) On the topic of aliases, it's worth noting that this isn't an issue with page table pages (only the linear mapping is used), but if we wanted to assigning a pkey to vmalloc areas we'd also have to amend the linear mapping. >> [...] >> >> # Threat model >> >> The proposed scheme aims at mitigating data-only attacks (e.g. >> use-after-free/cross-cache attacks). In other words, it is assumed that >> control flow is not corrupted, and that the attacker does not achieve >> arbitrary code execution. Nothing prevents the pkey register from being >> set to its most permissive state - the assumption is that the register >> is only modified on legitimate code paths. > Is the threat model that the attacker has already achieved full > read/write access to unprotected kernel data and should be stopped > from gaining write access to protected data? Or is the threat model > that the attacker has achieved some limited corruption, and this > series is intended to make it harder to either gain write access to > protected data or achieve full read/write access to unprotected data? The assumption is that the attacker has acquired a write primitive that could potentially allow corrupting any kernel data. The objective is to make it harder to exploit that primitive by making critical data immune to it. Nothing stops the attacker to turn to another (unprotected) target, but this is no different from hypervisor-based protection - the hope is that removing the low-hanging fruits makes it too difficult to build a complete exploit chain. - Kevin
[PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
The TQMa62xx is a SoM family with a pluggable board connector based on the
TI AM62x SoCs. Add DTS(I) for the AM625 (2x Cortex-A53) variant and its
combination with our MBa62xx carrier board.
Signed-off-by: Matthias Schiffer
---
arch/arm64/boot/dts/ti/Makefile | 1 +
.../boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts | 917 ++
arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi | 346 +++
3 files changed, 1264 insertions(+)
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index f71360f14f233..4d96c086e2daf 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -14,6 +14,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-csi2-ov5640.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-csi2-tevi-ov5640.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-phyboard-lyra-rdk.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-sk.dtb
+dtb-$(CONFIG_ARCH_K3) += k3-am625-tqma62xx-mba62xx.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-nonwifi-dahlia.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-nonwifi-dev.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-nonwifi-ivy.dtb
diff --git a/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
new file mode 100644
index 0..64ae1b13be15b
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts
@@ -0,0 +1,917 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Copyright (C) 2021-2022 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (c) 2023-2024 TQ-Systems GmbH , D-82229
Seefeld, Germany.
+ * Author: Matthias Schiffer
+ */
+
+/dts-v1/;
+
+#include
+#include
+#include
+#include
+#include
+#include "k3-am625-tqma62xx.dtsi"
+
+/ {
+ compatible = "tq,am625-tqma6254-mba62xx", "tq,am625-tqma6254",
+"ti,am625";
+ model = "TQ-Systems TQMa62xx SoM on MBa62xx carrier board";
+ chassis-type = "embedded";
+
+ aliases {
+ can0 = &mcu_mcan0;
+ can1 = &mcu_mcan1;
+ ethernet0 = &cpsw_port1;
+ ethernet1 = &cpsw_port2;
+ i2c1 = &main_i2c1;
+ mmc1 = &sdhci1;
+ mmc2 = &sdhci2;
+ serial0 = &main_uart0;
+ serial1 = &mcu_uart0;
+ spi1 = &main_spi0;
+ usb0 = &usb0;
+ usb1 = &usb1;
+ };
+
+ chosen {
+ stdout-path = &main_uart0;
+ };
+
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pinctrl-names = "default";
+ pinctrl-0 = <&backlight_pins>;
+ enable-gpios = <&main_gpio0 38 GPIO_ACTIVE_HIGH>;
+ status = "disabled";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_key_pins>;
+
+ user-button {
+ label = "USER_BUTTON";
+ linux,code = ;
+ gpios = <&main_gpio0 40 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ gpio-leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_led_pins>;
+
+ led-1 {
+ gpios = <&main_gpio0 41 GPIO_ACTIVE_HIGH>;
+ color = ;
+ function = LED_FUNCTION_INDICATOR;
+ };
+
+ led-2 {
+ gpios = <&main_gpio0 42 GPIO_ACTIVE_HIGH>;
+ color = ;
+ function = LED_FUNCTION_INDICATOR;
+ };
+ };
+
+ panel: panel {
+ pinctrl-names = "default";
+ pinctrl-0 = <&lvds_panel_pins>;
+ enable-gpios = <&main_gpio0 36 GPIO_ACTIVE_HIGH>;
+ power-supply = <®_lvds_pwr>;
+ };
+
+ fan0: pwm-fan {
+ compatible = "pwm-fan";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_fan_pins>;
+ fan-supply = <®_pwm_fan>;
+ #cooling-cells = <2>;
+ /* typical 25 kHz -> 40.000 nsec */
+ pwms = <&epwm0 1 4 PWM_POLARITY_INVERTED>;
+ cooling-levels = <0 32 64 128 196 240>;
+ pulses-per-revolution = <2>;
+ interrupt-parent = <&main_gpio1>;
+ interrupts = <30 IRQ_TYPE_EDGE_FALLING>;
+ status = "disabled";
+ };
+
+ wifi_pwrseq: pwrseq-wifi {
+ compatible = "mmc-pwrseq-simple";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mmc2_pwrseq_pins>;
+ reset-gpios = <&main_gpio0 44 GPIO_ACTIVE_HIGH>;
+ };
+
+ reg_1v8: regulator-1v8 {
+
[PATCH v2 4/5] arm64: dts: ti: k3-am62-wakeup: Add R5F device node
From: Hari Nagalla
AM62 SoCs have a single core R5F core in wakeup domain. This core is also
used as a device manager for the SoC.
Co-authored-by: Devarsh Thakkar
Signed-off-by: Hari Nagalla
[Matthias Schiffer: Updated to match latest submitted version v5 of "arm64:
dts: k3-am62a-wakeup: Add R5F device node"]
Signed-off-by: Matthias Schiffer
---
arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 24 ++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
index 9b8a1f85aa15c..619848e889cf9 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
@@ -106,6 +106,30 @@ wkup_rti0: watchdog@2b00 {
status = "reserved";
};
+ wkup_r5fss0: r5fss@7800 {
+ compatible = "ti,am62-r5fss";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x7800 0x00 0x7800 0x8000>,
+<0x7810 0x00 0x7810 0x8000>;
+ power-domains = <&k3_pds 119 TI_SCI_PD_EXCLUSIVE>;
+
+ wkup_r5fss0_core0: r5f@7800 {
+ compatible = "ti,am62-r5f";
+ reg = <0x7800 0x8000>,
+ <0x7810 0x8000>;
+ reg-names = "atcm", "btcm";
+ resets = <&k3_reset 121 1>;
+ firmware-name = "am62-wkup-r5f0_0-fw";
+ ti,sci = <&dmsc>;
+ ti,sci-dev-id = <121>;
+ ti,sci-proc-ids = <0x01 0xff>;
+ ti,atcm-enable = <1>;
+ ti,btcm-enable = <1>;
+ ti,loczrama = <1>;
+ };
+ };
+
wkup_vtm0: temperature-sensor@b0 {
compatible = "ti,j7200-vtm";
reg = <0x00 0xb0 0x00 0x400>,
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
[PATCH v2 3/5] arm64: dts: ti: k3-am62: Add DM R5 ranges in cbass
From: Devarsh Thakkar
Add DM R5F ATCM and BTCM ranges in cbass_wakeup and cbass_main.
Signed-off-by: Devarsh Thakkar
Signed-off-by: Dhruva Gole
Signed-off-by: Matthias Schiffer
---
arch/arm64/boot/dts/ti/k3-am62.dtsi | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am62.dtsi
b/arch/arm64/boot/dts/ti/k3-am62.dtsi
index bfb55ca113239..f01a594ba7f89 100644
--- a/arch/arm64/boot/dts/ti/k3-am62.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62.dtsi
@@ -86,7 +86,9 @@ cbass_main: bus@f {
/* Wakeup Domain Range */
<0x00 0x00b0 0x00 0x00b0 0x00 0x2400>, /*
VTM */
<0x00 0x2b00 0x00 0x2b00 0x00 0x00300400>,
-<0x00 0x4300 0x00 0x4300 0x00 0x0002>;
+<0x00 0x4300 0x00 0x4300 0x00 0x0002>,
+<0x00 0x7800 0x00 0x7800 0x00 0x8000>, /*
DM R5 ATCM */
+<0x00 0x7810 0x00 0x7810 0x00 0x8000>; /*
DM R5 BTCM */
cbass_mcu: bus@400 {
bootph-all;
@@ -103,7 +105,9 @@ cbass_wakeup: bus@b0 {
#size-cells = <2>;
ranges = <0x00 0x00b0 0x00 0x00b0 0x00
0x2400>, /* VTM */
<0x00 0x2b00 0x00 0x2b00 0x00
0x00300400>, /* Peripheral Window */
-<0x00 0x4300 0x00 0x4300 0x00
0x0002>;
+<0x00 0x4300 0x00 0x4300 0x00
0x0002>,
+<0x00 0x7800 0x00 0x7800 0x00
0x8000>, /* DM R5 ATCM */
+<0x00 0x7810 0x00 0x7810 0x00
0x8000>; /* DM R5 BTCM */
};
};
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
Re: [RFC PATCH 12/16] arm64: mm: Map p4d/pgd with privileged pkey
On Fri, Dec 06, 2024 at 10:11:06AM +, Kevin Brodsky wrote: > If CONFIG_KPKEYS_HARDENED_PGTABLES is enabled, map p4d/pgd pages > using a privileged pkey (KPKEYS_PKEY_PGTABLES), so that they can > only be written under guard(kpkeys_hardened_pgtables). > > The case where pgd is not page-sized is not currently handled - > this is pending support for pkeys in kmem_cache. > > This patch is a no-op if CONFIG_KPKEYS_HARDENED_PGTABLES is disabled > (default). Should not this live in pagetable_*_[cd]tor() in generic code?
Re: [RFC PATCH 13/16] arm64: mm: Reset pkey in __tlb_remove_table()
On Fri, Dec 06, 2024 at 10:11:07AM +, Kevin Brodsky wrote:
> Page table pages are typically freed via tlb_remove_table() and
> friends. Ensure that the linear mapping for those pages is reset to
> the default pkey when CONFIG_KPKEYS_HARDENED_PGTABLES is enabled.
>
> This patch is a no-op if CONFIG_KPKEYS_HARDENED_PGTABLES is disabled
> (default).
>
> Signed-off-by: Kevin Brodsky
> ---
> arch/arm64/include/asm/tlb.h | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
> index a947c6e784ed..d1611ffa6d91 100644
> --- a/arch/arm64/include/asm/tlb.h
> +++ b/arch/arm64/include/asm/tlb.h
> @@ -10,10 +10,14 @@
>
> #include
> #include
> +#include
>
> static inline void __tlb_remove_table(void *_table)
> {
> - free_page_and_swap_cache((struct page *)_table);
> + struct page *page = (struct page *)_table;
> +
> + kpkeys_unprotect_pgtable_memory((unsigned long)page_address(page), 1);
> + free_page_and_swap_cache(page);
> }
Same as for the others, perhaps stick this in generic code instead of in
the arch code?
[PATCH v2 1/5] dt-bindings: usb: dwc3: Allow connector in USB controller node
Allow specifying the connector directly in the USB controller node, as supported by other USB controller bindings. Signed-off-by: Matthias Schiffer Acked-by: Conor Dooley --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 1cd0ca90127d9..2976fb1a58061 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -68,6 +68,12 @@ properties: - enum: [bus_early, ref, suspend] - true + connector: +$ref: /schemas/connector/usb-connector.yaml# +description: Connector for dual role switch +type: object +unevaluatedProperties: false + dma-coherent: true extcon: -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
Re: [RFC PATCH 08/16] mm: Introduce kernel_pgtables_set_pkey()
On Fri, Dec 06, 2024 at 10:11:02AM +, Kevin Brodsky wrote: > kernel_pgtables_set_pkey() allows setting the pkey of all page table > pages in swapper_pg_dir, recursively. This will be needed by > kpkeys_hardened_pgtables, as it relies on all PTPs being mapped with > a non-default pkey. Those initial kernel page tables cannot > practically be assigned a non-default pkey right when they are > allocated, so mutating them during (early) boot is required. > > Signed-off-by: Kevin Brodsky > --- > > It feels that some sort of locking is called for in > kernel_pgtables_set_pkey(), but I couldn't figure out what would be > appropriate. init_mm.page_table_lock is typically the one used to serialize kernel page tables IIRC.
[PATCH v2 0/5] TQ-Systems TQMa62xx SoM and MBa62xx board
This adds Device Trees for out AM62x-based SoM TQMa62xx and its reference carrier board MBa62xx. Two of the patches are adapted from the TI vendor repo ti-linux-kernel to add RemoteProc/RPMsg support for the R5F core. A similar patch has been submitted for mainline by TI themselves for the closely related AM62A SoC. Not yet included are overlays to enable LVDS display output and MIPI-CSI camera input. Changes in v2: - Collected acks and reviews - Rebased onto v6.13-rc1 Devarsh Thakkar (1): arm64: dts: ti: k3-am62: Add DM R5 ranges in cbass Hari Nagalla (1): arm64: dts: ti: k3-am62-wakeup: Add R5F device node Matthias Schiffer (3): dt-bindings: usb: dwc3: Allow connector in USB controller node dt-bindings: arm: ti: Add compatible for AM625-based TQMa62xx SOM family and carrier board arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees .../devicetree/bindings/arm/ti/k3.yaml| 7 + .../devicetree/bindings/usb/snps,dwc3.yaml| 6 + arch/arm64/boot/dts/ti/Makefile | 1 + arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi| 24 + arch/arm64/boot/dts/ti/k3-am62.dtsi | 8 +- .../boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts | 917 ++ arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi | 346 +++ 7 files changed, 1307 insertions(+), 2 deletions(-) create mode 100644 arch/arm64/boot/dts/ti/k3-am625-tqma62xx-mba62xx.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-tqma62xx.dtsi -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH v2 2/5] dt-bindings: arm: ti: Add compatible for AM625-based TQMa62xx SOM family and carrier board
The TQMa62xx is a SoM family with a pluggable connector. The MBa62xx is the matching reference/starterkit carrier board. Signed-off-by: Matthias Schiffer Reviewed-by: Rob Herring (Arm) --- Documentation/devicetree/bindings/arm/ti/k3.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/ti/k3.yaml b/Documentation/devicetree/bindings/arm/ti/k3.yaml index 18f155cd06c84..07d98a67d967f 100644 --- a/Documentation/devicetree/bindings/arm/ti/k3.yaml +++ b/Documentation/devicetree/bindings/arm/ti/k3.yaml @@ -75,6 +75,13 @@ properties: - const: toradex,verdin-am62 # Verdin AM62 Module - const: ti,am625 + - description: K3 AM625 SoC on TQ-Systems TQMa62xx SoM +items: + - enum: + - tq,am625-tqma6254-mba62xx # MBa62xx base board + - const: tq,am625-tqma6254 + - const: ti,am625 + - description: K3 AM642 SoC items: - enum: -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
> +&cpsw_port1 {
> + phy-mode = "rgmii-rxid";
> + phy-handle = <&cpsw3g_phy0>;
> +};
> +
> +&cpsw_port2 {
> + phy-mode = "rgmii-rxid";
> + phy-handle = <&cpsw3g_phy3>;
> +};
rgmii-rxid is very odd.
> +
> +&cpsw3g_mdio {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_mdio1_pins>;
> +
> + cpsw3g_phy0: ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <0x0>;
> + reset-gpios = <&main_gpio1 11 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <1000>;
> + reset-deassert-us = <1000>;
> + ti,rx-internal-delay = ;
I guess this is the explanation.
What happens when you use rgmii-id, and don't have this delay here?
That would be normal.
Andrew
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
On Mon, 2024-12-09 at 14:24 +0100, Andrew Lunn wrote:
>
> > +&cpsw_port1 {
> > + phy-mode = "rgmii-rxid";
> > + phy-handle = <&cpsw3g_phy0>;
> > +};
> > +
> > +&cpsw_port2 {
> > + phy-mode = "rgmii-rxid";
> > + phy-handle = <&cpsw3g_phy3>;
> > +};
>
> rgmii-rxid is very odd.
>
> > +
> > +&cpsw3g_mdio {
> > + status = "okay";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&main_mdio1_pins>;
> > +
> > + cpsw3g_phy0: ethernet-phy@0 {
> > + compatible = "ethernet-phy-ieee802.3-c22";
> > + reg = <0x0>;
> > + reset-gpios = <&main_gpio1 11 GPIO_ACTIVE_LOW>;
> > + reset-assert-us = <1000>;
> > + reset-deassert-us = <1000>;
> > + ti,rx-internal-delay = ;
>
> I guess this is the explanation.
>
> What happens when you use rgmii-id, and don't have this delay here?
> That would be normal.
>
> Andrew
This is normal for AM62-based boards, see the DTSI of the TI reference
starterkit for example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi#n451
With rgmii-id, both ti,rx-internal-delay and ti,tx-internal-delay should be set.
As ti,*-internal-delay sets the delay on the PHY side, phy-mode "rgmii" is the
one that would not use either:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ti,dp83867.yaml#n78
At the end of the day, does it really matter as long as MAC and PHY agree on the
used mode? We copied this part of the hardware design from the TI reference
board, and did our hardware qualification with these settings, so I think it
makes sense to use the same phy-mode configuration.
Best regards,
Matthias
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
> Not our board, but the AM62 SoC. From the datasheet: > > "TXC is delayed internally before being driven to the RGMII[x]_TXC pin. This > internal delay is always enabled." So enabling the TX delay on the PHY side > would result in a double delay. phy-mode describes the board. If the board does not have extra long clock lines, phy-mode should be rgmii-id. The fact the MAC is doing something which no other MAC does should be hidden away in the MAC driver, as much as possible. The MAC driver should return -EINVAL with phy-mode rgmii, or rmgii-rxid, because the MAC driver is physically incapable of being used on a board which has extra long TX clock lines, which 'rmgii' or rgmii-rxid would indicate. Since the MAC driver is forcing the TX delay, it needs to take the value returned from of_get_phy_mode() and mask out the TX bit before passing it to the PHY. Now, it could be that history has got in the way. There are boards out there which have broken DT but work. Fixing the MAC driver to do the correct thing will break those boards. Vendors with low quality code which works, but not really. ~/linux/arch/arm64/boot/dts/ti$ grep rgmii k3-am625-* k3-am625-beagleplay.dts:phy-mode = "rgmii-rxid"; k3-am625-sk.dts:phy-mode = "rgmii-rxid"; Yep, these two have broken DT, they don't describe the board correctly. O.K. Can we fix this for you board? Yes, i think we can. If you take rmgii-rxid, aka PHY_INTERFACE_MODE_RGMII_RXID, and mask out the TX, you still get PHY_INTERFACE_MODE_RGMII_RXID. If you take rgmii-id, a.k.a. PHY_INTERFACE_MODE_RGMII_ID and mask out the TX, you get PHY_INTERFACE_MODE_RGMII_RXID, which is what you want. Please produce a patch to the MAC driver, explaining the horrible mess the vendor made, and how this fixes it, but should also not break other boards. > No such defaults exist in the DP83867 driver. If any rgmii-*id mode is used, > the > corresponding delays *must* be specified in the DTB: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/dp83867.c#n532 That is bad, different to pretty every other PHY driver :-( If you want, you could patch this driver as well, make it default to 2ns if delays are asked for. Andrew --- pw-bot: cr
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
On Mon, Dec 09, 2024 at 02:55:31PM +0100, Matthias Schiffer wrote:
> On Mon, 2024-12-09 at 14:24 +0100, Andrew Lunn wrote:
> >
> > > +&cpsw_port1 {
> > > + phy-mode = "rgmii-rxid";
> > > + phy-handle = <&cpsw3g_phy0>;
> > > +};
> > > +
> > > +&cpsw_port2 {
> > > + phy-mode = "rgmii-rxid";
> > > + phy-handle = <&cpsw3g_phy3>;
> > > +};
> >
> > rgmii-rxid is very odd.
> >
> > > +
> > > +&cpsw3g_mdio {
> > > + status = "okay";
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&main_mdio1_pins>;
> > > +
> > > + cpsw3g_phy0: ethernet-phy@0 {
> > > + compatible = "ethernet-phy-ieee802.3-c22";
> > > + reg = <0x0>;
> > > + reset-gpios = <&main_gpio1 11 GPIO_ACTIVE_LOW>;
> > > + reset-assert-us = <1000>;
> > > + reset-deassert-us = <1000>;
> > > + ti,rx-internal-delay = ;
> >
> > I guess this is the explanation.
> >
> > What happens when you use rgmii-id, and don't have this delay here?
> > That would be normal.
> >
> > Andrew
>
>
> This is normal for AM62-based boards, see the DTSI of the TI reference
> starterkit for example:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi#n451
>
> With rgmii-id, both ti,rx-internal-delay and ti,tx-internal-delay should be
> set.
> As ti,*-internal-delay sets the delay on the PHY side, phy-mode "rgmii" is the
> one that would not use either:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ti,dp83867.yaml#n78
>
> At the end of the day, does it really matter as long as MAC and PHY agree on
> the
> used mode? We copied this part of the hardware design from the TI reference
> board, and did our hardware qualification with these settings, so I think it
> makes sense to use the same phy-mode configuration.
What i try to achieve is every board uses the same configuration. The
PHY adds the delays, not the MAC. There are a few exceptions, because
a few cheap PHYs don't support delays, and so the MAC needs to add
them. But in general, any board i review, i always ask that the PHY
does the delay.
Also, don't put too much value in vendor code. Vendors don't care
about Linux has a whole, being uniform across all systems. Many
vendors do the minimum to get their stuff working, sometimes Monkeys
typing Shakespeare, and not a lot more.
I also find a lot of developers don't really understand what phy-mode
and PHY_INTERFACE_MODE_RGMII_* actually mean. phy-mode = 'rgmii' means
the board has extra long clock lines, so the MAC/PHY does not need to
add delays. rgmii-rxid means the board has an extra long rx clock
line, but a normal length tx clock line. Now, i doubt your board is
actually like this?
You want to correctly describe your hardware in DT, which i guess is
"rgmii-id". That means something, either the MAC or the PHY needs to
add delays. PHY_INTERFACE_MODE_RGMII_* is what is passed to the
PHY. To get it to add the 2ns delays, you pass
PHY_INTERFACE_MODE_RGMII_ID, and you should not need any additional
properties in DT, it should default to 2ns. If you need to tune the
delay, 2ns does not work, but you actually need 1.8ns etc, then you
can add additional parameters. But given you have
DP83867_RGMIIDCTL_2_00_NS, i doubt you need this.
Andrew
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
On Mon, 2024-12-09 at 15:42 +0100, Andrew Lunn wrote:
>
> On Mon, Dec 09, 2024 at 02:55:31PM +0100, Matthias Schiffer wrote:
> > On Mon, 2024-12-09 at 14:24 +0100, Andrew Lunn wrote:
> > >
> > > > +&cpsw_port1 {
> > > > + phy-mode = "rgmii-rxid";
> > > > + phy-handle = <&cpsw3g_phy0>;
> > > > +};
> > > > +
> > > > +&cpsw_port2 {
> > > > + phy-mode = "rgmii-rxid";
> > > > + phy-handle = <&cpsw3g_phy3>;
> > > > +};
> > >
> > > rgmii-rxid is very odd.
> > >
> > > > +
> > > > +&cpsw3g_mdio {
> > > > + status = "okay";
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&main_mdio1_pins>;
> > > > +
> > > > + cpsw3g_phy0: ethernet-phy@0 {
> > > > + compatible = "ethernet-phy-ieee802.3-c22";
> > > > + reg = <0x0>;
> > > > + reset-gpios = <&main_gpio1 11 GPIO_ACTIVE_LOW>;
> > > > + reset-assert-us = <1000>;
> > > > + reset-deassert-us = <1000>;
> > > > + ti,rx-internal-delay = ;
> > >
> > > I guess this is the explanation.
> > >
> > > What happens when you use rgmii-id, and don't have this delay here?
> > > That would be normal.
> > >
> > > Andrew
> >
> >
> > This is normal for AM62-based boards, see the DTSI of the TI reference
> > starterkit for example:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi#n451
> >
> > With rgmii-id, both ti,rx-internal-delay and ti,tx-internal-delay should be
> > set.
> > As ti,*-internal-delay sets the delay on the PHY side, phy-mode "rgmii" is
> > the
> > one that would not use either:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ti,dp83867.yaml#n78
> >
> > At the end of the day, does it really matter as long as MAC and PHY agree
> > on the
> > used mode? We copied this part of the hardware design from the TI reference
> > board, and did our hardware qualification with these settings, so I think it
> > makes sense to use the same phy-mode configuration.
>
> What i try to achieve is every board uses the same configuration. The
> PHY adds the delays, not the MAC. There are a few exceptions, because
> a few cheap PHYs don't support delays, and so the MAC needs to add
> them. But in general, any board i review, i always ask that the PHY
> does the delay.
>
> Also, don't put too much value in vendor code. Vendors don't care
> about Linux has a whole, being uniform across all systems. Many
> vendors do the minimum to get their stuff working, sometimes Monkeys
> typing Shakespeare, and not a lot more.
>
> I also find a lot of developers don't really understand what phy-mode
> and PHY_INTERFACE_MODE_RGMII_* actually mean. phy-mode = 'rgmii' means
> the board has extra long clock lines, so the MAC/PHY does not need to
> add delays. rgmii-rxid means the board has an extra long rx clock
> line, but a normal length tx clock line. Now, i doubt your board is
> actually like this?
Not our board, but the AM62 SoC. From the datasheet:
"TXC is delayed internally before being driven to the RGMII[x]_TXC pin. This
internal delay is always enabled." So enabling the TX delay on the PHY side
would result in a double delay.
>
> You want to correctly describe your hardware in DT, which i guess is
> "rgmii-id". That means something, either the MAC or the PHY needs to
> add delays. PHY_INTERFACE_MODE_RGMII_* is what is passed to the
> PHY. To get it to add the 2ns delays, you pass
> PHY_INTERFACE_MODE_RGMII_ID, and you should not need any additional
> properties in DT, it should default to 2ns. If you need to tune the
> delay, 2ns does not work, but you actually need 1.8ns etc, then you
> can add additional parameters. But given you have
> DP83867_RGMIIDCTL_2_00_NS, i doubt you need this.
No such defaults exist in the DP83867 driver. If any rgmii-*id mode is used, the
corresponding delays *must* be specified in the DTB:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/dp83867.c#n532
Best regards,
Matthias
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
RE: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
> ... which is why we documented it in Documentation/networking/phy.rst, > but it seems folk who run into RGMII stuff don't read that document. > > I'm wondering if we can come up with some kernel-doc compatible way to > document them in linux/phy.h instead, which may stand an improved > chance of being read? Move the relevant text from Documentation/networking/phy.rst into linux/phy.h as a linux doc format comment. "include" that header file into Documentation/networking/phy.rst with .. kernel-doc:: include/linux/phy.h -Tony
Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees
On Mon, Dec 09, 2024 at 03:42:52PM +0100, Andrew Lunn wrote: > I also find a lot of developers don't really understand what phy-mode > and PHY_INTERFACE_MODE_RGMII_* actually mean. ... which is why we documented it in Documentation/networking/phy.rst, but it seems folk who run into RGMII stuff don't read that document. I'm wondering if we can come up with some kernel-doc compatible way to document them in linux/phy.h instead, which may stand an improved chance of being read? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings
On Mon, 9 Dec 2024 12:59:40 -0800 Christopher Ferris wrote: > It looks like the way this was fixed in the ethtool.h uapi header was to > revert the usage of __struct_group. Should something similar happen for > pkt_cls.h? Or would it be easier to simply remove the usage of the TAG in > the _struct_group macro? Just to state it explicitly - are you running into a compilation issue with existing user space after updating pkt_cls.h?
Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings
On 09/12/24 15:14, Christopher Ferris wrote: Yes, when compiling Android, we have a C++ file that includes the pkt_cls.h directly to get access to some of the structures from that file. It currently gets the "types cannot be declared in an anonymous union" error due to the TAG part of the __struct_group usage not being empty in that file. (sigh) this should be reverted: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9c60712d71ff07197b2982899b9db28ed548ded -- Gustavo Christopher On Mon, Dec 9, 2024 at 1:10 PM Jakub Kicinski wrote: On Mon, 9 Dec 2024 12:59:40 -0800 Christopher Ferris wrote: It looks like the way this was fixed in the ethtool.h uapi header was to revert the usage of __struct_group. Should something similar happen for pkt_cls.h? Or would it be easier to simply remove the usage of the TAG in the _struct_group macro? Just to state it explicitly - are you running into a compilation issue with existing user space after updating pkt_cls.h?
Re: [PATCH v4 1/1] exec: seal system mappings
On Mon, Nov 25, 2024 at 12:49 PM wrote: > > From: Jeff Xu > > Seal vdso, vvar, sigpage, uprobes and vsyscall. > > Those mappings are readonly or executable only, sealing can protect > them from ever changing or unmapped during the life time of the process. > For complete descriptions of memory sealing, please see mseal.rst [1]. > > System mappings such as vdso, vvar, and sigpage (for arm) are > generated by the kernel during program initialization, and are > sealed after creation. > > Unlike the aforementioned mappings, the uprobe mapping is not > established during program startup. However, its lifetime is the same > as the process's lifetime [2]. It is sealed from creation. > > The vdso, vvar, sigpage, and uprobe mappings all invoke the > _install_special_mapping() function. As no other mappings utilize this > function, it is logical to incorporate sealing logic within > _install_special_mapping(). This approach avoids the necessity of > modifying code across various architecture-specific implementations. > > The vsyscall mapping, which has its own initialization function, is > sealed in the XONLY case, it seems to be the most common and secure > case of using vsyscall. > > It is important to note that the CHECKPOINT_RESTORE feature (CRIU) may > alter the mapping of vdso, vvar, and sigpage during restore > operations. Consequently, this feature cannot be universally enabled > across all systems. > ... > > +config SEAL_SYSTEM_MAPPINGS > + bool "seal system mappings" > + default n > + depends on 64BIT > + depends on ARCH_HAS_SEAL_SYSTEM_MAPPINGS > + depends on !CHECKPOINT_RESTORE Hi Jeff, I like the idea of this patchset, but I don’t like the idea of forcing users to choose between this security feature and checkpoint/restore functionality. We need to explore ways to make this feature work with checkpoint/restore. Relying on CAP_CHECKPOINT_RESTORE is the obvious approach. CRIU just needs to move these mappings, and it doesn't need to change their properties or modify their contents. With that in mind, here are two options: * Allow moving sealed mappings for processes with CAP_CHECKPOINT_RESTORE. * Allow temporarily "unsealing" mappings for processes with CAP_CHECKPOINT_RESTORE. CRIU could unseal mappings, move them, and then seal them back. Another approach might be to make this feature configurable on a per-process basis (e.g., via prctl). Once enabled for a process, it would be inherited by all its children. It can't be disabled unless a process has CAP_CHECKPOINT_RESTORE. I've added Mike, Dima, and Alex to the thread. They might have other ideas. Thanks, Andrei
[PATCH] iio: proximity: hx9023s: Added firmware file parsing functionality
Configuration information is now prioritized from the firmware file.
If the firmware file is missing or fails to parse, the driver falls
back to using the default configuration list for writing the settings.
Signed-off-by: Yasin Lee
---
drivers/iio/proximity/hx9023s.c | 96 ++---
1 file changed, 89 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
index 4021feb7a7ac..6cb1b688bfa9 100644
--- a/drivers/iio/proximity/hx9023s.c
+++ b/drivers/iio/proximity/hx9023s.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -100,6 +101,17 @@
#define HX9023S_INTERRUPT_MASK GENMASK(9, 0)
#define HX9023S_PROX_DEBOUNCE_MASK GENMASK(3, 0)
+#define FW_VER_OFFSET 2
+#define FW_REG_CNT_OFFSET 3
+#define FW_DATA_OFFSET 16
+
+struct hx9023s_bin {
+ u16 reg_count;
+ u16 fw_size;
+ u8 fw_ver;
+ u8 data[] __counted_by(fw_size);
+};
+
struct hx9023s_ch_data {
s16 raw; /* Raw Data*/
s16 lp; /* Low Pass Filter Data*/
@@ -998,6 +1010,80 @@ static int hx9023s_id_check(struct iio_dev *indio_dev)
return 0;
}
+static int hx9023s_bin_load(struct hx9023s_data *data,
+ struct
hx9023s_bin *bin)
+{
+ u8 *cfg_start = bin->data + FW_DATA_OFFSET;
+ u8 addr, val;
+ u16 i;
+ int ret;
+
+ for (i = 0; i < bin->reg_count; i++) {
+ addr = cfg_start[i * 2];
+ val = cfg_start[i * 2 + 1];
+ ret = regmap_write(data->regmap, addr, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ return ret;
+}
+
+static int hx9023s_send_cfg(const struct firmware *fw,
+ struct
hx9023s_data *data)
+{
+ if (!fw)
+ return -EINVAL;
+
+ struct hx9023s_bin *bin __free(kfree) =
+ kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
+ if (!bin)
+ return -ENOMEM;
+
+ memcpy(bin->data, fw->data, fw->size);
+ release_firmware(fw);
+
+ bin->fw_size = fw->size;
+ bin->fw_ver = bin->data[FW_VER_OFFSET];
+ bin->reg_count = get_unaligned_le16(bin->data + FW_REG_CNT_OFFSET);
+
+ return hx9023s_bin_load(data, bin);
+}
+
+static void hx9023s_cfg_update(const struct firmware *fw, void *context)
+{
+ struct hx9023s_data *data = context;
+ struct device *dev = regmap_get_device(data->regmap);
+ int ret;
+
+ if (!fw || !fw->data) {
+ dev_warn(dev, "No firmware\n");
+ goto no_fw;
+ }
+
+ ret = hx9023s_send_cfg(fw, data);
+ if (ret)
+ goto no_fw;
+
+ ret = regcache_sync(data->regmap);
+ if (ret)
+ dev_err(dev, "regcache sync failed\n");
+
+ return;
+
+no_fw:
+ ret = regmap_multi_reg_write(data->regmap, hx9023s_reg_init_list,
+
ARRAY_SIZE(hx9023s_reg_init_list));
+ if (ret) {
+ dev_err(dev, "Error loading default configuration\n");
+ return;
+ }
+
+ ret = regcache_sync(data->regmap);
+ if (ret)
+ dev_err(dev, "regcache sync failed\n");
+}
+
static int hx9023s_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -1036,18 +1122,14 @@ static int hx9023s_probe(struct i2c_client *client)
indio_dev->modes = INDIO_DIRECT_MODE;
i2c_set_clientdata(client, indio_dev);
- ret = regmap_multi_reg_write(data->regmap, hx9023s_reg_init_list,
-ARRAY_SIZE(hx9023s_reg_init_list));
- if (ret)
- return dev_err_probe(dev, ret, "device init failed\n");
-
ret = hx9023s_ch_cfg(data);
if (ret)
return dev_err_probe(dev, ret, "channel config failed\n");
- ret = regcache_sync(data->regmap);
+ ret = request_firmware_nowait(THIS_MODULE, true, "hx9023s.bin",
+ dev, GFP_KERNEL, data,
hx9023s_cfg_update);
if (ret)
- return dev_err_probe(dev, ret, "regcache sync failed\n");
+ return dev_err_probe(dev, ret, "reg config failed\n");
if (client->irq) {
ret = devm_request_threaded_irq(dev, client->irq,
---
base-commit: 8d4d26450d71289a35ff9e847675fd9c718798b8
change-id: 20241209-hx9023s-firmware-20241209-47411e8cda0b
Best regards,
--
Yasin Lee
Re: [PATCH v2] scsi: Replace zero-length array with flexible array member
On Sun, 10 Nov 2024 23:33:24 +0100, Thorsten Blum wrote: > Replace the deprecated zero-length array with a modern flexible array > member in the struct iscsi_bsg_host_vendor_reply. > > Applied to 6.14/scsi-queue, thanks! [1/1] scsi: Replace zero-length array with flexible array member https://git.kernel.org/mkp/scsi/c/cdb03e598750 -- Martin K. Petersen Oracle Linux Engineering
