Re: usb.c:undefined reference to `qe_immr'
Randy Dunlap writes: > [adding Cc's] > > > On 1/9/23 23:59, kernel test robot wrote: >> Hi Masahiro, >> >> FYI, the error/warning still remains. >> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >> master >> head: 5a41237ad1d4b62008f93163af1d9b1da90729d8 >> commit: 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b kbuild: link symbol CRCs at >> final link, removing CONFIG_MODULE_REL_CRCS >> date: 8 months ago >> config: powerpc-randconfig-r026-20230110 >> compiler: powerpc-linux-gcc (GCC) 12.1.0 >> reproduce (this is a W=1 build): >> wget >> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O >> ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b4537199a4a8480b8c3ba37a2d44765ce76cd9b >> git remote add linus >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >> git fetch --no-tags linus master >> git checkout 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b >> # save the config file >> mkdir build_dir && cp config build_dir/.config >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 >> O=build_dir ARCH=powerpc olddefconfig >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 >> O=build_dir ARCH=powerpc SHELL=/bin/bash >> >> If you fix the issue, kindly add following tag where applicable >> | Reported-by: kernel test robot >> >> All errors (new ones prefixed by >>): >> >>powerpc-linux-ld: powerpc-linux-ld: DWARF error: could not find abbrev >> number 74 >>drivers/soc/fsl/qe/usb.o: in function `qe_usb_clock_set': >>>> usb.c:(.text+0x1e): undefined reference to `qe_immr' >>>> powerpc-linux-ld: usb.c:(.text+0x2a): undefined reference to `qe_immr' >>>> powerpc-linux-ld: usb.c:(.text+0xbc): undefined reference to `qe_setbrg' >>>> powerpc-linux-ld: usb.c:(.text+0xca): undefined reference to `cmxgcr_lock' >>powerpc-linux-ld: usb.c:(.text+0xce): undefined reference to `cmxgcr_lock' >> > > .config extract: > > # > # NXP/Freescale QorIQ SoC drivers > # > # CONFIG_QUICC_ENGINE is not set > CONFIG_QE_USB=y > > > This is caused by (drivers/soc/fsl/qe/Kconfig): > > config QE_USB > bool > default y if USB_FSL_QE > help > QE USB Controller support > > which does not depend on QUICC_ENGINE, where the latter build provides > the missing symbols. So QE_USB should depend on QUICC_ENGINE no? cheers
Re: [PATCH v2 11/13] tty/serial: Call ->dtr_rts() parameter active consistently
On 10. 01. 23, 13:02, Ilpo Järvinen wrote: Convert various parameter names for ->dtr_rts() and related functions from onoff, on, and raise to active. Much better. Signed-off-by: Ilpo Järvinen Reviewed-by: Jiri Slaby -- js suse labs
Re: [PATCH 1/2] powerpc/64s/radix: Fix crash with unaligned relocated kernel
> On 10-Jan-2023, at 6:17 PM, Michael Ellerman wrote:
>
> If a relocatable kernel is loaded at an address that is not 2MB aligned
> and told not to relocate to zero, the kernel can crash due to
> mark_rodata_ro() incorrectly changing some read-write data to read-only.
>
> Scenarios where the misalignment can occur are when the kernel is
> loaded by kdump or using the RELOCATABLE_TEST config option.
>
> Example crash with the kernel loaded at 5MB:
>
> Run /sbin/init as init process
> BUG: Unable to handle kernel data access on write at 0xc0452000
> Faulting instruction address: 0xc05b6730
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
> CPU: 1 PID: 1 Comm: init Not tainted 6.2.0-rc1-00011-g349188be4841 #166
> Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202
> 0xf05 of:SLOF,git-5b4c5a hv:linux,kvm pSeries
> NIP: c05b6730 LR: c0ae9ab8 CTR: 0380
> REGS: c4503250 TRAP: 0300 Not tainted
> (6.2.0-rc1-00011-g349188be4841)
> MSR: 80009033 CR: 44288480 XER:
> CFAR: c05b66ec DAR: c0452000 DSISR: 0a00 IRQMASK: 0
> ...
> NIP memset+0x68/0x104
> LR zero_user_segments.constprop.0+0xa8/0xf0
> Call Trace:
>ext4_mpage_readpages+0x7f8/0x830
>ext4_readahead+0x48/0x60
>read_pages+0xb8/0x380
>page_cache_ra_unbounded+0x19c/0x250
>filemap_fault+0x58c/0xae0
>__do_fault+0x60/0x100
>__handle_mm_fault+0x1230/0x1a40
>handle_mm_fault+0x120/0x300
>___do_page_fault+0x20c/0xa80
>do_page_fault+0x30/0xc0
>data_access_common_virt+0x210/0x220
>
> This happens because mark_rodata_ro() tries to change permissions on the
> range _stext..__end_rodata, but _stext sits in the middle of the 2MB
> page from 4MB to 6MB:
>
> radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
> (exec)
> radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
> radix-mmu: Mapped 0x0040-0x0240 with 2.00 MiB pages
> (exec)
>
> The logic that changes the permissions assumes the linear mapping was
> split correctly at boot, so it marks the entire 2MB page read-only. That
> leads to the write fault above.
>
> To fix it, the boot time mapping logic needs to consider that if the
> kernel is running at a non-zero address then _stext is a boundary where
> it must split the mapping.
>
> That leads to the mapping being split correctly, allowing the rodata
> permission change to take happen correctly, with no spillover:
>
> radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
> (exec)
> radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
> radix-mmu: Mapped 0x0040-0x0050 with 64.0 KiB pages
> radix-mmu: Mapped 0x0050-0x0060 with 64.0 KiB pages
> (exec)
> radix-mmu: Mapped 0x0060-0x0240 with 2.00 MiB pages
> (exec)
>
> If the kernel is loaded at a 2MB aligned address, the mapping continues
> to use 2MB pages as before:
>
> radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
> (exec)
> radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
> radix-mmu: Mapped 0x0040-0x02c0 with 2.00 MiB pages
> (exec)
> radix-mmu: Mapped 0x02c0-0x0001 with 2.00 MiB pages
>
> Fixes: c55d7b5e6426 ("powerpc: Remove STRICT_KERNEL_RWX incompatibility with
> RELOCATABLE")
> Signed-off-by: Michael Ellerman
> ---
Tested successfully with different crash kernel memory values
Tested-by : Sachin Sant
- Sachin
Re: [PATCH v2] net: wan: Add checks for NULL. If uhdlc_priv_tsa != 1 then utdm is not initialized. And if ret != NULL then goto undo_uhdlc_init, where utdm is dereferenced. Same if dev == NULL.
On Tue, 10 Jan 2023 14:47:45 +0300 Esina Ekaterina wrote: > Subject: [PATCH v2] net: wan: Add checks for NULL. If uhdlc_priv_tsa != 1 > then utdm is not initialized. And if ret != NULL then goto undo_uhdlc_init, > where utdm is dereferenced. Same if dev == NULL. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Esina Ekaterina > > v2: Add check for NULL for unmap_si_regs > --- The format of your commit is wrong. You should make the commit message look like this in git: net: wan: Add checks for NULL If uhdlc_priv_tsa != 1 then utdm is not initialized. And if ret != NULL then goto undo_uhdlc_init, where utdm is dereferenced. Same if dev == NULL. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Esina Ekaterina --- v2: Add check for NULL for unmap_si_regs But the first line (subject) is still not specific enough. Refer to the bug that's being fixed, not how it's fixed. Also no braces needed around single-line if blocks.
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2023-01-06 at 21:49 +1100, Michael Ellerman wrote: > > > diff --git a/arch/powerpc/platforms/pseries/Kconfig > > b/arch/powerpc/platforms/pseries/Kconfig > > index a3b4d99567cb..94e08c405d50 100644 > > --- a/arch/powerpc/platforms/pseries/Kconfig > > +++ b/arch/powerpc/platforms/pseries/Kconfig > > @@ -162,6 +162,19 @@ config PSERIES_PLPKS > > > > If unsure, select N. > > > > +config PSERIES_PLPKS_SECVAR > > + depends on PSERIES_PLPKS > > + depends on PPC_SECURE_BOOT > > + bool "Support for the PLPKS secvar interface" > > + help > > + PowerVM can support dynamic secure boot with user-defined > > keys > > + through the PLPKS. Keystore objects used in dynamic > > secure boot > > + can be exposed to the kernel and userspace through the > > powerpc > > + secvar infrastructure. Select this to enable the PLPKS > > backend > > + for secvars for use in pseries dynamic secure boot. > > + > > + If unsure, select N. > > I don't think we need that config option at all, or if we do it > should > not be user selectable and just enabled automatically by > PSERIES_PLPKS. I actually think we should get rid of both PSERIES_PLPKS_SECVAR and PSERIES_PLPKS, and just use PPC_SECURE_BOOT / PPC_SECVAR_SYSFS. -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 08/41] mm: introduce CONFIG_PER_VMA_LOCK
On Tue, Jan 10, 2023 at 4:39 PM Davidlohr Bueso wrote: > > On Mon, 09 Jan 2023, Suren Baghdasaryan wrote: > > >This configuration variable will be used to build the support for VMA > >locking during page fault handling. > > > >This is enabled by default on supported architectures with SMP and MMU > >set. > > > >The architecture support is needed since the page fault handler is called > >from the architecture's page faulting code which needs modifications to > >handle faults under VMA lock. > > I don't think that per-vma locking should be something that is > user-configurable. > It should just be depdendant on the arch. So maybe just remove > CONFIG_PER_VMA_LOCK? Thanks for the suggestion! I would be happy to make that change if there are no objections. I think the only pushback might have been the vma size increase but with the latest optimization in the last patch maybe that's less of an issue? > > Thanks, > Davidlohr > > -- > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-team+unsubscr...@android.com. >
Re: [PATCH 08/41] mm: introduce CONFIG_PER_VMA_LOCK
On Mon, 09 Jan 2023, Suren Baghdasaryan wrote: This configuration variable will be used to build the support for VMA locking during page fault handling. This is enabled by default on supported architectures with SMP and MMU set. The architecture support is needed since the page fault handler is called from the architecture's page faulting code which needs modifications to handle faults under VMA lock. I don't think that per-vma locking should be something that is user-configurable. It should just be depdendant on the arch. So maybe just remove CONFIG_PER_VMA_LOCK? Thanks, Davidlohr
Re: [PATCH 2/4] dt-bindings: usb: Convert multiple "usb-ohci" bindings to DT schema
On Tue, Jan 10, 2023 at 5:18 PM Rob Herring wrote:
>
> "usb-ohci" is another "generic" OHCI controller compatible string used by
> several platforms. Add it to the generic-ohci.yaml schema and remove all
> the old binding docs.
>
> Marvell pxa-usb.txt has "usb-ohci" in the example, but actual users don't,
> so drop it.
>
> Signed-off-by: Rob Herring
> ---
> .../devicetree/bindings/powerpc/nintendo/wii.txt | 10 ---
> .../devicetree/bindings/usb/generic-ohci.yaml | 31 ---
> Documentation/devicetree/bindings/usb/ohci-nxp.txt | 24 ---
> .../devicetree/bindings/usb/ohci-omap3.txt | 15 --
> Documentation/devicetree/bindings/usb/pxa-usb.txt | 2 +-
> .../devicetree/bindings/usb/spear-usb.txt | 35
> --
> 6 files changed, 28 insertions(+), 89 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
> b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
> index c4d78f28d23c..3ff6ebbb4998 100644
> --- a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
> +++ b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
> @@ -97,16 +97,6 @@ Nintendo Wii device tree
> - reg : should contain the EXI registers location and length
> - interrupts : should contain the EXI interrupt
>
> -1.g) The Open Host Controller Interface (OHCI) nodes
> -
> - Represent the USB 1.x Open Host Controller Interfaces.
> -
> - Required properties:
> -
> - - compatible : should be "nintendo,hollywood-usb-ohci","usb-ohci"
> - - reg : should contain the OHCI registers location and length
> - - interrupts : should contain the OHCI interrupt
> -
> 1.h) The Enhanced Host Controller Interface (EHCI) node
>
>Represents the USB 2.0 Enhanced Host Controller Interface.
> diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> index 4fcbd0add49d..b898303381f8 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> @@ -6,9 +6,6 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: USB OHCI Controller
>
> -allOf:
> - - $ref: "usb-hcd.yaml"
> -
> maintainers:
>- Greg Kroah-Hartman
>
> @@ -49,7 +46,16 @@ properties:
>- ingenic,jz4740-ohci
>- snps,hsdk-v1.0-ohci
>- const: generic-ohci
> - - const: generic-ohci
> + - enum:
> + - generic-ohci
> + - ti,ohci-omap3
> + - items:
> + - enum:
> + - cavium,octeon-6335-ohci
> + - nintendo,hollywood-usb-ohci
> + - nxp,ohci-nxp
> + - st,spear600-ohci
> + - const: usb-ohci
>
>reg:
> maxItems: 1
> @@ -119,11 +125,28 @@ properties:
>- host
>- otg
>
> + transceiver:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> + The associated ISP1301 device. Necessary for the UDC controller for
> + connecting to the USB physical layer.
> +
> required:
>- compatible
>- reg
>- interrupts
>
> +allOf:
> + - $ref: usb-hcd.yaml
> + - if:
> + not:
> +properties:
> + compatible:
> +contains:
> + const: nxp,ohci-nxp
> +then:
Sigh. Need a 'properties' in here...
> + transceiver: false
> +
> additionalProperties: false
>
> examples:
> diff --git a/Documentation/devicetree/bindings/usb/ohci-nxp.txt
> b/Documentation/devicetree/bindings/usb/ohci-nxp.txt
> deleted file mode 100644
> index 71e28c1017ed..
> --- a/Documentation/devicetree/bindings/usb/ohci-nxp.txt
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -* OHCI controller, NXP ohci-nxp variant
> -
> -Required properties:
> -- compatible: must be "nxp,ohci-nxp"
> -- reg: physical base address of the controller and length of memory mapped
> - region.
> -- interrupts: The OHCI interrupt
> -- transceiver: phandle of the associated ISP1301 device - this is necessary
> for
> - the UDC controller for connecting to the USB physical layer
> -
> -Example (LPC32xx):
> -
> - isp1301: usb-transceiver@2c {
> - compatible = "nxp,isp1301";
> - reg = <0x2c>;
> - };
> -
> - ohci@3102 {
> - compatible = "nxp,ohci-nxp";
> - reg = <0x3102 0x300>;
> - interrupt-parent = <>;
> - interrupts = <0x3b 0>;
> - transceiver = <>;
> - };
> diff --git a/Documentation/devicetree/bindings/usb/ohci-omap3.txt
> b/Documentation/devicetree/bindings/usb/ohci-omap3.txt
> deleted file mode 100644
> index ce8c47cff6d0..
> --- a/Documentation/devicetree/bindings/usb/ohci-omap3.txt
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -OMAP HS USB OHCI controller (OMAP3 and later)
> -
> -Required properties:
> -
> -- compatible: should be "ti,ohci-omap3"
> -- reg: should contain
[PATCH 2/4] dt-bindings: usb: Convert multiple "usb-ohci" bindings to DT schema
"usb-ohci" is another "generic" OHCI controller compatible string used by
several platforms. Add it to the generic-ohci.yaml schema and remove all
the old binding docs.
Marvell pxa-usb.txt has "usb-ohci" in the example, but actual users don't,
so drop it.
Signed-off-by: Rob Herring
---
.../devicetree/bindings/powerpc/nintendo/wii.txt | 10 ---
.../devicetree/bindings/usb/generic-ohci.yaml | 31 ---
Documentation/devicetree/bindings/usb/ohci-nxp.txt | 24 ---
.../devicetree/bindings/usb/ohci-omap3.txt | 15 --
Documentation/devicetree/bindings/usb/pxa-usb.txt | 2 +-
.../devicetree/bindings/usb/spear-usb.txt | 35 --
6 files changed, 28 insertions(+), 89 deletions(-)
diff --git a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
index c4d78f28d23c..3ff6ebbb4998 100644
--- a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
+++ b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
@@ -97,16 +97,6 @@ Nintendo Wii device tree
- reg : should contain the EXI registers location and length
- interrupts : should contain the EXI interrupt
-1.g) The Open Host Controller Interface (OHCI) nodes
-
- Represent the USB 1.x Open Host Controller Interfaces.
-
- Required properties:
-
- - compatible : should be "nintendo,hollywood-usb-ohci","usb-ohci"
- - reg : should contain the OHCI registers location and length
- - interrupts : should contain the OHCI interrupt
-
1.h) The Enhanced Host Controller Interface (EHCI) node
Represents the USB 2.0 Enhanced Host Controller Interface.
diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
index 4fcbd0add49d..b898303381f8 100644
--- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
@@ -6,9 +6,6 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: USB OHCI Controller
-allOf:
- - $ref: "usb-hcd.yaml"
-
maintainers:
- Greg Kroah-Hartman
@@ -49,7 +46,16 @@ properties:
- ingenic,jz4740-ohci
- snps,hsdk-v1.0-ohci
- const: generic-ohci
- - const: generic-ohci
+ - enum:
+ - generic-ohci
+ - ti,ohci-omap3
+ - items:
+ - enum:
+ - cavium,octeon-6335-ohci
+ - nintendo,hollywood-usb-ohci
+ - nxp,ohci-nxp
+ - st,spear600-ohci
+ - const: usb-ohci
reg:
maxItems: 1
@@ -119,11 +125,28 @@ properties:
- host
- otg
+ transceiver:
+$ref: /schemas/types.yaml#/definitions/phandle
+description:
+ The associated ISP1301 device. Necessary for the UDC controller for
+ connecting to the USB physical layer.
+
required:
- compatible
- reg
- interrupts
+allOf:
+ - $ref: usb-hcd.yaml
+ - if:
+ not:
+properties:
+ compatible:
+contains:
+ const: nxp,ohci-nxp
+then:
+ transceiver: false
+
additionalProperties: false
examples:
diff --git a/Documentation/devicetree/bindings/usb/ohci-nxp.txt
b/Documentation/devicetree/bindings/usb/ohci-nxp.txt
deleted file mode 100644
index 71e28c1017ed..
--- a/Documentation/devicetree/bindings/usb/ohci-nxp.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-* OHCI controller, NXP ohci-nxp variant
-
-Required properties:
-- compatible: must be "nxp,ohci-nxp"
-- reg: physical base address of the controller and length of memory mapped
- region.
-- interrupts: The OHCI interrupt
-- transceiver: phandle of the associated ISP1301 device - this is necessary for
- the UDC controller for connecting to the USB physical layer
-
-Example (LPC32xx):
-
- isp1301: usb-transceiver@2c {
- compatible = "nxp,isp1301";
- reg = <0x2c>;
- };
-
- ohci@3102 {
- compatible = "nxp,ohci-nxp";
- reg = <0x3102 0x300>;
- interrupt-parent = <>;
- interrupts = <0x3b 0>;
- transceiver = <>;
- };
diff --git a/Documentation/devicetree/bindings/usb/ohci-omap3.txt
b/Documentation/devicetree/bindings/usb/ohci-omap3.txt
deleted file mode 100644
index ce8c47cff6d0..
--- a/Documentation/devicetree/bindings/usb/ohci-omap3.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-OMAP HS USB OHCI controller (OMAP3 and later)
-
-Required properties:
-
-- compatible: should be "ti,ohci-omap3"
-- reg: should contain one register range i.e. start and length
-- interrupts: description of the interrupt line
-
-Example for OMAP4:
-
-usbhsohci: ohci@4a064800 {
- compatible = "ti,ohci-omap3";
- reg = <0x4a064800 0x400>;
- interrupts = <0 76 0x4>;
-};
diff --git a/Documentation/devicetree/bindings/usb/pxa-usb.txt
b/Documentation/devicetree/bindings/usb/pxa-usb.txt
[PATCH 4/4] dt-bindings: usb: Convert Nuvoton EHCI to DT schema
The Nuvoton EHCI binding is just some compatible strings, so add it to the
generic-ehci.yaml schema.
Signed-off-by: Rob Herring
---
.../devicetree/bindings/usb/generic-ehci.yaml| 2 ++
.../devicetree/bindings/usb/npcm7xx-usb.txt | 20
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
index 85e41609b09b..1687c7e9302b 100644
--- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
@@ -76,6 +76,8 @@ properties:
- generic-ehci
- marvell,armada-3700-ehci
- marvell,orion-ehci
+ - nuvoton,npcm750-ehci
+ - nuvoton,npcm845-ehci
- usb-ehci
reg:
diff --git a/Documentation/devicetree/bindings/usb/npcm7xx-usb.txt
b/Documentation/devicetree/bindings/usb/npcm7xx-usb.txt
deleted file mode 100644
index 352a0a1e2f76..
--- a/Documentation/devicetree/bindings/usb/npcm7xx-usb.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-Nuvoton NPCM7XX SoC USB controllers:
--
-
-EHCI:
--
-
-Required properties:
-- compatible: should be one of
-"nuvoton,npcm750-ehci"
-"nuvoton,npcm845-ehci"
-- interrupts: Should contain the EHCI interrupt
-- reg:Physical address and length of the register set for the device
-
-Example:
-
- ehci1: usb@f0806000 {
- compatible = "nuvoton,npcm750-ehci";
- reg = <0xf0806000 0x1000>;
- interrupts = <0 61 4>;
- };
--
2.39.0
[PATCH 3/4] dt-bindings: usb: Convert Marvell Orion EHCI to DT schema
The Marvell Orion EHCI binding is just some compatible strings, so add it
to the generic-ehci.yaml schema.
Signed-off-by: Rob Herring
---
.../devicetree/bindings/usb/ehci-orion.txt | 22 --
.../devicetree/bindings/usb/generic-ehci.yaml | 2 ++
2 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt
b/Documentation/devicetree/bindings/usb/ehci-orion.txt
deleted file mode 100644
index 2855bae79fda..
--- a/Documentation/devicetree/bindings/usb/ehci-orion.txt
+++ /dev/null
@@ -1,22 +0,0 @@
-* EHCI controller, Orion Marvell variants
-
-Required properties:
-- compatible: must be one of the following
- "marvell,orion-ehci"
- "marvell,armada-3700-ehci"
-- reg: physical base address of the controller and length of memory mapped
- region.
-- interrupts: The EHCI interrupt
-
-Optional properties:
-- clocks: reference to the clock
-- phys: reference to the USB PHY
-- phy-names: name of the USB PHY, should be "usb"
-
-Example:
-
- ehci@5 {
- compatible = "marvell,orion-ehci";
- reg = <0x5 0x1000>;
- interrupts = <19>;
- };
diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
index 994818cb6044..85e41609b09b 100644
--- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
@@ -74,6 +74,8 @@ properties:
- const: usb-ehci
- enum:
- generic-ehci
+ - marvell,armada-3700-ehci
+ - marvell,orion-ehci
- usb-ehci
reg:
--
2.39.0
[PATCH 1/4] dt-bindings: usb: Remove obsolete brcm,bcm3384-usb.txt
The "brcm,bcm3384-ohci" and "brcm,bcm3384-ehci" compatibles are already documented in generic-ohci.yaml and generic-ehci.yaml, respectively, so remove the old txt binding. Signed-off-by: Rob Herring --- Documentation/devicetree/bindings/usb/brcm,bcm3384-usb.txt | 11 --- 1 file changed, 11 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/brcm,bcm3384-usb.txt b/Documentation/devicetree/bindings/usb/brcm,bcm3384-usb.txt deleted file mode 100644 index 452c45c7bf29.. --- a/Documentation/devicetree/bindings/usb/brcm,bcm3384-usb.txt +++ /dev/null @@ -1,11 +0,0 @@ -* Broadcom USB controllers - -Required properties: -- compatible: "brcm,bcm3384-ohci", "brcm,bcm3384-ehci" - - These currently use the generic-ohci and generic-ehci drivers. On some - systems, special handling may be needed in the following cases: - - - Restoring state after systemwide power save modes - - Sharing PHYs with the USBD (UDC) hardware - - Figuring out which controllers are disabled on ASIC bondout variants -- 2.39.0
[PATCH 0/4] dt-bindings: usb: Convert some more simple OHCI/EHCI bindings
The 'ohci-usb' compatible is another 'generic' compatible for OHCI, but isn't documented with a schema. Let's add it to generic-ohci.yaml schema. While looking at this, I found a few other USB host bindings which are simple enough to use the 'generic' schemas. Signed-off-by: Rob Herring --- Rob Herring (4): dt-bindings: usb: Remove obsolete brcm,bcm3384-usb.txt dt-bindings: usb: Convert multiple "usb-ohci" bindings to DT schema dt-bindings: usb: Convert Marvell Orion EHCI to DT schema dt-bindings: usb: Convert Nuvoton EHCI to DT schema .../devicetree/bindings/powerpc/nintendo/wii.txt | 10 --- .../devicetree/bindings/usb/brcm,bcm3384-usb.txt | 11 --- .../devicetree/bindings/usb/ehci-orion.txt | 22 -- .../devicetree/bindings/usb/generic-ehci.yaml | 4 +++ .../devicetree/bindings/usb/generic-ohci.yaml | 31 --- .../devicetree/bindings/usb/npcm7xx-usb.txt| 20 - Documentation/devicetree/bindings/usb/ohci-nxp.txt | 24 --- .../devicetree/bindings/usb/ohci-omap3.txt | 15 -- Documentation/devicetree/bindings/usb/pxa-usb.txt | 2 +- .../devicetree/bindings/usb/spear-usb.txt | 35 -- 10 files changed, 32 insertions(+), 142 deletions(-) --- base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2 change-id: 20230110-dt-usb-1c637752a83b Best regards, -- Rob Herring
Re: [powerpc] Boot failure kernel BUG at mm/usercopy.c:102
>> [ 0.445046] Code: 392990f8 4814 3d02ffe9 390827f0 7d074378 7d094378 >> 7c661b78 3c62ffe7 f9610060 386319f0 4bca6935 6000 <0fe0> >> 7c0802a6 >> [ 0.445061] ---[ end trace ]— >> Git bisect points to following patch: >> commit 317c8194e6aeb8b3b573ad139fc2a0635856498e >> rseq: Introduce feature size and alignment ELF auxiliary vector entries >> Reverting the patch helps boot the kernel. > > Can you try with the following fix ? > > https://lore.kernel.org/r/20230104192054.34046-1-mathieu.desnoy...@efficios.com > "rseq: Fix: Increase AT_VECTOR_SIZE_BASE to match rseq auxvec entries" > Thanks. Yes, this patch fixes the reported problem. - Sachin
Re: [PATCH 41/41] mm: replace rw_semaphore with atomic_t in vma_lock
On Tue, Jan 10, 2023 at 12:04 AM Vlastimil Babka wrote: > > On 1/9/23 21:53, Suren Baghdasaryan wrote: > > rw_semaphore is a sizable structure of 40 bytes and consumes > > considerable space for each vm_area_struct. However vma_lock has > > two important specifics which can be used to replace rw_semaphore > > with a simpler structure: > > 1. Readers never wait. They try to take the vma_lock and fall back to > > mmap_lock if that fails. > > 2. Only one writer at a time will ever try to write-lock a vma_lock > > because writers first take mmap_lock in write mode. > > Because of these requirements, full rw_semaphore functionality is not > > needed and we can replace rw_semaphore with an atomic variable. > > When a reader takes read lock, it increments the atomic unless the > > value is negative. If that fails read-locking is aborted and mmap_lock > > is used instead. > > When writer takes write lock, it resets atomic value to -1 if the > > current value is 0 (no readers). Since all writers take mmap_lock in > > write mode first, there can be only one writer at a time. If there > > are readers, writer will place itself into a wait queue using new > > mm_struct.vma_writer_wait waitqueue head. The last reader to release > > the vma_lock will signal the writer to wake up. > > vm_lock_seq is also moved into vma_lock and along with atomic_t they > > are nicely packed and consume 8 bytes, bringing the overhead from > > vma_lock from 44 to 16 bytes: > > > > slabinfo before the changes: > > ...: ... > > vm_area_struct...152 532 : ... > > > > slabinfo with vma_lock: > > ...: ... > > rw_semaphore ... 8 5121 : ... > > I guess the cache is called vma_lock, not rw_semaphore? Yes, sorry. Copy/paste error when combining the results. The numbers though look correct, so I did not screw up that part :) > > > vm_area_struct...160 512 : ... > > > > Assuming 4 vm_area_structs, memory consumption would be: > > baseline: 6040kB > > vma_lock (vm_area_structs+vma_lock): 6280kB+316kB=6596kB > > Total increase: 556kB > > > > atomic_t might overflow if there are many competing readers, therefore > > vma_read_trylock() implements an overflow check and if that occurs it > > restors the previous value and exits with a failure to lock. > > > > Signed-off-by: Suren Baghdasaryan > > This patch is indeed an interesting addition indeed, but I can't help but > think it obsoletes the previous one :) We allocate an extra 8 bytes slab > object for the lock, and the pointer to it is also 8 bytes, and requires an > indirection. The vma_lock cache is not cacheline aligned (otherwise it would > be a major waste), so we have potential false sharing with up to 7 other > vma_lock's. True, I thought long and hard about combining the last two patches but decided to keep them separate to document the intent. The previous patch splits the lock for performance reasons and this one is focused on memory consumption. I'm open to changing this if it's confusing. > I'd expect if the vma_lock was placed with the relatively cold fields of > vm_area_struct, it shouldn't cause much cache ping pong when working with > that vma. Even if we don't cache align the vma to save memory (would be 192 > bytes instead of 160 when aligned) and place the vma_lock and the cold > fields at the end of the vma, it may be false sharing the cacheline with the > next vma in the slab. I would love to combine the vma_lock with vm_area_struct and I spent several days trying different combinations to achieve decent performance. My best achieved result was when I placed the vm_lock into the third cache line at offset 192 and allocated vm_area_structs from cache-aligned slab (horrible memory waste with each vma consuming 256 bytes). Even then I see regression in pft-threads test on a NUMA machine (where cache-bouncing problem is most pronounced): This is the result with split vma locks (current version). The higher number the better: BASEPVL Hmean faults/sec-1469201.7282 ( 0.00%) 464453.3976 * -1.01%* Hmean faults/sec-4 1754465.6221 ( 0.00%) 1660688.0452 * -5.35%* Hmean faults/sec-7 2808141.6711 ( 0.00%) 2688910.6458 * -4.25%* Hmean faults/sec-12 3750307.7553 ( 0.00%) 3863490.2057 * 3.02%* Hmean faults/sec-21 4145672.4677 ( 0.00%) 3904532.7241 * -5.82%* Hmean faults/sec-30 3775722.5726 ( 0.00%) 3923225.3734 * 3.91%* Hmean faults/sec-48 4152563.5864 ( 0.00%) 4783720.6811 * 15.20%* Hmean faults/sec-56 4163868.7111 ( 0.00%) 4851473.7241 * 16.51%* Here are results with the vma locks integrated into cache-aligned vm_area_struct: BASE PVM_MERGED Hmean faults/sec-1469201.7282 ( 0.00%) 465268.1274 * -0.84%* Hmean faults/sec-4 1754465.6221 ( 0.00%) 1658538.0217 * -5.47%* Hmean faults/sec-7 2808141.6711 ( 0.00%) 2645016.1598 *
Re: [PATCH net-next 2/7] PCI: Remove PCI IDs used by the Sun Cassini driver
On 1/10/2023 7:26 AM, Bjorn Helgaas wrote: On Fri, Jan 06, 2023 at 02:00:15PM -0800, Anirudh Venkataramanan wrote: The previous patch removed the Cassini driver (drivers/net/ethernet/sun). With this, PCI_DEVICE_ID_NS_SATURN and PCI_DEVICE_ID_SUN_CASSINI are unused. Remove them. Cc: Leon Romanovsky Signed-off-by: Anirudh Venkataramanan --- include/linux/pci_ids.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index b362d90..eca2340 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -433,7 +433,6 @@ #define PCI_DEVICE_ID_NS_CS5535_AUDIO 0x002e #define PCI_DEVICE_ID_NS_CS5535_USB 0x002f #define PCI_DEVICE_ID_NS_GX_VIDEO 0x0030 -#define PCI_DEVICE_ID_NS_SATURN0x0035 #define PCI_DEVICE_ID_NS_SCx200_BRIDGE0x0500 #define PCI_DEVICE_ID_NS_SCx200_SMI 0x0501 #define PCI_DEVICE_ID_NS_SCx200_IDE 0x0502 @@ -1047,7 +1046,6 @@ #define PCI_DEVICE_ID_SUN_SABRE 0xa000 #define PCI_DEVICE_ID_SUN_HUMMINGBIRD 0xa001 #define PCI_DEVICE_ID_SUN_TOMATILLO 0xa801 -#define PCI_DEVICE_ID_SUN_CASSINI 0xabba I don't think there's value in removing these definitions. I would just leave them alone. This whole series was NACK'd so this patch isn't getting applied. Ani
Re: [powerpc] Boot failure kernel BUG at mm/usercopy.c:102
On 2023-01-10 05:42, Sachin Sant wrote: 6.2.0-rc3-next-20230109 fails to boot on powerpc with following: [ 0.444834] [ cut here ] [ 0.444838] kernel BUG at mm/usercopy.c:102! [ 0.444842] Oops: Exception in kernel mode, sig: 5 [#1] [ 0.444845] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries [ 0.444849] Modules linked in: [ 0.444853] CPU: 23 PID: 201 Comm: modprobe Not tainted 6.2.0-rc3-next-20230109 #1 [ 0.444858] Hardware name: IBM,9080-HEX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1030.00 (NH1030_026) hv:phyp pSeries [ 0.444862] NIP: c055a934 LR: c055a930 CTR: 00725d90 [ 0.444866] REGS: c7f937c0 TRAP: 0700 Not tainted (6.2.0-rc3-next-20230109) [ 0.444871] MSR: 80029033 CR: 28088822 XER: 0007 [ 0.444879] CFAR: c02012a8 IRQMASK: 0 [ 0.444879] GPR00: c055a930 c7f93a60 c13b0800 0066 [ 0.444879] GPR04: 7fff c7f93880 c7f93878 [ 0.444879] GPR08: 7fff c25e7150 c29672b8 [ 0.444879] GPR12: 48088824 c00e87bf6300 c017f458 c34b8100 [ 0.444879] GPR16: 00012f18eac0 7fffc5c095d0 7fffc5c095d8 00012f140040 [ 0.444879] GPR20: fcff 001f 5455 0008 [ 0.444879] GPR24: c723a0c0 7fffc5c09368 7fffc5c09370 [ 0.444879] GPR28: 0250 0001 c3017000 c723a0c0 [ 0.444922] NIP [c055a934] usercopy_abort+0xa4/0xb0 [ 0.444928] LR [c055a930] usercopy_abort+0xa0/0xb0 [ 0.444932] Call Trace: [ 0.444933] [c7f93a60] [c055a930] usercopy_abort+0xa0/0xb0 (unreliable) [ 0.444939] [c7f93ad0] [c050eeb8] __check_heap_object+0x198/0x1d0 [ 0.444945] [c7f93b10] [c055a7e0] __check_object_size+0x290/0x340 [ 0.444949] [c7f93b50] [c060eba4] create_elf_tables.isra.20+0xc04/0xc90 [ 0.444956] [c7f93c10] [c0610b2c] load_elf_binary+0xdac/0x1320 [ 0.444962] [c7f93d00] [c0571cf0] bprm_execve+0x3d0/0x7c0 [ 0.444966] [c7f93dc0] [c0572b9c] kernel_execve+0x1ac/0x270 [ 0.444971] [c7f93e10] [c017f5cc] call_usermodehelper_exec_async+0x17c/0x250 [ 0.444978] [c7f93e50] [c000e054] ret_from_kernel_thread+0x5c/0x64 [ 0.444983] --- interrupt: 0 at 0x0 [ 0.444986] NIP: LR: CTR: [ 0.444990] REGS: c7f93e80 TRAP: Not tainted (6.2.0-rc3-next-20230109) [ 0.444994] MSR: <> CR: XER: [ 0.444998] CFAR: IRQMASK: 0 [ 0.444998] GPR00: c7f94000 [ 0.444998] GPR04: [ 0.444998] GPR08: [ 0.444998] GPR12: c017f458 c34b8100 [ 0.444998] GPR16: [ 0.444998] GPR20: [ 0.444998] GPR24: [ 0.444998] GPR28: [ 0.445039] NIP [] 0x0 [ 0.445042] LR [] 0x0 [ 0.445044] --- interrupt: 0 [ 0.445046] Code: 392990f8 4814 3d02ffe9 390827f0 7d074378 7d094378 7c661b78 3c62ffe7 f9610060 386319f0 4bca6935 6000 <0fe0> 7c0802a6 [ 0.445061] ---[ end trace ]— Git bisect points to following patch: commit 317c8194e6aeb8b3b573ad139fc2a0635856498e rseq: Introduce feature size and alignment ELF auxiliary vector entries Reverting the patch helps boot the kernel. Can you try with the following fix ? https://lore.kernel.org/r/20230104192054.34046-1-mathieu.desnoy...@efficios.com "rseq: Fix: Increase AT_VECTOR_SIZE_BASE to match rseq auxvec entries" Peter, I suspect it would be good to merge that fix into tip/master though sched/core. Thanks, Mathieu Thanks -Sachin -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
Re: [PATCH net-next 2/7] PCI: Remove PCI IDs used by the Sun Cassini driver
On Fri, Jan 06, 2023 at 02:00:15PM -0800, Anirudh Venkataramanan wrote: > The previous patch removed the Cassini driver (drivers/net/ethernet/sun). > With this, PCI_DEVICE_ID_NS_SATURN and PCI_DEVICE_ID_SUN_CASSINI are > unused. Remove them. > > Cc: Leon Romanovsky > Signed-off-by: Anirudh Venkataramanan > --- > include/linux/pci_ids.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index b362d90..eca2340 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -433,7 +433,6 @@ > #define PCI_DEVICE_ID_NS_CS5535_AUDIO0x002e > #define PCI_DEVICE_ID_NS_CS5535_USB 0x002f > #define PCI_DEVICE_ID_NS_GX_VIDEO0x0030 > -#define PCI_DEVICE_ID_NS_SATURN 0x0035 > #define PCI_DEVICE_ID_NS_SCx200_BRIDGE 0x0500 > #define PCI_DEVICE_ID_NS_SCx200_SMI 0x0501 > #define PCI_DEVICE_ID_NS_SCx200_IDE 0x0502 > @@ -1047,7 +1046,6 @@ > #define PCI_DEVICE_ID_SUN_SABRE 0xa000 > #define PCI_DEVICE_ID_SUN_HUMMINGBIRD0xa001 > #define PCI_DEVICE_ID_SUN_TOMATILLO 0xa801 > -#define PCI_DEVICE_ID_SUN_CASSINI0xabba I don't think there's value in removing these definitions. I would just leave them alone.
Re: [PATCH 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS
On Tue, Jan 10, 2023 at 05:45:23AM -0600, Segher Boessenkool wrote: > On Mon, Jan 09, 2023 at 05:51:23PM -0700, Nathan Chancellor wrote: > > So for this patch, I have > > > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > > warns: > > > > clang-16: error: argument unused during compilation: '-s' > > [-Werror,-Wunused-command-line-argument] > > > > The compiler's '-s' flag is a linking option (it is passed along to the > > linker directly), which means it does nothing when the linker is not > > invoked by the compiler. The kernel builds all .o files with either '-c' > > or '-S', which do not run the linker, so '-s' can be safely dropped from > > ASFLAGS. > > > > as a new commit message. Is that sufficient for everyone? If so, I'll > > adjust the s390 commit to match, as it is the same exact problem. > > Almost? -S doesn't write .o files, it writes a .s file. To go from an > assembler file (.s, or .S if you want to run the C preprocessor on non-C > code for some strange reason, the assembler macro facilities are vastly > superior) to an object file is just -c as well. Heh, right, that is what I get for not paying attention and rushing at the end of my day :) thanks for being pendantic, I will get that ironed out for v2, which I should have out later today or tomorrow, time permitting. Cheers, Nathan
Re: [Intel-gfx] [RFC DO NOT MERGE] treewide: use __xchg in most obvious places
On Tue, Jan 10, 2023 at 01:46:37PM +0100, Andrzej Hajda wrote: > On 10.01.2023 12:07, Andy Shevchenko wrote: > > On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote: ... > > > + return __xchg(_chain->p_prod_elem, > > > + (void *)(((u8 *)p_chain->p_prod_elem) + > > > p_chain->elem_size)); > > > > Wondering if you still need a (void *) casting after the change. Ditto for > > the > > rest of similar cases. > > IMHO it is not needed also before the change and IIRC gcc has an extension > which allows to drop (u8 *) cast as well [1]. I guess you can drop at least the former one. > [1]: https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html ... > > Btw, is it done by coccinelle? If no, why not providing the script? > > Yes I have used cocci. My cocci skills are far from perfect, so I did not > want to share my dirty code, but this is nothing secret: Thank you! It's not about secrecy, it's about automation / error proofness. -- With Best Regards, Andy Shevchenko
[PATCH 1/2] powerpc/64s/radix: Fix crash with unaligned relocated kernel
If a relocatable kernel is loaded at an address that is not 2MB aligned
and told not to relocate to zero, the kernel can crash due to
mark_rodata_ro() incorrectly changing some read-write data to read-only.
Scenarios where the misalignment can occur are when the kernel is
loaded by kdump or using the RELOCATABLE_TEST config option.
Example crash with the kernel loaded at 5MB:
Run /sbin/init as init process
BUG: Unable to handle kernel data access on write at 0xc0452000
Faulting instruction address: 0xc05b6730
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
CPU: 1 PID: 1 Comm: init Not tainted 6.2.0-rc1-00011-g349188be4841 #166
Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 0xf05
of:SLOF,git-5b4c5a hv:linux,kvm pSeries
NIP: c05b6730 LR: c0ae9ab8 CTR: 0380
REGS: c4503250 TRAP: 0300 Not tainted
(6.2.0-rc1-00011-g349188be4841)
MSR: 80009033 CR: 44288480 XER:
CFAR: c05b66ec DAR: c0452000 DSISR: 0a00 IRQMASK: 0
...
NIP memset+0x68/0x104
LR zero_user_segments.constprop.0+0xa8/0xf0
Call Trace:
ext4_mpage_readpages+0x7f8/0x830
ext4_readahead+0x48/0x60
read_pages+0xb8/0x380
page_cache_ra_unbounded+0x19c/0x250
filemap_fault+0x58c/0xae0
__do_fault+0x60/0x100
__handle_mm_fault+0x1230/0x1a40
handle_mm_fault+0x120/0x300
___do_page_fault+0x20c/0xa80
do_page_fault+0x30/0xc0
data_access_common_virt+0x210/0x220
This happens because mark_rodata_ro() tries to change permissions on the
range _stext..__end_rodata, but _stext sits in the middle of the 2MB
page from 4MB to 6MB:
radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
(exec)
radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
radix-mmu: Mapped 0x0040-0x0240 with 2.00 MiB pages
(exec)
The logic that changes the permissions assumes the linear mapping was
split correctly at boot, so it marks the entire 2MB page read-only. That
leads to the write fault above.
To fix it, the boot time mapping logic needs to consider that if the
kernel is running at a non-zero address then _stext is a boundary where
it must split the mapping.
That leads to the mapping being split correctly, allowing the rodata
permission change to take happen correctly, with no spillover:
radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
(exec)
radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
radix-mmu: Mapped 0x0040-0x0050 with 64.0 KiB pages
radix-mmu: Mapped 0x0050-0x0060 with 64.0 KiB pages
(exec)
radix-mmu: Mapped 0x0060-0x0240 with 2.00 MiB pages
(exec)
If the kernel is loaded at a 2MB aligned address, the mapping continues
to use 2MB pages as before:
radix-mmu: Mapped 0x-0x0020 with 2.00 MiB pages
(exec)
radix-mmu: Mapped 0x0020-0x0040 with 2.00 MiB pages
radix-mmu: Mapped 0x0040-0x02c0 with 2.00 MiB pages
(exec)
radix-mmu: Mapped 0x02c0-0x0001 with 2.00 MiB pages
Fixes: c55d7b5e6426 ("powerpc: Remove STRICT_KERNEL_RWX incompatibility with
RELOCATABLE")
Signed-off-by: Michael Ellerman
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index cac727b01799..5a2384ed1727 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -262,6 +262,17 @@ print_mapping(unsigned long start, unsigned long end,
unsigned long size, bool e
static unsigned long next_boundary(unsigned long addr, unsigned long end)
{
#ifdef CONFIG_STRICT_KERNEL_RWX
+ unsigned long stext_phys;
+
+ stext_phys = __pa_symbol(_stext);
+
+ // Relocatable kernel running at non-zero real address
+ if (stext_phys != 0) {
+ // Start of relocated kernel text is a rodata boundary
+ if (addr < stext_phys)
+ return stext_phys;
+ }
+
if (addr < __pa_symbol(__srwx_boundary))
return __pa_symbol(__srwx_boundary);
#endif
--
2.39.0
[PATCH 2/2] powerpc/64s/radix: Fix RWX mapping with relocated kernel
If a relocatable kernel is loaded at a non-zero address and told not to
relocate to zero (kdump or RELOCATABLE_TEST), the mapping of the
interrupt code at zero is left with RWX permissions.
That is a security weakness, and leads to a warning at boot if
CONFIG_DEBUG_WX is enabled:
powerpc/mm: Found insecure W+X mapping at address
056435bc/0xc000
WARNING: CPU: 1 PID: 1 at arch/powerpc/mm/ptdump/ptdump.c:193
note_page+0x484/0x4c0
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc1-1-g8ae8e98aea82-dirty
#175
Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 0xf05
of:SLOF,git-dd0dca hv:linux,kvm pSeries
NIP: c04a1c34 LR: c04a1c30 CTR:
REGS: c3503770 TRAP: 0700 Not tainted
(6.2.0-rc1-1-g8ae8e98aea82-dirty)
MSR: 82029033 CR: 24000220 XER:
CFAR: c0545a58 IRQMASK: 0
...
NIP note_page+0x484/0x4c0
LR note_page+0x480/0x4c0
Call Trace:
note_page+0x480/0x4c0 (unreliable)
ptdump_pmd_entry+0xc8/0x100
walk_pgd_range+0x618/0xab0
walk_page_range_novma+0x74/0xc0
ptdump_walk_pgd+0x98/0x170
ptdump_check_wx+0x94/0x100
mark_rodata_ro+0x30/0x70
kernel_init+0x78/0x1a0
ret_from_kernel_thread+0x5c/0x64
The fix has two parts. Firstly the pages from zero up to the end of
interrupts need to be marked read-only, so that they are left with R-X
permissions. Secondly the mapping logic needs to be taught to ensure
there is a page boundary at the end of the interrupt region, so that the
permission change only applies to the interrupt text, and not the region
following it.
Fixes: c55d7b5e6426 ("powerpc: Remove STRICT_KERNEL_RWX incompatibility with
RELOCATABLE")
Signed-off-by: Michael Ellerman
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 5a2384ed1727..26245aaf12b8 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -234,6 +234,14 @@ void radix__mark_rodata_ro(void)
end = (unsigned long)__end_rodata;
radix__change_memory_range(start, end, _PAGE_WRITE);
+
+ for (start = PAGE_OFFSET; start < (unsigned long)_stext; start +=
PAGE_SIZE) {
+ end = start + PAGE_SIZE;
+ if (overlaps_interrupt_vector_text(start, end))
+ radix__change_memory_range(start, end, _PAGE_WRITE);
+ else
+ break;
+ }
}
void radix__mark_initmem_nx(void)
@@ -268,6 +276,11 @@ static unsigned long next_boundary(unsigned long addr,
unsigned long end)
// Relocatable kernel running at non-zero real address
if (stext_phys != 0) {
+ // The end of interrupts code at zero is a rodata boundary
+ unsigned long end_intr = __pa_symbol(__end_interrupts) -
stext_phys;
+ if (addr < end_intr)
+ return end_intr;
+
// Start of relocated kernel text is a rodata boundary
if (addr < stext_phys)
return stext_phys;
--
2.39.0
Re: [Intel-gfx] [RFC DO NOT MERGE] treewide: use __xchg in most obvious places
On 10.01.2023 12:07, Andy Shevchenko wrote:
On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:
This patch tries to show usability of __xchg helper.
It is not intended to be merged, but I can convert
it to proper patchset if necessary.
There are many more places where __xchg can be used.
This demo shows the most spectacular cases IMHO:
- previous value is returned from function,
- temporary variables are in use.
As a result readability is much better and diffstat is quite
nice, less local vars to look at.
In many cases whole body of functions is replaced
with __xchg(ptr, val), so as further refactoring the whole
function can be removed and __xchg can be called directly.
...
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long orig_ret_vaddr;
-
- orig_ret_vaddr = regs->ARM_lr;
- /* Replace the return addr with trampoline addr */
- regs->ARM_lr = trampoline_vaddr;
- return orig_ret_vaddr;
+ return __xchg(>ARM_lr, trampoline_vaddr);
}
If it's not a callback, the entire function can be killed.
And this is a good example of the function usage.
OTOH, these places might have a side effect (if it's in deep CPU
handlers), means we need to do this carefully.
...
static inline void *qed_chain_produce(struct qed_chain *p_chain)
{
- void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
+ void *p_prod_idx, *p_prod_page_idx;
if (is_chain_u16(p_chain)) {
if ((p_chain->u.chain16.prod_idx &
@@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain
*p_chain)
p_chain->u.chain32.prod_idx++;
}
- p_ret = p_chain->p_prod_elem;
- p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
- p_chain->elem_size);
-
- return p_ret;
+ return __xchg(_chain->p_prod_elem,
+ (void *)(((u8 *)p_chain->p_prod_elem) +
p_chain->elem_size));
Wondering if you still need a (void *) casting after the change. Ditto for the
rest of similar cases.
IMHO it is not needed also before the change and IIRC gcc has an
extension which allows to drop (u8 *) cast as well [1].
[1]: https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
}
...
Btw, is it done by coccinelle? If no, why not providing the script?
Yes I have used cocci. My cocci skills are far from perfect, so I did
not want to share my dirty code, but this is nothing secret:
@r1@
expression x, v;
local idexpression p;
@@
- p = x;
- x = v;
- return p;
+ return __xchg(, v);
@depends on r1@
expression e;
@@
__xchg(
- &*e,
+ e,
...)
@depends on r1@
expression t;
@@
- if (t) {
+ if (t)
return __xchg(...);
- }
@depends on r1@
type t;
identifier p;
expression e;
@@
(
- t p;
|
- t p = e;
)
... when != p
Regards
Andrzej
[PATCH v2 11/13] tty/serial: Call ->dtr_rts() parameter active consistently
Convert various parameter names for ->dtr_rts() and related functions
from onoff, on, and raise to active.
Signed-off-by: Ilpo Järvinen
---
drivers/char/pcmcia/synclink_cs.c | 6 +++---
drivers/mmc/core/sdio_uart.c | 6 +++---
drivers/staging/greybus/uart.c| 4 ++--
drivers/tty/amiserial.c | 4 ++--
drivers/tty/hvc/hvc_console.h | 2 +-
drivers/tty/hvc/hvc_iucv.c| 6 +++---
drivers/tty/mxser.c | 4 ++--
drivers/tty/n_gsm.c | 4 ++--
drivers/tty/serial/serial_core.c | 8
drivers/tty/synclink_gt.c | 4 ++--
include/linux/tty_port.h | 4 ++--
include/linux/usb/serial.h| 2 +-
12 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/char/pcmcia/synclink_cs.c
b/drivers/char/pcmcia/synclink_cs.c
index 46a0b586d234..1577eba6fe0e 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -378,7 +378,7 @@ static void async_mode(MGSLPC_INFO *info);
static void tx_timeout(struct timer_list *t);
static bool carrier_raised(struct tty_port *port);
-static void dtr_rts(struct tty_port *port, bool onoff);
+static void dtr_rts(struct tty_port *port, bool active);
#if SYNCLINK_GENERIC_HDLC
#define dev_to_port(D) (dev_to_hdlc(D)->priv)
@@ -2442,13 +2442,13 @@ static bool carrier_raised(struct tty_port *port)
return info->serial_signals & SerialSignal_DCD;
}
-static void dtr_rts(struct tty_port *port, bool onoff)
+static void dtr_rts(struct tty_port *port, bool active)
{
MGSLPC_INFO *info = container_of(port, MGSLPC_INFO, port);
unsigned long flags;
spin_lock_irqsave(>lock, flags);
- if (onoff)
+ if (active)
info->serial_signals |= SerialSignal_RTS | SerialSignal_DTR;
else
info->serial_signals &= ~(SerialSignal_RTS | SerialSignal_DTR);
diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
index c6b4b2b2a4b2..50536fe59f1a 100644
--- a/drivers/mmc/core/sdio_uart.c
+++ b/drivers/mmc/core/sdio_uart.c
@@ -542,20 +542,20 @@ static bool uart_carrier_raised(struct tty_port *tport)
/**
* uart_dtr_rts-port helper to set uart signals
* @tport: tty port to be updated
- * @onoff: set to turn on DTR/RTS
+ * @active: set to turn on DTR/RTS
*
* Called by the tty port helpers when the modem signals need to be
* adjusted during an open, close and hangup.
*/
-static void uart_dtr_rts(struct tty_port *tport, bool onoff)
+static void uart_dtr_rts(struct tty_port *tport, bool active)
{
struct sdio_uart_port *port =
container_of(tport, struct sdio_uart_port, port);
int ret = sdio_uart_claim_func(port);
if (ret)
return;
- if (!onoff)
+ if (!active)
sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
else
sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index 92d49740d5a4..20a34599859f 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -701,7 +701,7 @@ static int gb_tty_ioctl(struct tty_struct *tty, unsigned
int cmd,
return -ENOIOCTLCMD;
}
-static void gb_tty_dtr_rts(struct tty_port *port, bool on)
+static void gb_tty_dtr_rts(struct tty_port *port, bool active)
{
struct gb_tty *gb_tty;
u8 newctrl;
@@ -709,7 +709,7 @@ static void gb_tty_dtr_rts(struct tty_port *port, bool on)
gb_tty = container_of(port, struct gb_tty, port);
newctrl = gb_tty->ctrlout;
- if (on)
+ if (active)
newctrl |= (GB_UART_CTRL_DTR | GB_UART_CTRL_RTS);
else
newctrl &= ~(GB_UART_CTRL_DTR | GB_UART_CTRL_RTS);
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 29d4c554f6b8..d7515d61659e 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -1459,13 +1459,13 @@ static bool amiga_carrier_raised(struct tty_port *port)
return !(ciab.pra & SER_DCD);
}
-static void amiga_dtr_rts(struct tty_port *port, bool raise)
+static void amiga_dtr_rts(struct tty_port *port, bool active)
{
struct serial_state *info = container_of(port, struct serial_state,
tport);
unsigned long flags;
- if (raise)
+ if (active)
info->MCR |= SER_DTR|SER_RTS;
else
info->MCR &= ~(SER_DTR|SER_RTS);
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 6d3428bf868f..9668f821db01 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -66,7 +66,7 @@ struct hv_ops {
int (*tiocmset)(struct hvc_struct *hp, unsigned int set, unsigned int
clear);
/* Callbacks to handle tty ports */
- void (*dtr_rts)(struct hvc_struct *hp, bool raise);
+ void (*dtr_rts)(struct hvc_struct
[PATCH v2 07/13] tty: Convert ->dtr_rts() to take bool argument
Convert the raise/on parameter in ->dtr_rts() to bool through the
callchain. The parameter is used like bool. In USB serial, there
remains a few implicit bool -> larger type conversions because some
devices use u8 in their control messages.
In moxa_tiocmget(), dtr variable was reused for line status which
requires int so use a separate variable for status.
Reviewed-by: Jiri Slaby
Signed-off-by: Ilpo Järvinen
---
drivers/char/pcmcia/synclink_cs.c| 4 +--
drivers/mmc/core/sdio_uart.c | 4 +--
drivers/staging/greybus/uart.c | 2 +-
drivers/tty/amiserial.c | 2 +-
drivers/tty/hvc/hvc_console.c| 4 +--
drivers/tty/hvc/hvc_console.h| 2 +-
drivers/tty/hvc/hvc_iucv.c | 4 +--
drivers/tty/moxa.c | 52 +++-
drivers/tty/mxser.c | 2 +-
drivers/tty/n_gsm.c | 2 +-
drivers/tty/serial/serial_core.c | 8 ++---
drivers/tty/synclink_gt.c| 2 +-
drivers/tty/tty_port.c | 4 +--
drivers/usb/class/cdc-acm.c | 2 +-
drivers/usb/serial/ch341.c | 2 +-
drivers/usb/serial/cp210x.c | 4 +--
drivers/usb/serial/cypress_m8.c | 6 ++--
drivers/usb/serial/digi_acceleport.c | 6 ++--
drivers/usb/serial/f81232.c | 2 +-
drivers/usb/serial/f81534.c | 2 +-
drivers/usb/serial/ftdi_sio.c| 2 +-
drivers/usb/serial/ipw.c | 2 +-
drivers/usb/serial/keyspan.c | 2 +-
drivers/usb/serial/keyspan_pda.c | 2 +-
drivers/usb/serial/mct_u232.c| 4 +--
drivers/usb/serial/mxuport.c | 2 +-
drivers/usb/serial/pl2303.c | 2 +-
drivers/usb/serial/quatech2.c| 2 +-
drivers/usb/serial/sierra.c | 2 +-
drivers/usb/serial/spcp8x5.c | 2 +-
drivers/usb/serial/ssu100.c | 2 +-
drivers/usb/serial/upd78f0730.c | 6 ++--
drivers/usb/serial/usb-serial.c | 2 +-
drivers/usb/serial/usb-wwan.h| 2 +-
drivers/usb/serial/usb_wwan.c| 2 +-
drivers/usb/serial/xr_serial.c | 6 ++--
include/linux/tty_port.h | 4 +--
include/linux/usb/serial.h | 2 +-
38 files changed, 83 insertions(+), 81 deletions(-)
diff --git a/drivers/char/pcmcia/synclink_cs.c
b/drivers/char/pcmcia/synclink_cs.c
index 4391138e1b8a..46a0b586d234 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -378,7 +378,7 @@ static void async_mode(MGSLPC_INFO *info);
static void tx_timeout(struct timer_list *t);
static bool carrier_raised(struct tty_port *port);
-static void dtr_rts(struct tty_port *port, int onoff);
+static void dtr_rts(struct tty_port *port, bool onoff);
#if SYNCLINK_GENERIC_HDLC
#define dev_to_port(D) (dev_to_hdlc(D)->priv)
@@ -2442,7 +2442,7 @@ static bool carrier_raised(struct tty_port *port)
return info->serial_signals & SerialSignal_DCD;
}
-static void dtr_rts(struct tty_port *port, int onoff)
+static void dtr_rts(struct tty_port *port, bool onoff)
{
MGSLPC_INFO *info = container_of(port, MGSLPC_INFO, port);
unsigned long flags;
diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
index 47f58258d8ff..c6b4b2b2a4b2 100644
--- a/drivers/mmc/core/sdio_uart.c
+++ b/drivers/mmc/core/sdio_uart.c
@@ -548,14 +548,14 @@ static bool uart_carrier_raised(struct tty_port *tport)
* adjusted during an open, close and hangup.
*/
-static void uart_dtr_rts(struct tty_port *tport, int onoff)
+static void uart_dtr_rts(struct tty_port *tport, bool onoff)
{
struct sdio_uart_port *port =
container_of(tport, struct sdio_uart_port, port);
int ret = sdio_uart_claim_func(port);
if (ret)
return;
- if (onoff == 0)
+ if (!onoff)
sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
else
sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index 90ff07f2cbf7..92d49740d5a4 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -701,7 +701,7 @@ static int gb_tty_ioctl(struct tty_struct *tty, unsigned
int cmd,
return -ENOIOCTLCMD;
}
-static void gb_tty_dtr_rts(struct tty_port *port, int on)
+static void gb_tty_dtr_rts(struct tty_port *port, bool on)
{
struct gb_tty *gb_tty;
u8 newctrl;
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 01c4fd3ce7c8..29d4c554f6b8 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -1459,7 +1459,7 @@ static bool amiga_carrier_raised(struct tty_port *port)
return !(ciab.pra & SER_DCD);
}
-static void amiga_dtr_rts(struct tty_port *port, int raise)
+static void amiga_dtr_rts(struct tty_port *port, bool raise)
{
struct serial_state *info = container_of(port, struct serial_state,
Re: [PATCH 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS
On Mon, Jan 09, 2023 at 05:51:23PM -0700, Nathan Chancellor wrote: > So for this patch, I have > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > warns: > > clang-16: error: argument unused during compilation: '-s' > [-Werror,-Wunused-command-line-argument] > > The compiler's '-s' flag is a linking option (it is passed along to the > linker directly), which means it does nothing when the linker is not > invoked by the compiler. The kernel builds all .o files with either '-c' > or '-S', which do not run the linker, so '-s' can be safely dropped from > ASFLAGS. > > as a new commit message. Is that sufficient for everyone? If so, I'll > adjust the s390 commit to match, as it is the same exact problem. Almost? -S doesn't write .o files, it writes a .s file. To go from an assembler file (.s, or .S if you want to run the C preprocessor on non-C code for some strange reason, the assembler macro facilities are vastly superior) to an object file is just -c as well. > Alternatively, if '-s' should actually remain around, we could move it > to ldflags-y, which is added in patch 7. However, I assume that nobody > has noticed that it has not been doing its job for a while, so it should > be safe to remove. +1 Segher
[PATCH v2] net: wan: Add checks for NULL. If uhdlc_priv_tsa != 1 then utdm is not initialized. And if ret != NULL then goto undo_uhdlc_init, where utdm is dereferenced. Same if dev == NULL.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Esina Ekaterina
v2: Add check for NULL for unmap_si_regs
---
drivers/net/wan/fsl_ucc_hdlc.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 22edea6ca4b8..ed7886bde727 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1243,9 +1243,13 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
free_dev:
free_netdev(dev);
undo_uhdlc_init:
- iounmap(utdm->siram);
+ if (utdm != NULL) {
+ iounmap(utdm->siram);
+ }
unmap_si_regs:
- iounmap(utdm->si_regs);
+ if (utdm != NULL) {
+ iounmap(utdm->si_regs);
+ }
free_utdm:
if (uhdlc_priv->tsa)
kfree(utdm);
--
2.34.1
Re: [RFC DO NOT MERGE] treewide: use __xchg in most obvious places
On Tue, Jan 10, 2023 at 11:53:06AM +0100, Andrzej Hajda wrote:
> This patch tries to show usability of __xchg helper.
> It is not intended to be merged, but I can convert
> it to proper patchset if necessary.
>
> There are many more places where __xchg can be used.
> This demo shows the most spectacular cases IMHO:
> - previous value is returned from function,
> - temporary variables are in use.
>
> As a result readability is much better and diffstat is quite
> nice, less local vars to look at.
> In many cases whole body of functions is replaced
> with __xchg(ptr, val), so as further refactoring the whole
> function can be removed and __xchg can be called directly.
...
> arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
> struct pt_regs *regs)
> {
> - unsigned long orig_ret_vaddr;
> -
> - orig_ret_vaddr = regs->ARM_lr;
> - /* Replace the return addr with trampoline addr */
> - regs->ARM_lr = trampoline_vaddr;
> - return orig_ret_vaddr;
> + return __xchg(>ARM_lr, trampoline_vaddr);
> }
If it's not a callback, the entire function can be killed.
And this is a good example of the function usage.
OTOH, these places might have a side effect (if it's in deep CPU
handlers), means we need to do this carefully.
...
> static inline void *qed_chain_produce(struct qed_chain *p_chain)
> {
> - void *p_ret = NULL, *p_prod_idx, *p_prod_page_idx;
> + void *p_prod_idx, *p_prod_page_idx;
>
> if (is_chain_u16(p_chain)) {
> if ((p_chain->u.chain16.prod_idx &
> @@ -390,11 +391,8 @@ static inline void *qed_chain_produce(struct qed_chain
> *p_chain)
> p_chain->u.chain32.prod_idx++;
> }
>
> - p_ret = p_chain->p_prod_elem;
> - p_chain->p_prod_elem = (void *)(((u8 *)p_chain->p_prod_elem) +
> - p_chain->elem_size);
> -
> - return p_ret;
> + return __xchg(_chain->p_prod_elem,
> + (void *)(((u8 *)p_chain->p_prod_elem) +
> p_chain->elem_size));
Wondering if you still need a (void *) casting after the change. Ditto for the
rest of similar cases.
> }
...
Btw, is it done by coccinelle? If no, why not providing the script?
--
With Best Regards,
Andy Shevchenko
[RFC DO NOT MERGE] treewide: use __xchg in most obvious places
This patch tries to show usability of __xchg helper.
It is not intended to be merged, but I can convert
it to proper patchset if necessary.
There are many more places where __xchg can be used.
This demo shows the most spectacular cases IMHO:
- previous value is returned from function,
- temporary variables are in use.
As a result readability is much better and diffstat is quite
nice, less local vars to look at.
In many cases whole body of functions is replaced
with __xchg(ptr, val), so as further refactoring the whole
function can be removed and __xchg can be called directly.
Signed-off-by: Andrzej Hajda
---
arch/arm/probes/uprobes/core.c| 8 ++--
arch/csky/kernel/probes/uprobes.c | 9 ++---
arch/mips/kernel/irq_txx9.c | 7 ++-
arch/mips/kernel/process.c| 8 +++-
arch/mips/kernel/uprobes.c| 10 ++
arch/powerpc/include/asm/kvm_ppc.h| 7 ++-
arch/powerpc/kernel/uprobes.c | 10 ++
arch/powerpc/mm/init_64.c | 7 ++-
arch/riscv/kernel/probes/uprobes.c| 9 ++---
arch/s390/kernel/uprobes.c| 7 ++-
arch/s390/kvm/interrupt.c | 6 ++
arch/sh/kernel/traps_32.c | 6 ++
.../accessibility/speakup/speakup_dectlk.c| 7 ++-
drivers/accessibility/speakup/speakup_soft.c | 7 ++-
drivers/block/drbd/drbd_receiver.c| 5 ++---
drivers/cdrom/cdrom.c | 7 ++-
drivers/gpu/drm/drm_atomic_uapi.c | 14 +++---
drivers/iommu/iova.c | 7 ++-
drivers/misc/ti-st/st_core.c | 10 +++---
drivers/mtd/nand/raw/qcom_nandc.c | 11 ---
drivers/net/ethernet/ibm/ehea/ehea_main.c | 11 +++
.../microchip/sparx5/sparx5_calendar.c| 10 --
drivers/net/usb/rtl8150.c | 9 +++--
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 8 +++-
.../net/wireless/marvell/mwifiex/sta_ioctl.c | 7 +++
drivers/scsi/device_handler/scsi_dh_alua.c| 8 +++-
drivers/scsi/lpfc/lpfc_sli.c | 7 ++-
drivers/staging/rtl8192e/rtllib_tx.c | 7 ++-
drivers/tty/hvc/hvc_iucv.c| 8 +++-
drivers/video/fbdev/sis/sis_main.c| 6 ++
drivers/xen/grant-table.c | 6 ++
fs/namespace.c| 6 ++
include/linux/ptr_ring.h | 7 ++-
include/linux/qed/qed_chain.h | 19 +++
io_uring/io_uring.c | 7 ++-
mm/kmsan/init.c | 7 ++-
mm/memcontrol.c | 8 ++--
net/mac80211/rc80211_minstrel_ht.c| 6 ++
sound/pci/asihpi/hpidebug.c | 8 +++-
.../selftests/bpf/progs/dummy_st_ops.c| 7 ++-
40 files changed, 99 insertions(+), 225 deletions(-)
diff --git a/arch/arm/probes/uprobes/core.c b/arch/arm/probes/uprobes/core.c
index f5f790c6e5f896..77ce8ae431376d 100644
--- a/arch/arm/probes/uprobes/core.c
+++ b/arch/arm/probes/uprobes/core.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -61,12 +62,7 @@ unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long orig_ret_vaddr;
-
- orig_ret_vaddr = regs->ARM_lr;
- /* Replace the return addr with trampoline addr */
- regs->ARM_lr = trampoline_vaddr;
- return orig_ret_vaddr;
+ return __xchg(>ARM_lr, trampoline_vaddr);
}
int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
diff --git a/arch/csky/kernel/probes/uprobes.c
b/arch/csky/kernel/probes/uprobes.c
index 2d31a12e46cfee..775fe88b5f0016 100644
--- a/arch/csky/kernel/probes/uprobes.c
+++ b/arch/csky/kernel/probes/uprobes.c
@@ -3,6 +3,7 @@
* Copyright (C) 2014-2016 Pratyush Anand
*/
#include
+#include
#include
#include
#include
@@ -123,13 +124,7 @@ unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr,
struct pt_regs *regs)
{
- unsigned long ra;
-
- ra = regs->lr;
-
- regs->lr = trampoline_vaddr;
-
- return ra;
+ return __xchg(>lr, trampoline_vaddr);
}
int arch_uprobe_exception_notify(struct notifier_block *self,
diff --git a/arch/mips/kernel/irq_txx9.c b/arch/mips/kernel/irq_txx9.c
index af3ef4c9f7de1e..b5abe24ea7cfb9 100644
--- a/arch/mips/kernel/irq_txx9.c
+++ b/arch/mips/kernel/irq_txx9.c
@@ -15,6 +15,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -159,13 +160,9 @@ void __init txx9_irq_init(unsigned long baseaddr)
int __init txx9_irq_set_pri(int
[powerpc] Boot failure kernel BUG at mm/usercopy.c:102
6.2.0-rc3-next-20230109 fails to boot on powerpc with following: [ 0.444834] [ cut here ] [ 0.444838] kernel BUG at mm/usercopy.c:102! [ 0.444842] Oops: Exception in kernel mode, sig: 5 [#1] [ 0.444845] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries [ 0.444849] Modules linked in: [ 0.444853] CPU: 23 PID: 201 Comm: modprobe Not tainted 6.2.0-rc3-next-20230109 #1 [ 0.444858] Hardware name: IBM,9080-HEX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1030.00 (NH1030_026) hv:phyp pSeries [ 0.444862] NIP: c055a934 LR: c055a930 CTR: 00725d90 [ 0.444866] REGS: c7f937c0 TRAP: 0700 Not tainted (6.2.0-rc3-next-20230109) [ 0.444871] MSR: 80029033 CR: 28088822 XER: 0007 [ 0.444879] CFAR: c02012a8 IRQMASK: 0 [ 0.444879] GPR00: c055a930 c7f93a60 c13b0800 0066 [ 0.444879] GPR04: 7fff c7f93880 c7f93878 [ 0.444879] GPR08: 7fff c25e7150 c29672b8 [ 0.444879] GPR12: 48088824 c00e87bf6300 c017f458 c34b8100 [ 0.444879] GPR16: 00012f18eac0 7fffc5c095d0 7fffc5c095d8 00012f140040 [ 0.444879] GPR20: fcff 001f 5455 0008 [ 0.444879] GPR24: c723a0c0 7fffc5c09368 7fffc5c09370 [ 0.444879] GPR28: 0250 0001 c3017000 c723a0c0 [ 0.444922] NIP [c055a934] usercopy_abort+0xa4/0xb0 [ 0.444928] LR [c055a930] usercopy_abort+0xa0/0xb0 [ 0.444932] Call Trace: [ 0.444933] [c7f93a60] [c055a930] usercopy_abort+0xa0/0xb0 (unreliable) [ 0.444939] [c7f93ad0] [c050eeb8] __check_heap_object+0x198/0x1d0 [ 0.444945] [c7f93b10] [c055a7e0] __check_object_size+0x290/0x340 [ 0.444949] [c7f93b50] [c060eba4] create_elf_tables.isra.20+0xc04/0xc90 [ 0.444956] [c7f93c10] [c0610b2c] load_elf_binary+0xdac/0x1320 [ 0.444962] [c7f93d00] [c0571cf0] bprm_execve+0x3d0/0x7c0 [ 0.444966] [c7f93dc0] [c0572b9c] kernel_execve+0x1ac/0x270 [ 0.444971] [c7f93e10] [c017f5cc] call_usermodehelper_exec_async+0x17c/0x250 [ 0.444978] [c7f93e50] [c000e054] ret_from_kernel_thread+0x5c/0x64 [ 0.444983] --- interrupt: 0 at 0x0 [ 0.444986] NIP: LR: CTR: [ 0.444990] REGS: c7f93e80 TRAP: Not tainted (6.2.0-rc3-next-20230109) [ 0.444994] MSR: <> CR: XER: [ 0.444998] CFAR: IRQMASK: 0 [ 0.444998] GPR00: c7f94000 [ 0.444998] GPR04: [ 0.444998] GPR08: [ 0.444998] GPR12: c017f458 c34b8100 [ 0.444998] GPR16: [ 0.444998] GPR20: [ 0.444998] GPR24: [ 0.444998] GPR28: [ 0.445039] NIP [] 0x0 [ 0.445042] LR [] 0x0 [ 0.445044] --- interrupt: 0 [ 0.445046] Code: 392990f8 4814 3d02ffe9 390827f0 7d074378 7d094378 7c661b78 3c62ffe7 f9610060 386319f0 4bca6935 6000 <0fe0> 7c0802a6 [ 0.445061] ---[ end trace ]— Git bisect points to following patch: commit 317c8194e6aeb8b3b573ad139fc2a0635856498e rseq: Introduce feature size and alignment ELF auxiliary vector entries Reverting the patch helps boot the kernel. Thanks -Sachin
Re: [PATCH] net-wan: Add check for NULL for utdm in ucc_hdlc_probe
-Original Message-
From: Christophe
To: Ekaterina ; Zhao
Cc: lvc-project ; netdev
; linux-kernel ; Eric
; Jakub ; Paolo ;
linuxppc-dev ; David
Date: Monday, 9 January 2023 6:49 PM MSK
Subject: Re: [PATCH] net-wan: Add check for NULL for utdm in ucc_hdlc_probe
Le 23/12/2022 à 15:32, Ekaterina Esina a écrit :
> [Vous ne recevez pas souvent de courriers de ees...@astralinux.ru. Découvrez
> pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> If uhdlc_priv_tsa != 1 then utdm is not initialized.
> And if ret != NULL then goto undo_uhdlc_init, where utdm is dereferenced.
> Same if dev == NULL.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Ekaterina Esina
> ---
> drivers/net/wan/fsl_ucc_hdlc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 22edea6ca4b8..2ddb0f71e648 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -1243,7 +1243,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
> free_dev:
> free_netdev(dev);
> undo_uhdlc_init:
> - iounmap(utdm->siram);
> + if (utdm != NULL) {
> + iounmap(utdm->siram);
> + }
If utdm being NULL is a problem here, isn't it also a problem in the
iounmap below ?
> unmap_si_regs:
> iounmap(utdm->si_regs);
> free_utdm:
> --
> 2.30.2
> Yes, below the check is also needed. I guess I should send a new patch with
> both checks Best wishes, Ekaterina Esina
Re: [PATCH 15/15] backlight: backlight: Drop the deprecated fb_blank property
On Mon, 09 Jan 2023, Sam Ravnborg wrote: > Hi Daniel. > > On Mon, Jan 09, 2023 at 11:06:35AM +, Daniel Thompson wrote: > > On Sat, Jan 07, 2023 at 07:26:29PM +0100, Sam Ravnborg via B4 Submission > > Endpoint wrote: > > > From: Sam Ravnborg > > > > > > With all users gone remove the deprecated fb_blank member in > > > backlight_properties. > > > > > > Signed-off-by: Sam Ravnborg > > > Cc: Lee Jones > > > Cc: Daniel Thompson > > > Cc: Jingoo Han > > > > > > Reviewed-by: Daniel Thompson > > Thanks for the follow-up on all the backlight related patches. > > > > > > > PS Please don't treat this like a maintainer Acked-by: and merge it > >(Lee's not on holiday so work with Lee to figure out the merge > >strategy ;-) ). > Nope, I am aware that the usual pattern here and wait for Lee to show > up. It's on the list. Only 50 more reviews in the backlog now! > For this patch there is a bug as I need to update a comment. > I will fix this when I resend after all the patches in flight has > landed. So likely after the next merge window, -- Lee Jones [李琼斯]
Re: usb.c:undefined reference to `qe_immr'
[adding Cc's] On 1/9/23 23:59, kernel test robot wrote: > Hi Masahiro, > > FYI, the error/warning still remains. > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: 5a41237ad1d4b62008f93163af1d9b1da90729d8 > commit: 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b kbuild: link symbol CRCs at > final link, removing CONFIG_MODULE_REL_CRCS > date: 8 months ago > config: powerpc-randconfig-r026-20230110 > compiler: powerpc-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b4537199a4a8480b8c3ba37a2d44765ce76cd9b > git remote add linus > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > git fetch --no-tags linus master > git checkout 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 > O=build_dir ARCH=powerpc olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 > O=build_dir ARCH=powerpc SHELL=/bin/bash > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >powerpc-linux-ld: powerpc-linux-ld: DWARF error: could not find abbrev > number 74 >drivers/soc/fsl/qe/usb.o: in function `qe_usb_clock_set': >>> usb.c:(.text+0x1e): undefined reference to `qe_immr' >>> powerpc-linux-ld: usb.c:(.text+0x2a): undefined reference to `qe_immr' >>> powerpc-linux-ld: usb.c:(.text+0xbc): undefined reference to `qe_setbrg' >>> powerpc-linux-ld: usb.c:(.text+0xca): undefined reference to `cmxgcr_lock' >powerpc-linux-ld: usb.c:(.text+0xce): undefined reference to `cmxgcr_lock' > .config extract: # # NXP/Freescale QorIQ SoC drivers # # CONFIG_QUICC_ENGINE is not set CONFIG_QE_USB=y This is caused by (drivers/soc/fsl/qe/Kconfig): config QE_USB bool default y if USB_FSL_QE help QE USB Controller support which does not depend on QUICC_ENGINE, where the latter build provides the missing symbols. drivers/usb/gadget/udc/Kconfig: config USB_FSL_QE tristate "Freescale QE/CPM USB Device Controller" depends on FSL_SOC && (QUICC_ENGINE || CPM) depends on !64BIT || BROKEN CPM is set but QUICC_ENGINE is not set (by CONFIG_PPC_EP88XC=y). I don't know of a good fix for this build error. -- ~Randy
Re: [PATCH v1 06/10] powerpc/bpf: Perform complete extra passes to update addresses
Christophe Leroy wrote:
Le 13/12/2022 à 11:23, Naveen N. Rao a écrit :
Christophe Leroy wrote:
BPF core calls the jit compiler again for an extra pass in order
to properly set subprog addresses.
Unlike other architectures, powerpc only updates the addresses
during that extra pass. It means that holes must have been left
in the code in order to enable the maximum possible instruction
size.
In order avoid waste of space, and waste of CPU time on powerpc
processors on which the NOP instruction is not 0-cycle, perform
two real additional passes.
Signed-off-by: Christophe Leroy
---
arch/powerpc/net/bpf_jit_comp.c | 85 -
1 file changed, 85 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp.c
b/arch/powerpc/net/bpf_jit_comp.c
index 43e634126514..8833bf23f5aa 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -23,74 +23,6 @@ static void bpf_jit_fill_ill_insns(void *area,
unsigned int size)
memset32(area, BREAKPOINT_INSTRUCTION, size / 4);
}
-/* Fix updated addresses (for subprog calls, ldimm64, et al) during
extra pass */
-static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image,
- struct codegen_context *ctx, u32 *addrs)
-{
- const struct bpf_insn *insn = fp->insnsi;
- bool func_addr_fixed;
- u64 func_addr;
- u32 tmp_idx;
- int i, j, ret;
-
- for (i = 0; i < fp->len; i++) {
- /*
- * During the extra pass, only the branch target addresses for
- * the subprog calls need to be fixed. All other instructions
- * can left untouched.
- *
- * The JITed image length does not change because we already
- * ensure that the JITed instruction sequence for these calls
- * are of fixed length by padding them with NOPs.
- */
- if (insn[i].code == (BPF_JMP | BPF_CALL) &&
- insn[i].src_reg == BPF_PSEUDO_CALL) {
- ret = bpf_jit_get_func_addr(fp, [i], true,
- _addr,
- _addr_fixed);
I don't see you updating calls to bpf_jit_get_func_addr() in
bpf_jit_build_body() to set extra_pass to true. Afaics, that's required
to get the correct address to be branched to for subprogs.
I don't understand what you mean.
I am referring to the third parameter we pass to
bpf_jit_get_func_addr().
In bpf_jit_build_body(), we do:
case BPF_JMP | BPF_CALL:
ctx->seen |= SEEN_FUNC;
ret = bpf_jit_get_func_addr(fp, [i], false,
_addr,
_addr_fixed);
The third parameter (extra_pass) to bpf_jit_get_func_addr() is set to
false. In bpf_jit_get_func_addr(), we have:
*func_addr_fixed = insn->src_reg != BPF_PSEUDO_CALL;
if (!*func_addr_fixed) {
/* Place-holder address till the last pass has collected
* all addresses for JITed subprograms in which case we
* can pick them up from prog->aux.
*/
if (!extra_pass)
addr = NULL;
Before this patch series, in bpf_jit_fixup_addresses(), we were calling
bpf_jit_get_func_addr() with the third parameter set to true.
- Naveen
Re: [PATCH v2] powerpc/pseries: fix potential memory leak in init_cpu_associativity()
Wang Yufen wrote:
If the vcpu_associativity alloc memory successfully but the
pcpu_associativity fails to alloc memory, the vcpu_associativity
memory leaks.
Fixes: d62c8deeb6e6 ("powerpc/pseries: Provide vcpu dispatch statistics")
Signed-off-by: Wang Yufen
---
arch/powerpc/platforms/pseries/lpar.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Naveen N. Rao
- Naveen
Re: [PATCH v2 04/10] powerpc/8xx: Use a larger CPM1 command check mask
Le 06/01/2023 à 17:37, Herve Codina a écrit : > The CPM1 command mask is defined for use with the standard > CPM1 command register as described in the user's manual: >0 |13|47|8 11|12 14| 15| >RST|- |OPCODE|CH_NUM| -|FLG| > > In the QMC extension the CPM1 command register is redefined > (QMC supplement user's manuel) with the following mapping: >0 |13|47|8 13|14| 15| >RST|QMC OPCODE| 1110|CHANNEL_NUMBER| -|FLG| > > Extend the check command mask in order to support both the > standard CH_NUM field and the QMC extension CHANNEL_NUMBER > field. > > Signed-off-by: Herve Codina Acked-by: Christophe Leroy (As maintainer of LINUX FOR POWERPC EMBEDDED PPC8XX) > --- > arch/powerpc/platforms/8xx/cpm1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/platforms/8xx/cpm1.c > b/arch/powerpc/platforms/8xx/cpm1.c > index 8ef1f4392086..6b828b9f90d9 100644 > --- a/arch/powerpc/platforms/8xx/cpm1.c > +++ b/arch/powerpc/platforms/8xx/cpm1.c > @@ -100,7 +100,7 @@ int cpm_command(u32 command, u8 opcode) > int i, ret; > unsigned long flags; > > - if (command & 0xff0f) > + if (command & 0xff03) > return -EINVAL; > > spin_lock_irqsave(_lock, flags);
Re: [PATCH v2 08/10] dt-bindings: sound: Add support for QMC audio
Hi Krzysztof,
On Sun, 8 Jan 2023 16:16:24 +0100
Krzysztof Kozlowski wrote:
> On 06/01/2023 17:37, Herve Codina wrote:
> > The QMC (QUICC mutichannel controller) is a controller
> > present in some PowerQUICC SoC such as MPC885.
> > The QMC audio is an ASoC component that uses the QMC
> > controller to transfer the audio data.
> >
> > Signed-off-by: Herve Codina
> > ---
> > .../bindings/sound/fsl,qmc-audio.yaml | 110 ++
> > 1 file changed, 110 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml
> > b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml
> > new file mode 100644
> > index ..b3774be36c19
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml
> > @@ -0,0 +1,110 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,qmc-audio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: QMC audio
> > +
> > +maintainers:
> > + - Herve Codina
> > +
> > +description: |
> > + The QMC audio is an ASoC component which uses QMC (QUICC Multichannel
> > + Controller) channels to transfer the audio data.
> > + It provides as many DAI as the number of QMC channel used.
> > +
> > +properties:
> > + compatible:
> > +items:
>
> Drop items.
Will be dropped.
>
> > + - const: fsl,qmc-audio
> > +
> > + '#address-cells':
> > +const: 1
> > + '#size-cells':
> > +const: 0
> > + '#sound-dai-cells':
> > +const: 1
> > +
> > +patternProperties:
> > + "^dai@([0-9]|[1-5][0-9]|6[0-3])$":
> > +description:
> > + A DAI managed by this controller
> > +type: object
> > +
> > +properties:
> > + reg:
> > +minimum: 0
> > +maximum: 63
> > +description:
> > + The DAI number
> > +
> > + qmc-chan:
>
> Missing vendor prefix.
Will be changed to 'fsl,qmc-chan'
>
> > +$ref: /schemas/types.yaml#/definitions/phandle-array
>
> Why this is not a phandle?
I have try '$ref: /schemas/types.yaml#/definitions/phandle'
I have an error from make dt_binding_check:
dai@16:fsl,qmc-chan:0: [4294967295, 16] is too long
I need a phandle with an argument ie < 16>.
Is there an alternative to phandle-array to handle this case ?
>
> > +description: phandle to the QMC channel> +maxItems: 1
> > +
> > +required:
> > + - reg
> > + - qmc-chan
> > +
> > +required:
> > + - compatible
> > + - '#address-cells'
> > + - '#size-cells'
> > + - '#sound-dai-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +qmc_audio: qmc_audio {
>
> Same problem as in previous patch.
Changed to 'audio-controller'.
>
> > +compatible = "fsl,qmc-audio";
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +#sound-dai-cells = <1>;
> > +dai@16 {
> > +reg = <16>;
> > +qmc-chan = <_qmc 16>;
> > +};
> > +dai@17 {
> > +reg = <17>;
> > +qmc-chan = <_qmc 17>;
> > +};
> > +};
>
> Best regards,
> Krzysztof
>
Thanks for the review.
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2] hvc/xen: lock console list traversal
On 30.11.22 17:36, Roger Pau Monne wrote:
The currently lockless access to the xen console list in
vtermno_to_xencons() is incorrect, as additions and removals from the
list can happen anytime, and as such the traversal of the list to get
the private console data for a given termno needs to happen with the
lock held. Note users that modify the list already do so with the
lock taken.
Adjust current lock takers to use the _irq{save,restore} helpers,
since the context in which vtermno_to_xencons() is called can have
interrupts disabled. Use the _irq{save,restore} set of helpers to
switch the current callers to disable interrupts in the locked region.
I haven't checked if existing users could instead use the _irq
variant, as I think it's safer to use _irq{save,restore} upfront.
While there switch from using list_for_each_entry_safe to
list_for_each_entry: the current entry cursor won't be removed as
part of the code in the loop body, so using the _safe variant is
pointless.
Fixes: 02e19f9c7cac ('hvc_xen: implement multiconsole support')
Signed-off-by: Roger Pau Monné
Pushed to xen/tip.git for-linus-6.2
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v2 05/10] dt-bindings: soc: fsl: cpm_qe: Add QMC controller
Hi Krzysztof,
On Sun, 8 Jan 2023 16:14:47 +0100
Krzysztof Kozlowski wrote:
[...]
> > +
> > + interrupts:
> > +description: SCC interrupt line in the CPM interrupt controller
>
> Missing constraints.
'maxItems: 1' will be added in v3
>
> > +
> > + fsl,cpm-command:
> > +$ref: /schemas/types.yaml#/definitions/uint32
> > +description: Cf. soc/fsl/cpm_qe/cpm.txt
>
> Missing description.
'fsl,cpm-command' will be removed in v3.
The value needed is determined based on other information.
This is not needed in the DT.
>
> > +
> > + tsa:
> > +$ref: /schemas/types.yaml#/definitions/phandle
> > +description: phandle to the TSA
>
> Missing vendor prefix. Does not look like a generic property.
Will be be changed to 'fsl,tsa'
and also 'tsa-cell-id' will be changed to 'fsl,tsa-cell-id'
>
[...]
> > +
> > +patternProperties:
> > + "^channel@([0-9]|[1-5][0-9]|6[0-3])$":
> > +description:
> > + A channel managed by this controller
> > +type: object
> > +
> > +properties:
> > + reg:
> > +minimum: 0
> > +maximum: 63
> > +description:
> > + The channel number
> > +
> > + fsl,mode:
> > +$ref: /schemas/types.yaml#/definitions/string
> > +enum: [transparent, hdlc]
> > +default: transparent
> > +description: Operational mode
>
> And what do they mean?
I will change with
description: |
The channel operational mode
- hdlc: The channel handles HDLC frames
- transparent: The channel handles raw data without any processing
>
> > +
> > + fsl,reverse-data:
> > +$ref: /schemas/types.yaml#/definitions/flag
> > +description:
> > + The bit order as seen on the channels is reversed,
> > + transmitting/receiving the MSB of each octet first.
> > + This flag is used only in 'transparent' mode.
> > +
> > + tx-ts-mask:
>
> Missing vendor prefix.
Will be added, also on rx-ts-mask.
>
> > +$ref: /schemas/types.yaml#/definitions/uint64
> > +description:
> > + Channel assigned Tx time-slots within the Tx time-slots routed
> > + by the TSA to this cell.
> > +
> > + rx-ts-mask:
> > +$ref: /schemas/types.yaml#/definitions/uint64
> > +description:
> > + Channel assigned Rx time-slots within the Rx time-slots routed
> > + by the TSA to this cell.
> > +
> > +required:
> > + - reg
> > + - tx-ts-mask
> > + - rx-ts-mask
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - tsa
> > + - tsa-cell-id
> > + - '#address-cells'
> > + - '#size-cells'
> > + - '#chan-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +
> > +scc_qmc@a60 {
>
> No underscores in node names.
>
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Will be changed to qmc@a60
>
> > +compatible = "fsl,mpc885-scc-qmc", "fsl,cpm1-scc-qmc";
> > +reg = <0xa60 0x20>,
> > + <0x3f00 0xc0>,
> > + <0x2000 0x1000>;
> > +reg-names = "scc_regs", "scc_pram", "dpram";
> > +interrupts = <27>;
> > +interrupt-parent = <_PIC>;
> > +fsl,cpm-command = <0xc0>;
> > +
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +#chan-cells = <1>;
> > +
> > +tsa = <>;
> > +tsa-cell-id = ;
> > +
> > +channel@16 {
> > +/* Ch16 : First 4 even TS from all routed from TSA */
> > +reg = <16>;
> > +fsl,mode = "transparent";
> > +fsl,reverse-data;
> > +tx-ts-mask = <0x 0x00AA>;
> > +rx-ts-mask = <0x 0x00AA>;
>
> Keep case consistent. lower-case hex.
Will be fixed
>
> Best regards,
> Krzysztof
>
Thanks for the review,
Best regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 01/10] dt-bindings: soc: fsl: cpm_qe: Add TSA controller
Hi Krzysztof, On Sun, 8 Jan 2023 16:10:38 +0100 Krzysztof Kozlowski wrote: [...] > > + '#size-cells': > > +const: 0 > > + > > +patternProperties: > > + "^tdm@[0-1]$": > > Use consistent quotes - either ' or " Ok, I will change on v3. I will also change them on the other bindings present in the series. > > > +description: > > + The TDM managed by this controller > > +type: object > > + > > +properties: > > + reg: > > +minimum: 0 > > +maximum: 1 > > +description: > > + The TDM number for this TDM, 0 for TDMa and 1 for TDMb > > + > > + fsl,common-rxtx-pins: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: > > + Use common pins for both transmit and receive > > What are the "common" pins? Without this property device is using > uncommon pins? This does not make sense... Common in the "shared" sense. The hardware can use dedicated pins for Tx clock, Tx sync, Rx clock and Rx sync or use only 2 pins, Tx/Rx clock and Rx/Rx sync. Without the property, we use the 4 pins and with the property, we use 2 pins. > > > + > > + clocks: true > > + clock-names: true > > Both need constraints. The constraints are present later in the file as the number of clocks depends on the 'fsl,common-rxtx-pins' property. I will remove these two lines in the v3 series as they are not needed. 'clocks' and 'clock-names' are handled in the conditional part. > [...] > > + > > + fsl,rx-frame-sync-delay: > > +$ref: /schemas/types.yaml#/definitions/uint32 > > +enum: [0, 1, 2, 3] > > +default: 0 > > +description: | > > + Receive frame sync delay. > > Delay in what units? The unit is a number of bits. I will rename to fsl,rx-frame-sync-delay-bits and change the description to 'Receive frame sync delay in number of bits' I will do also the same for fsl,tx-frame-sync-delay property. > > > + Indicates the delay between the Rx sync and the first bit of the > > + Rx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay. > > + > > + fsl,tx-frame-sync-delay: > > +$ref: /schemas/types.yaml#/definitions/uint32 > > +enum: [0, 1, 2, 3] > > +default: 0 > > +description: | > > + Transmit frame sync delay. > > + Indicates the delay between the Tx sync and the first bit of the > > + Tx frame. 0 for no bit delay. 1, 2 or 3 for 1, 2 or 3 bits delay. > > + > > + fsl,clock-falling-edge: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: | > > + Data is sent on falling edge of the clock (and received on the > > + rising edge). > > + If 'clock-falling-edge' is not present, data is sent on the > > + rising edge (and received on the falling edge). > > + > > + fsl,fsync-rising-edge: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: > > + Frame sync pulses are sampled with the rising edge of the channel > > + clock. If 'fsync-rising-edge' is not present, pulses are sample > > + with e falling edge. > > + > > + fsl,double-speed-clock: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: > > + The channel clock is twice the data rate. > > + > > + fsl,grant-mode: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: > > + Grant mode enabled. > > This we know from property name. You need to describe what it is and > what it does. Instead of describing it, I will simply remove the property (I should have done already). I cannot test the 'grant mode' enabled with my hardware and so I prefer keeping it disabled. This property, if needed, could be add later setting it optional with default to 'disabled'. > > > + > > + tx_ts_routes: > > No underscores, missing vendor prefix. Indeed, will be change to fsl,tx-ts-routes (idem for rx_ts_routes). > > > +$ref: /schemas/types.yaml#/definitions/uint32-matrix > > +description: | > > + A list of tupple that indicates the Tx time-slots routes. > > +tx_ts_routes = > > + < 2 0 0>, /* The first 2 time slots are not used */ > > + < 3 1 0>, /* The next 3 ones are route to SCC2 */ > > + < 4 0 0>, /* The next 4 ones are not used */ > > + < 2 2 0>; /* The nest 2 ones are route to SCC3 */ > > +items: > > + items: > > +- description: > > +The number of time-slots > > + minimum: 1 > > + maximum: 64 > > +- description: | > > +The source serial interface (dt-bindings/soc/fsl-tsa.h > > +defines these values) > > + - 0: No destination > > + - 1: SCC2 > > + - 2: SCC3 > > +
Re: [PATCH 41/41] mm: replace rw_semaphore with atomic_t in vma_lock
On 1/9/23 21:53, Suren Baghdasaryan wrote: > rw_semaphore is a sizable structure of 40 bytes and consumes > considerable space for each vm_area_struct. However vma_lock has > two important specifics which can be used to replace rw_semaphore > with a simpler structure: > 1. Readers never wait. They try to take the vma_lock and fall back to > mmap_lock if that fails. > 2. Only one writer at a time will ever try to write-lock a vma_lock > because writers first take mmap_lock in write mode. > Because of these requirements, full rw_semaphore functionality is not > needed and we can replace rw_semaphore with an atomic variable. > When a reader takes read lock, it increments the atomic unless the > value is negative. If that fails read-locking is aborted and mmap_lock > is used instead. > When writer takes write lock, it resets atomic value to -1 if the > current value is 0 (no readers). Since all writers take mmap_lock in > write mode first, there can be only one writer at a time. If there > are readers, writer will place itself into a wait queue using new > mm_struct.vma_writer_wait waitqueue head. The last reader to release > the vma_lock will signal the writer to wake up. > vm_lock_seq is also moved into vma_lock and along with atomic_t they > are nicely packed and consume 8 bytes, bringing the overhead from > vma_lock from 44 to 16 bytes: > > slabinfo before the changes: > ...: ... > vm_area_struct...152 532 : ... > > slabinfo with vma_lock: > ...: ... > rw_semaphore ... 8 5121 : ... I guess the cache is called vma_lock, not rw_semaphore? > vm_area_struct...160 512 : ... > > Assuming 4 vm_area_structs, memory consumption would be: > baseline: 6040kB > vma_lock (vm_area_structs+vma_lock): 6280kB+316kB=6596kB > Total increase: 556kB > > atomic_t might overflow if there are many competing readers, therefore > vma_read_trylock() implements an overflow check and if that occurs it > restors the previous value and exits with a failure to lock. > > Signed-off-by: Suren Baghdasaryan This patch is indeed an interesting addition indeed, but I can't help but think it obsoletes the previous one :) We allocate an extra 8 bytes slab object for the lock, and the pointer to it is also 8 bytes, and requires an indirection. The vma_lock cache is not cacheline aligned (otherwise it would be a major waste), so we have potential false sharing with up to 7 other vma_lock's. I'd expect if the vma_lock was placed with the relatively cold fields of vm_area_struct, it shouldn't cause much cache ping pong when working with that vma. Even if we don't cache align the vma to save memory (would be 192 bytes instead of 160 when aligned) and place the vma_lock and the cold fields at the end of the vma, it may be false sharing the cacheline with the next vma in the slab. But that's a single vma, not up to 7, so it shouldn't be worse?
