* [PATCH v2 04/10] arm64: dts: microchip: sparx5_pcb135: add missing I2C mux unit addresses
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
The children of I2C mux should be named "i2c", according to DT schema
and bindings, and they should have unit address.
This fixes dtbs_check warnings like:
sparx5_pcb135.dtb: i2c0-imux@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'i2c_sfp1', 'i2c_sfp2', 'i2c_sfp3', 'i2c_sfp4' were unexpected)
and dtc W=1 warnings:
sparx5_pcb135_board.dtsi:172.23-180.4: Warning (simple_bus_reg): /axi@600000000/sfp-eth60: missing or empty reg/ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
index 82ce007d9959..bf51a6e11cf1 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
@@ -146,22 +146,22 @@ &i2c0_imux {
pinctrl-2 = <&i2cmux_s31>;
pinctrl-3 = <&i2cmux_s32>;
pinctrl-4 = <&i2cmux_pins_i>;
- i2c_sfp1: i2c_sfp1 {
+ i2c_sfp1: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp2: i2c_sfp2 {
+ i2c_sfp2: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp3: i2c_sfp3 {
+ i2c_sfp3: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp4: i2c_sfp4 {
+ i2c_sfp4: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
_______________________________________________
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 v2 03/10] arm64: dts: microchip: sparx5_pcb134: add missing I2C mux unit addresses
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
The children of I2C mux should be named "i2c", according to DT schema
and bindings, and they should have unit address.
This fixes dtbs_check warnings like:
sparx5_pcb134_emmc.dtb: i2c0-imux@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'i2c_sfp1', ...
and dtc W=1 warnings:
sparx5_pcb134_board.dtsi:548.23-555.4: Warning (simple_bus_reg): /axi@600000000/sfp-eth12: missing or empty reg/ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
.../dts/microchip/sparx5_pcb134_board.dtsi | 40 +++++++++----------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index f3e226de5e5e..e816e6e9d62d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -427,62 +427,62 @@ &i2c0_imux {
pinctrl-10 = <&i2cmux_10>;
pinctrl-11 = <&i2cmux_11>;
pinctrl-12 = <&i2cmux_pins_i>;
- i2c_sfp1: i2c_sfp1 {
+ i2c_sfp1: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp2: i2c_sfp2 {
+ i2c_sfp2: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp3: i2c_sfp3 {
+ i2c_sfp3: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp4: i2c_sfp4 {
+ i2c_sfp4: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp5: i2c_sfp5 {
+ i2c_sfp5: i2c@4 {
reg = <0x4>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp6: i2c_sfp6 {
+ i2c_sfp6: i2c@5 {
reg = <0x5>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp7: i2c_sfp7 {
+ i2c_sfp7: i2c@6 {
reg = <0x6>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp8: i2c_sfp8 {
+ i2c_sfp8: i2c@7 {
reg = <0x7>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp9: i2c_sfp9 {
+ i2c_sfp9: i2c@8 {
reg = <0x8>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp10: i2c_sfp10 {
+ i2c_sfp10: i2c@9 {
reg = <0x9>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp11: i2c_sfp11 {
+ i2c_sfp11: i2c@a {
reg = <0xa>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp12: i2c_sfp12 {
+ i2c_sfp12: i2c@b {
reg = <0xb>;
#address-cells = <1>;
#size-cells = <0>;
@@ -495,42 +495,42 @@ &gpio 60 GPIO_ACTIVE_HIGH
&gpio 61 GPIO_ACTIVE_HIGH
&gpio 54 GPIO_ACTIVE_HIGH>;
idle-state = <0x8>;
- i2c_sfp13: i2c_sfp13 {
+ i2c_sfp13: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp14: i2c_sfp14 {
+ i2c_sfp14: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp15: i2c_sfp15 {
+ i2c_sfp15: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp16: i2c_sfp16 {
+ i2c_sfp16: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp17: i2c_sfp17 {
+ i2c_sfp17: i2c@4 {
reg = <0x4>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp18: i2c_sfp18 {
+ i2c_sfp18: i2c@5 {
reg = <0x5>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp19: i2c_sfp19 {
+ i2c_sfp19: i2c@6 {
reg = <0x6>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp20: i2c_sfp20 {
+ i2c_sfp20: i2c@7 {
reg = <0x7>;
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
_______________________________________________
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 v2 02/10] arm64: dts: microchip: sparx5: correct serdes unit address
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
Unit address should match "reg" property, as reported by dtc W=1
warnings:
sparx5.dtsi:463.27-468.5: Warning (simple_bus_reg): /axi@600000000/serdes@10808000: simple-bus unit address format error, expected "610808000"
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 5d820da8c69d..c3029e0abacc 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -460,7 +460,7 @@ mdio3: mdio@61101031c {
reg = <0x6 0x1101031c 0x24>;
};
- serdes: serdes@10808000 {
+ serdes: serdes@610808000 {
compatible = "microchip,sparx5-serdes";
#phy-cells = <1>;
clocks = <&sys_clk>;
--
2.34.1
_______________________________________________
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 v2 01/10] arm64: dts: microchip: sparx5: fix mdio reg
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
Correct the reg address of mdio node to match unit address. Assume the
reg is not correct and unit address was correct, because there is
already node using the existing reg 0x110102d4.
sparx5.dtsi:443.25-451.5: Warning (simple_bus_reg): /axi@600000000/mdio@6110102f8: simple-bus unit address format error, expected "6110102d4"
Fixes: d0f482bb06f9 ("arm64: dts: sparx5: Add the Sparx5 switch node")
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Not tested on hardware
Changes in v2:
1. Add tags.
---
arch/arm64/boot/dts/microchip/sparx5.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 24075cd91420..5d820da8c69d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -447,7 +447,7 @@ mdio2: mdio@6110102f8 {
pinctrl-names = "default";
#address-cells = <1>;
#size-cells = <0>;
- reg = <0x6 0x110102d4 0x24>;
+ reg = <0x6 0x110102f8 0x24>;
};
mdio3: mdio@61101031c {
--
2.34.1
_______________________________________________
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 13/15] sh: Move defines needed for suppressing warning backtraces
From: Simon Horman @ 2024-04-05 18:31 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kselftest, David Airlie, Arnd Bergmann, Maíra Canal,
Dan Carpenter, Kees Cook, Daniel Diaz, David Gow, Arthur Grillo,
Brendan Higgins, Naresh Kamboju, Maarten Lankhorst, Andrew Morton,
Maxime Ripard, Ville Syrjälä, Daniel Vetter,
Thomas Zimmermann, dri-devel, kunit-dev, linux-arch,
linux-arm-kernel, linux-doc, linux-kernel, linux-parisc,
linuxppc-dev, linux-riscv, linux-s390, linux-sh, loongarch,
netdev, x86, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz
In-Reply-To: <20240403131936.787234-14-linux@roeck-us.net>
On Wed, Apr 03, 2024 at 06:19:34AM -0700, Guenter Roeck wrote:
> Declaring the defines needed for suppressing warning inside
> '#ifdef CONFIG_DEBUG_BUGVERBOSE' results in a kerneldoc warning.
>
> .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY().
> Prototype was for HAVE_BUG_FUNCTION() instead
>
> Move the defines above the kerneldoc entry for _EMIT_BUG_ENTRY
> to make kerneldoc happy.
>
> Reported-by: Simon Horman <horms@kernel.org>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v3: Added patch. Possibly squash into previous patch.
FWIIW, this looks good to me.
> arch/sh/include/asm/bug.h | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
> index 470ce6567d20..bf4947d51d69 100644
> --- a/arch/sh/include/asm/bug.h
> +++ b/arch/sh/include/asm/bug.h
> @@ -11,6 +11,15 @@
> #define HAVE_ARCH_BUG
> #define HAVE_ARCH_WARN_ON
>
> +#ifdef CONFIG_DEBUG_BUGVERBOSE
> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR "\t.long %O2\n"
> +#else
> +# define __BUG_FUNC_PTR
> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> +#endif /* CONFIG_DEBUG_BUGVERBOSE */
> +
> /**
> * _EMIT_BUG_ENTRY
> * %1 - __FILE__
> @@ -25,13 +34,6 @@
> */
> #ifdef CONFIG_DEBUG_BUGVERBOSE
>
> -#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> -# define HAVE_BUG_FUNCTION
> -# define __BUG_FUNC_PTR "\t.long %O2\n"
> -#else
> -# define __BUG_FUNC_PTR
> -#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> -
> #define _EMIT_BUG_ENTRY \
> "\t.pushsection __bug_table,\"aw\"\n" \
> "2:\t.long 1b, %O1\n" \
> --
> 2.39.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Krzysztof Kozlowski @ 2024-04-05 18:22 UTC (permalink / raw)
To: Gabriel FERNANDEZ, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
Alexandre Torgue, Philipp Zabel
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <285f2f64-58b0-4dd0-9f1a-89306a79d572@foss.st.com>
On 05/04/2024 14:54, Gabriel FERNANDEZ wrote:
>
> On 4/5/24 09:12, Krzysztof Kozlowski wrote:
>> On 02/04/2024 14:53, gabriel.fernandez@foss.st.com wrote:
>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>>
>>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
>>>
>>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
>> And except what Rob said, this does not look as a fix. How ABI break
>> could be a fix and what is even to fix here? Please describe the
>> observable bug, how it manifests itself and what is exactly the fix for
>> that bug.
> As I replied to Rob, there are no RCC STM32MP25 drivers already upstreamed.
>
> However, in my series, the DT binding was merged even though Stephen
> made some
>
> important remarks that needed to be taken into account.
>
> That's why I proposed a fix to update the documentation.
>
> To be sure, how would you like me to proceed?
You can send v3 and get exactly the same questions. Your commit msg must
answer to all such unusual questions. If maintainer asks something that
you need to explain, it is a hint for you that your commit msg is
inadequate.
Best regards,
Krzysztof
_______________________________________________
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 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Krzysztof Kozlowski @ 2024-04-05 18:21 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <ZhAO4YWuB8r8k+m8@lizhi-Precision-Tower-5810>
On 05/04/2024 16:46, Frank Li wrote:
> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
>> On 04/04/2024 18:19, Frank Li wrote:
>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
>>> i2c bus mux to connect both i2c devices. One will probe failure and other
>>> will probe success when devices driver check whoami. So one dtb can cover
>>> both board configuration.
>>
>> I don't understand it. Either you add real device or not. If one board
>> has two devices, then why do you need to check for failures?
>>
>> Anyway, don't add fake stuff to DTS.
>
> NAK can't resolve the problem. It should be common problem for long time
> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> chips on boards have been replace by some pin to pin compatible sensor. For
> example:
> old boards: use sensor A with address 0x1a
> new bench: use sensor B with address 0x1b.
>
> You can treat it as two kind boards, RevA or RevB. But most user want to
> use one dtb to handle such small differences. For this case, it should be
> simple. Just add a super set.
> i2c
> {
> sensorA@1a
> {
> }
> sensorB@1b
> {
> }
> }
>
> It also depend on whoami check by i2c devices. Only A or B will probe.
>
> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> allow the same i2c address in one i2c bus.
>
> You are feel to NAK my method, but I hope you also provide constructive
> solution to help resolve the problem.
Yes, we resolved it long time ago. Your bootloader can (usually easily)
detect revision of the board and load appropriate DTS or DTS+DTSO.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault (2)
From: Russell King (Oracle) @ 2024-04-05 18:19 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Puranjay Mohan, Mark Rutland, Andrew Morton,
linux-arm-kernel, syzbot, LKML, linux-mm, syzkaller-bugs, bpf
In-Reply-To: <CAEf4BzYNc-cxRu9qEe2DWdCBNwXAvpSBHKtUhXtoEhB_XNc1Gg@mail.gmail.com>
On Fri, Apr 05, 2024 at 10:50:30AM -0700, Andrii Nakryiko wrote:
> On Fri, Apr 5, 2024 at 9:30 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Apr 5, 2024 at 4:36 AM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Fri, Apr 05, 2024 at 12:02:36PM +0100, Mark Rutland wrote:
> > > > On Thu, Apr 04, 2024 at 03:57:04PM -0700, Alexei Starovoitov wrote:
> > > > > On Wed, Apr 3, 2024 at 6:56 PM Andrew Morton <akpm@linux-foundationorg> wrote:
> > > > > >
> > > > > > On Mon, 01 Apr 2024 22:19:25 -0700 syzbot <syzbot+186522670e6722692d86@syzkaller.appspotmail.com> wrote:
> > > > > >
> > > > > > > Hello,
> > > > > >
> > > > > > Thanks. Cc: bpf@vger.kernel.org
> > > > >
> > > > > I suspect the issue is not on bpf side.
> > > > > Looks like the bug is somewhere in arm32 bits.
> > > > > copy_from_kernel_nofault() is called from lots of places.
> > > > > bpf is just one user that is easy for syzbot to fuzz.
> > > > > Interestingly arm defines copy_from_kernel_nofault_allowed()
> > > > > that should have filtered out user addresses.
> > > > > In this case ffffffe9 is probably a kernel address?
> > > >
> > > > It's at the end of the kernel range, and it's ERR_PTR(-EINVAL).
> > > >
> > > > 0xffffffe9 is -0x16, which is -22, which is -EINVAL.
> > > >
> > > > > But the kernel is doing a write?
> > > > > Which makes no sense, since copy_from_kernel_nofault is probe reading.
> > > >
> > > > It makes perfect sense; the read from 'src' happened, then the kernel tries to
> > > > write the result to 'dst', and that aligns with the disassembly in the report
> > > > below, which I beleive is:
> > > >
> > > > 8: e4942000 ldr r2, [r4], #0 <-- Read of 'src', fault fixup is elsewhere
> > > > c: e3530000 cmp r3, #0
> > > > * 10: e5852000 str r2, [r5] <-- Write to 'dst'
> > > >
> > > > As above, it looks like 'dst' is ERR_PTR(-EINVAL).
> > > >
> > > > Are you certain that BPF is passing a sane value for 'dst'? Where does that
> > > > come from in the first place?
> > >
> > > It looks to me like it gets passed in from the BPF program, and the
> > > "type" for the argument is set to ARG_PTR_TO_UNINIT_MEM. What that
> > > means for validation purposes, I've no idea, I'm not a BPF hacker.
> > >
> > > Obviously, if BPF is allowing copy_from_kernel_nofault() to be passed
> > > an arbitary destination address, that would be a huge security hole.
> >
> > If that's the case that's indeed a giant security hole,
> > but I doubt it. We would be crashing other archs as well.
> > I cannot really tell whether arm32 JIT is on.
> > If it is, it's likely a bug there.
> > Puranjay,
> > could you please take a look.
> >
>
> I dumped the BPF program that repro.c is loading, it works on x86-64
> and there is nothing special there. We are probe-reading 5 bytes from
> somewhere into the stack. Everything is unaligned here, but stays
> within a well-defined memory slot.
>
> Note the r3 = (s8)r1, that's a new-ish thing, maybe bug is somewhere
> there (but then it would be JIT, not verifier itself)
>
> 0: (7a) *(u64 *)(r10 -8) = 896542069
> 1: (bf) r1 = r10
> 2: (07) r1 += -7
> 3: (b7) r2 = 5
> 4: (bf) r3 = (s8)r1
> 5: (85) call bpf_probe_read_kernel#-72390
Before jumping to conclusions, let's try to unravel what's going on
here.
We're calling bpf_probe_read_kernel(), and the arguments to this are:
void *dst ; r1
u32 size ; r2
const void *unsafe_ptr ; r3
The problem that has been reported is that the _store_ in
copy_from_kernel_nofault(). Thus it's the destination pointer that's
the proble, and thus that is the value that ends up in r1.
What we can also see in the dump is that the address being read from
is the same as the address being written, and these are both
0xffffffe9 or -22. This would mean that both r3 and r1 contain the
same value.
Unwinding the code further, r1 comes from r10 - 7. So r10 probably
was -15.
Neither of these are valid stack addresses on 32-bit ARM.
Now, to repeat the same question. Is the BPF JIT on for this test?
This is a crucial piece of information, because it tells us whether
we need to look at the JIT or whether there's a problem with the
BPF interpreter. Please answer this question.
The next question to BPF people is... what is r10? Is that supposed
to be the read-only frame pointer? If so, why is it called r10 and
not something more readable? I'm guessing that the definition is
BPF_REG_FP, but I'm grasping at straws here (BPF people, fix your
debug so people who don't know BPF inside out can understand it!)
If the BPF JIT is being used, I think the next thing which needs to
happen is that the BPF JIT debug needs to be enabled. If
/proc/sys/net/core/bpf_jit_enable contains a value greater than 1,
then the ARM assembly will be hexdumped. One of the annoying things
is going to be piecing the hexdump together, converting it into a
form that can then be turned into a binary file, to then be
disassembled by objdump.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
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 00/12] mm/gup: Unify hugetlb, part 2
From: Jason Gunthorpe @ 2024-04-05 18:16 UTC (permalink / raw)
To: Peter Xu
Cc: linux-mm, linux-kernel, linuxppc-dev, Michael Ellerman,
Christophe Leroy, Matthew Wilcox, Rik van Riel, Lorenzo Stoakes,
Axel Rasmussen, Yang Shi, John Hubbard, linux-arm-kernel,
Kirill A . Shutemov, Andrew Jones, Vlastimil Babka, Mike Rapoport,
Andrew Morton, Muchun Song, Christoph Hellwig, linux-riscv,
James Houghton, David Hildenbrand, Andrea Arcangeli,
Aneesh Kumar K . V, Mike Kravetz
In-Reply-To: <Zg8gEyE4o_VJsTmx@x1n>
On Thu, Apr 04, 2024 at 05:48:03PM -0400, Peter Xu wrote:
> On Tue, Mar 26, 2024 at 11:02:52AM -0300, Jason Gunthorpe wrote:
> > The more I look at this the more I think we need to get to Matthew's
> > idea of having some kind of generic page table API that is not tightly
> > tied to level. Replacing the hugetlb trick of 'everything is a PTE'
> > with 5 special cases in every place seems just horrible.
> >
> > struct mm_walk_ops {
> > int (*leaf_entry)(struct mm_walk_state *state, struct mm_walk *walk);
> > }
> >
> > And many cases really want something like:
> > struct mm_walk_state state;
> >
> > if (!mm_walk_seek_leaf(state, mm, address))
> > goto no_present
> > if (mm_walk_is_write(state)) ..
> >
> > And detailed walking:
> > for_each_pt_leaf(state, mm, address) {
> > if (mm_walk_is_write(state)) ..
> > }
> >
> > Replacing it with a mm_walk_state that retains the level or otherwise
> > to allow decoding any entry composes a lot better. Forced Loop
> > unrolling can get back to the current code gen in alot of places.
> >
> > It also makes the power stuff a bit nicer as the mm_walk_state could
> > automatically retain back pointers to the higher levels in the state
> > struct too...
> >
> > The puzzle is how to do it and still get reasonable efficient codegen,
> > many operations are going to end up switching on some state->level to
> > know how to decode the entry.
>
> These discussions are definitely constructive, thanks Jason. Very helpful.
>
> I thought about this last week but got interrupted. It does make sense to
> me; it looks pretty generic and it is flexible enough as a top design. At
> least that's what I thought.
Yeah, exactly..
> However now when I rethink about it, and look more into the code when I got
> the chance, it turns out this will be a major rewrite of mostly every
> walkers..
Indeed, it is why it may not be reasonable.
> Consider that what we (or.. I) want to teach the pXd layers are two things
> right now: (1) hugetlb mappings (2) MMIO (PFN) mappings. That mostly
> shares the generic concept when working on the mm walkers no matter which
> way to go, just different treatment on different type of mem. (2) is on
> top of current code and new stuff, while (1) is a refactoring to drop
> hugetlb_entry() hook point as the goal.
Right, I view this as a two pronged attack
One one front you teach the generic pXX_* macros to process huge pages
and push that around to the performance walkers like GUP
On another front you want to replace use of the hugepte with the new
walkers.
The challenge with the hugepte code is that it is all structured to
assume one API that works at all levels and that may be a hard fit to
replace with pXX_* functions.
The places that are easy to switch from hugetlb to pXX_* may as well
do so.
Other places maybe need a hugetlb replacement that has a similar
abstraction level of pointing to any page table level.
I think if you do the easy places for pXX conversion you will have a
good idea about what is needed for the hard places.
> Now the important question I'm asking myself is: do we really need huge p4d
> or even bigger?
Do we need huge p4d support with folios? Probably not..
huge p4d support for pfnmap, eg in VFIO. Yes I think that is possibly
interesting - but I wouldn't ask anyone to do the work :)
But then again we come back to power and its big list of page sizes
and variety :( Looks like some there have huge sizes at the pgd level
at least.
> So, can we over-engineer too much if we go the generic route now?
Yes we can, and it will probably be slow as well. The pXX macros are
the most efficient if code can be adapted to use them.
> Considering that we already have most of pmd/pud entries around in the mm
> walker ops.
Yeah, so you add pgd and maybe p4d and then we can don't need any
generic thing. If it is easy it would be nice.
Jason
_______________________________________________
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 bpf-next] arm64, bpf: add internal-only MOV instruction to resolve per-CPU addrs
From: Andrii Nakryiko @ 2024-04-05 18:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Puranjay Mohan, Catalin Marinas, Will Deacon, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Zi Shen Lim,
Xu Kuohai, Florent Revest, linux-arm-kernel, LKML, bpf
In-Reply-To: <CAADnVQ+MRAg2hKJfG_QUjXu8WHLb+7tpPgMOQv8rfCSmKU2w5Q@mail.gmail.com>
On Fri, Apr 5, 2024 at 8:48 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 2:17 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > Support an instruction for resolving absolute addresses of per-CPU
> > data from their per-CPU offsets. This instruction is internal-only and
> > users are not allowed to use them directly. They will only be used for
> > internal inlining optimizations for now between BPF verifier and BPF
> > JITs.
> >
> > Since commit 7158627686f0 ("arm64: percpu: implement optimised pcpu
> > access using tpidr_el1"), the per-cpu offset for the CPU is stored in
> > the tpidr_el1/2 register of that CPU.
> >
> > To support this BPF instruction in the ARM64 JIT, the following ARM64
> > instructions are emitted:
> >
> > mov dst, src // Move src to dst, if src != dst
> > mrs tmp, tpidr_el1/2 // Move per-cpu offset of the current cpu in tmp.
> > add dst, dst, tmp // Add the per cpu offset to the dst.
> >
> > If CONFIG_SMP is not defined, then nothing is emitted if src == dst, and
> > mov dst, src is emitted if dst != src.
> >
> > To measure the performance improvement provided by this change, the
> > benchmark in [1] was used:
> >
> > Before:
> > glob-arr-inc : 23.597 ± 0.012M/s
> > arr-inc : 23.173 ± 0.019M/s
> > hash-inc : 12.186 ± 0.028M/s
> >
> > After:
> > glob-arr-inc : 23.819 ± 0.034M/s
> > arr-inc : 23.285 ± 0.017M/s
> > hash-inc : 12.419 ± 0.011M/s
> >
> > [1] https://github.com/anakryiko/linux/commit/8dec900975ef
>
> You don't see as big of a gain, because bpf_get_smp_processor_id()
> is not inlined yet on arm64.
>
yep, would be nice to add ARM64 and RISC-V support there as well.
Though it feels that supporting this in BPF JIT directly might be
actually easier for RISC-V/ARM64, not sure?
> But even without it I expected bigger gains.
> Could you do 'perf report' before/after ?
> Just want to see what's on top.
I also did `bpftool p d x id <progid>` and `bpftool p d j id <progid>`
to validate expected inlined BPF instructions and jitted code. So it
might be a good idea to do that as well.
Either way, thanks for working on this!
>
> >
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> > ---
> > arch/arm64/include/asm/insn.h | 7 +++++++
> > arch/arm64/lib/insn.c | 11 +++++++++++
> > arch/arm64/net/bpf_jit.h | 6 ++++++
> > arch/arm64/net/bpf_jit_comp.c | 16 ++++++++++++++++
> > 4 files changed, 40 insertions(+)
> >
[...]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL] arm64 fix for 6.9-rc3
From: Catalin Marinas @ 2024-04-05 18:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Will Deacon, linux-arm-kernel, linux-kernel
Hi Linus,
Please pull the arm64 fix below. Thanks.
The following changes since commit 4cece764965020c22cff7665b18a012006359095:
Linux 6.9-rc1 (2024-03-24 14:10:05 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-fixes
for you to fetch changes up to b017a0cea627fcbe158fc2c214fe893e18c4d0c4:
arm64/ptrace: Use saved floating point state type to determine SVE layout (2024-04-03 15:02:00 +0100)
----------------------------------------------------------------
arm64/ptrace fix to use the correct SVE layout based on the saved
floating point state rather than the TIF_SVE flag. The latter may be
left on during syscalls even if the SVE state is discarded.
----------------------------------------------------------------
Mark Brown (1):
arm64/ptrace: Use saved floating point state type to determine SVE layout
arch/arm64/kernel/ptrace.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault (2)
From: Andrii Nakryiko @ 2024-04-05 17:50 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Russell King (Oracle), Puranjay Mohan, Mark Rutland,
Andrew Morton, linux-arm-kernel, syzbot, LKML, linux-mm,
syzkaller-bugs, bpf
In-Reply-To: <CAADnVQ+LKO2Y90DVZ4qQv3dXyuWKkvFqqJ0E_p_=qwscsvnaVg@mail.gmail.com>
On Fri, Apr 5, 2024 at 9:30 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 4:36 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Fri, Apr 05, 2024 at 12:02:36PM +0100, Mark Rutland wrote:
> > > On Thu, Apr 04, 2024 at 03:57:04PM -0700, Alexei Starovoitov wrote:
> > > > On Wed, Apr 3, 2024 at 6:56 PM Andrew Morton <akpm@linux-foundationorg> wrote:
> > > > >
> > > > > On Mon, 01 Apr 2024 22:19:25 -0700 syzbot <syzbot+186522670e6722692d86@syzkaller.appspotmail.com> wrote:
> > > > >
> > > > > > Hello,
> > > > >
> > > > > Thanks. Cc: bpf@vger.kernel.org
> > > >
> > > > I suspect the issue is not on bpf side.
> > > > Looks like the bug is somewhere in arm32 bits.
> > > > copy_from_kernel_nofault() is called from lots of places.
> > > > bpf is just one user that is easy for syzbot to fuzz.
> > > > Interestingly arm defines copy_from_kernel_nofault_allowed()
> > > > that should have filtered out user addresses.
> > > > In this case ffffffe9 is probably a kernel address?
> > >
> > > It's at the end of the kernel range, and it's ERR_PTR(-EINVAL).
> > >
> > > 0xffffffe9 is -0x16, which is -22, which is -EINVAL.
> > >
> > > > But the kernel is doing a write?
> > > > Which makes no sense, since copy_from_kernel_nofault is probe reading.
> > >
> > > It makes perfect sense; the read from 'src' happened, then the kernel tries to
> > > write the result to 'dst', and that aligns with the disassembly in the report
> > > below, which I beleive is:
> > >
> > > 8: e4942000 ldr r2, [r4], #0 <-- Read of 'src', fault fixup is elsewhere
> > > c: e3530000 cmp r3, #0
> > > * 10: e5852000 str r2, [r5] <-- Write to 'dst'
> > >
> > > As above, it looks like 'dst' is ERR_PTR(-EINVAL).
> > >
> > > Are you certain that BPF is passing a sane value for 'dst'? Where does that
> > > come from in the first place?
> >
> > It looks to me like it gets passed in from the BPF program, and the
> > "type" for the argument is set to ARG_PTR_TO_UNINIT_MEM. What that
> > means for validation purposes, I've no idea, I'm not a BPF hacker.
> >
> > Obviously, if BPF is allowing copy_from_kernel_nofault() to be passed
> > an arbitary destination address, that would be a huge security hole.
>
> If that's the case that's indeed a giant security hole,
> but I doubt it. We would be crashing other archs as well.
> I cannot really tell whether arm32 JIT is on.
> If it is, it's likely a bug there.
> Puranjay,
> could you please take a look.
>
I dumped the BPF program that repro.c is loading, it works on x86-64
and there is nothing special there. We are probe-reading 5 bytes from
somewhere into the stack. Everything is unaligned here, but stays
within a well-defined memory slot.
Note the r3 = (s8)r1, that's a new-ish thing, maybe bug is somewhere
there (but then it would be JIT, not verifier itself)
0: (7a) *(u64 *)(r10 -8) = 896542069
1: (bf) r1 = r10
2: (07) r1 += -7
3: (b7) r2 = 5
4: (bf) r3 = (s8)r1
5: (85) call bpf_probe_read_kernel#-72390
6: (b7) r0 = 0
7: (95) exit
_______________________________________________
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 1/2] serial: imx: Introduce timeout when waiting on transmitter empty
From: Fabio Estevam @ 2024-04-05 17:38 UTC (permalink / raw)
To: Esben Haabendal
Cc: Greg Kroah-Hartman, Jiri Slaby, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Marc Kleine-Budde, linux-kernel,
linux-serial, imx, linux-arm-kernel
In-Reply-To: <76cf9ce9cbf9dcdf78bc00ce7a919db1776ebce1.1712309058.git.esben@geanix.com>
On Fri, Apr 5, 2024 at 6:25 AM Esben Haabendal <esben@geanix.com> wrote:
>
> By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
s/potentital/potential
Could you elaborate on this deadlock? Have you seen it in practice?
Should a Fixes tag be added?
_______________________________________________
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 0/4] A new selftests/ directory for arm compatibility testing
From: Mark Brown @ 2024-04-05 17:37 UTC (permalink / raw)
To: Dev Jain
Cc: shuah, linux-arm-kernel, linux-kselftest, linux-kernel,
Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
Catalin.Marinas, will, mark.rutland, Russell King
In-Reply-To: <20240405084410.256788-1-dev.jain@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 892 bytes --]
On Fri, Apr 05, 2024 at 02:14:06PM +0530, Dev Jain wrote:
> This series introduces the selftests/arm directory, which tests 32 and 64-bit
> kernel compatibility with 32-bit ELFs running on the Aarch platform.
> The need for this bucket of tests is that 32 bit applications built on legacy
> ARM architecture must not break on the new Aarch64 platforms and the 64-bit
> kernel. The kernel must emulate the data structures, system calls and the
> registers according to Aarch32, when running a 32-bit process; this directory
> fills that testing requirement.
You should copy Russell King, the 32 bit architeture maintainer, on this
- I've added him here.
FWIW it was me who suggested this work, like Dev says having some
coverage of the corners of the 32 bit ABI feels especially sensible for
arm64 since compat mode could relatively easily get accidentally broken
making changes for 64 bit.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 18/25] dt-bindings: media: imx258: Add alternate compatible strings
From: Conor Dooley @ 2024-04-05 16:24 UTC (permalink / raw)
To: Dave Stevenson
Cc: git, linux-media, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <CAPY8ntC9SHJ6Ma17s0Vf2coB-0NUk-xgCLK9KCkxFMuXKHXNwg@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2440 bytes --]
On Fri, Apr 05, 2024 at 11:25:50AM +0100, Dave Stevenson wrote:
> Hi Conor
>
> On Wed, 3 Apr 2024 at 17:14, Conor Dooley <conor@kernel.org> wrote:
> >
> > On Wed, Apr 03, 2024 at 09:03:47AM -0600, git@luigi311.com wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >
> > > There are a number of variants of the imx258 modules that can not
> > > be differentiated at runtime, so add compatible strings for the
> > > PDAF variant.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Signed-off-by: Luis Garcia <git@luigi311.com>
> > > ---
> > > .../devicetree/bindings/media/i2c/sony,imx258.yaml | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > index bee61a443b23..c978abc0cdb3 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > @@ -13,11 +13,16 @@ description: |-
> > > IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > > type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > > is programmable through I2C interface. Image data is sent through MIPI
> > > - CSI-2.
> > > + CSI-2. The sensor exists in two different models, a standard variant
> > > + (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> > > + The camera module does not expose the model through registers, so the
> > > + exact model needs to be specified.
> > >
> > > properties:
> > > compatible:
> > > - const: sony,imx258
> > > + enum:
> > > + - sony,imx258
> > > + - sony,imx258-pdaf
> >
> > Does the pdaf variant support all of the features/is it register
> > compatible with the regular variant? If it is, the regular variant
> > should be a fallback compatible.
>
> It has the same register set, but certain registers have to be
> programmed differently so that the image is corrected for the
> partially shielded pixels used for phase detect auto focus (PDAF).
> Either compatible will "work" on either variant of the module, but
> you'll get weird image artifacts when using the wrong one.
To paraphase, a fallback compatible is not suitable.
Thanks Dave,
Conor.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
From: Andrzej Hajda @ 2024-04-05 16:20 UTC (permalink / raw)
To: Jani Nikula, Peter Zijlstra
Cc: Mark Rutland, linux-ia64, linux-sh, dri-devel, linux-kernel,
sparclinux, linux-riscv, linux-s390, linux-hexagon,
linux-snps-arc, intel-gfx, linux-xtensa, Arnd Bergmann,
Boqun Feng, linux-m68k, openrisc, loongarch, Rodrigo Vivi,
Andy Shevchenko, linux-arm-kernel, linux-parisc, linux-mips,
linux-alpha, Andrew Morton, linuxppc-dev
In-Reply-To: <87r0fjc1cz.fsf@intel.com>
On 05.04.2024 16:47, Jani Nikula wrote:
> On Mon, 27 Feb 2023, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote:
>>> On 22.02.2023 18:04, Peter Zijlstra wrote:
>>>> On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote:
>>>>
>>>>> Andrzej Hajda (7):
>>>>> arch: rename all internal names __xchg to __arch_xchg
>>>>> linux/include: add non-atomic version of xchg
>>>>> arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr
>>>>> llist: simplify __llist_del_all
>>>>> io_uring: use __xchg if possible
>>>>> qed: use __xchg if possible
>>>>> drm/i915/gt: use __xchg instead of internal helper
>>>> Nothing crazy in here I suppose, I somewhat wonder why you went through
>>>> the trouble, but meh.
>>> If you are asking why I have proposed this patchset, then the answer is
>>> simple, 1st I've tried to find a way to move internal i915 helper to core
>>> (see patch 7).
>>> Then I was looking for possible other users of this helper. And apparently
>>> there are many of them, patches 3-7 shows some.
>>>
>>>
>>>> You want me to take this through te locking tree (for the next cycle,
>>>> not this one) where I normally take atomic things or does someone else
>>>> want this?
>>> If you could take it I will be happy.
>> OK, I'll go queue it in tip/locking/core after -rc1. Thanks!
> Is this where the series fell between the cracks, or was there some
> follow-up that I missed?
>
> I think this would still be useful. Andrzej, would you mind rebasing and
> resending if there are no objections?
The patchset was rejected/dropped by Linus at the pull-request stage.
He didn't like many things, but the most __xchg name. However he was
quite positive about i915 name fetch_and_zero.
I can try to revive patchset with fetch_and_zero, and maybe
fetch_and_set, instead of __xchg.
Regards
Andrzej
>
> BR,
> Jani.
>
>
_______________________________________________
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 1/2] serial: imx: Introduce timeout when waiting on transmitter empty
From: Marc Kleine-Budde @ 2024-04-05 17:33 UTC (permalink / raw)
To: Esben Haabendal
Cc: Greg Kroah-Hartman, Jiri Slaby, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-kernel,
linux-serial, imx, linux-arm-kernel
In-Reply-To: <874jcf67xm.fsf@geanix.com>
[-- Attachment #1.1: Type: text/plain, Size: 1351 bytes --]
On 05.04.2024 19:22:29, Esben Haabendal wrote:
> Marc Kleine-Budde <mkl@pengutronix.de> writes:
>
> > On 05.04.2024 11:25:13, Esben Haabendal wrote:
> >> By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
> >> deadlock.
> >>
> >> In case of the timeout, there is not much we can do, so we simply ignore
> >> the transmitter state and optimistically try to continue.
> >>
> >> Signed-off-by: Esben Haabendal <esben@geanix.com>
> >> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >
> > Where's the cover letter and patch 2/2? Have a look at b4 [1], it's a
> > great tool to help you with sending git patch series.
>
> It is left out on purpose.
>
> This patch is a stand-alone patch as it is. The other part of the series
> you are talking about is not going to mainline for now. It needs still
> quite some work, and will only go in after all the other printk stuff.
>
> I hope we can merge this patch as it to mainline now, instead of piling
> up more than necessary in the rt tree.
Ok, then send it as patch 1/1.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v5] arm64/fpsimd: Suppress SVE access traps when loading FPSIMD state
From: Mark Brown @ 2024-04-05 17:23 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Benjamin Herrenschmidt, Dave Martin, linux-arm-kernel,
linux-kernel, Mark Brown
When we are in a syscall we take the opportunity to discard the SVE state,
saving only the FPSIMD subset of the register state. When we reload the
state from memory we reenable SVE access traps, stopping tracking SVE until
the task takes another SVE access trap. This means that for a task which is
actively using SVE many blocking system calls will have the additional
overhead of a SVE access trap.
As SVE deployment is progressing we are seeing much wider use of the SVE
instruction set, including performance optimised implementations of
operations like memset() and memcpy(), which mean that even tasks which are
not obviously floating point based can end up with substantial SVE usage.
It does not, however, make sense to just unconditionally use the full SVE
register state all the time since it is larger than the FPSIMD register
state so there is overhead saving and restoring it on context switch and
our requirement to flush the register state not shared with FPSIMD on
syscall also creates a noticeable overhead on system call.
I did some instrumentation which counted the number of SVE access traps
and the number of times we loaded FPSIMD only register state for each task.
Testing with Debian Bookworm this showed that during boot the overwhelming
majority of tasks triggered another SVE access trap more than 50% of the
time after loading FPSIMD only state with a substantial number near 100%,
though some programs had a very small number of SVE accesses most likely
from startup. There were few tasks in the range 5-45%, most tasks either
used SVE frequently or used it only a tiny proportion of times. As expected
older distributions which do not have the SVE performance work available
showed no SVE usage in general applications.
This indicates that there should be some useful benefit from reducing the
number of SVE access traps for blocking system calls like we did for non
blocking system calls in commit 8c845e273104 ("arm64/sve: Leave SVE enabled
on syscall if we don't context switch"). Let's do this with a timeout, when
we take a SVE access trap record a jiffies after which we'll reeanble SVE
traps then check this whenver we load a FPSIMD only floating point state
from memory. If the time has passed then we reenable traps, otherwise we
leave traps disabled and flush the non-shared register state like we would
on trap.
The timeout is currently set to a second, I pulled this number out of thin
air so there is doubtless some room for tuning. This means that for a
task which is actively using SVE the number of SVE access traps will be
substantially reduced but applications which use SVE only very
infrequently will avoid the overheads associated with tracking SVE state
after a second. The extra cost from additional tracking of SVE state
only occurs when a task is preempted so short running tasks should be
minimally affected.
There should be no functional change resulting from this, it is purely a
performance optimisation.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v5:
- Rebase onto v6.9-rc1.
- Use a timeout rather than number of state loads to decide when to
reenable traps.
- Link to v4: https://lore.kernel.org/r/20240122-arm64-sve-trap-mitigation-v4-1-54e0d78a3ae9@kernel.org
Changes in v4:
- Rebase onto v6.8-rc1.
- Link to v3: https://lore.kernel.org/r/20231113-arm64-sve-trap-mitigation-v3-1-4779c9382483@kernel.org
Changes in v3:
- Rebase onto v6.7-rc1.
- Link to v2: https://lore.kernel.org/r/20230913-arm64-sve-trap-mitigation-v2-1-1bdeff382171@kernel.org
Changes in v2:
- Rebase onto v6.6-rc1.
- Link to v1: https://lore.kernel.org/r/20230807-arm64-sve-trap-mitigation-v1-1-d92eed1d2855@kernel.org
---
arch/arm64/include/asm/processor.h | 1 +
arch/arm64/kernel/fpsimd.c | 42 ++++++++++++++++++++++++++++++++------
2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index f77371232d8c..7a6ed0551291 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -164,6 +164,7 @@ struct thread_struct {
unsigned int fpsimd_cpu;
void *sve_state; /* SVE registers, if any */
void *sme_state; /* ZA and ZT state, if any */
+ unsigned long sve_timeout; /* jiffies to drop TIF_SVE */
unsigned int vl[ARM64_VEC_MAX]; /* vector length */
unsigned int vl_onexec[ARM64_VEC_MAX]; /* vl after next exec */
unsigned long fault_address; /* fault info */
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index ebb0158997ca..22d7dc420e53 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -354,6 +354,7 @@ static void task_fpsimd_load(void)
{
bool restore_sve_regs = false;
bool restore_ffr;
+ unsigned long sve_vq_minus_one;
WARN_ON(!system_supports_fpsimd());
WARN_ON(preemptible());
@@ -365,18 +366,12 @@ static void task_fpsimd_load(void)
if (system_supports_sve() || system_supports_sme()) {
switch (current->thread.fp_type) {
case FP_STATE_FPSIMD:
- /* Stop tracking SVE for this task until next use. */
- if (test_and_clear_thread_flag(TIF_SVE))
- sve_user_disable();
break;
case FP_STATE_SVE:
if (!thread_sm_enabled(¤t->thread) &&
!WARN_ON_ONCE(!test_and_set_thread_flag(TIF_SVE)))
sve_user_enable();
- if (test_thread_flag(TIF_SVE))
- sve_set_vq(sve_vq_from_vl(task_get_sve_vl(current)) - 1);
-
restore_sve_regs = true;
restore_ffr = true;
break;
@@ -395,6 +390,15 @@ static void task_fpsimd_load(void)
}
}
+ /*
+ * If SVE has been enabled we may keep it enabled even if
+ * loading only FPSIMD state, so always set the VL.
+ */
+ if (system_supports_sve() && test_thread_flag(TIF_SVE)) {
+ sve_vq_minus_one = sve_vq_from_vl(task_get_sve_vl(current)) - 1;
+ sve_set_vq(sve_vq_minus_one);
+ }
+
/* Restore SME, override SVE register configuration if needed */
if (system_supports_sme()) {
unsigned long sme_vl = task_get_sme_vl(current);
@@ -421,6 +425,25 @@ static void task_fpsimd_load(void)
} else {
WARN_ON_ONCE(current->thread.fp_type != FP_STATE_FPSIMD);
fpsimd_load_state(¤t->thread.uw.fpsimd_state);
+
+ /*
+ * If the task had been using SVE we keep it enabled
+ * when loading FPSIMD only state for a period to
+ * minimise overhead for tasks actively using SVE,
+ * disabling it periodicaly to ensure that tasks that
+ * use SVE intermittently do eventually avoid the
+ * overhead of carrying SVE state. The timeout is
+ * initialised when we take a SVE trap in in
+ * do_sve_acc().
+ */
+ if (system_supports_sve() && test_thread_flag(TIF_SVE)) {
+ if (time_after(jiffies, current->thread.sve_timeout)) {
+ clear_thread_flag(TIF_SVE);
+ sve_user_disable();
+ } else {
+ sve_flush_live(true, sve_vq_minus_one);
+ }
+ }
}
}
@@ -1397,6 +1420,13 @@ void do_sve_acc(unsigned long esr, struct pt_regs *regs)
get_cpu_fpsimd_context();
+ /*
+ * We will keep SVE enabled when loading FPSIMD only state for
+ * the next second to minimise traps when userspace is
+ * actively using SVE.
+ */
+ current->thread.sve_timeout = jiffies + HZ;
+
if (test_and_set_thread_flag(TIF_SVE))
WARN_ON(1); /* SVE access shouldn't have trapped */
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20230807-arm64-sve-trap-mitigation-2e7e2663c849
Best regards,
--
Mark Brown <broonie@kernel.org>
_______________________________________________
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 1/2] serial: imx: Introduce timeout when waiting on transmitter empty
From: Esben Haabendal @ 2024-04-05 17:22 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Greg Kroah-Hartman, Jiri Slaby, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, linux-kernel,
linux-serial, imx, linux-arm-kernel
In-Reply-To: <20240405-impurity-emerald-f67dc37adf9b-mkl@pengutronix.de>
Marc Kleine-Budde <mkl@pengutronix.de> writes:
> On 05.04.2024 11:25:13, Esben Haabendal wrote:
>> By waiting at most 1 second for USR2_TXDC to be set, we avoid a potentital
>> deadlock.
>>
>> In case of the timeout, there is not much we can do, so we simply ignore
>> the transmitter state and optimistically try to continue.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>
> Where's the cover letter and patch 2/2? Have a look at b4 [1], it's a
> great tool to help you with sending git patch series.
It is left out on purpose.
This patch is a stand-alone patch as it is. The other part of the series
you are talking about is not going to mainline for now. It needs still
quite some work, and will only go in after all the other printk stuff.
I hope we can merge this patch as it to mainline now, instead of piling
up more than necessary in the rt tree.
/Esben
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v1 2/4] arm64: dts: freescale: imx8mp-verdin-dahlia: support sleep-moci
From: Stefan Eichenberger @ 2024-04-05 16:07 UTC (permalink / raw)
To: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini
Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
Stefan Eichenberger
In-Reply-To: <20240405160720.5977-1-eichest@gmail.com>
From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Previously, we had the sleep-moci pin set to always on. However, the
Dahlia carrier board supports disabling the sleep-moci when the system
is suspended to power down peripherals that support it. This reduces
overall power consumption. This commit adds support for this feature by
disabling the reg_force_sleep_moci regulator and adding two new
regulators for the USB hub and PCIe that can be turned off when the
system is suspended.
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
.../dts/freescale/imx8mp-verdin-dahlia.dtsi | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
index e68e0e6f21e9..abad1887040a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
@@ -32,6 +32,25 @@ simple-audio-card,cpu {
sound-dai = <&sai1>;
};
};
+
+ reg_usb_hub: regulator-usb-hub {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ /* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+ gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+ regulator-boot-on;
+ regulator-name = "HUB_PWR_EN";
+ };
+
+ reg_pcie: regulator-pcie {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ /* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+ gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+ regulator-boot-on;
+ regulator-name = "PCIE_1_PWR_EN";
+ startup-delay-us = <100000>;
+ };
};
&backlight {
@@ -117,6 +136,7 @@ wm8904_1a: audio-codec@1a {
/* Verdin PCIE_1 */
&pcie {
+ vpcie-supply = <®_pcie>;
status = "okay";
};
@@ -143,6 +163,11 @@ ®_usdhc2_vmmc {
vin-supply = <®_3p3v>;
};
+/* We support turning off sleep moci on Dahlia */
+®_force_sleep_moci {
+ status = "disabled";
+};
+
/* Verdin I2S_1 */
&sai1 {
assigned-clocks = <&clk IMX8MP_CLK_SAI1>;
@@ -186,6 +211,25 @@ &usb3_phy1 {
status = "okay";
};
+&usb_dwc3_1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_hub_3_0: usb-hub@1 {
+ compatible = "usb424,5744";
+ reg = <1>;
+ peer-hub = <&usb_hub_2_0>;
+ vdd-supply = <®_usb_hub>;
+ };
+
+ usb_hub_2_0: usb-hub@2 {
+ compatible = "usb424,2744";
+ reg = <2>;
+ peer-hub = <&usb_hub_3_0>;
+ vdd-supply = <®_usb_hub>;
+ };
+};
+
/* Verdin SD_1 */
&usdhc2 {
status = "okay";
--
2.40.1
_______________________________________________
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 v4 08/18] phy: ti: phy-j721e-wiz: add resume support
From: Andy Shevchenko @ 2024-04-05 17:14 UTC (permalink / raw)
To: Vinod Koul
Cc: Thomas Richard, Linus Walleij, Bartosz Golaszewski, Tony Lindgren,
Haojian Zhuang, Vignesh R, Aaro Koskinen, Janusz Krzysztofik,
Andi Shyti, Peter Rosin, Kishon Vijay Abraham I, Philipp Zabel,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, linux-gpio, linux-kernel, linux-arm-kernel,
linux-omap, linux-i2c, linux-phy, linux-pci, gregory.clement,
theo.lebrun, thomas.petazzoni, u-kumar1
In-Reply-To: <ZhAqWWdZvGcFDWlM@matsya>
On Fri, Apr 05, 2024 at 10:14:09PM +0530, Vinod Koul wrote:
> On 04-03-24, 16:35, Thomas Richard wrote:
...
> > +static int wiz_resume_noirq(struct device *dev)
>
> I think this should be annotated with __maybe_unused
No...
> > + .pm = pm_sleep_ptr(&wiz_pm_ops),
...because of magic of PTR_IF() here.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Philipp Stanner @ 2024-04-05 17:13 UTC (permalink / raw)
To: Linus Torvalds, Kent Overstreet
Cc: comex, Dr. David Alan Gilbert, Boqun Feng, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman,
Marco Elver, Mark Rutland, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Catalin Marinas, linux-arm-kernel, linux-fsdevel
In-Reply-To: <CAHk-=wgQy+FRKjO_BvZgZN56w6-+jDO8p-Mt=X=zM70CG=CVBQ@mail.gmail.com>
On Wed, 2024-03-27 at 12:07 -0700, Linus Torvalds wrote:
> On Wed, 27 Mar 2024 at 11:51, Kent Overstreet
> <kent.overstreet@linux.dev> wrote:
> >
> > On Wed, Mar 27, 2024 at 09:16:09AM -0700, comex wrote:
> > > Meanwhile, Rust intentionally lacks strict aliasing.
> >
> > I wasn't aware of this. Given that unrestricted pointers are a real
> > impediment to compiler optimization, I thought that with Rust we
> > were
> > finally starting to nail down a concrete enough memory model to
> > tackle
> > this safely. But I guess not?
>
> Strict aliasing is a *horrible* mistake.
>
> It's not even *remotely* "tackle this safely". It's the exact
> opposite. It's completely broken.
>
> Anybody who thinks strict aliasing is a good idea either
>
> (a) doesn't understand what it means
>
> (b) has been brainwashed by incompetent compiler people.
>
> it's a horrendous crock that was introduced by people who thought it
> was too complicated to write out "restrict" keywords, and that
> thought
> that "let's break old working programs and make it harder to write
> new
> programs" was a good idea.
>
> Nobody should ever do it. The fact that Rust doesn't do the C strict
> aliasing is a good thing. Really.
Btw, for the interested, that's a nice article on strict aliasing:
https://blog.regehr.org/archives/1307
Dennis Ritchie, the Man Himself, back in the 1980s pushed back quite
strongly on (different?) aliasing experiments:
https://www.yodaiken.com/2021/03/19/dennis-ritchie-on-alias-analysis-in-the-c-programming-language-1988/
No idea why they can't just leave C alone... It's not without reason
that new languages like Zig and Hare want to freeze the language
(standard) once they are released.
P.
>
> I suspect you have been fooled by the name. Because "strict aliasing"
> sounds like a good thing. It sounds like "I know these strictly can't
> alias". But despite that name, it's the complete opposite of that,
> and
> means "I will ignore actual real aliasing even if it exists, because
> I
> will make aliasing decisions on entirely made-up grounds".
>
> Just say no to strict aliasing. Thankfully, there's an actual
> compiler
> flag for that: -fno-strict-aliasing. It should absolutely have been
> the default.
>
> Linus
>
_______________________________________________
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 3/3] arm64: tlb: Allow range operation for MAX_TLBI_RANGE_PAGES
From: Catalin Marinas @ 2024-04-05 17:12 UTC (permalink / raw)
To: Gavin Shan
Cc: linux-arm-kernel, linux-kernel, will, akpm, maz, oliver.upton,
ryan.roberts, apopple, rananta, mark.rutland, v-songbaohua,
yangyicong, shahuang, yihyu, shan.gavin
In-Reply-To: <20240405035852.1532010-4-gshan@redhat.com>
On Fri, Apr 05, 2024 at 01:58:52PM +1000, Gavin Shan wrote:
> MAX_TLBI_RANGE_PAGES pages is covered by SCALE#3 and NUM#31 and it's
> supported now. Allow TLBI RANGE operation when the number of pages is
> equal to MAX_TLBI_RANGE_PAGES in __flush_tlb_range_nosync().
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
Verified this case as well, so:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
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 2/3] arm64: tlb: Improve __TLBI_VADDR_RANGE()
From: Catalin Marinas @ 2024-04-05 17:10 UTC (permalink / raw)
To: Gavin Shan
Cc: linux-arm-kernel, linux-kernel, will, akpm, maz, oliver.upton,
ryan.roberts, apopple, rananta, mark.rutland, v-songbaohua,
yangyicong, shahuang, yihyu, shan.gavin
In-Reply-To: <20240405035852.1532010-3-gshan@redhat.com>
On Fri, Apr 05, 2024 at 01:58:51PM +1000, Gavin Shan wrote:
> The macro returns the operand of TLBI RANGE instruction. A mask needs
> to be applied to each individual field upon producing the operand, to
> avoid the adjacent fields can interfere with each other when invalid
> arguments have been provided. The code looks more tidy at least with
> a mask and FIELD_PREP().
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
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 1/3] arm64: tlb: Fix TLBI RANGE operand
From: Catalin Marinas @ 2024-04-05 17:10 UTC (permalink / raw)
To: Gavin Shan
Cc: linux-arm-kernel, linux-kernel, will, akpm, maz, oliver.upton,
ryan.roberts, apopple, rananta, mark.rutland, v-songbaohua,
yangyicong, shahuang, yihyu, shan.gavin
In-Reply-To: <20240405035852.1532010-2-gshan@redhat.com>
On Fri, Apr 05, 2024 at 01:58:50PM +1000, Gavin Shan wrote:
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index 3b0e8248e1a4..a75de2665d84 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -161,12 +161,18 @@ static inline unsigned long get_trans_granule(void)
> #define MAX_TLBI_RANGE_PAGES __TLBI_RANGE_PAGES(31, 3)
>
> /*
> - * Generate 'num' values from -1 to 30 with -1 rejected by the
> - * __flush_tlb_range() loop below.
> + * Generate 'num' values from -1 to 31 with -1 rejected by the
> + * __flush_tlb_range() loop below. Its return value is only
> + * significant for a maximum of MAX_TLBI_RANGE_PAGES pages. If
> + * 'pages' is more than that, you must iterate over the overall
> + * range.
> */
> -#define TLBI_RANGE_MASK GENMASK_ULL(4, 0)
> -#define __TLBI_RANGE_NUM(pages, scale) \
> - ((((pages) >> (5 * (scale) + 1)) & TLBI_RANGE_MASK) - 1)
> +#define __TLBI_RANGE_NUM(pages, scale) \
> + ({ \
> + int __pages = min((pages), \
> + __TLBI_RANGE_PAGES(31, (scale))); \
> + (__pages >> (5 * (scale) + 1)) - 1; \
> + })
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
This looks correct to me as well. I spent a bit of time to update an old
CBMC model I had around. With the original __TLBI_RANGE_NUM indeed shows
'scale' becoming negative on the kvm_tlb_flush_vmid_range() path. The
patch above fixes it and it also allows the non-KVM path to use the
range TLBI for MAX_TLBI_RANGE_PAGES (as per patch 3).
FWIW, here's the model:
-----------------------8<--------------------------------------
// SPDX-License-Identifier: GPL-2.0-only
/*
* Check with:
* cbmc --unwind 6 tlbinval.c
*/
#define PAGE_SHIFT (12)
#define PAGE_SIZE (1 << PAGE_SHIFT)
#define VA_RANGE (1UL << 48)
#define SZ_64K 0x00010000
#define __round_mask(x, y) ((__typeof__(x))((y)-1))
#define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
#define round_down(x, y) ((x) & ~__round_mask(x, y))
#define min(x, y) (x <= y ? x : y)
#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
/* only masking out irrelevant bits */
#define __TLBI_RANGE_VADDR(addr, shift) ((addr) & ~((1UL << shift) - 1))
#define __TLBI_VADDR(addr) __TLBI_RANGE_VADDR(addr, PAGE_SHIFT)
#define __TLBI_RANGE_PAGES(num, scale) ((unsigned long)((num) + 1) << (5 * (scale) + 1))
#define MAX_TLBI_RANGE_PAGES __TLBI_RANGE_PAGES(31, 3)
#if 0
/* original code */
#define TLBI_RANGE_MASK 0x1fUL
#define __TLBI_RANGE_NUM(pages, scale) \
((((int)(pages) >> (5 * (scale) + 1)) & TLBI_RANGE_MASK) - 1)
#else
#define __TLBI_RANGE_NUM(pages, scale) \
({ \
int __pages = min((pages), \
__TLBI_RANGE_PAGES(31, (scale))); \
(__pages >> (5 * (scale) + 1)) - 1; \
})
#endif
const static _Bool lpa2 = 1;
const static _Bool kvm = 1;
static unsigned long inval_start;
static unsigned long inval_end;
static void tlbi(unsigned long start, unsigned long size)
{
unsigned long end = start + size;
if (inval_end == 0) {
inval_start = start;
inval_end = end;
return;
}
/* optimal invalidation */
__CPROVER_assert(start >= inval_end || end <= inval_start, "No overlapping TLBI range");
if (start < inval_start) {
__CPROVER_assert(end >= inval_start, "No TLBI range gaps");
inval_start = start;
}
if (end > inval_end) {
__CPROVER_assert(start <= inval_end, "No TLBI range gaps");
inval_end = end;
}
}
static void tlbi_range(unsigned long start, int num, int scale)
{
unsigned long size = __TLBI_RANGE_PAGES(num, scale) << PAGE_SHIFT;
tlbi(start, size);
}
static void __flush_tlb_range_op(unsigned long start, unsigned long pages,
unsigned long stride)
{
int num = 0;
int scale = 3;
int shift = lpa2 ? 16 : PAGE_SHIFT;
unsigned long addr;
while (pages > 0) {
if (pages == 1 ||
(lpa2 && start != ALIGN(start, SZ_64K))) {
addr = __TLBI_VADDR(start);
tlbi(addr, stride);
start += stride;
pages -= stride >> PAGE_SHIFT;
continue;
}
__CPROVER_assert(scale >= 0 && scale <= 3, "Scale in range");
num = __TLBI_RANGE_NUM(pages, scale);
__CPROVER_assert(num <= 31, "Num in range");
if (num >= 0) {
addr = __TLBI_RANGE_VADDR(start, shift);
tlbi_range(addr, num, scale);
start += __TLBI_RANGE_PAGES(num, scale) << PAGE_SHIFT;
pages -= __TLBI_RANGE_PAGES(num, scale);
}
scale--;
}
}
static void __flush_tlb_range(unsigned long start, unsigned long pages,
unsigned long stride)
{
if (pages > MAX_TLBI_RANGE_PAGES) {
tlbi(0, VA_RANGE);
return;
}
__flush_tlb_range_op(start, pages, stride);
}
void __kvm_tlb_flush_vmid_range(unsigned long start, unsigned long pages)
{
unsigned long stride;
stride = PAGE_SIZE;
start = round_down(start, stride);
__flush_tlb_range_op(start, pages, stride);
}
static void kvm_tlb_flush_vmid_range(unsigned long addr, unsigned long size)
{
unsigned long pages, inval_pages;
pages = size >> PAGE_SHIFT;
while (pages > 0) {
inval_pages = min(pages, MAX_TLBI_RANGE_PAGES);
__kvm_tlb_flush_vmid_range(addr, inval_pages);
addr += inval_pages << PAGE_SHIFT;
pages -= inval_pages;
}
}
static unsigned long nondet_ulong(void);
int main(void)
{
unsigned long stride = nondet_ulong();
unsigned long start = round_down(nondet_ulong(), stride);
unsigned long end = round_up(nondet_ulong(), stride);
unsigned long pages = (end - start) >> PAGE_SHIFT;
__CPROVER_assume(stride == PAGE_SIZE ||
stride == PAGE_SIZE << (PAGE_SHIFT - 3) ||
stride == PAGE_SIZE << (2 * (PAGE_SHIFT - 3)));
__CPROVER_assume(start < end);
__CPROVER_assume(end <= VA_RANGE);
if (kvm)
kvm_tlb_flush_vmid_range(start, pages << PAGE_SHIFT);
else
__flush_tlb_range(start, pages, stride);
__CPROVER_assert((inval_start == 0 && inval_end == VA_RANGE) ||
(inval_start == start && inval_end == end),
"Correct invalidation");
return 0;
}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox