* [PATCH] MAINTAINERS: Add entry for STM32 timer and lptimer drivers
From: Fabrice Gasnier @ 2018-05-15 15:33 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry to make myself a maintainer of STM32 timer and lptimer
drivers.
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b65225..94d8e47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13425,6 +13425,16 @@ T: git git://linuxtv.org/media_tree.git
S: Maintained
F: drivers/media/usb/stk1160/
+STM32 TIMER/LPTIMER DRIVERS
+M: Fabrice Gasnier <fabrice.gasnier@st.com>
+S: Maintained
+F: drivers/*/stm32-*timer*
+F: drivers/pwm/pwm-stm32*
+F: include/linux/*/stm32-*tim*
+F: Documentation/ABI/testing/*timer-stm32
+F: Documentation/devicetree/bindings/*/stm32-*timer
+F: Documentation/devicetree/bindings/pwm/pwm-stm32*
+
STMMAC ETHERNET DRIVER
M: Giuseppe Cavallaro <peppe.cavallaro@st.com>
M: Alexandre Torgue <alexandre.torgue@st.com>
--
1.9.1
^ permalink raw reply related
* [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device
From: Jingoo Han @ 2018-05-15 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5f2226585c6af9920b266d0503e32042d4c9e440.1526375226.git.jan.kiszka@siemens.com>
On Tuesday, May 15, 2018 5:07 AM, Jan Kiszka wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Another step towards a managed version of
> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> rather than just the OF node. This will allow to use managed resource
> allocation internally later on.
>
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
Best regards,
Jingoo Han
> ---
> drivers/pci/dwc/pcie-designware-host.c | 2 +-
> drivers/pci/host/pci-aardvark.c | 5 ++---
> drivers/pci/host/pci-ftpci100.c | 4 ++--
> drivers/pci/host/pci-v3-semi.c | 3 ++-
> drivers/pci/host/pci-versatile.c | 3 +--
> drivers/pci/host/pci-xgene.c | 3 ++-
> drivers/pci/host/pcie-altera.c | 5 ++---
> drivers/pci/host/pcie-iproc-platform.c | 4 ++--
> drivers/pci/host/pcie-rcar.c | 5 ++---
> drivers/pci/host/pcie-rockchip.c | 4 ++--
> drivers/pci/host/pcie-xilinx-nwl.c | 4 ++--
> drivers/pci/host/pcie-xilinx.c | 4 ++--
> drivers/pci/of.c | 9 +++++----
> include/linux/of_pci.h | 4 ++--
> 14 files changed, 29 insertions(+), 30 deletions(-)
>
^ permalink raw reply
* [PATCH v2 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled
From: David Lechner @ 2018-05-15 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515112224.30122-2-nsekhar@ti.com>
On 05/15/2018 06:22 AM, Sekhar Nori wrote:
> PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot
> be disabled. Mark it so to prevent unused clock disable
> infrastructure from disabling it.
>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
> v2: rebased to apply to v4.17-rc1. No functional change.
>
> drivers/clk/davinci/pll-dm646x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c
> index a61cc3256418..0ae827e3ce80 100644
> --- a/drivers/clk/davinci/pll-dm646x.c
> +++ b/drivers/clk/davinci/pll-dm646x.c
> @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = {
> .flags = 0,
> };
>
> -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0);
> +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED);
>
> int dm646x_pll2_init(struct device *dev, void __iomem *base)
> {
>
Reviewed-by: David Lechner <david@lechnology.com>
^ permalink raw reply
* [PATCH v2 2/2] clk: davinci: psc-dm365: fix few clocks
From: David Lechner @ 2018-05-15 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515112224.30122-3-nsekhar@ti.com>
On 05/15/2018 06:22 AM, Sekhar Nori wrote:
> Fix parent of EMAC and voice codec PSC clocks. Documentation is clear
> on EMAC clock parent, but its not fully clear on parent of voice codec
> clock. The implementation chosen is matches arch/arm/mach-davinci/dm365.c.
> Add a comment explaining this for posterity.
>
> There is only one power domain on DM365. Fix the power domain of voice
> codec and vpss dac modules.
>
> While at it, add a comment explaining how the parent of vpss dac clock was
> derived. Note that this patch does not touch the parent of vpss dac clock.
>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
> v2: add comments explaining parents of voice codec and vpss dac clocks
>
> drivers/clk/davinci/psc-dm365.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c
> index 3ad915f37376..289af3913fb0 100644
> --- a/drivers/clk/davinci/psc-dm365.c
> +++ b/drivers/clk/davinci/psc-dm365.c
> @@ -65,9 +65,22 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = {
> LPSC(31, 0, arm, pll2_sysclk2, NULL, LPSC_ALWAYS_ENABLED),
> LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0),
> LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0),
> - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0),
> - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0),
> - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0),
> + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0),
> + /*
> + * The TRM (ARM Subsystem User's Guide) shows two clocks input into
> + * voice codec module (PLL2 SYSCLK4 with a DIV2 and PLL1 SYSCLK4). Its
> + * not fully clear from documentation which clock should be considered
> + * as parent for PSC. The clock chosen here is to maintain
> + * compatibility with existing code in arch/arm/mach-davinci/dm365.c
> + */
> + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0),
> + /*
> + * Its not fully clear from TRM (ARM Subsystem User's Guide) as to what
> + * the parent of VPSS DAC LPSC should actually be. PLL1 SYSCLK3 feeds
> + * into HDVICP and MJCP. The clock chosen here is to remain compatible
> + * with code existing in arch/arm/mach-davinci/dm365.c
> + */
> + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0),
> LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0),
> LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0),
> { }
>
Reviewed-by: David Lechner <david@lechnology.com>
^ permalink raw reply
* [PATCH v10 02/27] clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
From: David Lechner @ 2018-05-15 15:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9203489d-0a5a-103a-67ea-d3e89bb7ebc4@ti.com>
On 05/15/2018 08:31 AM, Sekhar Nori wrote:
> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>> +void of_da850_pll0_init(struct device_node *node)
>> {
>> - return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
>> - &da850_pll0_obsclk_info,
>> - da850_pll0_sysclk_info, 7, base, cfgchip);
>> + void __iomem *base;
>> + struct regmap *cfgchip;
>> +
>> + base = of_iomap(node, 0);
>> + if (!base) {
>> + pr_err("%s: ioremap failed\n", __func__);
>> + return;
>> + }
>> +
>> + cfgchip = syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");
In your previous review, you pointed out that the error did not need to
be handled here because it is handled later in davinci_pll_clk_register().
We get a warning there because cfgchip is only needed for unlocking the
PLL for CPU frequency scaling and is not critical for operation of the
clocks.
>
> It will be nice to handle the error case here.
>
>> +
>> + of_davinci_pll_init(NULL, node, &da850_pll0_info,
>> + &da850_pll0_obsclk_info,
>> + da850_pll0_sysclk_info, 7, base, cfgchip);
>
> Apart from that, it looks good to me.
>
> Reviewed-by: Sekhar Nori <nsekhar@ti.com>
>
> Thanks,
> Sekhar
>
^ permalink raw reply
* [PATCH v3] coresight: documentation: update sysfs section
From: Mathieu Poirier @ 2018-05-15 16:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514141800.f1be30ce9d1f6ae20ca2d04c@arm.com>
On 14 May 2018 at 13:18, Kim Phillips <kim.phillips@arm.com> wrote:
> - Align and show updated ls devices output from the TC2, based on
> current driver
>
> - Provide an example from an ETMv4 based system (Juno)
>
> - Reflect changes to the way the RAM write pointer is accessed since
> it got changed in commit 7d83d17795ef ("coresight: tmc: adding sysFS
> management entries").
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
I'm good with this version - Jonathan, should I take this through my
tree of you'll take care of it?
In case of the former:
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> v3: address Randy Dunlap's showns->shown, corrected - and + line merging
> v2: address Mathieu's comment about clarifying the sinks on the Juno
> vs. TC2 platforms.
>
> Documentation/trace/coresight.txt | 43 +++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
> index 6f0120c3a4f1..15d2a0f1e1b8 100644
> --- a/Documentation/trace/coresight.txt
> +++ b/Documentation/trace/coresight.txt
> @@ -141,13 +141,25 @@ register the device with the core framework. The unregister function takes
> a reference to a "struct coresight_device", obtained at registration time.
>
> If everything goes well during the registration process the new devices will
> -show up under /sys/bus/coresight/devices, as showns here for a TC2 platform:
> +show up under /sys/bus/coresight/devices, as shown here for a TC2 platform:
>
> root:~# ls /sys/bus/coresight/devices/
> -replicator 20030000.tpiu 2201c000.ptm 2203c000.etm 2203e000.etm
> -20010000.etb 20040000.funnel 2201d000.ptm 2203d000.etm
> +20010000.etb 20040000.funnel 2201d000.ptm 2203d000.etm replicator
> +20030000.tpiu 2201c000.ptm 2203c000.etm 2203e000.etm
> root:~#
>
> +and here for a Juno platform:
> +
> +root at juno:~# ls /sys/bus/coresight/devices/
> +20010000.etf 20120000.replicator 22040000.etm 230c0000.funnel
> +20030000.tpiu 20130000.funnel 220c0000.funnel 23140000.etm
> +20040000.funnel 20140000.etf 22140000.etm 23240000.etm
> +20070000.etr 20150000.funnel 23040000.etm 23340000.etm
> +root at juno:~#
> +
> +Note that on Juno users can select the ETF, ETR and TPIU as a sink target while
> +on TC2, the ETB and TPIU can be selected.
> +
> The functions take a "struct coresight_device", which looks like this:
>
> struct coresight_desc {
> @@ -193,16 +205,16 @@ the information carried in "THIS_MODULE".
> How to use the tracer modules
> -----------------------------
>
> -Before trace collection can start, a coresight sink needs to be identify.
> +Before trace collection can start, a coresight sink needs to be identified.
> There is no limit on the amount of sinks (nor sources) that can be enabled at
> any given moment. As a generic operation, all device pertaining to the sink
> class will have an "active" entry in sysfs:
>
> root:/sys/bus/coresight/devices# ls
> -replicator 20030000.tpiu 2201c000.ptm 2203c000.etm 2203e000.etm
> -20010000.etb 20040000.funnel 2201d000.ptm 2203d000.etm
> +20010000.etb 20040000.funnel 2201d000.ptm 2203d000.etm replicator
> +20030000.tpiu 2201c000.ptm 2203c000.etm 2203e000.etm
> root:/sys/bus/coresight/devices# ls 20010000.etb
> -enable_sink status trigger_cntr
> +enable_sink mgmt power subsystem trigger_cntr uevent
> root:/sys/bus/coresight/devices# echo 1 > 20010000.etb/enable_sink
> root:/sys/bus/coresight/devices# cat 20010000.etb/enable_sink
> 1
> @@ -216,16 +228,13 @@ trigger a trace capture:
> root:/sys/bus/coresight/devices# echo 1 > 2201c000.ptm/enable_source
> root:/sys/bus/coresight/devices# cat 2201c000.ptm/enable_source
> 1
> -root:/sys/bus/coresight/devices# cat 20010000.etb/status
> -Depth: 0x2000
> -Status: 0x1
> -RAM read ptr: 0x0
> -RAM wrt ptr: 0x19d3 <----- The write pointer is moving
> -Trigger cnt: 0x0
> -Control: 0x1
> -Flush status: 0x0
> -Flush ctrl: 0x2001
> -root:/sys/bus/coresight/devices#
> +
> +Observe the write pointer moving:
> +
> +root:/sys/bus/coresight/devices# cat 20010000.etb/mgmt/rwp
> +0x1a8
> +root:/sys/bus/coresight/devices# cat 20010000.etb/mgmt/rwp
> +0x19a6
>
> Trace collection is stopped the same way:
>
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] arm64: dts: exynos: fix type of thermal trip points for Exynos7
From: Bartlomiej Zolnierkiewicz @ 2018-05-15 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJKOXPdW7Ovo9GW=29tqa3rEAkSFgrEzbxm_wdGmRpamgk2-ig@mail.gmail.com>
On Tuesday, May 15, 2018 02:59:33 PM Krzysztof Kozlowski wrote:
> On Tue, May 15, 2018 at 2:37 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > Currently Exynos thermal driver treats as "active" all specified
> > trip points before reaching maximum number of hardware supported
> > trip points.
>
> The trip point type describe the way how system should act - use
> active or passive cooling. It is independent of thermal driver. I do
OK, I see it now. Please disregard this patch.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* [PATCH v9 03/11] arm64: kexec_file: invoke the kernel without purgatory
From: James Morse @ 2018-05-15 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515044542.GC2737@linaro.org>
Hi Akashi,
On 15/05/18 05:45, AKASHI Takahiro wrote:
> On Fri, May 11, 2018 at 06:03:49PM +0100, James Morse wrote:
>> On 07/05/18 06:22, AKASHI Takahiro wrote:
>>> On Tue, May 01, 2018 at 06:46:06PM +0100, James Morse wrote:
>>>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>>>> diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
>>>>> index f76ea92dff91..f7dbba00be10 100644
>>>>> --- a/arch/arm64/kernel/machine_kexec.c
>>>>> +++ b/arch/arm64/kernel/machine_kexec.c
>>>>> @@ -205,10 +205,17 @@ void machine_kexec(struct kimage *kimage)
>>
>>>>> cpu_soft_restart(kimage != kexec_crash_image,
>>>>> - reboot_code_buffer_phys, kimage->head, kimage->start, 0);
>>>>> + reboot_code_buffer_phys, kimage->head, kimage->start,
>>>>> +#ifdef CONFIG_KEXEC_FILE
>>>>> + kimage->purgatory_info.purgatory_buf ?
>>>>> + 0 : kimage->arch.dtb_mem);
>>>>> +#else
>>>>> + 0);
>>>>> +#endif
>>
>>
>>>> purgatory_buf seems to only be set in kexec_purgatory_setup_kbuf(), called from
>>>> kexec_load_purgatory(), which we don't use. How does this get a value?
>>>>
>>>> Would it be better to always use kimage->arch.dtb_mem, and ensure that is 0 for
>>>> regular kexec (as we can't know where the dtb is)? (image_arg may then be a
>>>> better name).
>>>
>>> The problem is arch.dtb_mem is currently defined only if CONFIG_KEXEC_FILE.
>>
>> I thought it was ARCH_HAS_KIMAGE_ARCH, which we can define all the time if
>> that's what we want.
>>
>>
>>> So I would like to
>>> - merge this patch with patch#8
>>> - change the condition
>>> #ifdef CONFIG_KEXEC_FILE
>>> kimage->file_mode ? kimage->arch.dtb_mem : 0);
>>> #else
>>> 0);
>>> #endif
>>
>> If we can avoid even this #ifdef by always having kimage->arch, I'd prefer that.
>> If we do that 'dtb_mem' would need some thing that indicates its for kexec_file,
>> as kexec has a DTB too, we just don't know where it is...
>
> OK, but I want to have a minimum of kexec.arch always exist.
I'm curious, why? Its 32bytes that is allocated a maximum of twice.
(my questions on what needs to go in there were because it looked like a third
user was missing...)
> How about this?
>
> | struct kimage_arch {
> | phys_addr_t dtb_mem;
> | #ifdef CONFIG_KEXEC_FILE
#ifdef in structs just breeds more #ifdefs, as the code that accesses those
members has to be behind the same set of conditions.
Given this, I prefer the #ifdefs around cpu_soft_restart() as it doesn't force
us to add more #ifdefs later.
For either option without purgatory_info:
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
* [PATCH v9 04/11] arm64: kexec_file: allocate memory walking through memblock list
From: James Morse @ 2018-05-15 16:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515043538.GB2737@linaro.org>
Hi Akashi,
On 15/05/18 05:35, AKASHI Takahiro wrote:
> On Mon, May 07, 2018 at 02:59:07PM +0900, AKASHI Takahiro wrote:
>> On Tue, May 01, 2018 at 06:46:09PM +0100, James Morse wrote:
>>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>>> We need to prevent firmware-reserved memory regions, particularly EFI
>>>> memory map as well as ACPI tables, from being corrupted by loading
>>>> kernel/initrd (or other kexec buffers). We also want to support memory
>>>> allocation in top-down manner in addition to default bottom-up.
>>>> So let's have arm64 specific arch_kexec_walk_mem() which will search
>>>> for available memory ranges in usable memblock list,
>>>> i.e. !NOMAP & !reserved,
>>>
>>>> instead of system resource tree.
>>>
>>> Didn't we try to fix the system-resource-tree in order to fix regular-kexec to
>>> be safe in the EFI-memory-map/ACPI-tables case?
>>>
>>> It would be good to avoid having two ways of doing this, and I would like to
>>> avoid having extra arch code...
>>
>> I know what you mean.
>> /proc/iomem or system resource is, in my opinion, not the best place to
>> describe memory usage of kernel but rather to describe *physical* hardware
>> layout. As we are still discussing about "reserved" memory, I don't want
>> to depend on it.
I agree. We have funny stuff that isn't hardware-layout, but is important for
the next boot. The kernel doesn't have an ABI to support when it queries the
list itself.
>> Along with memblock list, we will have more accurate control over memory
>> usage.
>>> If the argument is walking memblock gives a better answer than the stringy
>>> walk_system_ram_res() thing, is there any mileage in moving this code into
>>> kexec_file.c, and using it if !IS_ENABLED(CONFIG_ARCH_DISCARD_MEMBLOCK)?
>>>
>>> This would save arm64/powerpc having near-identical implementations.
>>> 32bit arm keeps memblock if it has kexec, so it may be useful there too if
>>> kexec_file_load() support is added.
> If you don't have further objection, I will take memblock approach
> (with factoring out powerpc's arch_kexec_walk_mem()).
If we're agreed that the memblock walking is generic, then it would be quicker
to make the arm64 version as close as possible and merge them as a later series.
(saves a cross arch dependency)
With that,
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
* [PATCH V2 8/8] dt-bindings: stm32: add compatible for syscon
From: Christophe ROULLIER @ 2018-05-15 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507163537.GA15212@rob-hp-laptop>
Hi Rob,
I do not understand, so let me explain our status:
We have syscfg IP Harware in our SOC.
But we do not have SoC specific driver to manage syscfg, we are using a generic driver "syscon".
So can you tell me what you wish to describe this part in our SOC bindings ?
Thanks for your help.
Christophe.
-----Original Message-----
From: Rob Herring [mailto:robh at kernel.org]
Sent: lundi 7 mai 2018 18:36
To: Christophe ROULLIER <christophe.roullier@st.com>
Cc: mark.rutland at arm.com; mcoquelin.stm32 at gmail.com; Alexandre TORGUE <alexandre.torgue@st.com>; Peppe CAVALLARO <peppe.cavallaro@st.com>; devicetree at vger.kernel.org; andrew at lunn.ch; linux-arm-kernel at lists.infradead.org; netdev at vger.kernel.org
Subject: Re: [PATCH V2 8/8] dt-bindings: stm32: add compatible for syscon
On Wed, May 02, 2018 at 04:18:43PM +0200, Christophe Roullier wrote:
> This patch describes syscon DT bindings.
>
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---
> Documentation/devicetree/bindings/arm/stm32.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/stm32.txt
> b/Documentation/devicetree/bindings/arm/stm32.txt
> index 6808ed9..06e3834 100644
> --- a/Documentation/devicetree/bindings/arm/stm32.txt
> +++ b/Documentation/devicetree/bindings/arm/stm32.txt
> @@ -8,3 +8,7 @@ using one of the following compatible strings:
> st,stm32f746
> st,stm32h743
> st,stm32mp157
> +
> +Required nodes:
> +- syscon: the soc bus node must have a system controller node
> +pointing to the
> + global control registers, with the compatible string "syscon";
You misunderstood my prior comment. 'syscon' alone is not valid. You need SoC specific compatible string for it and 'stm32' is not SoC specific. IOW, the compatible property for a syscon should imply every single register field in the block.
Rob
^ permalink raw reply
* [PATCH v9 05/11] arm64: kexec_file: load initrd and device-tree
From: James Morse @ 2018-05-15 16:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180425062629.29404-6-takahiro.akashi@linaro.org>
Hi Akashi,
On 25/04/18 07:26, AKASHI Takahiro wrote:
> load_other_segments() is expected to allocate and place all the necessary
> memory segments other than kernel, including initrd and device-tree
> blob (and elf core header for crash).
> While most of the code was borrowed from kexec-tools' counterpart,
> users may not be allowed to specify dtb explicitly, instead, the dtb
> presented by a boot loader is reused.
(Nit: "a boot loader" -> "the original boot loader")
> arch_kimage_kernel_post_load_cleanup() is responsible for freeing arm64-
> specific data allocated in load_other_segments().
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index f9ebf54ca247..b3b9b1725d8a 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -13,7 +13,26 @@
> #include <linux/ioport.h>
> #include <linux/kernel.h>
> #include <linux/kexec.h>
> +#include <linux/libfdt.h>
> #include <linux/memblock.h>
> +#include <linux/of_fdt.h>
> +#include <linux/types.h>
> +#include <asm/byteorder.h>
> +
> +static int __dt_root_addr_cells;
> +static int __dt_root_size_cells;
> @@ -55,3 +74,144 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf,
>
> return ret;
> }
> +
> +static int setup_dtb(struct kimage *image,
> + unsigned long initrd_load_addr, unsigned long initrd_len,
> + char *cmdline, unsigned long cmdline_len,
> + char **dtb_buf, size_t *dtb_buf_len)
> +{
> + char *buf = NULL;
> + size_t buf_size;
> + int nodeoffset;
> + u64 value;
> + int range_len;
> + int ret;
> +
> + /* duplicate dt blob */
> + buf_size = fdt_totalsize(initial_boot_params);
> + range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
These two cells values are 0 here. Did you want
arch_kexec_file_init() in patch 7 in this patch?
Ah, range_len isn't used, so, did you want the cells values and this range_len
thing in in patch 7!?
> +
> + if (initrd_load_addr)
> + buf_size += fdt_prop_len("linux,initrd-start", sizeof(u64))
> + + fdt_prop_len("linux,initrd-end", sizeof(u64));
> +
> + if (cmdline)
> + buf_size += fdt_prop_len("bootargs", cmdline_len + 1);
I can't find where fdt_prop_len() .... oh, patch 7. fdt_prop_len() doesn't look
like the sort of thing that should be created here, but I agree there isn't an
existing API to do this.
(This must be why powerpc guesses that the fdt won't be more than double in size).
> + buf = vmalloc(buf_size);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto out_err;
> + }
> +
> + ret = fdt_open_into(initial_boot_params, buf, buf_size);
> + if (ret)
> + goto out_err;
> +
> + nodeoffset = fdt_path_offset(buf, "/chosen");
> + if (nodeoffset < 0)
> + goto out_err;
> +
> + /* add bootargs */
> + if (cmdline) {
> + ret = fdt_setprop(buf, nodeoffset, "bootargs",
> + cmdline, cmdline_len + 1);
fdt_setprop_string()?
> + if (ret)
> + goto out_err;
> + }
> +
> + /* add initrd-* */
> + if (initrd_load_addr) {
> + value = cpu_to_fdt64(initrd_load_addr);
> + ret = fdt_setprop(buf, nodeoffset, "linux,initrd-start",
> + &value, sizeof(value));
sizeof(value) was assumed to be the same as sizeof(u64) earlier.
fdt_setprop_u64()?
> + if (ret)
> + goto out_err;
> +
> + value = cpu_to_fdt64(initrd_load_addr + initrd_len);
> + ret = fdt_setprop(buf, nodeoffset, "linux,initrd-end",
> + &value, sizeof(value));
> + if (ret)
> + goto out_err;
> + }
> +
> + /* trim a buffer */
> + fdt_pack(buf);
> + *dtb_buf = buf;
> + *dtb_buf_len = fdt_totalsize(buf);
> +
> + return 0;
> +
> +out_err:
> + vfree(buf);
> + return ret;
> +}
While powerpc has some similar code for updating the initrd and cmdline, it
makes different assumptions about the size of the dt, and has different behavior
for memreserve. (looks like we don't expect the initramfs to be memreserved).
Lets leave unifying that stuff where possible for the future.
> +int load_other_segments(struct kimage *image,
> + char *initrd, unsigned long initrd_len,
> + char *cmdline, unsigned long cmdline_len)
> +{
> + struct kexec_segment *kern_seg;
> + struct kexec_buf kbuf;
> + unsigned long initrd_load_addr = 0;
> + char *dtb = NULL;
> + unsigned long dtb_len = 0;
> + int ret = 0;
> +
> + kern_seg = &image->segment[image->arch.kern_segment];
> + kbuf.image = image;
> + /* not allocate anything below the kernel */
> + kbuf.buf_min = kern_seg->mem + kern_seg->memsz;
> + /* load initrd */
> + if (initrd) {
> + kbuf.buffer = initrd;
> + kbuf.bufsz = initrd_len;
> + kbuf.memsz = initrd_len;
> + kbuf.buf_align = 0;
I'm surprised there initrd has no alignment requirement, but kexec_add_buffer()
rounds this up to PAGE_SIZE.
> + /* within 1GB-aligned window of up to 32GB in size */
> + kbuf.buf_max = round_down(kern_seg->mem, SZ_1G)
> + + (unsigned long)SZ_1G * 32;
> + kbuf.top_down = false;
> +
> + ret = kexec_add_buffer(&kbuf);
> + if (ret)
> + goto out_err;
> + initrd_load_addr = kbuf.mem;
> +
> + pr_debug("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> + initrd_load_addr, initrd_len, initrd_len);
> + }
> +
> + /* load dtb blob */
> + ret = setup_dtb(image, initrd_load_addr, initrd_len,
> + cmdline, cmdline_len, &dtb, &dtb_len);
> + if (ret) {
> + pr_err("Preparing for new dtb failed\n");
> + goto out_err;
> + }
> +
> + kbuf.buffer = dtb;
> + kbuf.bufsz = dtb_len;
> + kbuf.memsz = dtb_len;
> + /* not across 2MB boundary */
> + kbuf.buf_align = SZ_2M;
> + kbuf.buf_max = ULONG_MAX;
> + kbuf.top_down = true;
> +
> + ret = kexec_add_buffer(&kbuf);
> + if (ret)
> + goto out_err;
> + image->arch.dtb_mem = kbuf.mem;
> + image->arch.dtb_buf = dtb;
> +
> + pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> + kbuf.mem, dtb_len, dtb_len);
> +
> + return 0;
> +
> +out_err:
> + vfree(dtb);
> + image->arch.dtb_buf = NULL;
Won't kimage_file_post_load_cleanup() always be called if we return an error
here? Why not leave the free()ing until then?
> + return ret;
> +}
Thanks,
James
^ permalink raw reply
* [PATCH] KVM: arm/arm64: fix unaligned hva start and end in handle_hva_to_gpa
From: Suzuki K Poulose @ 2018-05-15 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5efb2da1-fed8-443b-d06b-cc03152b2c2e@gmail.com>
Hi Jia,
On 15/05/18 14:15, Jia He wrote:
>
>
> On 5/15/2018 8:38 PM, Jia He Wrote:
>> Hi Suzuki
>>
>> On 5/15/2018 4:36 PM, Suzuki K Poulose Wrote:
>>>
>>> Hi Jia
>>>
>>> On 05/15/2018 03:03 AM, Jia He wrote:
>>>> Hi Suzuki
>>>>
>>>> I will merge the other thread into this, and add the necessary CC list
>>>>
>>>> That WARN_ON call trace is very easy to reproduce in my armv8a server after I
>>>> start 20 guests
>>>>
>>>> and run memhog in the host. Of course, ksm should be enabled
>>>>
>>>> For you question about my inject fault debug patch:
>>>
>>>
>>> Thanks for the patch, comments below.
>>>
>>>>
>>>
>>> ...
>>>
>>>> index 7f6a944..ab8545e 100644
>>>> --- a/virt/kvm/arm/mmu.c
>>>> +++ b/virt/kvm/arm/mmu.c
>>>> @@ -290,12 +290,17 @@ static void unmap_stage2_puds(struct kvm *kvm, pgd_t *pgd,
>>>> ?? * destroying the VM), otherwise another faulting VCPU may come in and mess
>>>> ?? * with things behind our backs.
>>>> ?? */
>>>> +extern int trigger_by_ksm;
>>>> ??static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size)
>>>> ??{
>>>> ???????? pgd_t *pgd;
>>>> ???????? phys_addr_t addr = start, end = start + size;
>>>> ???????? phys_addr_t next;
>>>>
>>>> +?????? if(trigger_by_ksm) {
>>>> +?????????????? end -= 0x200;
>>>> +?????? }
>>>> +
>>>> ???????? assert_spin_locked(&kvm->mmu_lock);
>>>> ???????? pgd = kvm->arch.pgd + stage2_pgd_index(addr);
>>>> ???????? do {
>>>>
>>>> I need to point out that I never reproduced it without this debugging patch.
>>>
>>> That could trigger the panic iff your "size" <= 0x200, leading to the
>>> condition (end < start), which can make the loop go forever, as we
>>> do while(addr < end) and end up accessing something which may not be PGD entry
>>> and thus get a bad page with bad numbers all around. This case could be hit only
>>> with your change and the bug in the KSM which gives us an address near the page
>>> boundary.
>> No, I injected the fault on purpose to simulate the case when size is less than
>> PAGE_SIZE(eg. PAGE_SIZE-0x200=65024)
>> I ever got the panic info [1] *without* the debugging patch only once
>>
>> [1] https://lkml.org/lkml/2018/5/9/992
>>>
>>> So, I think we can safely ignore the PANIC().
>>> More below.
I am a bit confused now. Do you mean, the panic was triggered *without* the debugging
patch ? If that is the case, it is really worrying.
>>>
>>>
>>>>>> Suzuki, thanks for the comments.
>>>>>>
>>>>>> I proposed another ksm patch https://lkml.org/lkml/2018/5/3/1042
>>>>>> The root cause is ksm will add some extra flags to indicate that the page
>>>>>> is in/not_in the stable tree. This makes address not be aligned with PAGE_SIZE.
>>>>> Thanks for the pointer. In the future, please Cc the people relevant to the
>>>>> discussion in the patches.
>>>>>
>>>>>> ? From arm kvm mmu point of view, do you think handle_hva_to_gpa still need
>>>>>> to handle
>>>>>> the unalignment case?
>>>>> I don't think we should do that. Had we done this, we would never have caught
>>>>> this bug
>>>>> in KSM. Eventually if some other new implementation comes up with the a new
>>>>> notifier
>>>>> consumer which doesn't check alignment and doesn't WARN, it could simply do
>>>>> the wrong
>>>>> thing. So I believe what we have is a good measure to make sure that things are
>>>>> in the right order.
>>>>>
>>>>>> IMO, the PAGE_SIZE alignment is still needed because we should not let the
>>>>>> bottom function
>>>>>> kvm_age_hva_handler to handle the exception. Please refer to the
>>>>>> implementation in X86 and
>>>>>> powerpc kvm_handle_hva_range(). They both aligned the hva with
>>>>>> hva_to_gfn_memslot.
>>>>>>
>>>>> ? From an API perspective, you are passed on a "start" and "end" address. So,
>>>>> you could potentially
>>>>> do the wrong thing if you align the "start" and "end". May be those handlers
>>>>> should also do the
>>>>> same thing as we do.
>>>
>>>> But handle_hva_to_gpa has partially adjusted the alignment possibly:
>>>> ??? 1750???????? kvm_for_each_memslot(memslot, slots) {
>>>> ??? 1751???????????????? unsigned long hva_start, hva_end;
>>>> ??? 1752???????????????? gfn_t gpa;
>>>> ??? 1753
>>>> ??? 1754???????????????? hva_start = max(start, memslot->userspace_addr);
>>>> ??? 1755???????????????? hva_end = min(end, memslot->userspace_addr +
>>>> ??? 1756???????????????????????????? (memslot->npages << PAGE_SHIFT));
>>>>
>>>> at line 1755, let us assume that end=0x12340200 and
>>>> memslot->userspace_addr + (memslot->npages << PAGE_SHIFT)=0x12340000
>>>> Then, hva_start is not page_size aligned and hva_end is aligned, and the size
>>>> will be PAGE_SIZE-0x200,
>>>> just as what I had done in the inject fault debugging patch.
>>>
>>> Thats because we want to limit the handling of the hva/gpa range by memslot. So,
>>> we make sure we pass on the range within the given memslot
>>> to hva_to_gfn_memslot(). But we do iterate over the next memslot if the
>>> original range falls in to the next slot. So, in practice, there is no
>>> alignment/trimming of the range. Its just that we pass on the appropriate range
>>> for each slot.
>>>
>> Yes, I understand what the codes did in hva_to_gfn_memslot(). What I mean is
>> hva_end may be changed and (hva_end - hva_start) will not be same as the
>> parameter _size_ ?
>>
>>> ret |= handler(kvm, gpa, (u64)(hva_end - hva_start), data);
>>
>> Anyway, I have to admit that all the exceptions are originally caused by the
>> STABLE_FLAG in ksm code. What I want to discuss here is how to make arm kvm
>> handle the exception more gracefully.
Thats my point. We need the fix in ksm. Once we have the fix in place, do
we hit the WARN_ON() any more ?
>>
> Hi Suzuki
> How about this patch (balance of avoiding the WARN_ON storm and debugging
> convenience):
The problem with WARN_ON_ONCE() is, it could potentially suppress the different
call paths that could lead to the triggers. e.g, unmap_stage2_range() could be
called from different paths and having a WARN_ON_ONCE() could potentially
hide the other call paths.
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 7f6a944..4033946 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -297,6 +297,8 @@ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t
> start, u64 size)
> phys_addr_t next;
>
> assert_spin_locked(&kvm->mmu_lock);
> +
> + WARN_ON_ONCE(size & ~PAGE_MASK);
> pgd = kvm->arch.pgd + stage2_pgd_index(addr);
> do {
> /*
> @@ -1792,7 +1794,7 @@ static int kvm_set_spte_handler(struct kvm *kvm, gpa_t
> gpa, u64 size, void *data
> {
> pte_t *pte = (pte_t *)data;
>
> - WARN_ON(size != PAGE_SIZE);
> + WARN_ON_ONCE(size != PAGE_SIZE);
> /*
> * We can always call stage2_set_pte with KVM_S2PTE_FLAG_LOGGING_ACTIVE
> * flag clear because MMU notifiers will have unmapped a huge PMD before
> @@ -1823,7 +1825,7 @@ static int kvm_age_hva_handler(struct kvm *kvm, gpa_t gpa,
> u64 size, void *data)
> pmd_t *pmd;
> pte_t *pte;
>
> - WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
> + WARN_ON_ONCE(size != PAGE_SIZE && size != PMD_SIZE);
> pmd = stage2_get_pmd(kvm, NULL, gpa);
> if (!pmd || pmd_none(*pmd)) /* Nothing there */
> return 0;
> @@ -1843,7 +1845,7 @@ static int kvm_test_age_hva_handler(struct kvm *kvm, gpa_t
> gpa, u64 size, void *
> pmd_t *pmd;
> pte_t *pte;
>
> - WARN_ON(size != PAGE_SIZE && size != PMD_SIZE);
> + WARN_ON_ONCE(size != PAGE_SIZE && size != PMD_SIZE);
> pmd = stage2_get_pmd(kvm, NULL, gpa);
> if (!pmd || pmd_none(*pmd)) /* Nothing there */
> return 0;
>
Cheers
Suzuki
^ permalink raw reply
* [PATCH 06/18] arm64: move sve_user_{enable, disable} to <asm/fpsimd.h>
From: Mark Rutland @ 2018-05-15 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515121921.GN7753@e103592.cambridge.arm.com>
On Tue, May 15, 2018 at 01:19:26PM +0100, Dave Martin wrote:
> On Tue, May 15, 2018 at 11:39:36AM +0100, Mark Rutland wrote:
> > On Mon, May 14, 2018 at 12:06:50PM +0100, Dave Martin wrote:
> > > On Mon, May 14, 2018 at 10:46:28AM +0100, Mark Rutland wrote:
> > > > +static inline void sve_user_disable(void)
> > > > +{
> > > > + sysreg_clear_set(cpacr_el1, CPACR_EL1_ZEN_EL0EN, 0);
> > > > +}
> > > > +
> > > > +static inline void sve_user_enable(void)
> > > > +{
> > > > + sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
> > > > +}
> > > > +
> > > > /*
> > > > * Probing and setup functions.
> > > > * Calls to these functions must be serialised with one another.
> > > > @@ -107,6 +119,9 @@ static inline int sve_get_current_vl(void)
> > > > return -EINVAL;
> > > > }
> > > >
> > > > +static inline void sve_user_disable(void) { }
> > > > +static inline void sve_user_enable(void) { }
> > > > +
> > >
> > > Alternatively, just move the full definitions outside the #ifdef
> > > CONFIG_ARM64_SVE.
> >
> > Can do, though I was trying to keep the exsting pattern with empty
> > inlines for the !CONFIG_ARM64_SVE case.
>
> There isn't really a pattern. I tried to avoid dummy versions where
> there's no real reason to have them. I don't _think_ they're really
> needed here, unless I missed something. Did you get build failures
> without them?
I need *some* definition so that sve_user_reset() in the syscall path
can compile without ifdeferry.
In sve_user_reset() I first check system_supports_sve(), which checks
IS_ENABLED(CONFIG_ARM64_SVE), so the call should be optimised away when
!CONFIG_ARM64_SVE, but I need a prototype regardless.
> > > All calls to these should be shadowed by an if
> > > (system_supports_sve()) in any case, and setting/clearing ZEN_EL0EN
> > > in the CPACR_EL1 ought to be harmless now that the meaning of these
> > > bits architecturally committed.
> > >
> > > Ideally we would have a BUG_ON(!system_supports_sve()) in those
> > > functions, but we won't won't to pay the cost in a production kernel.
> >
> > Earlier I'd put BUILD_BUG() in the body for the !CONFIG_ARM64_SVE case,
> > to catch that kind of thing -- I could restore that.
>
> IIUC:
>
> if (0) {
> BUILD_BUG_ON(1);
> }
>
> can still fire, in which case it's futile checking for CONFIG_ARM64_SVE
> in most of the SVE support code.
We already rely on BUILD_BUG() not firing in paths that can be trivially
optimized away. e.g. in the cmpxchg code.
> > > > static inline void sve_init_vq_map(void) { }
> > > > static inline void sve_update_vq_map(void) { }
> > > > static inline int sve_verify_vq_map(void) { return 0; }
> > > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > > > index 088940387a4d..79a81c7d85c6 100644
> > > > --- a/arch/arm64/kernel/fpsimd.c
> > > > +++ b/arch/arm64/kernel/fpsimd.c
> > > > @@ -159,7 +159,6 @@ static void sve_free(struct task_struct *task)
> > > > __sve_free(task);
> > > > }
> > > >
> > > > -
> > >
> > > Hmmm, Ack. Check for conflicts with the KVM FPSIMD rework [1] (though
> > > trivial).
> >
> > I'll assume that Ack stands regardless. :)
>
> Actually, I was just commenting on the deleted blank line...
Ah. I've restored that now.
Thanks,
Mark.
^ permalink raw reply
* [PATCH 2/3] x86/mm: add TLB purge to free pmd/pte page interfaces
From: Kani, Toshi @ 2018-05-15 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515140549.GE18595@8bytes.org>
On Tue, 2018-05-15 at 16:05 +0200, Joerg Roedel wrote:
> On Mon, Apr 30, 2018 at 11:59:24AM -0600, Toshi Kani wrote:
> > int pud_free_pmd_page(pud_t *pud, unsigned long addr)
> > {
> > - pmd_t *pmd;
> > + pmd_t *pmd, *pmd_sv;
> > + pte_t *pte;
> > int i;
> >
> > if (pud_none(*pud))
> > return 1;
> >
> > pmd = (pmd_t *)pud_page_vaddr(*pud);
> > + pmd_sv = (pmd_t *)__get_free_page(GFP_KERNEL);
>
> So you need to allocate a page to free a page? It is better to put the
> pages into a list with a list_head on the stack.
The code should have checked if pmd_sv is NULL... I will update the
patch.
For performance, I do not think this page alloc is a problem. Unlike
pmd_free_pte_page(), pud_free_pmd_page() covers an extremely rare case.
Since pud requires 1GB-alignment, pud and pmd/pte mappings do not
share the same ranges within the vmalloc space. I had to instrument the
kernel to force them share the same ranges in order to test this patch.
> I am still on favour of just reverting the broken commit and do a
> correct and working fix for the/a merge window.
I will reorder the patch series, and change patch 3/3 to 1/3 so that we
can take it first to fix the BUG_ON on PAE. This revert will disable
2MB ioremap on PAE in some cases, but I do not think it's important on
PAE anyway.
I do not think revert on x86/64 is necessary and I am more worried about
disabling 2MB ioremap in some cases, which can be seen as degradation.
Patch 2/3 fixes a possible page-directory cache issue that I cannot hit
even though I put ioremap/iounmap with various sizes into a tight loop
for a day.
Thanks,
-Toshi
^ permalink raw reply
* [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()
From: Andy Shevchenko @ 2018-05-15 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3aeb2ed038cbce8fe744b614dc19d414555a7e8f.1526375226.git.jan.kiszka@siemens.com>
On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> - res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> + res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> if (!res) {
> err = -ENOMEM;
> - goto parse_failed;
> + goto failed;
> }
>
> err = of_pci_range_to_resource(&range, dev_node, res);
> if (err) {
> - kfree(res);
> + devm_kfree(dev, res);
> continue;
> }
Can't you rather make it better, i.e.
struct resource tmp;
...
err = of_pci_range_to_resource(&range, dev_node, &tmp);
if (err)
continue;
res = devm_kmemdump();
if (!res) {
ret = -ENOMEM;
goto failed;
}
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()
From: Andy Shevchenko @ 2018-05-15 16:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHp75VeUbjobse6uLeM4h_xXRUw+YUQUN+EOye67AJy77a5euQ@mail.gmail.com>
On Tue, May 15, 2018 at 7:48 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> + devm_kfree(dev, res);
devm_kfree() makes my eyes hurt.
> res = devm_kmemdump();
devm_kmemdup(&tmp...);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2 4/4] KVM: arm64: Add support for PUD hugepages at stage 2
From: Catalin Marinas @ 2018-05-15 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180501102659.13188-5-punit.agrawal@arm.com>
On Tue, May 01, 2018 at 11:26:59AM +0100, Punit Agrawal wrote:
> KVM currently supports PMD hugepages at stage 2. Extend the stage 2
> fault handling to add support for PUD hugepages.
>
> Addition of pud hugepage support enables additional hugepage
> sizes (e.g., 1G with 4K granule) which can be useful on cores that
> support mapping larger block sizes in the TLB entries.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 19 ++++++++++++
> arch/arm64/include/asm/kvm_mmu.h | 15 ++++++++++
> arch/arm64/include/asm/pgtable-hwdef.h | 4 +++
> arch/arm64/include/asm/pgtable.h | 2 ++
> virt/kvm/arm/mmu.c | 40 ++++++++++++++++++++++++--
> 5 files changed, 77 insertions(+), 3 deletions(-)
Since this patch touches a couple of core arm64 files:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH] ARM: dts: imx28-tx28: Fix alias DTC warnings
From: Fabio Estevam @ 2018-05-15 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515064234.GJ26863@dragon>
On Tue, May 15, 2018 at 3:42 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> - spi_gpio: spi-gpio {
>> + spi_gpio: spi {
>
> Not sure why the node name needs to be changed here.
Yes, this is a different fix and should be part of a different patch.
^ permalink raw reply
* [PATCH] arm64: export memblock_reserve()d regions via /proc/iomem
From: James Morse @ 2018-05-15 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507024024.GA11326@linaro.org>
Hi Akashi,
On 07/05/18 03:40, Akashi Takahiro wrote:
> On Wed, May 02, 2018 at 11:35:04AM +0100, James Morse wrote:
>> On 25/04/18 14:22, James Morse wrote:
>>> There has been some confusion around what is necessary to prevent kexec
>>> overwriting important memory regions. memblock: reserve, or nomap?
>>> Only memblock nomap regions are reported via /proc/iomem, kexec's
>>> user-space doesn't know about memblock_reserve()d regions.
>>> But this was always broken, as the UEFI memory map is also reserved
>>> and not marked as nomap.
>>> Lastly, we add the memblock_reserved() regions using
>>> reserve_region_with_split(), which will fill in the gaps between the
>>> existing named regions. (e.g. the regions occupied by the __init code).
>>> This call uses the slab allocator, so has to run from an initcall.
>> Re-reading Akashi's description of how kdump generates the ELF headers for
>> /proc/vmcore, this change might break kdump, as now the memblock_reserved()
>> regions may be missing from /proc/vmcore. (I'll test that).
> I also tested your patch, and there seems to be something wrong.
(excellent, I've been trying to remember how to build crash!)
> Actually, crash utility fails to read the core:
> crash-arm64: read error: kernel virtual address: ffff0000091fa998 \
> type: "shadow_timekeeper xtime_sec"
> crash-arm64: read error: kernel virtual address: ffff0000091fda48 \
> type: "high_memory"
> Adding some debug messages shows that "shadow_timekeeper" variable
> belongs to the range (A), which is originally part of "Kernel data"
> and should have been unmarked from "reserved" list.
Heh, so it caught fire even earlier. I was expecting only the per-cpu regions to
go missing.
Yes, the problems are going to be spurious-reserved and regions that aren't
reserved anymore, but were when the /proc/iomem list was generated.
>> Unless there is a 'name' for a region that kexec-tools interprets as "don't
>> overwrite this, but do include it in the ELF header", then we're stuck. We can't
>> fix kexec without breaking kdump, we have to change user-space.
>>
>> Of the two, missing data from kdump vmcore would be preferable to failed-to-boot
>> kexec.
>
> Well, kexec has had this potential bug since the day One in mainline.
> (we were then using "boot wrapper" instead of normal boot loaders though.)
> Even if we accept your assertion above now, future new-comers may
> "rediscover" a kdump bug and make a complaint time to time.
> So I think that we would be better off fixing the issue right now
> completely with accompanying user-space change.
I agree we need to fix this 'now'. But changing user-space means people still
hit this bug, and are told 'update both user-space and your kernel'.
This is because kexec-tools is using the /proc/iomem list for two things:
1 don't write here, you need it to boot.
2 preserve this memory, you need it in the vmcore.
I think kdump is less common and more best-effort, my preference would be:
>> If we have to change user-space, I'd like to make use of the kernels
>> ability to generate the ELF header itself, (e.g. kexec_file_load()).
For kexec-tools to load the vmcore data it currently generates itself from
/sys/kernel/kdump_elf_header or similar.
This splits the 1&2 cases above, meaning the kernel can export information for
(1) via /proc/iomem and (2) via the new file.
old kexec-tools still works for kexec, but hits the problem you found for kdump.
new kexec-tools aware of (2) works in both cases.
But merging this patch to fix old kexec-tools will prevent us doing anything
'neater', and any fix for old-kexec is going to break kdump with the same
user-space.
For your proposed kexec-tool and kernel changes, how will old kexec-tools work?
Is it going to be any worse than today?
Thanks,
James
^ permalink raw reply
* [PATCH v9 07/11] arm64: kexec_file: add crash dump support
From: James Morse @ 2018-05-15 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180425062629.29404-8-takahiro.akashi@linaro.org>
Hi Akashi,
On 25/04/18 07:26, AKASHI Takahiro wrote:
> Enabling crash dump (kdump) includes
> * prepare contents of ELF header of a core dump file, /proc/vmcore,
> using crash_prepare_elf64_headers(), and
> * add two device tree properties, "linux,usable-memory-range" and
> "linux,elfcorehdr", which represent repsectively a memory range
(Nit: respectively)
> to be used by crash dump kernel and the header's location
> arch/arm64/include/asm/kexec.h | 4 +
> arch/arm64/kernel/kexec_image.c | 9 +-
> arch/arm64/kernel/machine_kexec_file.c | 202 +++++++++++++++++++++++++
In this patch, machine_kexec_file.c gains its own private fdt array encoder.
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 37c0a9dc2e47..ec674f4d267c 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -76,6 +81,78 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> return ret;
> }
>
> +static int __init arch_kexec_file_init(void)
> +{
> + /* Those values are used later on loading the kernel */
> + __dt_root_addr_cells = dt_root_addr_cells;
> + __dt_root_size_cells = dt_root_size_cells;
> +
> + return 0;
> +}
> +late_initcall(arch_kexec_file_init);
If we need these is it worth taking them out of __initdata? I note they've been
'temporary' for quite a long time.
> +
> +#define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
> +#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
> +
> +static int fdt_prop_len(const char *prop_name, int len)
> +{
> + return (strlen(prop_name) + 1) +
> + sizeof(struct fdt_property) +
> + FDT_TAGALIGN(len);
> +}
This stuff should really be in libfdt.h Those macros come from
libfdt_internal.h, so we're probably doing something wrong here.
> +static bool cells_size_fitted(unsigned long base, unsigned long size)
> +{
> + /* if *_cells >= 2, cells can hold 64-bit values anyway */
> + if ((__dt_root_addr_cells == 1) && (base >= (1ULL << 32)))
> + return false;
> +
> + if ((__dt_root_size_cells == 1) && (size >= (1ULL << 32)))
> + return false;
Using '> U32_MAX' here may be more readable.
> + return true;
> +}
> +
> +static void fill_property(void *buf, u64 val64, int cells)
> +{
> + u32 val32;
> +
> + if (cells == 1) {
> + val32 = cpu_to_fdt32((u32)val64);
> + memcpy(buf, &val32, sizeof(val32));
> + } else {
> + memset(buf, 0, cells * sizeof(u32) - sizeof(u64));
> + buf += cells * sizeof(u32) - sizeof(u64);
Is this trying to clear the 'top' cells and shuffle the pointer to point at the
'bottom' 2? I'm pretty sure this isn't endian safe.
Do we really expect a system to have #address-cells > 2?
> + val64 = cpu_to_fdt64(val64);
> + memcpy(buf, &val64, sizeof(val64));
> + }
> +}
> +
> +static int fdt_setprop_range(void *fdt, int nodeoffset, const char *name,
> + unsigned long addr, unsigned long size)
(the device-tree spec describes a 'ranges' property, which had me confused. This
is encoding a prop-encoded-array)
> +{
> + void *buf, *prop;
> + size_t buf_size;
> + int result;
> +
> + buf_size = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
> + prop = buf = vmalloc(buf_size);
virtual memory allocation for something less than PAGE_SIZE?
> + if (!buf)
> + return -ENOMEM;
> +
> + fill_property(prop, addr, __dt_root_addr_cells);
> + prop += __dt_root_addr_cells * sizeof(u32);
> +
> + fill_property(prop, size, __dt_root_size_cells);
> +
> + result = fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
> +
> + vfree(buf);
> +
> + return result;
> +}
Doesn't this stuff belong in libfdt? I guess there is no 'add array element' api
because this the first time we've wanted to create a node with more than
key=fixed-size-value.
I don't think this belongs in arch C code. Do we have a plan for getting libfdt
to support encoding prop-arrays? Can we put it somewhere anyone else duplicating
this will find it, until we can (re)move it?
I have no idea how that happens... it looks like the devicetree list is the
place to ask.
> static int setup_dtb(struct kimage *image,
> unsigned long initrd_load_addr, unsigned long initrd_len,
> char *cmdline, unsigned long cmdline_len,
> @@ -88,10 +165,26 @@ static int setup_dtb(struct kimage *image,
> int range_len;
> int ret;
>
> + /* check ranges against root's #address-cells and #size-cells */
> + if (image->type == KEXEC_TYPE_CRASH &&
> + (!cells_size_fitted(image->arch.elf_load_addr,
> + image->arch.elf_headers_sz) ||
> + !cells_size_fitted(crashk_res.start,
> + crashk_res.end - crashk_res.start + 1))) {
> + pr_err("Crash memory region doesn't fit into DT's root cell sizes.\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
To check I've understood this properly: This can happen if the firmware provided
a DTB with 32bit address/size cells, but at least some of the memory requires 64
bit address/size cells. This could only happen on a UEFI system where the
firmware-DTB doesn't describe memory. ACPI-only systems would have the EFIstub DT.
> /* duplicate dt blob */
> buf_size = fdt_totalsize(initial_boot_params);
> range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
>
> + if (image->type == KEXEC_TYPE_CRASH)
> + buf_size += fdt_prop_len("linux,elfcorehdr", range_len)
> + + fdt_prop_len("linux,usable-memory-range",
> + range_len);
> +
> if (initrd_load_addr)
> buf_size += fdt_prop_len("linux,initrd-start", sizeof(u64))
> + fdt_prop_len("linux,initrd-end", sizeof(u64));
> @@ -113,6 +206,23 @@ static int setup_dtb(struct kimage *image,
> if (nodeoffset < 0)
> goto out_err;
>
> + if (image->type == KEXEC_TYPE_CRASH) {
> + /* add linux,elfcorehdr */
> + ret = fdt_setprop_range(buf, nodeoffset, "linux,elfcorehdr",
> + image->arch.elf_load_addr,
> + image->arch.elf_headers_sz);
> + if (ret)
> + goto out_err;
> +
> + /* add linux,usable-memory-range */
> + ret = fdt_setprop_range(buf, nodeoffset,
> + "linux,usable-memory-range",
> + crashk_res.start,
> + crashk_res.end - crashk_res.start + 1);
Don't you need to add "linux,usable-memory-range" to the buf_size estimate?
> + if (ret)
> + goto out_err;
> + }
> @@ -148,17 +258,109 @@ static int setup_dtb(struct kimage *image,
> +static struct crash_mem *get_crash_memory_ranges(void)
> +{
> + unsigned int nr_ranges;
> + struct crash_mem *cmem;
> +
> + nr_ranges = 1; /* for exclusion of crashkernel region */
> + walk_system_ram_res(0, -1, &nr_ranges, get_nr_ranges_callback);
> +
> + cmem = vmalloc(sizeof(struct crash_mem) +
> + sizeof(struct crash_mem_range) * nr_ranges);
> + if (!cmem)
> + return NULL;
> +
> + cmem->max_nr_ranges = nr_ranges;
> + cmem->nr_ranges = 0;
> + walk_system_ram_res(0, -1, cmem, add_mem_range_callback);
> +
> + /* Exclude crashkernel region */
> + if (crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end)) {
> + vfree(cmem);
> + return NULL;
> + }
> +
> + return cmem;
> +}
Could this function be included in prepare_elf_headers() so that the alloc() and
free() occur together.
> +static int prepare_elf_headers(void **addr, unsigned long *sz)
> +{
> + struct crash_mem *cmem;
> + int ret = 0;
> +
> + cmem = get_crash_memory_ranges();
> + if (!cmem)
> + return -ENOMEM;
> +
> + ret = crash_prepare_elf64_headers(cmem, true, addr, sz);
> +
> + vfree(cmem);
> + return ret;
> +}
All this is moving memory-range information from core-code's
walk_system_ram_res() into core-code's struct crash_mem, and excluding
crashk_res, which again is accessible to the core code.
It looks like this is duplicated in arch/x86 and arch/arm64 because arm64
doesn't have a second 'crashk_low_res' region, and always wants elf64, instead
of when IS_ENABLED(CONFIG_X86_64).
If we can abstract just those two, more of this could be moved to core code
where powerpc can make use of it if they want to support kdump with
kexec_file_load().
But, its getting late for cross-architecture dependencies, lets put that on the
for-later list. (assuming there isn't a powerpc-kdump series out there adding a
third copy of this)
Thanks,
James
^ permalink raw reply
* [PATCH v2 4/4] KVM: arm64: Add support for PUD hugepages at stage 2
From: Punit Agrawal @ 2018-05-15 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515165620.37l2epyno3vdcisu@armageddon.cambridge.arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> On Tue, May 01, 2018 at 11:26:59AM +0100, Punit Agrawal wrote:
>> KVM currently supports PMD hugepages at stage 2. Extend the stage 2
>> fault handling to add support for PUD hugepages.
>>
>> Addition of pud hugepage support enables additional hugepage
>> sizes (e.g., 1G with 4K granule) which can be useful on cores that
>> support mapping larger block sizes in the TLB entries.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Christoffer Dall <christoffer.dall@arm.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> ---
>> arch/arm/include/asm/kvm_mmu.h | 19 ++++++++++++
>> arch/arm64/include/asm/kvm_mmu.h | 15 ++++++++++
>> arch/arm64/include/asm/pgtable-hwdef.h | 4 +++
>> arch/arm64/include/asm/pgtable.h | 2 ++
>> virt/kvm/arm/mmu.c | 40 ++++++++++++++++++++++++--
>> 5 files changed, 77 insertions(+), 3 deletions(-)
>
> Since this patch touches a couple of core arm64 files:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Thanks Catalin.
I've posted a v3 with minor changes yesterday[0]. Can you comment there?
Or maybe Marc can apply the tag while merging the patches.
[0] https://lkml.org/lkml/2018/5/14/912
^ permalink raw reply
* [PATCH v9 07/11] arm64: kexec_file: add crash dump support
From: James Morse @ 2018-05-15 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180425062629.29404-8-takahiro.akashi@linaro.org>
Hi guys,
(CC: +RobH, devicetree list)
On 25/04/18 07:26, AKASHI Takahiro wrote:
> Enabling crash dump (kdump) includes
> * prepare contents of ELF header of a core dump file, /proc/vmcore,
> using crash_prepare_elf64_headers(), and
> * add two device tree properties, "linux,usable-memory-range" and
> "linux,elfcorehdr", which represent repsectively a memory range
> to be used by crash dump kernel and the header's location
kexec_file_load() on arm64 needs to be able to create a prop encoded array to
the FDT, but there doesn't appear to be a libfdt helper to do this.
Akashi's code below adds fdt_setprop_range() to the arch code, and duplicates
bits of libfdt_internal.h to do the work.
How should this be done? I'm assuming this is something we need a new API in
libfdt.h for. How do these come about, and is there an interim step we can use
until then?
Thanks!
James
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 37c0a9dc2e47..ec674f4d267c 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -76,6 +81,78 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> return ret;
> }
>
> +static int __init arch_kexec_file_init(void)
> +{
> + /* Those values are used later on loading the kernel */
> + __dt_root_addr_cells = dt_root_addr_cells;
> + __dt_root_size_cells = dt_root_size_cells;
> +
> + return 0;
> +}
> +late_initcall(arch_kexec_file_init);
> +
> +#define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
> +#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
> +
> +static int fdt_prop_len(const char *prop_name, int len)
> +{
> + return (strlen(prop_name) + 1) +
> + sizeof(struct fdt_property) +
> + FDT_TAGALIGN(len);
> +}
> +
> +static bool cells_size_fitted(unsigned long base, unsigned long size)
> +{
> + /* if *_cells >= 2, cells can hold 64-bit values anyway */
> + if ((__dt_root_addr_cells == 1) && (base >= (1ULL << 32)))
> + return false;
> +
> + if ((__dt_root_size_cells == 1) && (size >= (1ULL << 32)))
> + return false;
> +
> + return true;
> +}
> +
> +static void fill_property(void *buf, u64 val64, int cells)
> +{
> + u32 val32;
> +
> + if (cells == 1) {
> + val32 = cpu_to_fdt32((u32)val64);
> + memcpy(buf, &val32, sizeof(val32));
> + } else {
> + memset(buf, 0, cells * sizeof(u32) - sizeof(u64));
> + buf += cells * sizeof(u32) - sizeof(u64);
> +
> + val64 = cpu_to_fdt64(val64);
> + memcpy(buf, &val64, sizeof(val64));
> + }
> +}
> +
> +static int fdt_setprop_range(void *fdt, int nodeoffset, const char *name,
> + unsigned long addr, unsigned long size)
> +{
> + void *buf, *prop;
> + size_t buf_size;
> + int result;
> +
> + buf_size = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
> + prop = buf = vmalloc(buf_size);
> + if (!buf)
> + return -ENOMEM;
> +
> + fill_property(prop, addr, __dt_root_addr_cells);
> + prop += __dt_root_addr_cells * sizeof(u32);
> +
> + fill_property(prop, size, __dt_root_size_cells);
> +
> + result = fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
> +
> + vfree(buf);
> +
> + return result;
> +}
> +
> static int setup_dtb(struct kimage *image,
> unsigned long initrd_load_addr, unsigned long initrd_len,
> char *cmdline, unsigned long cmdline_len,
> @@ -88,10 +165,26 @@ static int setup_dtb(struct kimage *image,
> int range_len;
> int ret;
>
> + /* check ranges against root's #address-cells and #size-cells */
> + if (image->type == KEXEC_TYPE_CRASH &&
> + (!cells_size_fitted(image->arch.elf_load_addr,
> + image->arch.elf_headers_sz) ||
> + !cells_size_fitted(crashk_res.start,
> + crashk_res.end - crashk_res.start + 1))) {
> + pr_err("Crash memory region doesn't fit into DT's root cell sizes.\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
> +
> /* duplicate dt blob */
> buf_size = fdt_totalsize(initial_boot_params);
> range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
>
> + if (image->type == KEXEC_TYPE_CRASH)
> + buf_size += fdt_prop_len("linux,elfcorehdr", range_len)
> + + fdt_prop_len("linux,usable-memory-range",
> + range_len);
> +
> if (initrd_load_addr)
> buf_size += fdt_prop_len("linux,initrd-start", sizeof(u64))
> + fdt_prop_len("linux,initrd-end", sizeof(u64));
> @@ -113,6 +206,23 @@ static int setup_dtb(struct kimage *image,
> if (nodeoffset < 0)
> goto out_err;
>
> + if (image->type == KEXEC_TYPE_CRASH) {
> + /* add linux,elfcorehdr */
> + ret = fdt_setprop_range(buf, nodeoffset, "linux,elfcorehdr",
> + image->arch.elf_load_addr,
> + image->arch.elf_headers_sz);
> + if (ret)
> + goto out_err;
> +
> + /* add linux,usable-memory-range */
> + ret = fdt_setprop_range(buf, nodeoffset,
> + "linux,usable-memory-range",
> + crashk_res.start,
> + crashk_res.end - crashk_res.start + 1);
> + if (ret)
> + goto out_err;
> + }
> +
> /* add bootargs */
> if (cmdline) {
> ret = fdt_setprop(buf, nodeoffset, "bootargs",
^ permalink raw reply
* [PATCH 0/3] coresight: Don't use contextID with PID namespaces
From: Mathieu Poirier @ 2018-05-15 17:13 UTC (permalink / raw)
To: linux-arm-kernel
Since the in-kernel value of a PID differs from what is seen from
PID namespaces, using contextID tracing with PID namespaces makes the
feature confusing to use and potentially subject to leaking out internal
kernel information.
This set returns an error if contextID and PID namespaces are used in
conjunction and gets rid of the vpid-to-pid translation function as it
is no longer needed.
Thanks,
Mathieu
Mathieu Poirier (3):
coresight: etm3x: Don't use contextID with PID namespaces
coresight: etm4x: Don't use contextID with PID namespaces
coresight: Remove function coresight_vpid_to_pid()
drivers/hwtracing/coresight/coresight-etm.h | 3 --
.../hwtracing/coresight/coresight-etm3x-sysfs.c | 43 +++++++++++++++++---
drivers/hwtracing/coresight/coresight-etm3x.c | 4 +-
.../hwtracing/coresight/coresight-etm4x-sysfs.c | 47 +++++++++++++++++-----
drivers/hwtracing/coresight/coresight-etm4x.h | 3 --
include/linux/coresight.h | 20 ---------
6 files changed, 76 insertions(+), 44 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] coresight: etm3x: Don't use contextID with PID namespaces
From: Mathieu Poirier @ 2018-05-15 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526404417-32507-1-git-send-email-mathieu.poirier@linaro.org>
Tracers can trigger trace acquisition based on contextID value, something
that isn't useful when PID namespaces are enabled. Indeed the PID value
of a process has a different representation in the kernel and the PID
namespace, making the feature confusing and potentially leaking internal
kernel information.
As such simply return an error when the feature is being used from a
PID namespace other than the default one.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm.h | 3 --
.../hwtracing/coresight/coresight-etm3x-sysfs.c | 43 +++++++++++++++++++---
drivers/hwtracing/coresight/coresight-etm3x.c | 4 +-
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index e8b4549e30e2..79e1ad860d8a 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -168,8 +168,6 @@
* @seq_curr_state: current value of the sequencer register.
* @ctxid_idx: index for the context ID registers.
* @ctxid_pid: value for the context ID to trigger on.
- * @ctxid_vpid: Virtual PID seen by users if PID namespace is enabled, otherwise
- * the same value of ctxid_pid.
* @ctxid_mask: mask applicable to all the context IDs.
* @sync_freq: Synchronisation frequency.
* @timestamp_event: Defines an event that requests the insertion
@@ -202,7 +200,6 @@ struct etm_config {
u32 seq_curr_state;
u8 ctxid_idx;
u32 ctxid_pid[ETM_MAX_CTXID_CMP];
- u32 ctxid_vpid[ETM_MAX_CTXID_CMP];
u32 ctxid_mask;
u32 sync_freq;
u32 timestamp_event;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 9435c1481f61..75487b3fad86 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -4,6 +4,7 @@
* Author: Mathieu Poirier <mathieu.poirier@linaro.org>
*/
+#include <linux/pid_namespace.h>
#include <linux/pm_runtime.h>
#include <linux/sysfs.h>
#include "coresight-etm.h"
@@ -1025,8 +1026,15 @@ static ssize_t ctxid_pid_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
+ /*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
spin_lock(&drvdata->spinlock);
- val = config->ctxid_vpid[config->ctxid_idx];
+ val = config->ctxid_pid[config->ctxid_idx];
spin_unlock(&drvdata->spinlock);
return sprintf(buf, "%#lx\n", val);
@@ -1037,19 +1045,28 @@ static ssize_t ctxid_pid_store(struct device *dev,
const char *buf, size_t size)
{
int ret;
- unsigned long vpid, pid;
+ unsigned long pid;
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
- ret = kstrtoul(buf, 16, &vpid);
+ /*
+ * When contextID tracing is enabled the tracers will insert the
+ * value found in the contextID register in the trace stream. But if
+ * a process is in a namespace the PID of that process as seen from the
+ * namespace won't be what the kernel sees, something that makes the
+ * feature confusing and can potentially leak kernel only information.
+ * As such refuse to use the feature if @current is not in the initial
+ * PID namespace.
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
+ ret = kstrtoul(buf, 16, &pid);
if (ret)
return ret;
- pid = coresight_vpid_to_pid(vpid);
-
spin_lock(&drvdata->spinlock);
config->ctxid_pid[config->ctxid_idx] = pid;
- config->ctxid_vpid[config->ctxid_idx] = vpid;
spin_unlock(&drvdata->spinlock);
return size;
@@ -1063,6 +1080,13 @@ static ssize_t ctxid_mask_show(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
+ /*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
val = config->ctxid_mask;
return sprintf(buf, "%#lx\n", val);
}
@@ -1076,6 +1100,13 @@ static ssize_t ctxid_mask_store(struct device *dev,
struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etm_config *config = &drvdata->config;
+ /*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
ret = kstrtoul(buf, 16, &val);
if (ret)
return ret;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index 15ed64d51a5b..7c74263c333d 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -230,10 +230,8 @@ void etm_set_default(struct etm_config *config)
config->seq_curr_state = 0x0;
config->ctxid_idx = 0x0;
- for (i = 0; i < ETM_MAX_CTXID_CMP; i++) {
+ for (i = 0; i < ETM_MAX_CTXID_CMP; i++)
config->ctxid_pid[i] = 0x0;
- config->ctxid_vpid[i] = 0x0;
- }
config->ctxid_mask = 0x0;
/* Setting default to 1024 as per TRM recommendation */
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] coresight: etm4x: Don't use contextID with PID namespaces
From: Mathieu Poirier @ 2018-05-15 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526404417-32507-1-git-send-email-mathieu.poirier@linaro.org>
As with ETM3x, the ETM4x tracers can trigger trace acquisition based on
contextID value, something that isn't useful when PID namespaces are
enabled. Indeed the PID value of a process has a different representation
in the kernel and the PID namespace, making the feature confusing and
potentially leaking internal kernel information.
As such simply return an error when the feature is being used from a
PID namespace other than the default one.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../hwtracing/coresight/coresight-etm4x-sysfs.c | 47 +++++++++++++++++-----
drivers/hwtracing/coresight/coresight-etm4x.h | 3 --
2 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 4eb8da785ce0..a0365e23678e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -4,6 +4,7 @@
* Author: Mathieu Poirier <mathieu.poirier@linaro.org>
*/
+#include <linux/pid_namespace.h>
#include <linux/pm_runtime.h>
#include <linux/sysfs.h>
#include "coresight-etm4x.h"
@@ -250,10 +251,8 @@ static ssize_t reset_store(struct device *dev,
}
config->ctxid_idx = 0x0;
- for (i = 0; i < drvdata->numcidc; i++) {
+ for (i = 0; i < drvdata->numcidc; i++)
config->ctxid_pid[i] = 0x0;
- config->ctxid_vpid[i] = 0x0;
- }
config->ctxid_mask0 = 0x0;
config->ctxid_mask1 = 0x0;
@@ -1637,9 +1636,16 @@ static ssize_t ctxid_pid_show(struct device *dev,
struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etmv4_config *config = &drvdata->config;
+ /*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
spin_lock(&drvdata->spinlock);
idx = config->ctxid_idx;
- val = (unsigned long)config->ctxid_vpid[idx];
+ val = (unsigned long)config->ctxid_pid[idx];
spin_unlock(&drvdata->spinlock);
return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
}
@@ -1649,26 +1655,35 @@ static ssize_t ctxid_pid_store(struct device *dev,
const char *buf, size_t size)
{
u8 idx;
- unsigned long vpid, pid;
+ unsigned long pid;
struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etmv4_config *config = &drvdata->config;
/*
+ * When contextID tracing is enabled the tracers will insert the
+ * value found in the contextID register in the trace stream. But if
+ * a process is in a namespace the PID of that process as seen from the
+ * namespace won't be what the kernel sees, something that makes the
+ * feature confusing and can potentially leak kernel only information.
+ * As such refuse to use the feature if @current is not in the initial
+ * PID namespace.
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
+ /*
* only implemented when ctxid tracing is enabled, i.e. at least one
* ctxid comparator is implemented and ctxid is greater than 0 bits
* in length
*/
if (!drvdata->ctxid_size || !drvdata->numcidc)
return -EINVAL;
- if (kstrtoul(buf, 16, &vpid))
+ if (kstrtoul(buf, 16, &pid))
return -EINVAL;
- pid = coresight_vpid_to_pid(vpid);
-
spin_lock(&drvdata->spinlock);
idx = config->ctxid_idx;
config->ctxid_pid[idx] = (u64)pid;
- config->ctxid_vpid[idx] = (u64)vpid;
spin_unlock(&drvdata->spinlock);
return size;
}
@@ -1682,6 +1697,13 @@ static ssize_t ctxid_masks_show(struct device *dev,
struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
struct etmv4_config *config = &drvdata->config;
+ /*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
spin_lock(&drvdata->spinlock);
val1 = config->ctxid_mask0;
val2 = config->ctxid_mask1;
@@ -1699,6 +1721,13 @@ static ssize_t ctxid_masks_store(struct device *dev,
struct etmv4_config *config = &drvdata->config;
/*
+ * Don't use contextID tracing if coming from a PID namespace. See
+ * comment in ctxid_pid_store().
+ */
+ if (task_active_pid_ns(current) != &init_pid_ns)
+ return -EINVAL;
+
+ /*
* only implemented when ctxid tracing is enabled, i.e. at least one
* ctxid comparator is implemented and ctxid is greater than 0 bits
* in length
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index b7c4a6f6c6b9..52786e9d8926 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -230,8 +230,6 @@
* @addr_type: Current status of the comparator register.
* @ctxid_idx: Context ID index selector.
* @ctxid_pid: Value of the context ID comparator.
- * @ctxid_vpid: Virtual PID seen by users if PID namespace is enabled, otherwise
- * the same value of ctxid_pid.
* @ctxid_mask0:Context ID comparator mask for comparator 0-3.
* @ctxid_mask1:Context ID comparator mask for comparator 4-7.
* @vmid_idx: VM ID index selector.
@@ -274,7 +272,6 @@ struct etmv4_config {
u8 addr_type[ETM_MAX_SINGLE_ADDR_CMP];
u8 ctxid_idx;
u64 ctxid_pid[ETMv4_MAX_CTXID_CMP];
- u64 ctxid_vpid[ETMv4_MAX_CTXID_CMP];
u32 ctxid_mask0;
u32 ctxid_mask1;
u8 vmid_idx;
--
2.7.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