* [PATCH v4 08/18] arm: sun8i: orangepi-pc: Enable dwmac-sun8i
From: Maxime Ripard @ 2017-04-12 12:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412111400.2296-9-clabbe.montjoie@gmail.com>
Hi Corentin,
On Wed, Apr 12, 2017 at 01:13:50PM +0200, Corentin Labbe wrote:
> The dwmac-sun8i hardware is present on the Orange PI PC.
> It uses the internal PHY.
>
> This patch create the needed emac node.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> index f148111..746c25a 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> @@ -53,6 +53,7 @@
>
> aliases {
> serial0 = &uart0;
> + ethernet0 = &emac;
Sorry for not spotting this earlier...
This should be ordered by alphabetical order,
> };
>
> chosen {
> @@ -184,3 +185,10 @@
> /* USB VBUS is always on */
> status = "okay";
> };
> +
> +&emac {
> + phy-handle = <&int_mii_phy>;
> + phy-mode = "mii";
> + allwinner,leds-active-low;
> + status = "okay";
> +};
And the node here as well.
Almost all your other DT patches also have this issue, please fix it
in all of them.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/36268261/attachment.sig>
^ permalink raw reply
* [PATCH v4 13/18] arm64: allwinner: sun50i-a64: add dwmac-sun8i Ethernet driver
From: Maxime Ripard @ 2017-04-12 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412111400.2296-14-clabbe.montjoie@gmail.com>
On Wed, Apr 12, 2017 at 01:13:55PM +0200, Corentin Labbe wrote:
> The dwmac-sun8i is an Ethernet MAC that supports 10/100/1000 Mbit
> connections. It is very similar to the device found in the Allwinner
> H3, but lacks the internal 100 Mbit PHY and its associated control
> bits.
> This adds the necessary bits to the Allwinner A64 SoC .dtsi, but keeps
> it disabled at this level.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 37 +++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 0b0f4ab..2569827 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -287,6 +287,23 @@
> bias-pull-up;
> };
>
> + rmii_pins: rmii_pins {
> + pins = "PD10", "PD11", "PD13", "PD14",
> + "PD17", "PD18", "PD19", "PD20",
> + "PD22", "PD23";
Please align the wrapped lines on the first pin.
> + function = "emac";
> + drive-strength = <40>;
Do you actually need that for all the boards, or only a few of them?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/24b56b06/attachment.sig>
^ permalink raw reply
* [PATCH v4 14/18] arm: allwinner: pine64: Enable dwmac-sun8i
From: Maxime Ripard @ 2017-04-12 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412111400.2296-15-clabbe.montjoie@gmail.com>
On Wed, Apr 12, 2017 at 01:13:56PM +0200, Corentin Labbe wrote:
> The dwmac-sun8i hardware is present on the pine64
> It uses an external PHY via RMII.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
The architecture in your commit title in arm64, not arm (the next
patch also have this issue).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/8032d65e/attachment.sig>
^ permalink raw reply
* [PATCH v4 18/18] arm: multi_v7: Enable dwmac-sun8i driver on multi_v7_defconfig
From: Maxime Ripard @ 2017-04-12 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412111400.2296-19-clabbe.montjoie@gmail.com>
On Wed, Apr 12, 2017 at 01:14:00PM +0200, Corentin Labbe wrote:
> Enable the dwmac-sun8i driver in the multi_v7 default configuration
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Maybe we should enable that in arm64's defconfig too?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/3dd08306/attachment.sig>
^ permalink raw reply
* [PATCH] arm64: kernel: restrict /dev/mem read() calls to linear region
From: Domenico Andreoli @ 2017-04-12 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8zJqGE7bGF0DkDon2Ojhfoo76bm2ebfkqf3Cj05S5sDQ@mail.gmail.com>
On Wed, Apr 12, 2017 at 10:37:46AM +0100, Ard Biesheuvel wrote:
> On 12 April 2017 at 10:29, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Wed, Apr 12, 2017 at 10:33:43AM +0200, Domenico Andreoli wrote:
> >> On Wed, Apr 12, 2017 at 09:26:06AM +0100, Ard Biesheuvel wrote:
> >> > When running lscpu on an AArch64 system that has SMBIOS version 2.0
> >> > tables, it will segfault in the following way:
> >> >
> >> > Unable to handle kernel paging request at virtual address ffff8000bfff0000
> >> > pgd = ffff8000f9615000
> >> > [ffff8000bfff0000] *pgd=0000000000000000
> >> > Internal error: Oops: 96000007 [#1] PREEMPT SMP
> >> > Modules linked in:
> >> > CPU: 0 PID: 1284 Comm: lscpu Not tainted 4.11.0-rc3+ #103
> >> > Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
> >> > task: ffff8000fa78e800 task.stack: ffff8000f9780000
> >> > PC is at __arch_copy_to_user+0x90/0x220
> >> > LR is at read_mem+0xcc/0x140
> >> >
> >> > This is caused by the fact that lspci issues a read() on /dev/mem at the
> >> > offset where it expects to find the SMBIOS structure array. However, this
> >> > region is classified as EFI_RUNTIME_SERVICE_DATA (as per the UEFI spec),
> >> > and so it is omitted from the linear mapping.
> >> >
> >> > So let's restrict /dev/mem read/write access to those areas that are
> >> > covered by the linear region.
> >> >
> >> > Reported-by: Alexander Graf <agraf@suse.de>
> >> > Fixes: 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> > ---
> >> > arch/arm64/mm/mmap.c | 9 +++------
> >> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
> >> > index 7b0d55756eb1..2956240d17d7 100644
> >> > --- a/arch/arm64/mm/mmap.c
> >> > +++ b/arch/arm64/mm/mmap.c
> >> > @@ -18,6 +18,7 @@
> >> >
> >> > #include <linux/elf.h>
> >> > #include <linux/fs.h>
> >> > +#include <linux/memblock.h>
> >> > #include <linux/mm.h>
> >> > #include <linux/mman.h>
> >> > #include <linux/export.h>
> >> > @@ -103,12 +104,8 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
> >> > */
> >> > int valid_phys_addr_range(phys_addr_t addr, size_t size)
> >> > {
> >> > - if (addr < PHYS_OFFSET)
> >> > - return 0;
> >> > - if (addr + size > __pa(high_memory - 1) + 1)
> >> > - return 0;
> >> > -
> >> > - return 1;
> >> > + return memblock_is_map_memory(addr) &&
> >> > + memblock_is_map_memory(addr + size - 1);
> >> > }
> >> >
> >> > /*
> >>
> >> Does arch/arm/mm/mmap.c need the same treatment?
> >
> > high_memory is supposed to be the virtual address of the end of the
> > linearly mapped region. ARM conforms to that.
> >
>
> Yes, but with UEFI boot, there will be holes in the linear region.
> This is due to the fact that those regions may have special
> significance to the firmware, and may be mapped with mismatched
> attributes.
>
> As it happens, the UEFI spec recommends that SMBIOS tables (which only
> have significance to the OS) are covered by a EFI_RUNTIME_SERVICE_DATA
> region, which is one of the types we mark MEMBLOCK_NOMAP, and omit
> from the linear region.
>
> So when lspcu goes looking for the SMBIOS tables, using the read()
> interface to /dev/mem, it will dereference a physical pointer into an
> unmapped region. This will undoubtedly produce a similar result to the
> above if we don't account for NOMAP regions.
>
> However, we have no DMI/SMBIOS support yet for ARM (nor ACPI support),
> so this particular issue does not exist on ARM atm. But read() calls
> to /dev/mem may indeed provoke a similar splat when (ab)used in a
> similar manner.
I was exactly thinking at this last abuse indeed. Maybe other archs
are affected.
thanks,
Domenico
--
3B10 0CA1 8674 ACBA B4FE FCD2 CE5B CF17 9960 DE13
^ permalink raw reply
* [PATCH 1/2] dt-bindings: Document STM32 I2S bindings
From: Olivier MOYSAN @ 2017-04-12 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412113251.nfyk5sw6c5pnpmwr@sirena.org.uk>
Hello Mark,
On 04/12/2017 01:32 PM, Mark Brown wrote:
> On Wed, Apr 12, 2017 at 08:30:31AM +0000, Olivier MOYSAN wrote:
>> On 04/11/2017 11:10 PM, Mark Brown wrote:
>
>>> That sounds wrong, I'd expect this wiring to be done statically as part
>>> of the .dtsi for the SoC (or just grabbed as needed at runtime if
>>> things are flexbile enough) rather than being a configuration thing done
>>> per board... I had thought that this was configuration reflecting
>>> different ways of taping out the IP with different feature sets, is that
>>> not the case?
>
>> This configuration is board dependent. The IP may be used as rx, tx or
>> fd depending on board. So I think it can make sense to have a DMA
>> configuration linked to board, and to set IP mode accordingly.
>
> It is totally normal to just not use one direction in a given system, we
> don't normally need to do anything special to handle things. I'm a bit
> confused as to what's different here and needs configuring?
>
The IP does not provide an audio channel configured either as rx or tx.
I agree, that in such case, the cpu driver does not generally need
to worry about direction and there is nothing special required to handle it.
Here the IP provides 2 channels, 1 tx and 1 rx, which may be active or
not. The driver has to know which channel is used, and if both channels
have to be managed. The affected registers depend on selected channel.
Moreover specific processing has to be performed if both channels are used.
BRs
olivier
^ permalink raw reply
* stmmac still supporting spear600 ?
From: Thomas Petazzoni @ 2017-04-12 13:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0468e2fb-5a7d-97ec-c51c-2436a13dda69@st.com>
Hello,
Thanks again for your answer, sorry for the delay, I was away from the
spear600 board for a while.
On Mon, 3 Apr 2017 08:16:50 +0200, Giuseppe CAVALLARO wrote:
> I tested the SMSC on other platform (+ stmmac), not on SPEAr.
OK. But I believe there might be a SPEAr specific issue here, which
might explain why you don't reproduce the problem.
> ok for reset, keep the radar on clock. Hmm, can you attach a piece of
> log file to see the failure?
During the boot, nothing bad:
libphy: Fixed MDIO Bus: probed
stmmaceth e0800000.ethernet: no reset control found
stmmac - user ID: 0x10, Synopsys ID: 0x32
Ring mode enabled
DMA HW capability register supported Normal descriptors
libphy: stmmac: probed
eth0: PHY ID 0007c0c4 at 31 IRQ POLL (stmmac-0:1f) active
Then, when upping the interface:
# ifconfig eth0 up
eth0: device MAC address 00:30:d3:21:22:60
stmmaceth e0800000.ethernet: Failed to reset the dma
stmmac_hw_setup: DMA engine initialization failed
stmmac_open: Hw setup failed
SIOCSIFFLAGS: Device or resource busy
As I said earlier, the "Failed to reset the dma" is because
dwmac_dma_reset() returns -EBUSY because the DMA reset bit never
clears. Again, we see the same behavior in U-Boot (DMA reset bit never
clears), but Ethernet does work in U-Boot.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 2/7] Documentation: dt: Remove bindings for STM32 pinctrl
From: Alexandre Torgue @ 2017-04-12 13:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170410202054.urednfofxyb5iyou@rob-hp-laptop>
Hi Rob
On 04/10/2017 10:20 PM, Rob Herring wrote:
> On Fri, Apr 07, 2017 at 02:42:59PM +0200, Alexandre TORGUE wrote:
>> Remove "ngpios" bindings definition as it is no more used in stm32 pinctrl
>> driver.
>
> I read the subject as "rm
> Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt". You can
> be more specific:
>
> dt-bindings: pinctrl: remove ngpios from stm32-pinctrl binding
>
> With that,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
Thanks for review. I think that documentations patches (patch 2 & 3)
will be merged by Linus W.
Linus, do you want a V3 to fix commit header ?
Regards
Alex
^ permalink raw reply
* [PATCH 1/2] drivers: pci: do not disregard parent resources starting at 0x0
From: Lorenzo Pieralisi @ 2017-04-12 13:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411163313.18577-2-ard.biesheuvel@linaro.org>
[+Yinghai, Bjorn]
On Tue, Apr 11, 2017 at 05:33:12PM +0100, Ard Biesheuvel wrote:
> Commit f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for
> allocation") updated the logic that iterates over all bus resources
> and compares them to a given resource, in order to decide whether one
> is the parent of the latter.
>
> This change inadvertently causes pci_find_parent_resource() to disregard
> resources starting at address 0x0, resulting in an error such as the one
> below on ARM systems whose I/O window starts at 0x0.
>
> pci_bus 0000:00: root bus resource [mem 0x10000000-0x3efeffff window]
> pci_bus 0000:00: root bus resource [io 0x0000-0xffff window]
> pci_bus 0000:00: root bus resource [mem 0x8000000000-0xffffffffff window]
> pci_bus 0000:00: root bus resource [bus 00-0f]
> pci 0000:00:01.0: PCI bridge to [bus 01]
> pci 0000:00:02.0: PCI bridge to [bus 02]
> pci 0000:00:03.0: PCI bridge to [bus 03]
> pci 0000:00:03.0: can't claim BAR 13 [io 0x0000-0x0fff]: no compatible bridge window
> pci 0000:03:01.0: can't claim BAR 0 [io 0x0000-0x001f]: no compatible bridge window
>
> While this never happens on x86, it is perfectly legal in general for a
> PCI MMIO or IO window to start at address 0x0, and it was supported in
> the code before commit f44116ae8818.
>
> So let's drop the test for res->start != 0; resource_contains() already
> checks whether [start, end) completely covers the resource, and so it
> should be redundant.
>
> Fixes: f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for allocation")
I know this code fixes IO claiming on ARM/ARM64 (well, it fixes nothing
because we never claim resources on ARM/ARM64 apart from kvmtool and
generic host bridge), my _big_ worry is that it can cause endless
regressions on other arches, in any case I would be really really
careful about adding a Fixes: tag to it.
Yinghai, Bjorn ?
Thanks,
Lorenzo
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/pci/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7904d02ffdb9..53a41b1f7ef7 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -454,7 +454,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> pci_bus_for_each_resource(bus, r, i) {
> if (!r)
> continue;
> - if (res->start && resource_contains(r, res)) {
> + if (resource_contains(r, res)) {
>
> /*
> * If the window is prefetchable but the BAR is
> --
> 2.9.3
>
^ permalink raw reply
* [PATCH v3] arm64: dts: rk3399: add support for firefly-rk3399 board
From: Heiko Stuebner @ 2017-04-12 13:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491796213-4115-1-git-send-email-kever.yang@rock-chips.com>
Hi Kever,
Am Montag, 10. April 2017, 11:50:13 CEST schrieb Kever Yang:
> Firefly-rk3399 is a bord from T-Firefly, you can find detail about
> it here:
> http://en.t-firefly.com/en/firenow/Firefly_RK3399/
>
> This patch add basic node for the board and make it able to bring
> up.
>
> Peripheral works:
> - usb hub which connect to ehci controller;
> - UART2 debug
> - eMMC
> - PCIe
>
> Not work:
> - USB 3.0 HOST, type-C port
> - sdio, sd-card
>
> Not test for other peripheral:
> - HDMI
> - Ethernet
> - OPTICAL
> - WiFi/BT
> - MIPI CSI/DSI
> - IR
> - EDP/DP
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
applied for 4.13, as we're a bit late for 4.12, with the following changes:
- commit subject
- dropped status from backlight (as there is no disabled common node
and it's specific to the firefly itself)
- quite some reordering of properties
- reordered regulator nodes per their addresses: 0x1b < 0x40
- dropped obsolete regulator-compatible properties
- fixed gpio-irq on the mpu6500
- dropped out-of-tree orientation properties of mpu6500
--> please provide the optional "mount-matrix" in a follow-up patch
see bindings/iio/imu/inv_mpu6050.txt
- dropped rockchip,i2s-broken-burst-len;
That change never made it into the mainline kernel
- fixed pcie pinctrl indentation
- dropped wireless-bluetooth uart-gpios pinctrl
- dropped supports-emmc property
Please try to be a bit more careful when porting stuff from device kernels
with respect to properties not found in the mainline kernel and please
also double-check in [0] that I didn't break anything.
Thanks
Heiko
[0] https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=495a3c891a696b62465d71b1a125e3424352028b
^ permalink raw reply
* [PATCH] serial: imx: Enable RTSD only when needed
From: Romain Perier @ 2017-04-12 13:30 UTC (permalink / raw)
To: linux-arm-kernel
From: Nandor Han <nandor.han@ge.com>
Currently, this IRQ is always enabled. Some devices might mux these pins
to other I/Os, like I2C. This could lead to spurious interrupts.
This commit makes this IRQ optional, by using the field have_rtscts.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/tty/serial/imx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e3e152cb..6dd674f 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1294,7 +1294,9 @@ static int imx_startup(struct uart_port *port)
imx_enable_dma(sport);
temp = readl(sport->port.membase + UCR1);
- temp |= UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN;
+ temp |= UCR1_RRDYEN | UCR1_UARTEN;
+ if (sport->have_rtscts)
+ temp |= UCR1_RTSDEN;
writel(temp, sport->port.membase + UCR1);
--
2.9.3
^ permalink raw reply related
* [PATCH v2 2/2] dt-bindings: document: add firefly-rk3399 board support
From: Heiko Stuebner @ 2017-04-12 13:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491384800-22412-2-git-send-email-kever.yang@rock-chips.com>
Hi Kever,
Am Mittwoch, 5. April 2017, 17:33:20 CEST schrieb Kever Yang:
> Use "firefly,firefly-rk3399" compatible string for firefly-rk3399 board.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
you forgot to send the binding in your v3 series, so I've applied this
one for v4.13 with Rob's Ack.
Heiko
^ permalink raw reply
* [PATCH v2 3/7] includes: dt-bindings: Rename STM32F429 pinctrl DT bindings
From: Alexandre Torgue @ 2017-04-12 13:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170410202736.zy7naw74hvrdmfzk@rob-hp-laptop>
Hi Rob,
On 04/10/2017 10:27 PM, Rob Herring wrote:
> On Fri, Apr 07, 2017 at 02:43:00PM +0200, Alexandre TORGUE wrote:
>> STM32F4 MCU series is composed of several SOC (STM32F429, STM32F469, ...).
>> Most of muxing definition are identical. So to avoid to duplicate bindings
>> definition, this patch create common definitions.
>
> This is a lot of churn. Some confirmation that the resultant dtb is the
> same before and after would be nice. Perhaps the script you used to
> convert this as well.
I tried to use fdtdump but it seems bugged. So I used directly dtc
binary to (re)generate dts files (before and after apply the series) and
I compared "pinmux" field in both case.
Example on stm32f469-disco:
./scripts/dtc/dtc -I dtb -O dts -o stm32f469-disco-after.dts
stm32f469-disco-after.dtb
./scripts/dtc/dtc -I dtb -O dts -o stm32f469-disco-before.dts
stm32f469-disco-before.dtb
cat stm32f469-disco-after.dts | grep pinmux
cat stm32f469-disco-before.dts | grep pinmux
regards
alex
>
> Rob
>
^ permalink raw reply
* [PATCH V14 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Borislav Petkov @ 2017-04-12 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1490729440-32591-3-git-send-email-tbaicar@codeaurora.org>
> Subject: [PATCH V14 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
Use a verb in your patch subjects: "Add support for ..." or so.
On Tue, Mar 28, 2017 at 01:30:32PM -0600, Tyler Baicar wrote:
> Currently when a RAS error is reported it is not timestamped.
What is a RAS error? You mean a hardware error?
> The ACPI 6.1 spec adds the timestamp field to the generic error
> data entry v3 structure. The timestamp of when the firmware
> generated the error is now being reported.
So what this patch does doesn't have a lot to to do with the Subject?
Please state what the patch does in the Subject.
Also, your commit message talks about adding timestamp but the patch
does more. You need to state that too and explain what this patch does
actually.
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Reviewed-by: James Morse <james.morse@arm.com>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/acpi/apei/ghes.c | 9 ++++---
> drivers/firmware/efi/cper.c | 63 +++++++++++++++++++++++++++++++++++----------
> include/acpi/ghes.h | 22 ++++++++++++++++
> 3 files changed, 77 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 0241e36..9ddbb93 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -421,7 +421,8 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
> int flags = -1;
> int sec_sev = ghes_severity(gdata->error_severity);
> struct cper_sec_mem_err *mem_err;
> - mem_err = (struct cper_sec_mem_err *)(gdata + 1);
> +
> + mem_err = acpi_hest_generic_data_payload(gdata);
>
> if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
> return;
> @@ -458,7 +459,8 @@ static void ghes_do_proc(struct ghes *ghes,
> if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> CPER_SEC_PLATFORM_MEM)) {
> struct cper_sec_mem_err *mem_err;
> - mem_err = (struct cper_sec_mem_err *)(gdata+1);
> +
> + mem_err = acpi_hest_generic_data_payload(gdata);
> ghes_edac_report_mem_error(ghes, sev, mem_err);
>
> arch_apei_report_mem_error(sev, mem_err);
> @@ -468,7 +470,8 @@ static void ghes_do_proc(struct ghes *ghes,
> else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> CPER_SEC_PCIE)) {
> struct cper_sec_pcie *pcie_err;
> - pcie_err = (struct cper_sec_pcie *)(gdata+1);
> +
> + pcie_err = acpi_hest_generic_data_payload(gdata);
> if (sev == GHES_SEV_RECOVERABLE &&
> sec_sev == GHES_SEV_RECOVERABLE &&
> pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index d425374..8fa4e23 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -32,6 +32,9 @@
> #include <linux/acpi.h>
> #include <linux/pci.h>
> #include <linux/aer.h>
> +#include <linux/printk.h>
> +#include <linux/bcd.h>
> +#include <acpi/ghes.h>
>
> #define INDENT_SP " "
>
> @@ -386,13 +389,37 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
> pfx, pcie->bridge.secondary_status, pcie->bridge.control);
> }
>
> +static void cper_estatus_print_section_v300(const char *pfx,
> + const struct acpi_hest_generic_data_v300 *gdata)
Yuck, acpi_hest_generic_data_v300. Can we make those struct names smaller pls?
And v300 is just silly.
And then align args at opening brace.
> +{
> + __u8 hour, min, sec, day, mon, year, century, *timestamp;
> +
> + if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
> + timestamp = (__u8 *)&(gdata->time_stamp);
> + sec = bcd2bin(timestamp[0]);
> + min = bcd2bin(timestamp[1]);
> + hour = bcd2bin(timestamp[2]);
> + day = bcd2bin(timestamp[4]);
> + mon = bcd2bin(timestamp[5]);
> + year = bcd2bin(timestamp[6]);
> + century = bcd2bin(timestamp[7]);
Align those vertically on the = sign.
> + printk("%stime: %7s %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
> + 0x01 & *(timestamp + 3) ? "precise" : "",
Move that test in a separate if-statement - that printk is unreadable as
it is. Also, the test bit always comes second.
> century,
> + year, mon, day, hour, min, sec);
> + }
> +}
> +
> static void cper_estatus_print_section(
> - const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
> + const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
static void
cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata,
int sec_no)
looks much better.
> {
> uuid_le *sec_type = (uuid_le *)gdata->section_type;
> __u16 severity;
> char newpfx[64];
>
> + if (acpi_hest_generic_data_version(gdata) >= 3)
Jeez, that macro name is like a one-lined book!
Let's make that "hest_gdata_ver()" or something else shorter.
> + cper_estatus_print_section_v300(pfx,
> + (const struct acpi_hest_generic_data_v300 *)gdata);
> +
> severity = gdata->error_severity;
> printk("%s""Error %d, type: %s\n", pfx, sec_no,
> cper_severity_str(severity));
> @@ -403,14 +430,18 @@ static void cper_estatus_print_section(
>
> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
> - struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
> + struct cper_sec_proc_generic *proc_err;
> +
> + proc_err = acpi_hest_generic_data_payload(gdata);
This looks like an unrelated change. The payload function addition and the
conversion of the code to use it should be a separate patch. And shorten that
function name too pls.
> printk("%s""section_type: general processor error\n", newpfx);
> if (gdata->error_data_length >= sizeof(*proc_err))
> cper_print_proc_generic(newpfx, proc_err);
> else
> goto err_section_too_small;
> } else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
> - struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
> + struct cper_sec_mem_err *mem_err;
> +
> + mem_err = acpi_hest_generic_data_payload(gdata);
> printk("%s""section_type: memory error\n", newpfx);
> if (gdata->error_data_length >=
> sizeof(struct cper_sec_mem_err_old))
> @@ -419,7 +450,9 @@ static void cper_estatus_print_section(
> else
> goto err_section_too_small;
> } else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
> - struct cper_sec_pcie *pcie = (void *)(gdata + 1);
> + struct cper_sec_pcie *pcie;
> +
> + pcie = acpi_hest_generic_data_payload(gdata);
> printk("%s""section_type: PCIe error\n", newpfx);
> if (gdata->error_data_length >= sizeof(*pcie))
> cper_print_pcie(newpfx, pcie, gdata);
> @@ -438,7 +471,7 @@ void cper_estatus_print(const char *pfx,
> const struct acpi_hest_generic_status *estatus)
> {
> struct acpi_hest_generic_data *gdata;
> - unsigned int data_len, gedata_len;
> + unsigned int data_len;
> int sec_no = 0;
> char newpfx[64];
> __u16 severity;
> @@ -451,12 +484,13 @@ void cper_estatus_print(const char *pfx,
> printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
> data_len = estatus->data_length;
> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
> +
> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> - while (data_len >= sizeof(*gdata)) {
> - gedata_len = gdata->error_data_length;
> +
> + while (data_len >= acpi_hest_generic_data_size(gdata)) {
> cper_estatus_print_section(newpfx, gdata, sec_no);
> - data_len -= gedata_len + sizeof(*gdata);
> - gdata = (void *)(gdata + 1) + gedata_len;
> + data_len -= acpi_hest_generic_data_record_size(gdata);
> + gdata = acpi_hest_generic_data_next(gdata);
> sec_no++;
> }
> }
> @@ -486,12 +520,13 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
> return rc;
> data_len = estatus->data_length;
> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
> - while (data_len >= sizeof(*gdata)) {
> - gedata_len = gdata->error_data_length;
> - if (gedata_len > data_len - sizeof(*gdata))
> +
> + while (data_len >= acpi_hest_generic_data_size(gdata)) {
> + gedata_len = acpi_hest_generic_data_error_length(gdata);
> + if (gedata_len > data_len - acpi_hest_generic_data_size(gdata))
> return -EINVAL;
> - data_len -= gedata_len + sizeof(*gdata);
> - gdata = (void *)(gdata + 1) + gedata_len;
> + data_len -= gedata_len + acpi_hest_generic_data_size(gdata);
> + gdata = acpi_hest_generic_data_next(gdata);
> }
> if (data_len)
> return -EINVAL;
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 68f088a..6ae318b 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -12,6 +12,18 @@
> #define GHES_TO_CLEAR 0x0001
> #define GHES_EXITING 0x0002
>
> +#define acpi_hest_generic_data_error_length(gdata) \
> + (((struct acpi_hest_generic_data *)(gdata))->error_data_length)
> +#define acpi_hest_generic_data_size(gdata) \
> + ((acpi_hest_generic_data_version(gdata) >= 3) ? \
> + sizeof(struct acpi_hest_generic_data_v300) : \
> + sizeof(struct acpi_hest_generic_data))
> +#define acpi_hest_generic_data_record_size(gdata) \
> + (acpi_hest_generic_data_size(gdata) + \
> + acpi_hest_generic_data_error_length(gdata))
> +#define acpi_hest_generic_data_next(gdata) \
> + ((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
This is one unreadable pile of too long names with a clearly redundant
and too long prefix. Please shorten it all.
> +
> struct ghes {
> union {
> struct acpi_hest_generic *generic;
> @@ -73,3 +85,13 @@ static inline void ghes_edac_unregister(struct ghes *ghes)
> {
> }
> #endif
> +
> +#define acpi_hest_generic_data_version(gdata) \
> + (gdata->revision >> 8)
> +
> +static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data *gdata)
Lemme try to shorten it:
static inline void *acpi_hest_get_payload(struct acpi_hest_gdata *d)
{
if (hest_gdata_ver(d) >= 3)
return (void *)(((struct acpi_hest_gdata_v3 *)d) + 1);
else
return d + 1;
}
Now this is much more readable IMO. You can actually see what's going
on. And you still know what the struct names are.
So let's drop all that unnecessary too long prefixing and make the
code readable. That cper thing needs a lot more scrubbing, of course,
but some other day.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* [PATCH V2] clk: hi6220: Add the hi655x's pmic clock
From: Lee Jones @ 2017-04-12 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c1dba250-a60c-febc-a5c3-5ba645a61ce0@linaro.org>
On Wed, 12 Apr 2017, Daniel Lezcano wrote:
> On 12/04/2017 10:00, Lee Jones wrote:
> > On Tue, 11 Apr 2017, Daniel Lezcano wrote:
> >
> >> On Tue, Apr 11, 2017 at 03:06:13PM +0100, Lee Jones wrote:
> >>> On Sat, 08 Apr 2017, Daniel Lezcano wrote:
> >>>
> >>>> The hi655x multi function device is a PMIC providing regulators.
> >>>>
> >>>> The PMIC also provides a clock for the WiFi and the Bluetooth, let's implement
> >>>> this clock in order to add it in the hi655x MFD and allow proper wireless
> >>>> initialization.
> >>>>
> >>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> [ ... ]
>
> >> Do you want to drop it from your tree or shall I resubmit a V3 without the mfd
> >> change?
> >
> > The latter please.
>
> Ok.
>
> What about the DT binding change?
If I haven't merged it already, you'll need to re-send that
(separately) too.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] serial: imx: Enable RTSD only when needed
From: Fabio Estevam @ 2017-04-12 13:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412133015.23185-1-romain.perier@collabora.com>
On Wed, Apr 12, 2017 at 10:30 AM, Romain Perier
<romain.perier@collabora.com> wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> Currently, this IRQ is always enabled. Some devices might mux these pins
> to other I/Os, like I2C. This could lead to spurious interrupts.
>
> This commit makes this IRQ optional, by using the field have_rtscts.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-12 13:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412094454.GA31143@red-moon>
On Wed, 2017-04-12 at 10:44 +0100, Lorenzo Pieralisi wrote:
> > This is a problem to be solved by the bridge itself. If ARM has a
> > mapping attribute to make stores non-posted, keep this an ARM specific
> > attribute at this stage I'd say.
>
> I can't. Some PCI host bridge drivers (DT) are shared between ARM/ARM64
> and live in drivers (and are also reused on some arches eg microblaze)
> and they use ioremap() to map config space.
>
> So the idea behind this series was to add an interface (that is
> overriden on ARM/ARM64), it started as a PCI specific interface (v1) and
> evolved to ioremap_nopost() (v2).
Then provide some kind of HAVE_* or config option indicating that this
mapping attribute exist so the bridge code at least has the option of
implementing an "alternative" mechanism when it doesn't rather than
silently being wrong :-)
(Or just return NULL, that works too)
Cheers,
Ben.
^ permalink raw reply
* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-12 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412113124.GZ17774@n2100.armlinux.org.uk>
On Wed, 2017-04-12 at 12:31 +0100, Russell King - ARM Linux wrote:
> default implementation should fail if it's not supportable on all
> architectures.? However, when we have existing drivers using an
> interface that doesn't provide the semantics they already require,
> then it makes no sense to effectively break these drivers on a range
> of existing architectures.
>
> The question really is - what's the best way to solve the problem
> with
> existing drivers without breaking them.? I suspect that, sadly, the
> only realistic way forward here is via the litter-drivers-with-ifdefs
> approach since you don't like providing a default implementation that
> is compatible with what these drivers are already doing.
Then make ioremap_nopost return NULL when the arch doesn't have
the right semantic.
The driver than can *chose* to either silently fallback to ioremap,
which has served us well for a long time despite being theorically in
violation of the spec, or do funny things like read back some register
after every config write to ensure ordering etc...
I much prefer that approach than having some generic ioremap function
that exposes a semantic that silently provides a weaker one on some
architecture.
At least we make the failure explicit, and the driver can take
alternate (possibly sub-optimal) action if it chooses to do so.
Cheers,
Ben.
^ permalink raw reply
* [PATCH 2/7] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver
From: Robin Murphy @ 2017-04-12 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170329124432.27457-3-antoine.tenart@free-electrons.com>
Hi Antoine,
Bit of a drive-by, but since I have it in my head that crypto drivers
are a hotspot for dodgy DMA usage (in part due to the hardware often
being a bit esoteric with embedded RAMs and such), this caught my eye
and I thought I'd give this a quick once-over to check for anything
smelly. Unfortunately, I was not disappointed... ;)
On 29/03/17 13:44, Antoine Tenart wrote:
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
> new file mode 100644
> index 000000000000..00f3f2c85d05
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel.c
[...]
> +int safexcel_invalidate_cache(struct crypto_async_request *async,
> + struct safexcel_context *ctx,
> + struct safexcel_crypto_priv *priv,
> + dma_addr_t ctxr_dma, int ring,
> + struct safexcel_request *request)
> +{
> + struct safexcel_command_desc *cdesc;
> + struct safexcel_result_desc *rdesc;
> + phys_addr_t ctxr_phys;
> + int ret = 0;
> +
> + ctxr_phys = dma_to_phys(priv->dev, ctxr_dma);
No no no. This is a SWIOTLB-specific (or otherwise arch-private,
depending on implementation) helper, not a DMA API function, and should
not be called directly by drivers. There is no guarantee it will give
the result you expect, or even compile, in all cases (if the kbuild
robot hasn't already revealed via the COMPILE_TEST dependency).
That said, AFAICS ctxr_phys ends up in a descriptor which is given to
the device, so trying to use a physical address is wrong and it should
still be the DMA address anyway.
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* prepare command descriptor */
> + cdesc = safexcel_add_cdesc(priv, ring, true, true, 0, 0, 0, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + ret = PTR_ERR(cdesc);
> + goto unlock;
> + }
> +
> + cdesc->control_data.type = EIP197_TYPE_EXTENDED;
> + cdesc->control_data.options = 0;
> + cdesc->control_data.refresh = 0;
> + cdesc->control_data.control0 = CONTEXT_CONTROL_INV_TR;
> +
> + /* prepare result descriptor */
> + rdesc = safexcel_add_rdesc(priv, ring, true, true, 0, 0);
> +
> + if (IS_ERR(rdesc)) {
> + ret = PTR_ERR(rdesc);
> + goto cdesc_rollback;
> + }
> +
> + request->req = async;
> + goto unlock;
> +
> +cdesc_rollback:
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +
> +unlock:
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> + return ret;
> +}
[...]
> +static int safexcel_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct safexcel_crypto_priv *priv;
> + int i, ret;
> +
> + priv = devm_kzalloc(dev, sizeof(struct safexcel_crypto_priv),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(priv->base)) {
> + dev_err(dev, "failed to get resource\n");
> + return PTR_ERR(priv->base);
> + }
> +
> + priv->clk = of_clk_get(dev->of_node, 0);
> + if (!IS_ERR(priv->clk)) {
> + ret = clk_prepare_enable(priv->clk);
> + if (ret) {
> + dev_err(dev, "unable to enable clk (%d)\n", ret);
> + return ret;
> + }
> + } else {
> + /* The clock isn't mandatory */
> + if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + }
You should call dma_set_mask_and_coherent() before any DMA API calls,
both to confirm DMA really is going to work all, and also (since this IP
apparently supports >32-bit addresses) to describe the full inherent
addressing capability, not least to avoid wasting time/space with
unnecessary bounce buffering otherwise.
> + priv->context_pool = dmam_pool_create("safexcel-context", dev,
> + sizeof(struct safexcel_context_record),
> + 1, 0);
> + if (!priv->context_pool) {
> + ret = -ENOMEM;
> + goto err_clk;
> + }
> +
> + safexcel_configure(priv);
> +
> + for (i = 0; i < priv->config.rings; i++) {
> + char irq_name[6] = {0}; /* "ringX\0" */
> + char wq_name[9] = {0}; /* "wq_ringX\0" */
> + int irq;
> + struct safexcel_ring_irq_data *ring_irq;
> +
> + ret = safexcel_init_ring_descriptors(priv,
> + &priv->ring[i].cdr,
> + &priv->ring[i].rdr);
> + if (ret)
> + goto err_pool;
> +
> + ring_irq = devm_kzalloc(dev, sizeof(struct safexcel_ring_irq_data),
> + GFP_KERNEL);
> + if (!ring_irq) {
> + ret = -ENOMEM;
> + goto err_pool;
> + }
> +
> + ring_irq->priv = priv;
> + ring_irq->ring = i;
> +
> + snprintf(irq_name, 6, "ring%d", i);
> + irq = safexcel_request_ring_irq(pdev, irq_name, safexcel_irq_ring,
> + ring_irq);
> +
> + if (irq < 0)
> + goto err_pool;
> +
> + priv->ring[i].work_data.priv = priv;
> + priv->ring[i].work_data.ring = i;
> + INIT_WORK(&priv->ring[i].work_data.work, safexcel_handle_result_work);
> +
> + snprintf(wq_name, 9, "wq_ring%d", i);
> + priv->ring[i].workqueue = create_singlethread_workqueue(wq_name);
> + if (!priv->ring[i].workqueue) {
> + ret = -ENOMEM;
> + goto err_pool;
> + }
> +
> + INIT_LIST_HEAD(&priv->ring[i].list);
> + spin_lock_init(&priv->ring[i].lock);
> + spin_lock_init(&priv->ring[i].egress_lock);
> + }
> +
> + platform_set_drvdata(pdev, priv);
> + atomic_set(&priv->ring_used, 0);
> +
> + spin_lock_init(&priv->lock);
> + crypto_init_queue(&priv->queue, EIP197_DEFAULT_RING_SIZE);
> +
> + ret = safexcel_hw_init(priv);
> + if (ret) {
> + dev_err(dev, "EIP h/w init failed (%d)\n", ret);
> + goto err_pool;
> + }
> +
> + ret = safexcel_register_algorithms(priv);
> + if (ret) {
> + dev_err(dev, "Failed to register algorithms (%d)\n", ret);
> + goto err_pool;
> + }
> +
> + return 0;
> +
> +err_pool:
> + dma_pool_destroy(priv->context_pool);
You used dmam_pool_create(), so not only is an explicit destroy
unnecessary, but doing so with the non-managed version is actively
harmful, since devres would end up double-freeing the pool after you return.
> +err_clk:
> + clk_disable_unprepare(priv->clk);
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
> new file mode 100644
> index 000000000000..ffe39aaabe1c
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel.h
[...]
> +/* Result data */
> +struct result_data_desc {
> + u32 packet_length:17;
> + u32 error_code:15;
> +
> + u8 bypass_length:4;
> + u8 e15:1;
> + u16 rsvd0;
> + u8 hash_bytes:1;
> + u8 hash_length:6;
> + u8 generic_bytes:1;
> + u8 checksum:1;
> + u8 next_header:1;
> + u8 length:1;
> +
> + u16 application_id;
> + u16 rsvd1;
> +
> + u32 rsvd2;
> +} __packed;
> +
> +
> +/* Basic Result Descriptor format */
> +struct safexcel_result_desc {
> + u32 particle_size:17;
> + u8 rsvd0:3;
> + u8 descriptor_overflow:1;
> + u8 buffer_overflow:1;
> + u8 last_seg:1;
> + u8 first_seg:1;
> + u16 result_size:8;
> +
> + u32 rsvd1;
> +
> + u32 data_lo;
> + u32 data_hi;
> +
> + struct result_data_desc result_data;
> +} __packed;
> +
> +struct safexcel_token {
> + u32 packet_length:17;
> + u8 stat:2;
> + u16 instructions:9;
> + u8 opcode:4;
> +} __packed;
Using bitfields in hardware descriptors seems pretty risky, since you've
no real guarantee two compilers will lay them out the same way (or that
either would be correct WRT what the hardware expects). I'd be more
inclined to construct all these fields with explicit masking and shifting.
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> new file mode 100644
> index 000000000000..ff42e45ae43e
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
[...]
> +static int safexcel_aes_send(struct crypto_async_request *async,
> + int ring, struct safexcel_request *request,
> + int *commands, int *results)
> +{
> + struct ablkcipher_request *req = ablkcipher_request_cast(async);
> + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
> + struct safexcel_crypto_priv *priv = ctx->priv;
> + struct safexcel_command_desc *cdesc;
> + struct safexcel_result_desc *rdesc;
> + struct scatterlist *sg;
> + phys_addr_t ctxr_phys;
> + int nr_src, nr_dst, n_cdesc = 0, n_rdesc = 0, queued = req->nbytes;
> + int i, ret = 0;
> +
> + request->req = &req->base;
> +
> + if (req->src == req->dst) {
> + nr_src = sg_nents_for_len(req->src, req->nbytes);
> + nr_dst = nr_src;
> +
> + if (dma_map_sg(priv->dev, req->src, nr_src, DMA_BIDIRECTIONAL) <= 0)
Nit: you only need to check for zero/nonzero to determine failure
(similarly below) - dma_map_sg() cannot return negative values.
Bigger complaint: you should not ignore the successful return value and
rely on it being equal to nr_src - please see the description of
dma_map_sg() in Documenatation/DMA-API.txt
> + return -EINVAL;
> + } else {
> + nr_src = sg_nents_for_len(req->src, req->nbytes);
> + nr_dst = sg_nents_for_len(req->dst, req->nbytes);
> +
> + if (dma_map_sg(priv->dev, req->src, nr_src, DMA_TO_DEVICE) <= 0)
> + return -EINVAL;
> +
> + if (dma_map_sg(priv->dev, req->dst, nr_dst, DMA_FROM_DEVICE) <= 0) {
> + dma_unmap_sg(priv->dev, req->src, nr_src,
> + DMA_TO_DEVICE);
> + return -EINVAL;
> + }
> + }
> +
> + ctxr_phys = dma_to_phys(priv->dev, ctx->base.ctxr_dma);
Once again, wrong and probably unnecessary.
> + memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);
> +
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* command descriptors */
> + for_each_sg(req->src, sg, nr_src, i) {
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
> + int len = sg_dma_len(sg);
If dma_map_sg() coalesced any entries into the same mapping such that
count < nents, these could well give bogus values toward the end of the
list once i >= count(if you're lucky, at least len might be 0).
> + /* Do not overflow the request */
> + if (queued - len < 0)
> + len = queued;
> +
> + cdesc = safexcel_add_cdesc(priv, ring, !n_cdesc, !(queued - len),
> + sg_phys, len, req->nbytes, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + /* No space left in the command descriptor ring */
> + ret = PTR_ERR(cdesc);
> + goto cdesc_rollback;
> + }
> + n_cdesc++;
> +
> + if (n_cdesc == 1) {
> + safexcel_context_control(ctx, cdesc);
> + safexcel_cipher_token(ctx, async, cdesc, req->nbytes);
> + }
> +
> + queued -= len;
> + if (!queued)
> + break;
> + }
> +
> + /* result descriptors */
> + for_each_sg(req->dst, sg, nr_dst, i) {
> + bool first = !i, last = (i == nr_dst - 1);
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
And again, no. Also note that sg_phys() already exists as a function, so
I imagine sparse could get quite cross here.
> + u32 len = sg_dma_len(sg);
Plus the same comment again about potentially having iterated past the
point where these are valid.
> +
> + rdesc = safexcel_add_rdesc(priv, ring, first, last, sg_phys, len);
> + if (IS_ERR(rdesc)) {
> + /* No space left in the result descriptor ring */
> + ret = PTR_ERR(rdesc);
> + goto rdesc_rollback;
> + }
> + n_rdesc++;
> + }
> +
> + ctx->base.handle_result = safexcel_handle_result;
> +
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + *commands = n_cdesc;
> + *results = nr_dst;
> + return 0;
> +
> +rdesc_rollback:
> + for (i = 0; i < n_rdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].rdr);
> +cdesc_rollback:
> + for (i = 0; i < n_cdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + if (req->src == req->dst) {
> + dma_unmap_sg(priv->dev, req->src, nr_src, DMA_BIDIRECTIONAL);
> + } else {
> + dma_unmap_sg(priv->dev, req->src, nr_src, DMA_TO_DEVICE);
> + dma_unmap_sg(priv->dev, req->dst, nr_dst, DMA_FROM_DEVICE);
I will note that the unmap_sg calls *should* have the same nents value
as originally passed to map_sg, so these remain correct.
> + }
> +
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
> new file mode 100644
> index 000000000000..1f44e0a2ddf1
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
[...]
> +static int safexcel_ahash_send(struct crypto_async_request *async, int ring,
> + struct safexcel_request *request, int *commands,
> + int *results)
> +{
> + struct ahash_request *areq = ahash_request_cast(async);
> + struct crypto_ahash *ahash = crypto_ahash_reqtfm(areq);
> + struct safexcel_ahash_req *req = ahash_request_ctx(areq);
> + struct safexcel_ahash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(areq));
> + struct safexcel_crypto_priv *priv = ctx->priv;
> + struct safexcel_command_desc *cdesc, *first_cdesc = NULL;
> + struct safexcel_result_desc *rdesc;
> + struct scatterlist *sg;
> + int i, nents, queued, len, cache_len, extra, n_cdesc = 0, ret = 0;
> + phys_addr_t ctxr_phys = 0;
> +
> + queued = len = req->len - req->processed;
> + if (queued < crypto_ahash_blocksize(ahash))
> + cache_len = queued;
> + else
> + cache_len = queued - areq->nbytes;
> +
> + /*
> + * If this is not the last request and the queued data does not fit
> + * into full blocks, cache it for the next send() call.
> + */
> + extra = queued & (crypto_ahash_blocksize(ahash) - 1);
> + if (!req->last_req && extra) {
> + sg_pcopy_to_buffer(areq->src, sg_nents(areq->src),
> + req->cache_next, extra, areq->nbytes - extra);
> +
> + queued -= extra;
> + len -= extra;
> + }
> +
> + request->req = &areq->base;
> + ctx->base.handle_result = safexcel_handle_result;
> + ctxr_phys = dma_to_phys(priv->dev, ctx->base.ctxr_dma);
No.
> + spin_lock_bh(&priv->ring[ring].egress_lock);
> +
> + /* Add a command descriptor for the cached data, if any */
> + if (cache_len) {
> + ctx->base.cache_dma = dma_map_single(priv->dev, req->cache,
This is pretty dodgy, since req->cache is inside a live data structure,
adjoining parts of which are updated whilst still mapped (the update of
req->processed below). You just about get away without data corruption
since we *probably* don't invalidate anything when unmapping
DMA_TO_DEVICE, and the coherency hardware *probably* doesn't do anything
crazy, but you've no real guarantee of that - any DMA buffer should
really be separately kmalloced. "That's a nice dirty cache line you've
got there, it'd be a shame if anything were to happen to it..."
> + cache_len, DMA_TO_DEVICE);
> + if (dma_mapping_error(priv->dev, ctx->base.cache_dma)) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ctx->base.cache_sz = cache_len;
> + first_cdesc = safexcel_add_cdesc(priv, ring, 1,
> + (cache_len == len),
> + dma_to_phys(priv->dev, ctx->base.cache_dma),
No.
> + cache_len, len, ctxr_phys);
> + if (IS_ERR(first_cdesc)) {
> + ret = PTR_ERR(first_cdesc);
> + goto free_cache;
> + }
> + n_cdesc++;
> +
> + queued -= cache_len;
> + if (!queued)
> + goto send_command;
> + }
> +
> + /* Now handle the current ahash request buffer(s) */
> + nents = sg_nents_for_len(areq->src, areq->nbytes);
> + if (dma_map_sg(priv->dev, areq->src, nents, DMA_TO_DEVICE) <= 0) {
Same comments about the return value.
> + ret = -ENOMEM;
> + goto cdesc_rollback;
> + }
> +
> + for_each_sg(areq->src, sg, nents, i) {
> + phys_addr_t sg_phys = dma_to_phys(priv->dev, sg_dma_address(sg));
No.
> + int sglen = sg_dma_len(sg);
> +
> + /* Do not overflow the request */
> + if (queued - sglen < 0)
> + sglen = queued;
> +
> + cdesc = safexcel_add_cdesc(priv, ring, !n_cdesc,
> + !(queued - sglen), sg_phys, sglen,
> + len, ctxr_phys);
> + if (IS_ERR(cdesc)) {
> + ret = PTR_ERR(cdesc);
> + goto cdesc_rollback;
> + }
> + n_cdesc++;
> +
> + if (n_cdesc == 1)
> + first_cdesc = cdesc;
> +
> + queued -= sglen;
> + if (!queued)
> + break;
> + }
> +
> +send_command:
> + /* Setup the context options */
> + safexcel_context_control(ctx, req, first_cdesc, req->state_sz,
> + crypto_ahash_blocksize(ahash));
> +
> + /* Add the token */
> + safexcel_hash_token(first_cdesc, len, req->state_sz);
> +
> + ctx->base.result_dma = dma_map_single(priv->dev, areq->result,
> + req->state_sz, DMA_FROM_DEVICE);
> + if (dma_mapping_error(priv->dev, ctx->base.result_dma)) {
> + ret = -EINVAL;
> + goto cdesc_rollback;
> + }
> +
> + /* Add a result descriptor */
> + rdesc = safexcel_add_rdesc(priv, ring, 1, 1,
> + dma_to_phys(priv->dev, ctx->base.result_dma),
> + req->state_sz);
> + if (IS_ERR(rdesc)) {
> + ret = PTR_ERR(rdesc);
> + goto cdesc_rollback;
> + }
> +
> + req->processed += len;
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> +
> + *commands = n_cdesc;
> + *results = 1;
> + return 0;
> +
> +cdesc_rollback:
> + for (i = 0; i < n_cdesc; i++)
> + safexcel_ring_rollback_wptr(priv, &priv->ring[ring].cdr);
> +free_cache:
> + if (ctx->base.cache_dma)
> + dma_unmap_single(priv->dev, ctx->base.cache_dma,
> + ctx->base.cache_sz, DMA_TO_DEVICE);
> +
> +unlock:
> + spin_unlock_bh(&priv->ring[ring].egress_lock);
> + return ret;
> +}
[...]
> diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
> new file mode 100644
> index 000000000000..5b4ba7a8cc0a
> --- /dev/null
> +++ b/drivers/crypto/inside-secure/safexcel_ring.c
[...]
> +int safexcel_init_ring_descriptors(struct safexcel_crypto_priv *priv,
> + struct safexcel_ring *cdr,
> + struct safexcel_ring *rdr)
> +{
> + cdr->offset = sizeof(u32) * priv->config.cd_offset;
> + cdr->base = dmam_alloc_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + &cdr->base_dma, GFP_KERNEL);
> + if (!cdr->base)
> + return -ENOMEM;
> + cdr->write = cdr->base;
> + cdr->base_end = cdr->base + cdr->offset * EIP197_DEFAULT_RING_SIZE;
> + cdr->read = cdr->base;
> +
> + rdr->offset = sizeof(u32) * priv->config.rd_offset;
> + rdr->base = dmam_alloc_coherent(priv->dev,
> + rdr->offset * EIP197_DEFAULT_RING_SIZE,
> + &rdr->base_dma, GFP_KERNEL);
> + if (!rdr->base) {
> + dmam_free_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + cdr->base, cdr->base_dma);
Returning an error here will abort your probe routine, so devres will
clean these up there and then - there's no need to do free anything
explicitly. That's rather the point of using devm_*() to begin with.
> + return -ENOMEM;
> + }
> + rdr->write = rdr->base;
> + rdr->base_end = rdr->base + rdr->offset * EIP197_DEFAULT_RING_SIZE;
> + rdr->read = rdr->base;
> +
> + return 0;
> +}
> +
> +void safexcel_free_ring_descriptors(struct safexcel_crypto_priv *priv,
> + struct safexcel_ring *cdr,
> + struct safexcel_ring *rdr)
> +{
> + dmam_free_coherent(priv->dev,
> + cdr->offset * EIP197_DEFAULT_RING_SIZE,
> + cdr->base, cdr->base_dma);
> + dmam_free_coherent(priv->dev,
> + rdr->offset * EIP197_DEFAULT_RING_SIZE,
> + rdr->base, rdr->base_dma);
> +}
Again, this is only called at device teardown, so the whole function is
redundant.
Robin.
^ permalink raw reply
* rfc: [PATCH v2] gpio: Allow hogged gpios to be requested
From: Linus Walleij @ 2017-04-12 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9805e56d-ebc9-c95b-9f5d-194a2de40899@rempel-privat.de>
On Wed, Apr 12, 2017 at 10:59 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
> i need to take over the work on this patch set and need to know if it still
> upstreamable:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/364227.html
Initial values are still OK I guess, just as I describe.
But primariliy write to linux-gpio at vger.kernel.org.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] Revert "arm64: Increase the max granular size"
From: Chalamarla, Tirumalesh @ 2017-04-12 14:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <08fa98de-760b-15bc-5220-fa449b08c118@codeaurora.org>
On 4/11/17, 10:13 PM, "linux-arm-kernel on behalf of Imran Khan" <linux-arm-kernel-bounces at lists.infradead.org on behalf of kimran@codeaurora.org> wrote:
On 4/7/2017 7:36 AM, Ganesh Mahendran wrote:
> 2017-04-06 23:58 GMT+08:00 Catalin Marinas <catalin.marinas@arm.com>:
>> On Thu, Apr 06, 2017 at 12:52:13PM +0530, Imran Khan wrote:
>>> On 4/5/2017 10:13 AM, Imran Khan wrote:
>>>>> We may have to revisit this logic and consider L1_CACHE_BYTES the
>>>>> _minimum_ of cache line sizes in arm64 systems supported by the kernel.
>>>>> Do you have any benchmarks on Cavium boards that would show significant
>>>>> degradation with 64-byte L1_CACHE_BYTES vs 128?
>>>>>
>>>>> For non-coherent DMA, the simplest is to make ARCH_DMA_MINALIGN the
>>>>> _maximum_ of the supported systems:
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
>>>>> index 5082b30bc2c0..4b5d7b27edaf 100644
>>>>> --- a/arch/arm64/include/asm/cache.h
>>>>> +++ b/arch/arm64/include/asm/cache.h
>>>>> @@ -18,17 +18,17 @@
>>>>>
>>>>> #include <asm/cachetype.h>
>>>>>
>>>>> -#define L1_CACHE_SHIFT 7
>>>>> +#define L1_CACHE_SHIFT 6
>>>>> #define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>>>>>
>>>>> /*
>>>>> * Memory returned by kmalloc() may be used for DMA, so we must make
>>>>> - * sure that all such allocations are cache aligned. Otherwise,
>>>>> - * unrelated code may cause parts of the buffer to be read into the
>>>>> - * cache before the transfer is done, causing old data to be seen by
>>>>> - * the CPU.
>>>>> + * sure that all such allocations are aligned to the maximum *known*
>>>>> + * cache line size on ARMv8 systems. Otherwise, unrelated code may cause
>>>>> + * parts of the buffer to be read into the cache before the transfer is
>>>>> + * done, causing old data to be seen by the CPU.
>>>>> */
>>>>> -#define ARCH_DMA_MINALIGN L1_CACHE_BYTES
>>>>> +#define ARCH_DMA_MINALIGN (128)
>>>>>
>>>>> #ifndef __ASSEMBLY__
>>>>>
>>>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>>>> index 392c67eb9fa6..30bafca1aebf 100644
>>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>>> @@ -976,9 +976,9 @@ void __init setup_cpu_features(void)
>>>>> if (!cwg)
>>>>> pr_warn("No Cache Writeback Granule information, assuming
>>>>> cache line size %d\n",
>>>>> cls);
>>>>> - if (L1_CACHE_BYTES < cls)
>>>>> - pr_warn("L1_CACHE_BYTES smaller than the Cache Writeback Granule (%d < %d)\n",
>>>>> - L1_CACHE_BYTES, cls);
>>>>> + if (ARCH_DMA_MINALIGN < cls)
>>>>> + pr_warn("ARCH_DMA_MINALIGN smaller than the Cache Writeback Granule (%d < %d)\n",
>>>>> + ARCH_DMA_MINALIGN, cls);
>>>>> }
>>>>>
>>>>> static bool __maybe_unused
>>>>
>>>> This change was discussed at: [1] but was not concluded as apparently no one
>>>> came back with test report and numbers. After including this change in our
>>>> local kernel we are seeing significant throughput improvement. For example with:
>>>>
>>>> iperf -c 192.168.1.181 -i 1 -w 128K -t 60
>>>>
>>>> The average throughput is improving by about 30% (230Mbps from 180Mbps).
>>>> Could you please let us know if this change can be included in upstream kernel.
>>>>
>>>> [1]: https://groups.google.com/forum/#!topic/linux.kernel/P40yDB90ePs
>>>
>>> Could you please provide some feedback about the above mentioned query ?
>>
>> Do you have an explanation on the performance variation when
>> L1_CACHE_BYTES is changed? We'd need to understand how the network stack
>> is affected by L1_CACHE_BYTES, in which context it uses it (is it for
>> non-coherent DMA?).
>
> network stack use SKB_DATA_ALIGN to align.
> ---
> #define SKB_DATA_ALIGN(X) (((X) + (SMP_CACHE_BYTES - 1)) & \
> ~(SMP_CACHE_BYTES - 1))
>
> #define SMP_CACHE_BYTES L1_CACHE_BYTES
> ---
> I think this is the reason of performance regression.
>
Yes this is the reason for performance regression. Due to increases L1 cache alignment the
object is coming from next kmalloc slab and skb->truesize is changing from 2304 bytes to
4352 bytes. This in turn increases sk_wmem_alloc which causes queuing of less send buffers.
We tried different benchmarks and found none which really affects with Cache line change. If there is no correctness issue,
I think we are fine with reverting the patch.
Though I still think it is beneficiary to do some more investigation for the perf loss, who knows 32 bit align or no align might
Give even more perf benefit.
Thanks,
Tirumalesh.
>>
>> The Cavium guys haven't shown any numbers (IIUC) to back the
>> L1_CACHE_BYTES performance improvement but I would not revert the
>> original commit since ARCH_DMA_MINALIGN definitely needs to cover the
>> maximum available cache line size, which is 128 for them.
>
> how about define L1_CACHE_SHIFT like below:
> ---
> #ifdef CONFIG_ARM64_L1_CACHE_SHIFT
> #define L1_CACHE_SHIFT CONFIG_ARM64_L1_CACHE_SHIFT
> #else
> #define L1_CACHE_SHIFT 7
> endif
> ---
>
> Thanks
>
>>
>> --
>> Catalin
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/3] arm64: Add support for handling memory corruption
From: Punit Agrawal @ 2017-04-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
With support for contiguous hugepages being turned off[0], some of the
problems arising from swap entries go away[1]. This simplifies the
changes needed to enable memory corruption handling for arm64 (done in
this seris).
In this series, we updates huge_pte_offset() to correctly deal with
swap entries (Patch 1). This function will need to be updated when
contiguous hugepages are re-enabled.
Patch 2 adds support to send SIGBUS to processes that have their
memory corrupted. With the prerequisites in place, enable memory
corruption handling for arm64 (patch 3).
Thanks,
Punit
[0] https://lkml.org/lkml/2017/4/7/486
[1] https://lkml.org/lkml/2017/4/5/402
Jonathan (Zhixiong) Zhang (2):
arm64: hwpoison: add VM_FAULT_HWPOISON[_LARGE] handling
arm64: kconfig: allow support for memory failure handling
Punit Agrawal (1):
arm64: hugetlb: Fix huge_pte_offset to return poisoned page table
entries
arch/arm64/Kconfig | 1 +
arch/arm64/mm/fault.c | 22 +++++++++++++++++++---
arch/arm64/mm/hugetlbpage.c | 20 +++++++++++++++-----
3 files changed, 35 insertions(+), 8 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/3] arm64: hugetlb: Fix huge_pte_offset to return poisoned page table entries
From: Punit Agrawal @ 2017-04-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412140459.21824-1-punit.agrawal@arm.com>
When memory failure is enabled, a poisoned hugepage pte is marked as a
swap entry. huge_pte_offset() does not return the poisoned page table
entries when it encounters PUD/PMD hugepages.
This behaviour of huge_pte_offset() leads to error such as below when
munmap is called on poisoned hugepages.
[ 344.165544] mm/pgtable-generic.c:33: bad pmd 000000083af00074.
Fix huge_pte_offset() to return the poisoned pte which is then
appropriately handled by the generic layer code.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: David Woods <dwoods@mellanox.com>
---
arch/arm64/mm/hugetlbpage.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 7514a000e361..5f1832165d69 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -143,15 +143,24 @@ pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
pr_debug("%s: addr:0x%lx pgd:%p\n", __func__, addr, pgd);
if (!pgd_present(*pgd))
return NULL;
- pud = pud_offset(pgd, addr);
- if (!pud_present(*pud))
- return NULL;
- if (pud_huge(*pud))
+ pud = pud_offset(pgd, addr);
+ /*
+ * In case of HW Poisoning, a hugepage pud/pmd can contain
+ * poisoned entries. Poisoned entries are marked as swap
+ * entries.
+ *
+ * For puds/pmds that are not present, check to see if it
+ * could be a swap entry (!present and !none).
+ */
+ if ((!pte_present(pud_pte(*pud)) && !pud_none(*pud)) || pud_huge(*pud))
return (pte_t *)pud;
+
pmd = pmd_offset(pud, addr);
- if (!pmd_present(*pmd))
+ if (pmd_none(*pmd))
return NULL;
+ if (!pmd_present(*pmd) && !pmd_none(*pmd))
+ return (pte_t *)pmd;
if (pte_cont(pmd_pte(*pmd))) {
pmd = pmd_offset(
@@ -160,6 +169,7 @@ pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
}
if (pmd_huge(*pmd))
return (pte_t *)pmd;
+
pte = pte_offset_kernel(pmd, addr);
if (pte_present(*pte) && pte_cont(*pte)) {
pte = pte_offset_kernel(
--
2.11.0
^ permalink raw reply related
* [PATCH 2/3] arm64: hwpoison: add VM_FAULT_HWPOISON[_LARGE] handling
From: Punit Agrawal @ 2017-04-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412140459.21824-1-punit.agrawal@arm.com>
From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
Add VM_FAULT_HWPOISON[_LARGE] handling to the arm64 page fault
handler. Handling of VM_FAULT_HWPOISON[_LARGE] is very similar
to VM_FAULT_OOM, the only difference is that a different si_code
(BUS_MCEERR_AR) is passed to user space and si_addr_lsb field is
initialized.
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
(fix new __do_user_fault call-site)
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
arch/arm64/mm/fault.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 1b35b8bddbfb..7684219d8cc4 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -31,6 +31,7 @@
#include <linux/highmem.h>
#include <linux/perf_event.h>
#include <linux/preempt.h>
+#include <linux/hugetlb.h>
#include <asm/bug.h>
#include <asm/cpufeature.h>
@@ -207,10 +208,11 @@ static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
*/
static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
unsigned int esr, unsigned int sig, int code,
- struct pt_regs *regs)
+ struct pt_regs *regs, int fault)
{
struct siginfo si;
const struct fault_info *inf;
+ unsigned int lsb = 0;
if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
inf = esr_to_fault_info(esr);
@@ -227,6 +229,17 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
si.si_errno = 0;
si.si_code = code;
si.si_addr = (void __user *)addr;
+ /*
+ * Either small page or large page may be poisoned.
+ * In other words, VM_FAULT_HWPOISON_LARGE and
+ * VM_FAULT_HWPOISON are mutually exclusive.
+ */
+ if (fault & VM_FAULT_HWPOISON_LARGE)
+ lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
+ else if (fault & VM_FAULT_HWPOISON)
+ lsb = PAGE_SHIFT;
+ si.si_addr_lsb = lsb;
+
force_sig_info(sig, &si, tsk);
}
@@ -242,7 +255,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
*/
if (user_mode(regs)) {
inf = esr_to_fault_info(esr);
- __do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs);
+ __do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs, 0);
} else
__do_kernel_fault(mm, addr, esr, regs);
}
@@ -444,6 +457,9 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
*/
sig = SIGBUS;
code = BUS_ADRERR;
+ } else if (fault & (VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE)) {
+ sig = SIGBUS;
+ code = BUS_MCEERR_AR;
} else {
/*
* Something tried to access memory that isn't in our memory
@@ -454,7 +470,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
SEGV_ACCERR : SEGV_MAPERR;
}
- __do_user_fault(tsk, addr, esr, sig, code, regs);
+ __do_user_fault(tsk, addr, esr, sig, code, regs, fault);
return 0;
no_context:
--
2.11.0
^ permalink raw reply related
* [PATCH 3/3] arm64: kconfig: allow support for memory failure handling
From: Punit Agrawal @ 2017-04-12 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170412140459.21824-1-punit.agrawal@arm.com>
From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
Declare ARCH_SUPPORTS_MEMORY_FAILURE, as arm64 does support
memory failure recovery attempt.
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
(Dropped changes to ACPI APEI Kconfig and updated commit log)
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3741859765cf..993a5fd85452 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -19,6 +19,7 @@ config ARM64
select ARCH_HAS_STRICT_MODULE_RWX
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_SUPPORTS_MEMORY_FAILURE
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_NUMA_BALANCING
select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox