* [PATCH 0/7] Add USB remote wakeup driver
From: Rob Herring @ 2017-12-15 20:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512809136-2779-1-git-send-email-chunfeng.yun@mediatek.com>
On Sat, Dec 09, 2017 at 04:45:29PM +0800, Chunfeng Yun wrote:
> These patches introduce the SSUSB and SPM glue layer driver which is
> used to support usb remote wakeup. Usually the glue layer is put into
> a system controller, such as PERICFG module.
> The old way to support usb wakeup is put into SSUSB controller drivers,
> including xhci-mtk driver and mtu3 driver, but there are some problems:
> 1. can't disdinguish the relation between glue layer and SSUSB IP
> when SoCs supports multi SSUSB IPs;
> 2. duplicated code for wakeup are put into both xhci-mtk and mtu3
> drivers;
> 3. the glue layer may vary on different SoCs with SSUSB IP, and will
> make SSUSB controller drivers complicated;
> In order to resolve these problems, it's useful to make the glue layer
> transparent by extracting a seperated driver, meanwhile to reduce the
> duplicated code and simplify SSUSB controller drivers.
Both the driver and binding look overly complicated to me when it looks
like you just have 2 versions of enable/disable functions which modify
a single register. The complexity may be justified if this was a common
binding and driver, but it is not.
You already have a phandle to the system controller. Can't you add cells
to it to handle any differences between instances? That and SoC specific
compatible strings should be enough to handle differences.
Rob
^ permalink raw reply
* WARNING: suspicious RCU usage
From: Fabio Estevam @ 2017-12-15 20:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215182340.GA30443@linux.vnet.ibm.com>
Hi Paul,
On Fri, Dec 15, 2017 at 4:23 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> How about this one, also untested etc.?
This one gives a build warning:
arch/arm/kernel/smp.c: In function ?__cpu_die?:
arch/arm/kernel/smp.c:262:5: warning: ?ret? may be used uninitialized
in this function [-Wmaybe-uninitialized]
if (!ret) {
^
but when I run suspend/resume I don't see the RCU warning with this
patch applied.
Thanks
^ permalink raw reply
* [PATCH 1/3] dt-bindings: gpu: mali-utgard: add rockchip,rk3328-mali compatible
From: Rob Herring @ 2017-12-15 20:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171209000738.32187-2-heiko@sntech.de>
On Sat, Dec 09, 2017 at 01:07:36AM +0100, Heiko Stuebner wrote:
> The rk3328 quad-core Cortex A53 uses a Mali-450MP2 with 2 PPs, so
> add a compatible for it.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 1 +
> 1 file changed, 1 insertion(+)
For the series,
Reviewed-by: Rob Herring <robh@kernel.org>
Though I don't think it's really necessary to enable the gpu per board
as it has no pinout. Default enabled would be better IMO.
Rob
^ permalink raw reply
* [PATCH v4 2/2] ARM64: dts: meson-axg: add pinctrl DT info for Meson-AXG SoC
From: Kevin Hilman @ 2017-12-15 20:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171208015418.9632-3-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> From: Xingyu Chen <xingyu.chen@amlogic.com>
>
> Add new pinctrl DT info for the Amlogic's Meson-AXG SoC.
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 44 ++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index e7213eb53958..7b24f83ab4bf 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -7,6 +7,7 @@
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/gpio/meson-axg-gpio.h>
FYI: I dropped this line, since it's not used (yet) and it causes an
unncessary dependency on an external tree. It can be added back as soon
as there are users.
Kevin
^ permalink raw reply
* [PATCH] fix perl locale warnings in arch/arm/boot/
From: Pavel Machek @ 2017-12-15 20:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171127094620.GA18072@amd>
On Mon 2017-11-27 10:46:20, Pavel Machek wrote:
>
> Commit 429f7a062e3b5cf6fcf01eb00600cee5fe4d751f introduced perl into
> arch/arm/boot/compressed/Makefile, which unfortunately leads to locale
> warnings.
>
> Fix it by setting default locale.
>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
RMK apparently can't be bothered to fix the stuff he broke. Can
someone take the patch? Arm-soc? Linus? Andrew?
Thanks,
Pavel
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 45a6b9b..4656e98 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -118,7 +118,7 @@ asflags-y := -DZIMAGE
>
> # Supply kernel BSS size to the decompressor via a linker symbol.
> KBSS_SZ = $(shell $(CROSS_COMPILE)nm $(obj)/../../../../vmlinux | \
> - perl -e 'while (<>) { \
> + LC_ALL=C perl -e 'while (<>) { \
> $$bss_start=hex($$1) if /^([[:xdigit:]]+) B __bss_start$$/; \
> $$bss_end=hex($$1) if /^([[:xdigit:]]+) B __bss_stop$$/; \
> }; printf "%d\n", $$bss_end - $$bss_start;')
>
>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171215/6fe8baf2/attachment.sig>
^ permalink raw reply
* [PATCH v3 3/8] PCI: brcmstb: Add Broadcom STB PCIe host controller driver
From: Bjorn Helgaas @ 2017-12-15 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANCKTBvFhHb57JdG01aiSypBAcUeLzY3Dhuh5xP-_F5k_NcAhA@mail.gmail.com>
On Fri, Dec 15, 2017 at 02:53:57PM -0500, Jim Quinlan wrote:
> On Thu, Dec 14, 2017 at 5:51 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Wed, Dec 13, 2017 at 06:53:53PM -0500, Jim Quinlan wrote:
> >> On Tue, Dec 12, 2017 at 5:16 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> > On Tue, Nov 14, 2017 at 05:12:07PM -0500, Jim Quinlan wrote:
> >> >> This commit adds the basic Broadcom STB PCIe controller. Missing is
> >> >> the ability to process MSI and also handle dma-ranges for inbound
> >> >> memory accesses. These two functionalities are added in subsequent
> >> >> commits.
> >> >>
> >> >> The PCIe block contains an MDIO interface. This is a local interface
> >> >> only accessible by the PCIe controller. It cannot be used or shared
> >> >> by any other HW. As such, the small amount of code for this
> >> >> controller is included in this driver as there is little upside to put
> >> >> it elsewhere.
> >> ...
> >
> >> >> +static bool brcm_pcie_valid_device(struct brcm_pcie *pcie, struct pci_bus *bus,
> >> >> + int dev)
> >> >> +{
> >> >> + if (pci_is_root_bus(bus)) {
> >> >> + if (dev > 0)
> >> >> + return false;
> >> >> + } else {
> >> >> + /* If there is no link, then there is no device */
> >> >> + if (!brcm_pcie_link_up(pcie))
> >> >> + return false;
> >> >
> >> > This is racy, since the link can go down after you check but before
> >> > you do the config access. I assume your hardware can deal with a
> >> > config access that targets a link that is down?
> >>
> >> Yes, that can happen but there is really nothing that can be done if
> >> the link goes down in that vulnerability window. What do you suggest
> >> doing?
> >
> > Most hardware drops writes and returns ~0 on reads if the link is
> > down. I assume your hardware does something similar, and that should
> > be enough. You shouldn't have to check whether the link is up.
> Unfortunately our HW is quite unforgiving and effects a synchronous or
> asynchronous abort when doing a config access when the link or power
> has gone down on the EP. I will open a discussion with the PCIe HW
> folks regarding why our controller does not behave like "most
> hardware". Thanks, Jim
I mentioned in the other thread [1] that I think the best way to
handle this is to figure out how to deal with the abort cleanly.
Using a test like *_pcie_link_up() to try to avoid it is a 99%
solution that means we'll see rare failures that are very difficult to
reproduce and debug.
> > The hardware might report errors, e.g., via AER, if the link is down.
> > And we might not not handle those nicely. If we have issues there, we
> > should find out and fix them.
[1] https://lkml.kernel.org/r/20171214225821.GN30595 at bhelgaas-glaptop.roam.corp.google.com
^ permalink raw reply
* [PATCH v7 0/6] add clk controller driver for Meson-AXG SoC
From: Kevin Hilman @ 2017-12-15 20:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513245826.32163.7.camel@baylibre.com>
Jerome Brunet <jbrunet@baylibre.com> writes:
> On Mon, 2017-12-11 at 22:13 +0800, Yixun Lan wrote:
>> Qiufang Dai (3):
>> clk: meson-axg: add clocks dt-bindings required header
>> clk: meson-axg: add clock controller drivers
>> arm64: dts: meson-axg: add clock DT info for Meson AXG SoC
>>
>> Yixun Lan (3):
>> clk: meson: make the spinlock naming more specific
>> dt-bindings: clock: add compatible variant for the Meson-AXG
>> arm64: dts: meson-axg: switch uart_ao clock to CLK81
>>
>> .../bindings/clock/amlogic,gxbb-clkc.txt | 7 +-
>> arch/arm64/Kconfig.platforms | 1 +
>> arch/arm64/boot/dts/amlogic/meson-axg-s400.dts | 2 +
>> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 19 +-
>> drivers/clk/meson/Kconfig | 8 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/axg.c | 936 +++++++++++++++++++++
>> drivers/clk/meson/axg.h | 126 +++
>> drivers/clk/meson/clkc.h | 2 +-
>> drivers/clk/meson/gxbb.c | 112 +--
>> drivers/clk/meson/meson8b.c | 24 +-
>> include/dt-bindings/clock/axg-clkc.h | 71 ++
>> 12 files changed, 1234 insertions(+), 75 deletions(-)
>> create mode 100644 drivers/clk/meson/axg.c
>> create mode 100644 drivers/clk/meson/axg.h
>> create mode 100644 include/dt-bindings/clock/axg-clkc.h
>
> Kevin,
>
> I took the first 4 patches through clk-meson. I left the last 2 for you.
>
> I have applied the DT bindings update separately.
> Let me know if you have dependency on these new bindings and need a tag.
Yes, I will need a tag on the headers since both the "switch uart_ao"
patch and a subsequent patch to add DT for SPICC are using new clock
IDs.
Thanks,
Kevin
^ permalink raw reply
* [PATCH v3 3/8] PCI: brcmstb: Add Broadcom STB PCIe host controller driver
From: Jim Quinlan @ 2017-12-15 19:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214225133.GM30595@bhelgaas-glaptop.roam.corp.google.com>
On Thu, Dec 14, 2017 at 5:51 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Dec 13, 2017 at 06:53:53PM -0500, Jim Quinlan wrote:
>> On Tue, Dec 12, 2017 at 5:16 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > On Tue, Nov 14, 2017 at 05:12:07PM -0500, Jim Quinlan wrote:
>> >> This commit adds the basic Broadcom STB PCIe controller. Missing is
>> >> the ability to process MSI and also handle dma-ranges for inbound
>> >> memory accesses. These two functionalities are added in subsequent
>> >> commits.
>> >>
>> >> The PCIe block contains an MDIO interface. This is a local interface
>> >> only accessible by the PCIe controller. It cannot be used or shared
>> >> by any other HW. As such, the small amount of code for this
>> >> controller is included in this driver as there is little upside to put
>> >> it elsewhere.
>> ...
>
>> >> +static bool brcm_pcie_valid_device(struct brcm_pcie *pcie, struct pci_bus *bus,
>> >> + int dev)
>> >> +{
>> >> + if (pci_is_root_bus(bus)) {
>> >> + if (dev > 0)
>> >> + return false;
>> >> + } else {
>> >> + /* If there is no link, then there is no device */
>> >> + if (!brcm_pcie_link_up(pcie))
>> >> + return false;
>> >
>> > This is racy, since the link can go down after you check but before
>> > you do the config access. I assume your hardware can deal with a
>> > config access that targets a link that is down?
>>
>> Yes, that can happen but there is really nothing that can be done if
>> the link goes down in that vulnerability window. What do you suggest
>> doing?
>
> Most hardware drops writes and returns ~0 on reads if the link is
> down. I assume your hardware does something similar, and that should
> be enough. You shouldn't have to check whether the link is up.
Unfortunately our HW is quite unforgiving and effects a synchronous or
asynchronous abort when doing a config access when the link or power
has gone down on the EP. I will open a discussion with the PCIe HW
folks regarding why our controller does not behave like "most
hardware". Thanks, Jim
>
> The hardware might report errors, e.g., via AER, if the link is down.
> And we might not not handle those nicely. If we have issues there, we
> should find out and fix them.
>
> I see that dwc, altera, rockchip, and xilinx all do check for link up
> there, too. I can't remember if they had a legitimate reason, or if I
> just missed it.
>
>> >> +static void brcm_pcie_remove_controller(struct brcm_pcie *pcie)
>> >> +{
>> >> + struct list_head *pos, *q;
>> >> + struct brcm_pcie *tmp;
>> >> +
>> >> + mutex_lock(&brcm_pcie_lock);
>> >> + list_for_each_safe(pos, q, &brcm_pcie) {
>> >> + tmp = list_entry(pos, struct brcm_pcie, list);
>> >> + if (tmp == pcie) {
>> >> + list_del(pos);
>> >> + if (list_empty(&brcm_pcie))
>> >> + num_memc = 0;
>> >> + break;
>> >> + }
>> >> + }
>> >> + mutex_unlock(&brcm_pcie_lock);
>> >
>> > I'm missing something. I don't see that num_memc is ever set to
>> > anything *other* than zero.
>> The num_memc is set and used in the dma commit. I will remove its
>> declaration from this commit.
>
> Thanks, that will make the patches much easier to read.
>
>> >> + pcie->id = of_get_pci_domain_nr(dn);
>> >
>> > Why do you call of_get_pci_domain_nr() directly? No other driver
>> > does.
>>
>> We use the domain as the controller number (id). We use the id to
>> identify and fix a HW bug that only affects the 2nd controller; see
>> the clause
>> " } else if (of_machine_is_compatible("brcm,bcm7278a0")) {".
>
> pci_register_host_bridge() already sets bus->domain_nr for every host
> bridge. That path calls of_get_pci_domain_nr() eventually. But I
> guess that's too late for your use case, because you have this:
>
> brcm_pcie_probe
> brcm_pcie_setup
> brcm_pcie_bridge_sw_init_set
> if (of_machine_is_compatible("brcm,bcm7278a0")) {
> offset = pcie->id ? ... <--- use
> pci_scan_root_bus_bridge
> pci_register_host_bridge
> bus->domain_nr = pci_bus_find_domain_nr <--- available
>
> I guess you haven't added a binding for brcm,bcm7278a0 yet?
>
> I'm not really sure that using the linux,pci-domain DT property is the
> best way to distinguish the two controllers. Maybe Rob has an
> opinion?
>
>> >> + if (pcie->id < 0)
>> >> + return pcie->id;
>
> Bjorn
^ permalink raw reply
* [PATCH v3] arm: imx: dts: Use lower case for bindings notation
From: Fabio Estevam @ 2017-12-15 19:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215191930.11410-1-malat@debian.org>
On Fri, Dec 15, 2017 at 5:19 PM, Mathieu Malaterre <malat@debian.org> wrote:
> Improve the DTS files using lower case to fix the following dtc warnings:
>
> Warning (simple_bus_reg): Node /XXX@<UPPER> simple-bus unit address format error, expected "<lower>"
>
> Converted using the following command:
>
> find . -type f \( -iname *.dts -o -iname *.dtsi \) -exec sed -i -e "s/@\([0-9a-fA-FxX\.;:#]+\)\s*{/@\L\1 {/g" -e "s/@0x\(.*\) {/@\1 {/g" -e "s/@0+\(.*\) {/@\1 {/g" {} +^C
>
> For simplicity, two sed expressions were used to solve each warnings separately.
>
> To make the regex expression more robust a few other issues were resolved,
> namely setting unit-address to lower case, and adding a whitespace before the
> the opening curly brace:
>
> https://elinux.org/Device_Tree_Linux#Linux_conventions
>
> This is a follow up to commit 4c9847b7375a ("dt-bindings: Remove leading 0x from bindings notation")
>
> Reported-by: David Daney <ddaney@caviumnetworks.com>
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH v2] ARM64: dts: meson-axg: add PWM DT info for Meson-Axg SoC
From: Kevin Hilman @ 2017-12-15 19:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215024739.106705-1-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> From: Jian Hu <jian.hu@amlogic.com>
>
> Add PWM DT info for the Amlogic's Meson-Axg SoC.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>
> ---
> Changes in v2 since [1]
> - drop clock DT info from soc.dtsi
Applied to v4.16/dt64,
Thanks,
Kevin
^ permalink raw reply
* [PATCH v3 1/8] SOC: brcmstb: add memory API
From: Jim Quinlan @ 2017-12-15 19:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215171440.GB32131@red-moon>
On Fri, Dec 15, 2017 at 12:14 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, Dec 12, 2017 at 03:14:04PM -0600, Bjorn Helgaas wrote:
>> [+cc Lorenzo]
>>
>> On Tue, Dec 12, 2017 at 03:53:28PM -0500, Jim Quinlan wrote:
>> > On Tue, Dec 5, 2017 at 3:59 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > > On Tue, Nov 14, 2017 at 05:12:05PM -0500, Jim Quinlan wrote:
>> > >> From: Florian Fainelli <f.fainelli@gmail.com>
>> > >>
>> > >> This commit adds a memory API suitable for ascertaining the sizes of
>> > >> each of the N memory controllers in a Broadcom STB chip. Its first
>> > >> user will be the Broadcom STB PCIe root complex driver, which needs
>> > >> to know these sizes to properly set up DMA mappings for inbound
>> > >> regions.
>> > >>
>> > >> We cannot use memblock here or anything like what Linux provides
>> > >> because it collapses adjacent regions within a larger block, and here
>> > >> we actually need per-memory controller addresses and sizes, which is
>> > >> why we resort to manual DT parsing.
>> > >>
>> > >> Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
>> > >> ---
>> > >> drivers/soc/bcm/brcmstb/Makefile | 2 +-
>> > >> drivers/soc/bcm/brcmstb/memory.c | 172 +++++++++++++++++++++++++++++++++++++++
>> > >> include/soc/brcmstb/memory_api.h | 25 ++++++
>> > >> 3 files changed, 198 insertions(+), 1 deletion(-)
>> > >> create mode 100644 drivers/soc/bcm/brcmstb/memory.c
>> > >> create mode 100644 include/soc/brcmstb/memory_api.h
>> > >>
>> > >> diff --git a/drivers/soc/bcm/brcmstb/Makefile b/drivers/soc/bcm/brcmstb/Makefile
>> > >> index 9120b27..4cea7b6 100644
>> > >> --- a/drivers/soc/bcm/brcmstb/Makefile
>> > >> +++ b/drivers/soc/bcm/brcmstb/Makefile
>> > >> @@ -1 +1 @@
>> > >> -obj-y += common.o biuctrl.o
>> > >> +obj-y += common.o biuctrl.o memory.o
>> > >> diff --git a/drivers/soc/bcm/brcmstb/memory.c b/drivers/soc/bcm/brcmstb/memory.c
>> > >> new file mode 100644
>> > >> index 0000000..eb647ad9
>> > >> --- /dev/null
>> > >> +++ b/drivers/soc/bcm/brcmstb/memory.c
>> > >
>> > > I sort of assume based on [1] that every new file should have an SPDX
>> > > identifier ("The Linux kernel requires the precise SPDX identifier in
>> > > all source files") and that the actual text of the GPL can be omitted.
>> > >
>> > > Only a few files in drivers/pci currently have an SPDX identifier. I
>> > > don't know if that's oversight or work-in-progress or what.
>> > >
>> > > [1] https://lkml.kernel.org/r/20171204212120.484179273 at linutronix.de
>> > >
>> >
>> > Bjorn, Did you get a chance to review the other commits of this
>> > submission (V3)? I would like any additional feedback before I send
>> > out a V4 with SPDX fixes. Thanks, JimQ
>>
>> Lorenzo is taking over drivers/pci/host/* and he'll no doubt have some
>> comments when he gets to this. I'll point out a few quick formatting
>> things in the meantime.
>
> I need some time to review the code but overall I am quite worried about
> patches 1 and 4 in particular, it is ok to have platform host bridge
> drivers but we can't rewrite a DMA layer for a specific host bridge, I
> really do not like that - it is just not manageable from a maintenance
> perspective for the mainline kernel.
>
> Lorenzo
Hi Lorenzo,
First I note that the file drivers/pci/host/vmd.c appears to do the
same thing -- rewrite a layer over the DMA ops. Secondly, there seems
to be no other way to accomplish what we need to do, especially that
will work with ARM, ARM64, and MIPs. Someone raised the same point
you did and suggested I involve ARM/ARM64 maintainers, so I expanded
my "--to" list to include Russell. I'm open to ideas. I've asked the
HW PCIe folks to redesign the controller to accommodate an
identity-map for inbound memory, but it will be a while if that
happens, if ever. --Jim
^ permalink raw reply
* [PATCH v4 0/2] dt: add pinctrl driver for Meson-AXG SoC
From: Kevin Hilman @ 2017-12-15 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171208015418.9632-1-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> This is DT part patchset for adding pinctrl support for
> the Amlogic's Meson-AXG SoC.
>
> Changes since v3 at [3]
> -- rebase to khilman's v4.16/dt64 branch and re-send
> -- add Rob's Ack
>
> Changes since v2 at [2]:
> -- Resend this patch series due to fail to send patch [2/2]
>
> Changes since v1 at [1]:
> -- Separate DT part patches
> -- Add Neil Armstrong's Ack
>
> [3]
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005392.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005393.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005394.html
>
> [2]
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005390.html
>
> [1]
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005270.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005271.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005272.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005273.html
> http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005274.html
>
> Xingyu Chen (2):
> documentation: Add compatibles for Amlogic Meson AXG pin controllers
> ARM64: dts: meson-axg: add pinctrl DT info for Meson-AXG SoC
Applied both to v4.14/dt64
Normally, the documentation patch should go with the driver, but since
Linus has already merged the driver, this time I'll take it with the DT
itself.
Kevin
^ permalink raw reply
* [Question ]: Avoid kernel panic when killing an application if happen RAS page table error
From: Matthew Wilcox @ 2017-12-15 19:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A3419F3.1030804@arm.com>
On Fri, Dec 15, 2017 at 06:52:35PM +0000, James Morse wrote:
> Leaking any memory that isn't marked as poisoned isn't a good idea.
>
> What you would need is a way to know from the struct_page that: this page is
> is page-table, and which struct_mm it belongs to. (If its the kernel's init_mm:
> panic()).
> Next you need a way to find all the other pages of page-table without walking
> them. With these three pieces of information you can free all the unaffected
> memory, with even more work you can probably regenerate the corrupted page.
>
> It's going to be complicated to do, I don't think its worth the effort.
We can find a bit in struct page that we guarantee will only be set if
this is allocated as a pagetable. Bit 1 of the third union is currently
available (compound_head is a pointer if bit 0 is set, so nothing is
using bit 1). We can put a pointer to the mm_struct in the same word.
Finding all the allocated pages will be the tricky bit. We could put a
list_head into struct page; perhaps in the same spot as page_deferred_list
for tail pages. Then we can link all the pagetables belonging to
this mm together and tear them all down if any of them get an error.
They'll repopulate on demand. It won't be quick or scalable, but when
the alternative is death, it looks relatively attractive.
^ permalink raw reply
* [PATCH v3 2/2] ARM64: dts: meson-axg: enable ethernet for A113D S400 board
From: Kevin Hilman @ 2017-12-15 19:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215021014.231308-3-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> This is tested in the S400 dev board which use a RTL8211F PHY,
> and the pins connect to the 'eth_rgmii_y_pins' group.
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> arch/arm64/boot/dts/amlogic/meson-axg-s400.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
> index 70eca1f8736a..b8c4f1913d28 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
> @@ -20,3 +20,10 @@
> &uart_AO {
> status = "okay";
> };
> +
> +ðmac {
> + status = "okay";
> + phy-mode = "rgmii";
> + pinctrl-0 = <ð_rgmii_y_pins>;
> + pinctrl-names = "default";
> +};
Minor nit: we try to keep these sorted alphabetically. Can you move
this above the uart_A0 one?
Note: if PATCH 1/1 had applied cleanly, I would have fixed this up
myself and not required a respin.
Kevin
^ permalink raw reply
* [PATCH v3 1/2] ARM64: dts: meson-axg: add ethernet mac controller
From: Kevin Hilman @ 2017-12-15 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215021014.231308-2-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> Add DT info for the stmmac ethernet MAC which found in
> the Amlogic's Meson-AXG SoC, also describe the ethernet
> pinctrl & clock information here.
>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
This patch does not apply, and dependencies are not described.
> ---
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 54 ++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index d356ce74ad89..94c4972222b7 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -7,6 +7,7 @@
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/axg-clkc.h>
>
> / {
> compatible = "amlogic,meson-axg";
> @@ -148,6 +149,19 @@
> #address-cells = <0>;
> };
>
> + ethmac: ethernet at ff3f0000 {
> + compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
> + reg = <0x0 0xff3f0000 0x0 0x10000
> + 0x0 0xff634540 0x0 0x8>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "macirq";
> + clocks = <&clkc CLKID_ETH>,
> + <&clkc CLKID_FCLK_DIV2>,
> + <&clkc CLKID_MPLL2>;
> + clock-names = "stmmaceth", "clkin0", "clkin1";
> + status = "disabled";
> + };
> +
> hiubus: bus at ff63c000 {
> compatible = "simple-bus";
> reg = <0x0 0xff63c000 0x0 0x1c00>;
Based on the hiubus node, presumably this depends on the patch from the
clock series.
> @@ -194,6 +208,46 @@
> #gpio-cells = <2>;
> gpio-ranges = <&pinctrl_periphs 0 0 86>;
> };
I'm not sure where this part is coming from, but it causes the rest of
it to not apply.
Please be sure to describe all dependencies.
Kevin
> +
> + eth_rgmii_x_pins: eth-x-rgmii {
> + mux {
> + groups = "eth_mdio_x",
> + "eth_mdc_x",
> + "eth_rgmii_rx_clk_x",
> + "eth_rx_dv_x",
> + "eth_rxd0_x",
> + "eth_rxd1_x",
> + "eth_rxd2_rgmii",
> + "eth_rxd3_rgmii",
> + "eth_rgmii_tx_clk",
> + "eth_txen_x",
> + "eth_txd0_x",
> + "eth_txd1_x",
> + "eth_txd2_rgmii",
> + "eth_txd3_rgmii";
> + function = "eth";
> + };
> + };
> +
> + eth_rgmii_y_pins: eth-y-rgmii {
> + mux {
> + groups = "eth_mdio_y",
> + "eth_mdc_y",
> + "eth_rgmii_rx_clk_y",
> + "eth_rx_dv_y",
> + "eth_rxd0_y",
> + "eth_rxd1_y",
> + "eth_rxd2_rgmii",
> + "eth_rxd3_rgmii",
> + "eth_rgmii_tx_clk",
> + "eth_txen_y",
> + "eth_txd0_y",
> + "eth_txd1_y",
> + "eth_txd2_rgmii",
> + "eth_txd3_rgmii";
> + function = "eth";
> + };
> + };
> };
> };
^ permalink raw reply
* [PATCH 1/2] serial: stm32: add default console
From: Greg Kroah-Hartman @ 2017-12-15 19:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512146211-29086-2-git-send-email-ludovic.Barre@st.com>
On Fri, Dec 01, 2017 at 05:36:50PM +0100, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch adds by default the console support
> on stm32.
Why? 'default y' should only mean "machine will not boot without this".
And I think your machine will boot without the console, right? :)
thanks,
greg k-h
^ permalink raw reply
* DT dtc warnings
From: Alexandre Belloni @ 2017-12-15 19:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJKOXPcf_zf3X8i9xVpK_BSvnR-78otwmaiOERDYTT2tC3VC2A@mail.gmail.com>
On 15/12/2017 at 08:34:55 +0100, Krzysztof Kozlowski wrote:
> On Fri, Dec 15, 2017 at 8:29 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > On 15/12/2017 at 08:23:39 +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Dec 15, 2017 at 12:02 AM, Fabio Estevam <festevam@gmail.com> wrote:
> >> > On Thu, Dec 14, 2017 at 7:19 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >> >
> >> >> Thanks for reply!
> >> >>
> >> >> Isn't this property of a SoC? The registers used by
> >> >> syscon-poweroff/reboot are part of SoC power management unit. It does
> >> >> not refer to any externals. Why then it should be put outside of soc?
> >> >
> >> > If these nodes have registers, then they should have a unit address
> >> > and reg property.
> >>
> >> That's the point - they do not have unit address.
> >>
> >
> > Should they be put under the syscon they are using?
>
> They are not using syscon but regmap provided by such external IP
> block (for example this:
> http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/exynos3250.dtsi#L153).
> I guess you are proposing something like on imx7s:
> http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/imx7s.dtsi#L539
>
Yeah, exactly.
Another example here:
http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/gemini.dtsi#L32
It seems your poweroff and reboot bits are in registers that are in the
pmu_system_controller so it makes sense to put them under it. I would
even remove the regmap property and get the regmap from the parent
first. This can easily be done while keeping the ABI backward
compatibility.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 0/2] Use SPDX-License-Identifier for rockchip devicetree files
From: Brian Norris @ 2017-12-15 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <24788EB1-9868-41E9-B97C-7D85D8C3761D@theobroma-systems.com>
On Fri, Dec 15, 2017 at 05:27:52PM +0100, klaus.goger at theobroma-systems.com wrote:
> > On 15.12.2017, at 16:20, Heiko St?bner <heiko@sntech.de> wrote:
> > Am Freitag, 15. Dezember 2017, 15:42:48 CET schrieb Philippe Ombredanne:
> >> On Fri, Dec 15, 2017 at 3:28 PM, Heiko St?bner <heiko@sntech.de> wrote:
> >>> Am Freitag, 15. Dezember 2017, 14:45:34 CET schrieb Philippe Ombredanne:
> >>>> On Fri, Dec 15, 2017 at 12:44 PM, Klaus Goger
> >>>> <klaus.goger@theobroma-systems.com> wrote:
> >>>>> This patch series replaces all the license text in rockchip devicetree
> >>>>> files text with a proper SPDX-License-Identifier.
> >>>>> It follows the guidelines submitted[1] by Thomas Gleixner that are not
> >>>>> yet merged.
[...]
> I added the full list from the get_maintainers script. Some of the original authors
> got dropped as the current contribution level dropped below the scripts limit.
> I added the missing email addresses from the copyright headers to the CC list.
>
> Convenience links to the original patches for the added people:
>
> https://patchwork.kernel.org/patch/10114845/
> https://patchwork.kernel.org/patch/10114843/
I can't possibly miss my chance to spam dozens of people! I don't really
care at all about this change, so pick whichever of the following will
get these patches out of my mailbox quicker (choice is at the
maintainers' discretion):
Acked-by: Brian Norris <briannorris@chromium.org>
Nacked-by: Brian Norris <briannorris@chromium.org>
Viewed-with-strong-indifference-by: Brian Norris <briannorris@chromium.org>
for both patches. (I was CC'd only on 1, but I contributed to both
sets.)
Brian
^ permalink raw reply
* [PATCH v3] arm: imx: dts: Use lower case for bindings notation
From: Mathieu Malaterre @ 2017-12-15 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171215124631.30132-1-malat@debian.org>
Improve the DTS files using lower case to fix the following dtc warnings:
Warning (simple_bus_reg): Node /XXX@<UPPER> simple-bus unit address format error, expected "<lower>"
Converted using the following command:
find . -type f \( -iname *.dts -o -iname *.dtsi \) -exec sed -i -e "s/@\([0-9a-fA-FxX\.;:#]+\)\s*{/@\L\1 {/g" -e "s/@0x\(.*\) {/@\1 {/g" -e "s/@0+\(.*\) {/@\1 {/g" {} +^C
For simplicity, two sed expressions were used to solve each warnings separately.
To make the regex expression more robust a few other issues were resolved,
namely setting unit-address to lower case, and adding a whitespace before the
the opening curly brace:
https://elinux.org/Device_Tree_Linux#Linux_conventions
This is a follow up to commit 4c9847b7375a ("dt-bindings: Remove leading 0x from bindings notation")
Reported-by: David Daney <ddaney@caviumnetworks.com>
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v2: Remove invalid patch for imx7s.dtsi, remove duplicate patch for imx7d.dtsi
v3: Clarify the commit message
arch/arm/boot/dts/imx6q-display5.dtsi | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-display5.dtsi b/arch/arm/boot/dts/imx6q-display5.dtsi
index 4084de43d4d9..09085fde3341 100644
--- a/arch/arm/boot/dts/imx6q-display5.dtsi
+++ b/arch/arm/boot/dts/imx6q-display5.dtsi
@@ -255,7 +255,7 @@
pinctrl-0 = <&pinctrl_i2c1>;
status = "okay";
- codec: tfa9879 at 6C {
+ codec: tfa9879 at 6c {
#sound-dai-cells = <0>;
compatible = "nxp,tfa9879";
reg = <0x6C>;
--
2.11.0
^ permalink raw reply related
* [PATCH v7 6/6] arm64: dts: meson-axg: switch uart_ao clock to CLK81
From: Kevin Hilman @ 2017-12-15 19:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b4e29e22-c0e9-1085-fbb7-eabc5c361445@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> On 12/15/17 00:47, Jerome Brunet wrote:
>> On Mon, 2017-12-11 at 22:13 +0800, Yixun Lan wrote:
>>> Switch the uart_ao pclk to CLK81 since the clock driver is ready.
>>> Also move the clock info to the board.dts instead in the soc.dtsi.
>>
>> Same comment as for ethmac, is it really wise ?
>> Isn't the clock setup the same for the axg family ?
>>
> HI Jerome:
> yes, should be same for AXG family
>
>
> HI Kevin:
> could you take the patch [5/6]? then I just need to resend for this one
Yes, I've applied PATCH 5/6.
Kevin
^ permalink raw reply
* [PATCH v7 5/6] arm64: dts: meson-axg: add clock DT info for Meson AXG SoC
From: Kevin Hilman @ 2017-12-15 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211141348.22048-6-yixun.lan@amlogic.com>
Yixun Lan <yixun.lan@amlogic.com> writes:
> From: Qiufang Dai <qiufang.dai@amlogic.com>
>
> Try to add Hiubus DT info, and also enable clock DT info
> for the Amlogic's Meson-AXG SoC.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Qiufang Dai <qiufang.dai@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Applied to v4.16/dt64,
Thanks,
Kevin
> ---
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index b932a784b02a..6fe5ee0c144e 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -148,6 +148,20 @@
> #address-cells = <0>;
> };
>
> + hiubus: bus at ff63c000 {
> + compatible = "simple-bus";
> + reg = <0x0 0xff63c000 0x0 0x1c00>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0x0 0x0 0x0 0xff63c000 0x0 0x1c00>;
> +
> + clkc: clock-controller at 0 {
> + compatible = "amlogic,axg-clkc";
> + #clock-cells = <1>;
> + reg = <0x0 0x0 0x0 0x320>;
> + };
> + };
> +
> mailbox: mailbox at ff63dc00 {
> compatible = "amlogic,meson-gx-mhu", "amlogic,meson-gxbb-mhu";
> reg = <0 0xff63dc00 0 0x400>;
^ permalink raw reply
* [PATCH v8 7/7] arm64: kvm: handle SError Interrupt by categorization
From: James Morse @ 2017-12-15 18:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4b37e86d-eee3-c51e-eceb-5d0c7ad12886@huawei.com>
Hi gengdongjiu,
On 07/12/17 06:37, gengdongjiu wrote:
> I understand you most idea.
>
> But In the Qemu one signal type can only correspond to one behavior, can not correspond to two behaviors,
> otherwise Qemu will do not know how to do.
>
> For the Qemu, if it receives the SIGBUS_MCEERR_AR signal, it will populate the CPER
> records and inject a SEA to guest through KVM IOCTL "KVM_SET_ONE_REG"; if receives the SIGBUS_MCEERR_AO
> signal, it will record the CPER and trigger a IRQ to notify guest, as shown below:
>
> SIGBUS_MCEERR_AR trigger Synchronous External Abort.
> SIGBUS_MCEERR_AO trigger GPIO IRQ.
>
> For the SIGBUS_MCEERR_AO and SIGBUS_MCEERR_AR, we have already specify trigger method, which all
>
> not involve _trigger_ an SError.
It's a policy choice. How does your virtual CPU notify RAS errors to its virtual
software? You could use SError for SIGBUS_MCEERR_AR, it depends on what type of
CPU you are trying to emulate.
I'd suggest using NOTIFY_SEA for SIGBUS_MCEERR_AR as it avoids problems where
the guest doesn't take the SError immediately, instead tries to re-execute the
code KVM has unmapped from stage2 because its corrupt. (You could detect this
happening in Qemu and try something else)
Synchronous/asynchronous external abort matters to the CPU, but once the error
has been notified to software the reasons for this distinction disappear. Once
the error has been handled, all trace of this distinction is gone.
CPER records only describe component failures. You are trying to re-create some
state that disappeared with one of the firmware-first abstractions. Trying to
re-create this information isn't worth the effort as the distinction doesn't
matter to linux, only to the CPU.
> so there is no chance for Qemu to trigger the SError when gets the SIGBUS_MCEERR_A{O,R}.
You mean there is no reason for Qemu to trigger an SError when it gets a signal
from the kernel.
The reasons the CPU might have to generate an SError don't apply to linux and
KVM user space. User-space will never get a signal for an uncontained error, we
will always panic(). We can't give user-space a signal for imprecise exceptions,
as it can't return from the signal. The classes of error that are left are
covered by polled/irq and NOTIFY_SEA.
Qemu can decide to generate RAS SErrors for SIGBUS_MCEERR_AR if it really wants
to, (but I don't think you should, the kernel may have unmapped the page at PC
from stage2 due to corruption).
I think the problem here is you're applying the CPU->software behaviour and
choices to software->software. By the time user-space gets the error, the
behaviour is different.
Thanks,
James
^ permalink raw reply
* DT dtc warnings
From: Rob Herring @ 2017-12-15 18:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJKOXPcf_zf3X8i9xVpK_BSvnR-78otwmaiOERDYTT2tC3VC2A@mail.gmail.com>
On Fri, Dec 15, 2017 at 1:34 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Fri, Dec 15, 2017 at 8:29 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>> On 15/12/2017 at 08:23:39 +0100, Krzysztof Kozlowski wrote:
>>> On Fri, Dec 15, 2017 at 12:02 AM, Fabio Estevam <festevam@gmail.com> wrote:
>>> > On Thu, Dec 14, 2017 at 7:19 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> >
>>> >> Thanks for reply!
>>> >>
>>> >> Isn't this property of a SoC? The registers used by
>>> >> syscon-poweroff/reboot are part of SoC power management unit. It does
>>> >> not refer to any externals. Why then it should be put outside of soc?
>>> >
>>> > If these nodes have registers, then they should have a unit address
>>> > and reg property.
>>>
>>> That's the point - they do not have unit address.
>>>
>>
>> Should they be put under the syscon they are using?
+1
> They are not using syscon but regmap provided by such external IP
> block (for example this:
> http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/exynos3250.dtsi#L153).
> I guess you are proposing something like on imx7s:
> http://elixir.free-electrons.com/linux/v4.15-rc3/source/arch/arm/boot/dts/imx7s.dtsi#L539
Yes, but the regmap property is pointless. It's the parent!
> That makes sense... I am not sure how this would be related to the
> warning itself but anyway it looks logically.
They just have to be under a node that is not a simple-bus.
"soc" nodes with a simple-bus compatible don't really mean everything
in the SoC, but just all (or some part of) the memory mapped space.
There's many board specific settings within those nodes. We don't
really split up top-level things into board and soc levels.
Rob
^ permalink raw reply
* [Question ]: Avoid kernel panic when killing an application if happen RAS page table error
From: James Morse @ 2017-12-15 18:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eab54efe-0ab4-bf6a-5831-128ff02a018b@huawei.com>
Hi gengdongjiu,
On 15/12/17 02:00, gengdongjiu wrote:
> change the mail title and resend.
(please don't do this, we all got the first version)
> If the user space application happen page table RAS error,Memory error handler(memory_failure()) will
> do nothing except making a poisoned page flag,
Yes, because user-space process's page tables are kernel memory.
memory_failure() depends on the system being able to contain these faults,
giving us another RAS exception if we touch the page again.
> and fault handler in arch/arm64/mm/fault.c
> will deliver a signal to kill this application. when this application exits, it will call unmap_vmas ()
> to release his vma resource, but here it will touch the error page table
again, then will
> trigger RAS error again, so this application cannot be killed and system will be panic, the log is shown in [2].
Kernel memory is corrupt, we panic().
You want to add a distinction to handle user-space process's page tables:
> As shown the stack in [1], unmap_page_range() will touch the error page table, so system will panic,
> there are some simple way to avoid this panic and avoid change much about
> the memory management.
> 1. put the tasks to dead status, not run it again.
> 2. not release the page table for this task.
>
> Of cause, above methods may happen memory leakage. do you have good suggestion about how to solve it?, or do you think this panic is expected behavior? thanks.
I don't think this is worth the effort, the page tables are small compared to
the memory they map. Even if this were fixed, you still have the chance of other
kernel memory being corrupted.
Leaking any memory that isn't marked as poisoned isn't a good idea.
What you would need is a way to know from the struct_page that: this page is
is page-table, and which struct_mm it belongs to. (If its the kernel's init_mm:
panic()).
Next you need a way to find all the other pages of page-table without walking
them. With these three pieces of information you can free all the unaffected
memory, with even more work you can probably regenerate the corrupted page.
It's going to be complicated to do, I don't think its worth the effort.
Thanks,
James
^ permalink raw reply
* [PATCH] arm64: fpsimd: Fix state leakage when migrating after sigreturn
From: Dave Martin @ 2017-12-15 18:34 UTC (permalink / raw)
To: linux-arm-kernel
When refactoring the sigreturn code to handle SVE, I changed the
sigreturn implementation to store the new FPSIMD state from the
user sigframe into task_struct before reloading the state into the
CPU regs. This makes it easier to convert the data for SVE when
needed.
However, it turns out that the fpsimd_state structure passed into
fpsimd_update_current_state is not fully initialised, so assigning
the structure as a whole corrupts current->thread.fpsimd_state.cpu
with uninitialised data.
This means that if the garbage data written to .cpu happens to be a
valid cpu number, and the task is subsequently migrated to the cpu
identified by the that number, and then tries to enter userspace,
the CPU FPSIMD regs will be assumed to be correct for the task and
not reloaded as they should be. This can result in returning to
userspace with the FPSIMD registers containing data that is stale or
that belongs to another task or to the kernel.
Knowingly handing around a kernel structure that is incompletely
initialised with user data is a potential source of mistakes,
especially across source file boundaries. To help avoid a repeat
of this issue, this patch adapts the relevant internal API to hand
around the user-accessible subset only: struct user_fpsimd_state.
To avoid future surprises, this patch also converts all uses of
struct fpsimd_state that really only access the user subset, to use
struct user_fpsimd_state. A few missing consts are added to
function prototypes for good measure.
Thanks to Will for spotting the cause of the bug here.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 8cd969d28fd2 ("arm64/sve: Signal handling support")
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
This can be applied either on top of Will's point fix, or separately.
The rebase on top should be pretty trivial.
arch/arm64/include/asm/fpsimd.h | 2 +-
arch/arm64/kernel/fpsimd.c | 4 ++--
arch/arm64/kernel/signal.c | 7 ++++---
arch/arm64/kernel/signal32.c | 5 +++--
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index 74f3439..8857a0f 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -71,7 +71,7 @@ extern void fpsimd_flush_thread(void);
extern void fpsimd_signal_preserve_current_state(void);
extern void fpsimd_preserve_current_state(void);
extern void fpsimd_restore_current_state(void);
-extern void fpsimd_update_current_state(struct fpsimd_state *state);
+extern void fpsimd_update_current_state(struct user_fpsimd_state const *state);
extern void fpsimd_flush_task_state(struct task_struct *target);
extern void sve_flush_cpu_state(void);
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 540a1e0..55fb544 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1036,14 +1036,14 @@ void fpsimd_restore_current_state(void)
* flag that indicates that the FPSIMD register contents are the most recent
* FPSIMD state of 'current'
*/
-void fpsimd_update_current_state(struct fpsimd_state *state)
+void fpsimd_update_current_state(struct user_fpsimd_state const *state)
{
if (!system_supports_fpsimd())
return;
local_bh_disable();
- current->thread.fpsimd_state = *state;
+ current->thread.fpsimd_state.user_fpsimd = *state;
if (system_supports_sve() && test_thread_flag(TIF_SVE))
fpsimd_to_sve(current);
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index b120111..f60c052 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -178,7 +178,8 @@ static void __user *apply_user_offset(
static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
{
- struct fpsimd_state *fpsimd = ¤t->thread.fpsimd_state;
+ struct user_fpsimd_state const *fpsimd =
+ ¤t->thread.fpsimd_state.user_fpsimd;
int err;
/* copy the FP and status/control registers */
@@ -195,7 +196,7 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
{
- struct fpsimd_state fpsimd;
+ struct user_fpsimd_state fpsimd;
__u32 magic, size;
int err = 0;
@@ -266,7 +267,7 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user)
{
int err;
unsigned int vq;
- struct fpsimd_state fpsimd;
+ struct user_fpsimd_state fpsimd;
struct sve_context sve;
if (__copy_from_user(&sve, user->sve, sizeof(sve)))
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 22711ee..a124140 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -228,7 +228,8 @@ union __fpsimd_vreg {
static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame)
{
- struct fpsimd_state *fpsimd = ¤t->thread.fpsimd_state;
+ struct user_fpsimd_state const *fpsimd =
+ ¤t->thread.fpsimd_state.user_fpsimd;
compat_ulong_t magic = VFP_MAGIC;
compat_ulong_t size = VFP_STORAGE_SIZE;
compat_ulong_t fpscr, fpexc;
@@ -277,7 +278,7 @@ static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame)
static int compat_restore_vfp_context(struct compat_vfp_sigframe __user *frame)
{
- struct fpsimd_state fpsimd;
+ struct user_fpsimd_state fpsimd;
compat_ulong_t magic = VFP_MAGIC;
compat_ulong_t size = VFP_STORAGE_SIZE;
compat_ulong_t fpscr;
--
2.1.4
^ 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