Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Geert Uytterhoeven @ 2017-04-24 10:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <58F06092.9080409@mentor.com>

Hi Jiada,

On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie@kernel.org>  wrote:
>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang at mentor.com wrote:
>>>> From: Jiada Wang<jiada_wang@mentor.com>
>>>>
>>>> v1:
>>>>    add Slave mode support in SPI core
>>>>    spidev create slave device when SPI controller work in slave mode
>>>>    spi-imx support to work in slave mode
>>>
>>> Adding Geert who also had a series doing this in progress that was
>>> getting very near to being merged.
>>
>> Thank you!
>>
>> Actually my plan is to fix the last remaining issues and resubmit for
>> v4.13.
>
> I noticed your patch set for SPI slave support,
> (I am sure you can find out some of the change
> in this patch set is based on your work).
> we have similar requirement to add slave mode support to ecspi IP on imx6
> Soc.
>
> Our use case is to use spidev as an interface to communicate with external
> SPI master devices.
> meanwhile the SPI bus controller can also act as master device to send data
> to other
> SPI slave devices on the board.

That sounds a bit hackish to me. SPI was never meant to be a multi-master bus.
While it can be done, you will need external synchronization (signals) to
avoid conflicts between the SPI masters.

> I found in your implementation, SPI bus controller is limited to either work
> in master mode or
> slave mode, is there any reasoning to not configure SPI mode based on SPI
> devices use case?

If you really need both master and slave support, you can use 2 subnodes
in DT, the first representing the master, the second the slave.

Mark, what's your opinion about this?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] arm64: fix the overlap between the kernel image and vmalloc address
From: Mark Rutland @ 2017-04-24 10:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493025729-21505-1-git-send-email-zhongjiang@huawei.com>

Hi,

Thanks for reporting the problematic usage of is_vmalloc_addr() and
vmalloc_to_page() here. That is a real problem that we need to address.

On Mon, Apr 24, 2017 at 05:22:09PM +0800, zhongjiang wrote:
> From: zhong jiang <zhongjiang@huawei.com>
>
> Recently, xiaojun report the following issue.
>
> [ 4544.984139] Unable to handle kernel paging request at virtual address ffff804392800000
> [ 4544.991995] pgd = ffff80096745f000
> [ 4544.995369] [ffff804392800000] *pgd=0000000000000000

> [ 4545.425416] fc00: ffff0000081fdfd0 0000ffffa9c87440
> [ 4545.430248] [<ffff0000083a1000>] __memcpy+0x100/0x180
> [ 4545.435253] [<ffff000008270f64>] read_kcore+0x21c/0x3b0
> [ 4545.440429] [<ffff00000826340c>] proc_reg_read+0x64/0x90
> [ 4545.445691] [<ffff0000081fb83c>] __vfs_read+0x1c/0x108
> [ 4545.450779] [<ffff0000081fcb28>] vfs_read+0x80/0x130
> [ 4545.455696] [<ffff0000081fe014>] SyS_read+0x44/0xa0
> [ 4545.460528] [<ffff000008082f30>] el0_svc_naked+0x24/0x28
> [ 4545.465790] Code: d503201f d503201f d503201f d503201f (a8c12027)
> [ 4545.471852] ---[ end trace 4d1897f94759f461 ]---
> [ 4545.476435] note: cat[8976] exited with preempt_count 2
>
> I find the issue is introduced when applying commit f9040773b7bb
> ("arm64: move kernel image to base of vmalloc area"). This patch
> make the kernel image overlap with vmalloc area. It will result in
> vmalloc area have the huge page table. but the vmalloc_to_page is
> not realize the change. and the function is public to any arch.

So the issue is that we have the callchain below for a kernel image
address:

read_kcore()
->is_vmalloc_or_module_addr() // returns true
->vread()
-->aligned_vread()
--->vmalloc_to_page()

In is_vmalloc{,or_module}_addr() we just check the addr against
VMALLOC_START and VMALLOC_END, so they will return true for a kernel
image address.

Then, we call vmalloc_to_page(). While this only handles mappings made
at page granularity, the kernel image mapping may have used sections. So
this tries a bogus walk to the pte level.

Evidently, we assume that any memory in the vmalloc area (or module
areas) is mapped at page granularity. Is that always the case?

AFAICT, memremap'd memory isn't necessarily, but vread() should skip
that due to the VM_IOREMAP flag on the vma. The KASAN region should be
below MODULES_VADDR on arm64. I'm not sure if there's anything else.

Does it make sense to teach vmalloc_to_page() about section mappings?

Should we special-case kernel image handling, e.g. with new
is_kernel_image_addr() / kernel_image_to_page() helpers?

Do we need to shuffle things around such that the kernel image is not
between VMALLOC_START and VMALLOC_END?

> I fix it by change the init mapping to make it keep the accordance
> with vmalloc area mapping.
>
> Fixes: f9040773b7bb ("arm64: move kernel image to base of vmalloc area")
> Reported-by: tan xiaojun <tanxiaojun@huawei.com>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  arch/arm64/mm/mmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e4..2d8b34d 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -185,7 +185,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>
>               /* try section mapping first */
>               if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
> -                   !page_mappings_only) {
> +                   !page_mappings_only && !is_vmalloc_addr((void *)addr)) {
>                       /*
>                        * Set the contiguous bit for the subsequent group of
>                        * PMDs if its size and alignment are appropriate.
> @@ -256,7 +256,8 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
>               /*
>                * For 4K granule only, attempt to put down a 1GB block
>                */
> -             if (use_1G_block(addr, next, phys) && !page_mappings_only) {
> +             if (use_1G_block(addr, next, phys) && !page_mappings_only &&
> +                                     !is_vmalloc_addr((void *)addr)) {
>                       pud_set_huge(pud, phys, prot);
>

This will force the kernel image mappings to use page granularity, which
will come at a significant TLB pressure cost, and would be incredibly
unfortunate.

I would rather we solved this through other means.

Thanks,
Mark.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply

* [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
From: Guillaume Tucker @ 2017-04-24 10:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b8cf7acd-83e9-49fb-0800-19a76b8dcce5@collabora.com>

On 18/04/17 10:15, Guillaume Tucker wrote:
> Hi Rob,
>
> On 04/04/17 03:00, Rob Herring wrote:
>> On Sun, Apr 02, 2017 at 08:59:44AM +0100, Guillaume Tucker wrote:

>>> +- operating-points : Refer to Documentation/devicetree/bindings/power/opp.txt
>>> +  for details.
>>
>> Is this going to be sufficient vs. operating-points-v2? Or should it be
>> a power domain with OPPs?
>
> In principle, switching to operating-points-v2 should be very
> straightforward.  I have smoke-tested the driver with an
> operating-points-v2 table and a phandle to it inside the gpu node
> in place of operating-points and it seems to be working fine.  At
> least it parsed the OPPs and got initialised correctly.
>
> My understanding is that operating-points (v1) are not deprecated
> so we could keep the bindings as-is, but please let me know
> otherwise and I can try to address that in my next patch version.
> In the documentation, it should only be the case of replacing
> operating-points with operating-points-v2.

While the opp bindings documentation doesn't mention anything
about operating-points being deprecated, the code and comments in
of.c are pretty clear about this:

	/*
	 * OPPs have two version of bindings now. The older one is deprecated,
	 * try for the new binding first.
	 */

So I shall use operating-points-v2 in my patch v4.

Thanks,
Guillaume

^ permalink raw reply

* [PATCH] arm64: dts: rockchip: update cpu opp table for rk3399 op1
From: Heiko Stübner @ 2017-04-24 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9edc6ebc-8aab-03df-49b2-759ed8581a6c@163.com>

Am Montag, 24. April 2017, 18:25:49 CEST schrieb Caesar Wang:
> ? 2017?04?24? 17:15, Heiko St?bner ??:
> > Am Montag, 24. April 2017, 16:49:08 CEST schrieb Caesar Wang:
> >> ? 2017?04?24? 16:26, Heiko St?bner ??:
> >>> Hi Caesar,
> >>> 
> >>> Am Montag, 24. April 2017, 14:18:50 CEST schrieb Caesar Wang:
> >>>> Update the cpu opp table for rk3399 op1.
> >>> 
> >>> Ideally this should contain something about the "why".
> >>> Are these new voltage settings safer to operate under?
> >> 
> >> The before opp table is for earlier batch of rk3399 SoCs,  that's no
> >> enough for the current and
> >> newer batch of rk3399 op1. In order to suit for the rk3399 op1, we need
> >> to little voltages changed.
> > 
> > just to make sure, this is also safe for all the non-chromebook rk3399
> > socs
> > (like the firefly and tv-boxes, etc), right?
> 
> No,  just for chromebook.
> 
> Maybe, we need support two table for rk3399 SoCs.
> For chromebook supporting 2GHz frequency and others support 1.8GHz
> frequency.

Definitly. So please move the chromebook opp-table to something like a rk3399-
op1-opp.dtsi and adapt rk3399-opp.dtsi to be safe for all rk3399 variants.


Thanks
Heiko

> 
> For example:
> ....
>    opp03 {
>                opp-hz = /bits/ 64 <1008000000>;
>                opp-microvolt = <925000 925000 1350000>;
>                opp-microvolt-l0 = <925000 925000 1350000>;
>                opp-microvolt-l1 = <900000 900000 1350000>;
>            };
> ....
> 
> 
> 
> -Caesar
> 
> > Thanks
> > Heiko
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* [PATCH v4 3/6] mfd: exynos-lpass: Remove pad retention control
From: Lee Jones @ 2017-04-24 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4646acab-261e-32ea-df63-6f8333f64e2d@samsung.com>

On Wed, 19 Apr 2017, Marek Szyprowski wrote:

> Hi Lee,
> 
> On 2017-04-11 14:02, Lee Jones wrote:
> > On Thu, 23 Mar 2017, Marek Szyprowski wrote:
> > 
> > > Pad retention should be controlled from pin control driver, so remove it
> > > from Exynos LPASS driver. After this change, no more access to PMU regmap
> > > is needed, so remove also the code for handling PMU regmap.
> > > 
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >   .../bindings/mfd/samsung,exynos5433-lpass.txt           |  2 --
> > >   drivers/mfd/exynos-lpass.c                              | 17 -----------------
> > >   include/linux/mfd/syscon/exynos5-pmu.h                  |  3 ---
> > >   3 files changed, 22 deletions(-)
> > Applied, thanks.
> 
> When can I expect this (and the remaining patches from this patchset) to
> appear
> in your -next branch?

Now. ;)

[sorry for the delay, I was on Honeymoon last week]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] arm64: dts: rockchip: update cpu opp table for rk3399 op1
From: Eddie Cai @ 2017-04-24 10:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3119382.DFiP2J6FnE@diego>

Hi Heiko

2017-04-24 17:15 GMT+08:00 Heiko St?bner <heiko@sntech.de>:
> Am Montag, 24. April 2017, 16:49:08 CEST schrieb Caesar Wang:
>> ? 2017?04?24? 16:26, Heiko St?bner ??:
>> > Hi Caesar,
>> >
>> > Am Montag, 24. April 2017, 14:18:50 CEST schrieb Caesar Wang:
>> >> Update the cpu opp table for rk3399 op1.
>> >
>> > Ideally this should contain something about the "why".
>> > Are these new voltage settings safer to operate under?
>>
>> The before opp table is for earlier batch of rk3399 SoCs,  that's no
>> enough for the current and
>> newer batch of rk3399 op1. In order to suit for the rk3399 op1, we need
>> to little voltages changed.
>
> just to make sure, this is also safe for all the non-chromebook rk3399 socs
> (like the firefly and tv-boxes, etc), right?
This is only for op1. other 3399s should have a different opp table
>
>
> Thanks
> Heiko
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* [linux-sunxi] Re: [PATCH v5 02/11] clk: sunxi-ng: add support for DE2 CCU
From: icenowy at aosc.io @ 2017-04-24 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170424085109.p44bmzbyjkuf7ckv@lukather>

? 2017-04-24 16:51?Maxime Ripard ???
> Hi,
> 
> On Sun, Apr 23, 2017 at 06:37:45PM +0800, Icenowy Zheng wrote:
>> +static const struct of_device_id sunxi_de2_clk_ids[] = {
>> +	{
>> +		.compatible = "allwinner,sun8i-a83t-de2-clk",
>> +		.data = &sun8i_a83t_de2_clk_desc,
>> +	},
>> +	{
>> +		.compatible = "allwinner,sun50i-h5-de2-clk",
>> +		.data = &sun50i_a64_de2_clk_desc,
>> +	},
>> +	/*
>> +	 * The Allwinner A64 SoC needs some bit to be poke in syscon to make
>> +	 * DE2 really working.
>> +	 * So there's currently no A64 compatible here.
>> +	 * H5 shares the same reset line with A64, so here H5 is using the
>> +	 * clock description of A64.
>> +	 */
>> +	{ }
>> +};
> 
> So that A64 driver would require more than just what you defined in
> the binding in order to operate?

Yes. When trying to do A64 driver, I will send out first a patch to
add the needed binding bit.

> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply

* [linux-sunxi] Re: [PATCH v3 02/12] arm64: allwinner: a64: add NMI controller on A64
From: icenowy at aosc.io @ 2017-04-24 10:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170424071746.u2lrk43kmvvd7m25@lukather>

? 2017-04-24 15:17?Maxime Ripard ???
> On Thu, Apr 20, 2017 at 03:03:38PM +0800, icenowy at aosc.io wrote:
>> ? 2017-04-20 13:58?Maxime Ripard ???
>> > On Tue, Apr 18, 2017 at 06:56:43PM +0800, Icenowy Zheng wrote:
>> > >
>> > >
>> > > ? 2017?4?18? GMT+08:00 ??3:00:16, Maxime Ripard
>> > > <maxime.ripard@free-electrons.com> ??:
>> > > >On Mon, Apr 17, 2017 at 07:57:37PM +0800, Icenowy Zheng wrote:
>> > > >> Allwinner A64 SoC features a NMI controller, which is usually
>> > > >connected
>> > > >> to the AXP PMIC.
>> > > >>
>> > > >> Add support for it.
>> > > >>
>> > > >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > >> Acked-by: Chen-Yu Tsai <wens@csie.org>
>> > > >> ---
>> > > >> Changes in v2:
>> > > >> - Added Chen-Yu's ACK.
>> > > >>
>> > > >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
>> > > >>  1 file changed, 8 insertions(+)
>> > > >>
>> > > >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > > >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > > >> index 05ec9fc5e81f..53c18ca372ea 100644
>> > > >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > > >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > > >> @@ -403,6 +403,14 @@
>> > > >>  				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
>> > > >>  		};
>> > > >>
>> > > >> +		nmi_intc: interrupt-controller at 01f00c0c {
>> > > >> +			compatible = "allwinner,sun6i-a31-sc-nmi";
>> > > >> +			interrupt-controller;
>> > > >> +			#interrupt-cells = <2>;
>> > > >> +			reg = <0x01f00c0c 0x38>;
>> > > >
>> > > >The base address is not correct, and there's uncertainty on whether
>> > > >this is this particular controller or not. Did you even test this?
>> > >
>> > > Tested by axp20x-pek.
>> >
>> > Still, the base address is wrong, which is yet another hint that this
>> > is not the same interrupt controller, and just works by accident.
>> 
>> No, it's the same as other post-sun6i device trees.
>> See other post-sun6i device trees: (or maybe they're all wrong, but
>> as we have no document for it, we should temporarily keep them)
>> 
>> sun6i-a31.dtsi
>> ```
>> 		nmi_intc: interrupt-controller at 01f00c0c {
>> 			compatible = "allwinner,sun6i-a31-sc-nmi";
>> 			interrupt-controller;
>> 			#interrupt-cells = <2>;
>> 			reg = <0x01f00c0c 0x38>;
>> 			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
>> 		};
>> ```
>> 
>> sun8i-a23-a33.dtsi
>> ```
>> 		nmi_intc: interrupt-controller at 01f00c0c {
>> 			compatible = "allwinner,sun6i-a31-sc-nmi";
>> 			interrupt-controller;
>> 			#interrupt-cells = <2>;
>> 			reg = <0x01f00c0c 0x38>;
>> 			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
>> 		};
>> ```
>> 
>> But according to the BSP device tree, the base address should be
>> 0x01f00c00. Should I send some patch to fix all of them? (but it will
>> break device tree compatibility)
> 
> I'm really not a big fan of "if we see something that is broken, just
> let it rot" to be honest.
> 
> We have no idea how this controller works exactly, just like we have
> no idea if it is exactly the same controller or not.
> 
> The only thing we have today is the memory map, and it tells us that
> it has more registers than what you express here.
> 
> Because of the DT backward compatibility, you have to think of it the
> other way around: what will happen if it turns out we need to setup
> any register outside of that region you described in the DT, in
> something like a year or so?
> 
> We can't, really. While if you have the full memory region from the
> beginning, then you just have to add a single writel in your driver.

So things are now already broken, and we may need to fix also A31 and
A23/33.

How should we do this?

> 
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RFC] minimum gcc version for kernel: raise to gcc-4.3 or 4.6?
From: Geert Uytterhoeven @ 2017-04-24 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAK8P3a32Y0AiKyVaotYb14uZZ1jM=nwhFD1vyP56epW1Y9KqSw@mail.gmail.com>

Hi Arnd,

On Mon, Apr 24, 2017 at 11:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sun, Apr 23, 2017 at 10:13 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Sat, Apr 22, 2017 at 5:30 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> Based on what I found so far, gcc-4 can be pretty much ruled out from
>>> being the minimum version based on the number of failures I got.
>>> It's much better than 3.4 but much worse than 4.1 or 4.2 which seem
>>> fixable on MIPS and x86 at least, and may or may not work depending
>>> on configuration. So the best two possible (but conflicting) answers I
>>> have are
>>>
>>> a) There are known users on gcc-4.1, and we never break things that
>>>     work for users, so gcc-4.1 (or possibly 4.0 if a user shows up) would
>>>     be the minimum version.
>>> b) gcc-4.1 and 4.2 have too many problems, so users are better off
>>>     when we tell them to upgrade to something newer, and a minimum
>>>     version of gcc-4.3 has fewer surprises. We should remove all
>>>     workarounds for pre-gcc-4.3 compilers and just force a build error
>>>     message.
>>
>> If there's no real good reason (brokenness) to deprecate gcc-4.1, I would not
>> do it.I guess most people using old compilers know what they're doing.
>
> What I'm trying to find out first is whether "people regularly using 10+
> year old compilers for the latest kernels" is a strict subset of "people in
> Geert's household".

Fair enough.

>> My main motivation for keep on using gcc-4.1 is that it gives many warnings
>> that were disabled in later gcc versions.  I do look at all new warnings, and
>> send patches when they are real bugs, or are trivial to silence.
>
> What kind of warnings do you see that disappeared with later versions?
> Do you know what caused them to disappear in later versions (different
> optimization decisions, warning getting disabled by default but still available
> for turning on manually, ...)? Do you know if the disabled warnings are
> still there in gcc-4.3 (I can try it out if you give me examples)?

Mostly the "may be used uninitialized" warnings. I believe they were disabled
in gcc-4.3 (4.2?) and later due to too many false positives, which is not an
issue for me, as I look at differences.
They were re-enabled lately (with much less false-positives), that's why you
see them, and fix them.

For example, do you see the warning fixed by commit 1b8c1012142d8322
("drivers: net: xgene: Simplify xgene_enet_setup_mss() to kill warning")
with gcc-4.3? Yes, that was a false positive.

Or see commit cc4a7ffe02c95f53 ("spi: fsl-lpspi: Pre-initialize ret in
fsl_lpspi_transfer_one_msg()"). That one was a real bug.

I don't see that in any of the kisskb build logs, and they use gcc-4.2.4 for
avr32. So having gcc-4.2 or gcc-4.3 in a farm won't help.

And as long as I find real bugs this way, I'd like to continue doing it.

>> BTW, below is the diff I use to avoid an ICE.
>> After that, it builds and (test)boots fine on ARAnyM ;-)
>
> I guess this means that even your builds require extra patches and you
> can't strictly build a defconfig and expect that to work ;-)

No sane people enable GFS in defconfig, so it's not affected.

Oh wait, some mips, powerpc, s390, and tile do ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v4 7/7] ARM: dma-mapping: Remove traces of NOMMU code
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

DMA operations for NOMMU case have been just factored out into
separate compilation unit, so don't keep dead code.

Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mm/dma-mapping.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 475811f..cd90338 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -344,8 +344,6 @@ static void __dma_free_buffer(struct page *page, size_t size)
 	}
 }
 
-#ifdef CONFIG_MMU
-
 static void *__alloc_from_contiguous(struct device *dev, size_t size,
 				     pgprot_t prot, struct page **ret_page,
 				     const void *caller, bool want_vaddr,
@@ -647,22 +645,6 @@ static inline pgprot_t __get_dma_pgprot(unsigned long attrs, pgprot_t prot)
 	return prot;
 }
 
-#define nommu() 0
-
-#else	/* !CONFIG_MMU */
-
-#define nommu() 1
-
-#define __get_dma_pgprot(attrs, prot)				__pgprot(0)
-#define __alloc_remap_buffer(dev, size, gfp, prot, ret, c, wv)	NULL
-#define __alloc_from_pool(size, ret_page)			NULL
-#define __alloc_from_contiguous(dev, size, prot, ret, c, wv, coherent_flag, gfp)	NULL
-#define __free_from_pool(cpu_addr, size)			do { } while (0)
-#define __free_from_contiguous(dev, page, cpu_addr, size, wv)	do { } while (0)
-#define __dma_free_remap(cpu_addr, size)			do { } while (0)
-
-#endif	/* CONFIG_MMU */
-
 static void *__alloc_simple_buffer(struct device *dev, size_t size, gfp_t gfp,
 				   struct page **ret_page)
 {
@@ -805,7 +787,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 
 	if (cma)
 		buf->allocator = &cma_allocator;
-	else if (nommu() || is_coherent)
+	else if (is_coherent)
 		buf->allocator = &simple_allocator;
 	else if (allowblock)
 		buf->allocator = &remap_allocator;
@@ -854,8 +836,7 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 		 void *cpu_addr, dma_addr_t dma_addr, size_t size,
 		 unsigned long attrs)
 {
-	int ret = -ENXIO;
-#ifdef CONFIG_MMU
+	int ret;
 	unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
 	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	unsigned long pfn = dma_to_pfn(dev, dma_addr);
@@ -870,10 +851,6 @@ static int __arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 				      vma->vm_end - vma->vm_start,
 				      vma->vm_page_prot);
 	}
-#else
-	ret = vm_iomap_memory(vma, vma->vm_start,
-			      (vma->vm_end - vma->vm_start));
-#endif	/* CONFIG_MMU */
 
 	return ret;
 }
@@ -892,9 +869,7 @@ int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 		 void *cpu_addr, dma_addr_t dma_addr, size_t size,
 		 unsigned long attrs)
 {
-#ifdef CONFIG_MMU
 	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
-#endif	/* CONFIG_MMU */
 	return __arm_dma_mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
 }
 
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 6/7] ARM: NOMMU: Set ARM_DMA_MEM_BUFFERABLE for M-class cpus
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

Now, we have dedicated non-cacheable region for consistent DMA
operations. However, that region can still be marked as bufferable by
MPU, so it'd be safer to have barriers by default.

Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 arch/arm/mm/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index d731f28..7e357c6 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -1049,8 +1049,8 @@ config ARM_L1_CACHE_SHIFT
 	default 5
 
 config ARM_DMA_MEM_BUFFERABLE
-	bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
-	default y if CPU_V6 || CPU_V6K || CPU_V7
+	bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K || CPU_V7M) && !CPU_V7
+	default y if CPU_V6 || CPU_V6K || CPU_V7 || CPU_V7M
 	help
 	  Historically, the kernel has used strongly ordered mappings to
 	  provide DMA coherent memory.  With the advent of ARMv7, mapping
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 5/7] ARM: NOMMU: Introduce dma operations for noMMU
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

R/M classes of cpus can have memory covered by MPU which in turn might
configure RAM as Normal i.e. bufferable and cacheable. It breaks
dma_alloc_coherent() and friends, since data can stuck in caches now
or be buffered.

This patch factors out DMA support for NOMMU configuration into
separate entity which provides dedicated dma_ops. We have to handle
there several cases:
- configurations with MMU/MPU setup
- configurations without MMU/MPU setup
- special case for M-class, since caches and MPU there are optional

In general we rely on default DMA area for coherent allocations or/and
per-device memory reserves suitable for coherent DMA, so if such
regions are set coherent allocations go from there.

In case MMU/MPU was not setup we fallback to normal page allocator for
DMA memory allocation.

In case we run M-class cpus, for configuration without cache support
(like Cortex-M3/M4) dma operations are forced to be coherent and wired
with dma-noop (such decision is made based on cacheid global
variable); however, if caches are detected there and no DMA coherent
region is given (either default or per-device), dma is disallowed even
MPU is not set - it is because M-class implement system memory map
which defines part of address space as Normal memory.

Reported-by: Alexandre Torgue <alexandre.torgue@st.com>
Reported-by: Andras Szemzo <sza@esh.hu>
Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/Kconfig                   |   1 +
 arch/arm/include/asm/dma-mapping.h |   2 +-
 arch/arm/mm/Makefile               |   5 +-
 arch/arm/mm/dma-mapping-nommu.c    | 253 +++++++++++++++++++++++++++++++++++++
 4 files changed, 257 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mm/dma-mapping-nommu.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6ab63fa..8f0b6ca 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -22,6 +22,7 @@ config ARM
 	select CLONE_BACKWARDS
 	select CPU_PM if (SUSPEND || CPU_IDLE)
 	select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
+	select DMA_NOOP_OPS if !MMU
 	select EDAC_SUPPORT
 	select EDAC_ATOMIC_SCRUB
 	select GENERIC_ALLOCATOR
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 7166569..63270de 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -20,7 +20,7 @@ static inline const struct dma_map_ops *__generic_dma_ops(struct device *dev)
 {
 	if (dev && dev->dma_ops)
 		return dev->dma_ops;
-	return &arm_dma_ops;
+	return IS_ENABLED(CONFIG_MMU) ? &arm_dma_ops : &dma_noop_ops;
 }
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 54857bc..ea80df7 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -2,9 +2,8 @@
 # Makefile for the linux arm-specific parts of the memory manager.
 #
 
-obj-y				:= dma-mapping.o extable.o fault.o init.o \
-				   iomap.o
-
+obj-y				:= extable.o fault.o init.o iomap.o
+obj-y				+= dma-mapping$(MMUEXT).o
 obj-$(CONFIG_MMU)		+= fault-armv.o flush.o idmap.o ioremap.o \
 				   mmap.o pgd.o mmu.o pageattr.o
 
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
new file mode 100644
index 0000000..3ba3003
--- /dev/null
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -0,0 +1,253 @@
+/*
+ *  Based on linux/arch/arm/mm/dma-mapping.c
+ *
+ *  Copyright (C) 2000-2004 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/export.h>
+#include <linux/mm.h>
+#include <linux/dma-mapping.h>
+#include <linux/scatterlist.h>
+
+#include <asm/cachetype.h>
+#include <asm/cacheflush.h>
+#include <asm/outercache.h>
+#include <asm/cp15.h>
+
+#include "dma.h"
+
+/*
+ *  dma_noop_ops is used if
+ *   - MMU/MPU is off
+ *   - cpu is v7m w/o cache support
+ *   - device is coherent
+ *  otherwise arm_nommu_dma_ops is used.
+ *
+ *  arm_nommu_dma_ops rely on consistent DMA memory (please, refer to
+ *  [1] on how to declare such memory).
+ *
+ *  [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+ */
+
+static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
+				 dma_addr_t *dma_handle, gfp_t gfp,
+				 unsigned long attrs)
+
+{
+	const struct dma_map_ops *ops = &dma_noop_ops;
+
+	/*
+	 * We are here because:
+	 * - no consistent DMA region has been defined, so we can't
+	 *   continue.
+	 * - there is no space left in consistent DMA region, so we
+	 *   only can fallback to generic allocator if we are
+	 *   advertised that consistency is not required.
+	 */
+
+	if (attrs & DMA_ATTR_NON_CONSISTENT)
+		return ops->alloc(dev, size, dma_handle, gfp, attrs);
+
+	WARN_ON_ONCE(1);
+	return NULL;
+}
+
+static void arm_nommu_dma_free(struct device *dev, size_t size,
+			       void *cpu_addr, dma_addr_t dma_addr,
+			       unsigned long attrs)
+{
+	const struct dma_map_ops *ops = &dma_noop_ops;
+
+	if (attrs & DMA_ATTR_NON_CONSISTENT)
+		ops->free(dev, size, cpu_addr, dma_addr, attrs);
+	else
+		WARN_ON_ONCE(1);
+
+	return;
+}
+
+static int arm_nommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+			      void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			      unsigned long attrs)
+{
+	const struct dma_map_ops *ops = &dma_noop_ops;
+	int ret;
+
+	if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
+		return ret;
+
+	if (attrs & DMA_ATTR_NON_CONSISTENT)
+		return ops->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
+
+	WARN_ON_ONCE(1);
+	return -ENXIO;
+}
+
+static void __dma_page_cpu_to_dev(phys_addr_t paddr, size_t size,
+				  enum dma_data_direction dir)
+{
+	dmac_map_area(__va(paddr), size, dir);
+
+	if (dir == DMA_FROM_DEVICE)
+		outer_inv_range(paddr, paddr + size);
+	else
+		outer_clean_range(paddr, paddr + size);
+}
+
+static void __dma_page_dev_to_cpu(phys_addr_t paddr, size_t size,
+				  enum dma_data_direction dir)
+{
+	if (dir != DMA_TO_DEVICE) {
+		outer_inv_range(paddr, paddr + size);
+		dmac_unmap_area(__va(paddr), size, dir);
+	}
+}
+
+static dma_addr_t arm_nommu_dma_map_page(struct device *dev, struct page *page,
+					 unsigned long offset, size_t size,
+					 enum dma_data_direction dir,
+					 unsigned long attrs)
+{
+	dma_addr_t handle = page_to_phys(page) + offset;
+
+	__dma_page_cpu_to_dev(handle, size, dir);
+
+	return handle;
+}
+
+static void arm_nommu_dma_unmap_page(struct device *dev, dma_addr_t handle,
+				     size_t size, enum dma_data_direction dir,
+				     unsigned long attrs)
+{
+	__dma_page_dev_to_cpu(handle, size, dir);
+}
+
+
+static int arm_nommu_dma_map_sg(struct device *dev, struct scatterlist *sgl,
+				int nents, enum dma_data_direction dir,
+				unsigned long attrs)
+{
+	int i;
+	struct scatterlist *sg;
+
+	for_each_sg(sgl, sg, nents, i) {
+		sg_dma_address(sg) = sg_phys(sg);
+		sg_dma_len(sg) = sg->length;
+		__dma_page_cpu_to_dev(sg_dma_address(sg), sg_dma_len(sg), dir);
+	}
+
+	return nents;
+}
+
+static void arm_nommu_dma_unmap_sg(struct device *dev, struct scatterlist *sgl,
+				   int nents, enum dma_data_direction dir,
+				   unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nents, i)
+		__dma_page_dev_to_cpu(sg_dma_address(sg), sg_dma_len(sg), dir);
+}
+
+static void arm_nommu_dma_sync_single_for_device(struct device *dev,
+		dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+	__dma_page_cpu_to_dev(handle, size, dir);
+}
+
+static void arm_nommu_dma_sync_single_for_cpu(struct device *dev,
+		dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+	__dma_page_cpu_to_dev(handle, size, dir);
+}
+
+static void arm_nommu_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
+					     int nents, enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nents, i)
+		__dma_page_cpu_to_dev(sg_dma_address(sg), sg_dma_len(sg), dir);
+}
+
+static void arm_nommu_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl,
+					  int nents, enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nents, i)
+		__dma_page_dev_to_cpu(sg_dma_address(sg), sg_dma_len(sg), dir);
+}
+
+const struct dma_map_ops arm_nommu_dma_ops = {
+	.alloc			= arm_nommu_dma_alloc,
+	.free			= arm_nommu_dma_free,
+	.mmap			= arm_nommu_dma_mmap,
+	.map_page		= arm_nommu_dma_map_page,
+	.unmap_page		= arm_nommu_dma_unmap_page,
+	.map_sg			= arm_nommu_dma_map_sg,
+	.unmap_sg		= arm_nommu_dma_unmap_sg,
+	.sync_single_for_device	= arm_nommu_dma_sync_single_for_device,
+	.sync_single_for_cpu	= arm_nommu_dma_sync_single_for_cpu,
+	.sync_sg_for_device	= arm_nommu_dma_sync_sg_for_device,
+	.sync_sg_for_cpu	= arm_nommu_dma_sync_sg_for_cpu,
+};
+EXPORT_SYMBOL(arm_nommu_dma_ops);
+
+static const struct dma_map_ops *arm_nommu_get_dma_map_ops(bool coherent)
+{
+	return coherent ? &dma_noop_ops : &arm_nommu_dma_ops;
+}
+
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+			const struct iommu_ops *iommu, bool coherent)
+{
+	const struct dma_map_ops *dma_ops;
+
+	if (IS_ENABLED(CONFIG_CPU_V7M)) {
+		/*
+		 * Cache support for v7m is optional, so can be treated as
+		 * coherent if no cache has been detected. Note that it is not
+		 * enough to check if MPU is in use or not since in absense of
+		 * MPU system memory map is used.
+		 */
+		dev->archdata.dma_coherent = (cacheid) ? coherent : true;
+	} else {
+		/*
+		 * Assume coherent DMA in case MMU/MPU has not been set up.
+		 */
+		dev->archdata.dma_coherent = (get_cr() & CR_M) ? coherent : true;
+	}
+
+	dma_ops = arm_nommu_get_dma_map_ops(dev->archdata.dma_coherent);
+
+	set_dma_ops(dev, dma_ops);
+}
+
+void arch_teardown_dma_ops(struct device *dev)
+{
+}
+
+int dma_supported(struct device *dev, u64 mask)
+{
+	return 1;
+}
+
+EXPORT_SYMBOL(dma_supported);
+
+#define PREALLOC_DMA_DEBUG_ENTRIES	4096
+
+static int __init dma_debug_do_init(void)
+{
+	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
+	return 0;
+}
+core_initcall(dma_debug_do_init);
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 4/7] drivers: dma-coherent: Introduce default DMA pool
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

This patch introduces default coherent DMA pool similar to default CMA
area concept. To keep other users safe code kept under CONFIG_ARM.

Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 .../bindings/reserved-memory/reserved-memory.txt   |  3 ++
 drivers/base/dma-coherent.c                        | 59 +++++++++++++++++++---
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index 3da0ebd..16291f2 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -68,6 +68,9 @@ Linux implementation note:
 - If a "linux,cma-default" property is present, then Linux will use the
   region for the default pool of the contiguous memory allocator.
 
+- If a "linux,dma-default" property is present, then Linux will use the
+  region for the default pool of the consistent DMA allocator.
+
 Device node references to reserved memory
 -----------------------------------------
 Regions in the /reserved-memory node may be referenced by other device
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 99c9695..2ae24c2 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -19,6 +19,15 @@ struct dma_coherent_mem {
 	bool		use_dev_dma_pfn_offset;
 };
 
+static struct dma_coherent_mem *dma_coherent_default_memory __ro_after_init;
+
+static inline struct dma_coherent_mem *dev_get_coherent_memory(struct device *dev)
+{
+	if (dev && dev->dma_mem)
+		return dev->dma_mem;
+	return dma_coherent_default_memory;
+}
+
 static inline dma_addr_t dma_get_device_base(struct device *dev,
 					     struct dma_coherent_mem * mem)
 {
@@ -93,6 +102,9 @@ static void dma_release_coherent_memory(struct dma_coherent_mem *mem)
 static int dma_assign_coherent_memory(struct device *dev,
 				      struct dma_coherent_mem *mem)
 {
+	if (!dev)
+		return -ENODEV;
+
 	if (dev->dma_mem)
 		return -EBUSY;
 
@@ -171,15 +183,12 @@ EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
 int dma_alloc_from_coherent(struct device *dev, ssize_t size,
 				       dma_addr_t *dma_handle, void **ret)
 {
-	struct dma_coherent_mem *mem;
+	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
 	int order = get_order(size);
 	unsigned long flags;
 	int pageno;
 	int dma_memory_map;
 
-	if (!dev)
-		return 0;
-	mem = dev->dma_mem;
 	if (!mem)
 		return 0;
 
@@ -233,7 +242,7 @@ EXPORT_SYMBOL(dma_alloc_from_coherent);
  */
 int dma_release_from_coherent(struct device *dev, int order, void *vaddr)
 {
-	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
+	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
 
 	if (mem && vaddr >= mem->virt_base && vaddr <
 		   (mem->virt_base + (mem->size << PAGE_SHIFT))) {
@@ -267,7 +276,7 @@ EXPORT_SYMBOL(dma_release_from_coherent);
 int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma,
 			   void *vaddr, size_t size, int *ret)
 {
-	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
+	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
 
 	if (mem && vaddr >= mem->virt_base && vaddr + size <=
 		   (mem->virt_base + (mem->size << PAGE_SHIFT))) {
@@ -297,6 +306,8 @@ EXPORT_SYMBOL(dma_mmap_from_coherent);
 #include <linux/of_fdt.h>
 #include <linux/of_reserved_mem.h>
 
+static struct reserved_mem *dma_reserved_default_memory __initdata;
+
 static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
 {
 	struct dma_coherent_mem *mem = rmem->priv;
@@ -318,7 +329,8 @@ static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
 static void rmem_dma_device_release(struct reserved_mem *rmem,
 				    struct device *dev)
 {
-	dev->dma_mem = NULL;
+	if (dev)
+		dev->dma_mem = NULL;
 }
 
 static const struct reserved_mem_ops rmem_dma_ops = {
@@ -338,6 +350,12 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem)
 		pr_err("Reserved memory: regions without no-map are not yet supported\n");
 		return -EINVAL;
 	}
+
+	if (of_get_flat_dt_prop(node, "linux,dma-default", NULL)) {
+		WARN(dma_reserved_default_memory,
+		     "Reserved memory: region for default DMA coherent area is redefined\n");
+		dma_reserved_default_memory = rmem;
+	}
 #endif
 
 	rmem->ops = &rmem_dma_ops;
@@ -345,5 +363,32 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem)
 		&rmem->base, (unsigned long)rmem->size / SZ_1M);
 	return 0;
 }
+
+static int __init dma_init_reserved_memory(void)
+{
+	const struct reserved_mem_ops *ops;
+	int ret;
+
+	if (!dma_reserved_default_memory)
+		return -ENOMEM;
+
+	ops = dma_reserved_default_memory->ops;
+
+	/*
+	 * We rely on rmem_dma_device_init() does not propagate error of
+	 * dma_assign_coherent_memory() for "NULL" device.
+	 */
+	ret = ops->device_init(dma_reserved_default_memory, NULL);
+
+	if (!ret) {
+		dma_coherent_default_memory = dma_reserved_default_memory->priv;
+		pr_info("DMA: default coherent area is set\n");
+	}
+
+	return ret;
+}
+
+core_initcall(dma_init_reserved_memory);
+
 RESERVEDMEM_OF_DECLARE(dma, "shared-dma-pool", rmem_dma_setup);
 #endif
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 3/7] drivers: dma-coherent: Account dma_pfn_offset when used with device tree
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

dma_declare_coherent_memory() and friends are designed to account
difference in CPU and device addresses. However, when it is used with
reserved memory regions there is assumption that CPU and device have
the same view on address space. This assumption gets invalid when
reserved memory for coherent DMA allocations is referenced by device
with non-empty "dma-range" property.

Simply feeding device address as rmem->base + dev->dma_pfn_offset
would not work due to reserved memory region can be shared, so this
patch turns device address to be expressed with help of CPU address
and device's dma_pfn_offset in case memory reservation has been done
via device tree; non device tree users continue to use the old scheme.

Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Roger Quadros <rogerq@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 drivers/base/dma-coherent.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e6..99c9695 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -16,8 +16,18 @@ struct dma_coherent_mem {
 	int		flags;
 	unsigned long	*bitmap;
 	spinlock_t	spinlock;
+	bool		use_dev_dma_pfn_offset;
 };
 
+static inline dma_addr_t dma_get_device_base(struct device *dev,
+					     struct dma_coherent_mem * mem)
+{
+	if (mem->use_dev_dma_pfn_offset)
+		return (mem->pfn_base - dev->dma_pfn_offset) << PAGE_SHIFT;
+	else
+		return mem->device_base;
+}
+
 static bool dma_init_coherent_memory(
 	phys_addr_t phys_addr, dma_addr_t device_addr, size_t size, int flags,
 	struct dma_coherent_mem **mem)
@@ -133,7 +143,7 @@ void *dma_mark_declared_memory_occupied(struct device *dev,
 		return ERR_PTR(-EINVAL);
 
 	spin_lock_irqsave(&mem->spinlock, flags);
-	pos = (device_addr - mem->device_base) >> PAGE_SHIFT;
+	pos = PFN_DOWN(device_addr - dma_get_device_base(dev, mem));
 	err = bitmap_allocate_region(mem->bitmap, pos, get_order(size));
 	spin_unlock_irqrestore(&mem->spinlock, flags);
 
@@ -186,7 +196,7 @@ int dma_alloc_from_coherent(struct device *dev, ssize_t size,
 	/*
 	 * Memory was found in the per-device area.
 	 */
-	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
+	*dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
 	*ret = mem->virt_base + (pageno << PAGE_SHIFT);
 	dma_memory_map = (mem->flags & DMA_MEMORY_MAP);
 	spin_unlock_irqrestore(&mem->spinlock, flags);
@@ -299,6 +309,7 @@ static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
 			&rmem->base, (unsigned long)rmem->size / SZ_1M);
 		return -ENODEV;
 	}
+	mem->use_dev_dma_pfn_offset = true;
 	rmem->priv = mem;
 	dma_assign_coherent_memory(dev, mem);
 	return 0;
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 2/7] dma: Add simple dma_noop_mmap
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

This patch adds a simple implementation of mmap to dma_noop_ops.

Cc: Joerg Roedel <jroedel@suse.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Reported-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 lib/dma-noop.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lib/dma-noop.c b/lib/dma-noop.c
index ff4ef5e..0acc3f6 100644
--- a/lib/dma-noop.c
+++ b/lib/dma-noop.c
@@ -66,6 +66,26 @@ static int dma_noop_supported(struct device *dev, u64 mask)
 	return 1;
 }
 
+static int dma_noop_mmap(struct device *dev, struct vm_area_struct *vma,
+			 void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			 unsigned long attrs)
+{
+	unsigned long user_count = vma_pages(vma);
+	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+	unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
+	unsigned long off = vma->vm_pgoff;
+	int ret = -ENXIO;
+
+	if (off < count && user_count <= (count - off)) {
+		ret = remap_pfn_range(vma, vma->vm_start,
+				      pfn + off,
+				      user_count << PAGE_SHIFT,
+				      vma->vm_page_prot);
+	}
+
+	return ret;
+}
+
 const struct dma_map_ops dma_noop_ops = {
 	.alloc			= dma_noop_alloc,
 	.free			= dma_noop_free,
@@ -73,6 +93,7 @@ const struct dma_map_ops dma_noop_ops = {
 	.map_sg			= dma_noop_map_sg,
 	.mapping_error		= dma_noop_mapping_error,
 	.dma_supported		= dma_noop_supported,
+	.mmap			= dma_noop_mmap,
 };
 
 EXPORT_SYMBOL(dma_noop_ops);
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 1/7] dma: Take into account dma_pfn_offset
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493029017-31382-1-git-send-email-vladimir.murzin@arm.com>

Even though dma-noop-ops assumes 1:1 memory mapping DMA memory range
can be different to RAM. For example, ARM STM32F4 MCU offers the
possibility to remap SDRAM from 0xc000_0000 to 0x0 to get CPU
performance boost, but DMA continue to see SDRAM at 0xc000_0000. This
difference in mapping is handled via device-tree "dma-range" property
which leads to dev->dma_pfn_offset is set nonzero. To handle such
cases take dma_pfn_offset into account.

Cc: Joerg Roedel <jroedel@suse.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Reported-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Tested-by: Andras Szemzo <sza@esh.hu>
Tested-by: Alexandre TORGUE <alexandre.torgue@st.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 lib/dma-noop.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/dma-noop.c b/lib/dma-noop.c
index de26c8b..ff4ef5e 100644
--- a/lib/dma-noop.c
+++ b/lib/dma-noop.c
@@ -7,6 +7,7 @@
 #include <linux/mm.h>
 #include <linux/dma-mapping.h>
 #include <linux/scatterlist.h>
+#include <linux/pfn.h>
 
 static void *dma_noop_alloc(struct device *dev, size_t size,
 			    dma_addr_t *dma_handle, gfp_t gfp,
@@ -16,7 +17,8 @@ static void *dma_noop_alloc(struct device *dev, size_t size,
 
 	ret = (void *)__get_free_pages(gfp, get_order(size));
 	if (ret)
-		*dma_handle = virt_to_phys(ret);
+		*dma_handle = virt_to_phys(ret) - PFN_PHYS(dev->dma_pfn_offset);
+
 	return ret;
 }
 
@@ -32,7 +34,7 @@ static dma_addr_t dma_noop_map_page(struct device *dev, struct page *page,
 				      enum dma_data_direction dir,
 				      unsigned long attrs)
 {
-	return page_to_phys(page) + offset;
+	return page_to_phys(page) + offset - PFN_PHYS(dev->dma_pfn_offset);
 }
 
 static int dma_noop_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
@@ -47,7 +49,7 @@ static int dma_noop_map_sg(struct device *dev, struct scatterlist *sgl, int nent
 
 		BUG_ON(!sg_page(sg));
 		va = sg_virt(sg);
-		sg_dma_address(sg) = (dma_addr_t)virt_to_phys(va);
+		sg_dma_address(sg) = (dma_addr_t)(virt_to_phys(va) - PFN_PHYS(dev->dma_pfn_offset));
 		sg_dma_len(sg) = sg->length;
 	}
 
-- 
2.0.0

^ permalink raw reply related

* [PATCH v4 0/7] ARM: Fix dma_alloc_coherent() and friends for NOMMU
From: Vladimir Murzin @ 2017-04-24 10:16 UTC (permalink / raw)
  To: linux-arm-kernel

It seem that addition of cache support for M-class CPUs uncovered
latent bug in DMA usage. NOMMU memory model has been treated as being
always consistent; however, for R/M CPU classes memory can be covered
by MPU which in turn might configure RAM as Normal i.e. bufferable and
cacheable. It breaks dma_alloc_coherent() and friends, since data can
stuck in caches now or be buffered.

This patch set is trying to address the issue by providing region of
memory suitable for consistent DMA operations. It is supposed that
such region is marked by MPU as non-cacheable. Robin suggested to
advertise such memory as reserved shared-dma-pool, rather then using
homebrew command line option, and extend dma-coherent to provide
default DMA area in the similar way as it is done for CMA (PATCH
4/7). It allows us to offload all bookkeeping on generic coherent DMA
framework, and it seems that it might be reused by other architectures
like c6x and blackfin.

While reviewing/testing previous vesrions of the patch set it turned
out that dma-coherent does not take into account "dma-ranges" device
tree property, so it is addressed in PATCH 3/7.

For ARM, dedicated DMA region is required for cases other than:
 - MMU/MPU is off
 - cpu is v7m w/o cache support
 - device is coherent

In case one of the above conditions is true dma operations are forced
to be coherent and wired with dma_noop_ops.

To make life easier NOMMU dma operations are kept in separate
compilation unit.

Since the issue was reported in the same time as Benjamin sent his
patch [1] to allow mmap for NOMMU, his case is also addressed in this
series (PATCH 1/7 and PATCH 2/7).

Thanks!

[1] http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8633/1

Cc: Joerg Roedel <jroedel@suse.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Roger Quadros <rogerq@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Doug Ledford <dledford@redhat.com>

Changelog:
	    v3 -> v4
	       - rebased on v4.11-rc7
	       - made CONFIG_ARM_DMA_MEM_BUFFERABLE optional for CPU_V7M
	       - added Arnd's Acked-by

	    v2 -> v3
	       - fixed warnings reported by Alexandre and kbuild robot

	    v1 -> v2
	       - rebased on v4.11-rc1
	       - added Robin's Reviewed-by
	       - dedicated flag is introduced to use dev->dma_pfn_offset
	         rather than mem->device_base in case memory region is
		 configured via device tree (so Tested-by discarded there)

	RFC v6 -> v1
	       - dropped RFC tag
	       - added Alexandre's Tested-by


Vladimir Murzin (7):
  dma: Take into account dma_pfn_offset
  dma: Add simple dma_noop_mmap
  drivers: dma-coherent: Account dma_pfn_offset when used with device
    tree
  drivers: dma-coherent: Introduce default DMA pool
  ARM: NOMMU: Introduce dma operations for noMMU
  ARM: NOMMU: Set ARM_DMA_MEM_BUFFERABLE for M-class cpus
  ARM: dma-mapping: Remove traces of NOMMU code

 .../bindings/reserved-memory/reserved-memory.txt   |   3 +
 arch/arm/Kconfig                                   |   1 +
 arch/arm/include/asm/dma-mapping.h                 |   2 +-
 arch/arm/mm/Kconfig                                |   4 +-
 arch/arm/mm/Makefile                               |   5 +-
 arch/arm/mm/dma-mapping-nommu.c                    | 253 +++++++++++++++++++++
 arch/arm/mm/dma-mapping.c                          |  29 +--
 drivers/base/dma-coherent.c                        |  74 +++++-
 lib/dma-noop.c                                     |  29 ++-
 9 files changed, 355 insertions(+), 45 deletions(-)
 create mode 100644 arch/arm/mm/dma-mapping-nommu.c

-- 
2.0.0

^ permalink raw reply

* [PATCH] clk: meson: gxbb: fix build error without RESET_CONTROLLER
From: Neil Armstrong @ 2017-04-24 10:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170424100542.20226-1-tobias.regnery@gmail.com>

On 04/24/2017 12:05 PM, Tobias Regnery wrote:
> With CONFIG_RESET_CONTROLLER=n we see the following link error in the
> meson gxbb clk driver:
> 
> drivers/built-in.o: In function 'gxbb_aoclkc_probe':
> drivers/clk/meson/gxbb-aoclk.c:161: undefined reference to 'devm_reset_controller_register'
> 
> Fix this by selecting the reset controller subsystem.
> 
> Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
> ---
>  drivers/clk/meson/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 19480bcc7046..2f29ee1a4d00 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -14,6 +14,7 @@ config COMMON_CLK_MESON8B
>  config COMMON_CLK_GXBB
>  	bool
>  	depends on COMMON_CLK_AMLOGIC
> +	select RESET_CONTROLLER
>  	help
>  	  Support for the clock controller on AmLogic S905 devices, aka gxbb.
>  	  Say Y if you want peripherals and CPU frequency scaling to work.
> 

Hi Tobias,

I was going to push this, thanks for the patch !

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

^ permalink raw reply

* [PATCH 2/2] kvm: arm/arm64: Fix race in resetting stage2 PGD
From: Suzuki K Poulose @ 2017-04-24 10:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493028624-29837-1-git-send-email-suzuki.poulose@arm.com>

In kvm_free_stage2_pgd() we check the stage2 PGD before holding
the lock and proceed to take the lock if it is valid. And we unmap
the page tables, followed by releasing the lock. We reset the PGD
only after dropping this lock, which could cause a race condition
where another thread waiting on the lock could potentially see that
the PGD is still valid and proceed to perform a stage2 operation.

This patch moves the stage2 PGD manipulation under the lock.

Reported-by: Alexander Graf <agraf@suse.de>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm/kvm/mmu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 582a972..9c4026d 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -835,16 +835,18 @@ void stage2_unmap_vm(struct kvm *kvm)
  */
 void kvm_free_stage2_pgd(struct kvm *kvm)
 {
-	if (kvm->arch.pgd == NULL)
-		return;
+	void *pgd = NULL;
 
 	spin_lock(&kvm->mmu_lock);
-	unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
+	if (kvm->arch.pgd) {
+		unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
+		pgd = kvm->arch.pgd;
+		kvm->arch.pgd = NULL;
+	}
 	spin_unlock(&kvm->mmu_lock);
-
 	/* Free the HW pgd, one page at a time */
-	free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
-	kvm->arch.pgd = NULL;
+	if (pgd)
+		free_pages_exact(pgd, S2_PGD_SIZE);
 }
 
 static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] kvm: Fix mmu_notifier release race
From: Suzuki K Poulose @ 2017-04-24 10:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493028624-29837-1-git-send-email-suzuki.poulose@arm.com>

The KVM uses mmu_notifier (wherever available) to keep track
of the changes to the mm of the guest. The guest shadow page
tables are released when the VM exits via mmu_notifier->ops.release().
There is a rare chance that the mmu_notifier->release could be
called more than once via two different paths, which could end
up in use-after-free of kvm instance (such as [0]).

e.g:

thread A                                        thread B
-------                                         --------------

 get_signal->                                   kvm_destroy_vm()->
 do_exit->                                        mmu_notifier_unregister->
 exit_mm->                                        kvm_arch_flush_shadow_all()->
 exit_mmap->                                      spin_lock(&kvm->mmu_lock)
 mmu_notifier_release->                           ....
  kvm_arch_flush_shadow_all()->                   .....
  ... spin_lock(&kvm->mmu_lock)                   .....
                                                  spin_unlock(&kvm->mmu_lock)
                                                kvm_arch_free_kvm()
   *** use after free of kvm ***

This patch attempts to solve the problem by holding a reference to the KVM
for the mmu_notifier, which is dropped only from notifier->ops.release().
This will ensure that the KVM struct is available till we reach the
kvm_mmu_notifier_release, and the kvm_destroy_vm is called only from/after
it. So, we can unregister the notifier with no_release option and hence
avoiding the race above. However, we need to make sure that the KVM is
freed only after the mmu_notifier has finished processing the notifier due to
the following possible path of execution :

mmu_notifier_release -> kvm_mmu_notifier_release -> kvm_put_kvm ->
  kvm_destroy_vm -> kvm_arch_free_kvm

[0] http://lkml.kernel.org/r/CAAeHK+x8udHKq9xa1zkTO6ax5E8Dk32HYWfaT05FMchL2cr48g at mail.gmail.com

Fixes: commit 85db06e514422 ("KVM: mmu_notifiers release method")
Reported-by: andreyknvl at google.com
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Kr?m?? <rkrcmar@redhat.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: andreyknvl at google.com
Cc: Marc Zyngier <marc.zyngier@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 59 ++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d025074..561e968 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -424,6 +424,7 @@ struct kvm {
 	struct mmu_notifier mmu_notifier;
 	unsigned long mmu_notifier_seq;
 	long mmu_notifier_count;
+	struct rcu_head mmu_notifier_rcu;
 #endif
 	long tlbs_dirty;
 	struct list_head devices;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 88257b3..2c3fdd4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -471,6 +471,7 @@ static void kvm_mmu_notifier_release(struct mmu_notifier *mn,
 	idx = srcu_read_lock(&kvm->srcu);
 	kvm_arch_flush_shadow_all(kvm);
 	srcu_read_unlock(&kvm->srcu, idx);
+	kvm_put_kvm(kvm);
 }
 
 static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
@@ -486,8 +487,46 @@ static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
 
 static int kvm_init_mmu_notifier(struct kvm *kvm)
 {
+	int rc;
 	kvm->mmu_notifier.ops = &kvm_mmu_notifier_ops;
-	return mmu_notifier_register(&kvm->mmu_notifier, current->mm);
+	rc = mmu_notifier_register(&kvm->mmu_notifier, current->mm);
+	/*
+	 * We hold a reference to KVM here to make sure that the KVM
+	 * doesn't get free'd before ops->release() completes.
+	 */
+	if (!rc)
+		kvm_get_kvm(kvm);
+	return rc;
+}
+
+static void kvm_free_vm_rcu(struct rcu_head *rcu)
+{
+	struct kvm *kvm = container_of(rcu, struct kvm, mmu_notifier_rcu);
+	kvm_arch_free_vm(kvm);
+}
+
+static void kvm_flush_shadow_mmu(struct kvm *kvm)
+{
+	/*
+	 * We hold a reference to kvm instance for mmu_notifier and is
+	 * only released when ops->release() is called via exit_mmap path.
+	 * So, when we reach here ops->release() has been called already, which
+	 * flushes the shadow page tables. Hence there is no need to call the
+	 * release() again when we unregister the notifier. However, we need
+	 * to delay freeing up the kvm until the release() completes, since
+	 * we could reach here via :
+	 *  kvm_mmu_notifier_release() -> kvm_put_kvm() -> kvm_destroy_vm()
+	 */
+	mmu_notifier_unregister_no_release(&kvm->mmu_notifier, kvm->mm);
+}
+
+static void kvm_free_vm(struct kvm *kvm)
+{
+	/*
+	 * Wait until the mmu_notifier has finished the release().
+	 * See comments above in kvm_flush_shadow_mmu.
+	 */
+	mmu_notifier_call_srcu(&kvm->mmu_notifier_rcu, kvm_free_vm_rcu);
 }
 
 #else  /* !(CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER) */
@@ -497,6 +536,16 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
 	return 0;
 }
 
+static void kvm_flush_shadow_mmu(struct kvm *kvm)
+{
+	kvm_arch_flush_shadow_all(kvm);
+}
+
+static void kvm_free_vm(struct kvm *kvm)
+{
+	kvm_arch_free_vm(kvm);
+}
+
 #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
 
 static struct kvm_memslots *kvm_alloc_memslots(void)
@@ -733,18 +782,14 @@ static void kvm_destroy_vm(struct kvm *kvm)
 		kvm->buses[i] = NULL;
 	}
 	kvm_coalesced_mmio_free(kvm);
-#if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
-	mmu_notifier_unregister(&kvm->mmu_notifier, kvm->mm);
-#else
-	kvm_arch_flush_shadow_all(kvm);
-#endif
+	kvm_flush_shadow_mmu(kvm);
 	kvm_arch_destroy_vm(kvm);
 	kvm_destroy_devices(kvm);
 	for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
 		kvm_free_memslots(kvm, kvm->memslots[i]);
 	cleanup_srcu_struct(&kvm->irq_srcu);
 	cleanup_srcu_struct(&kvm->srcu);
-	kvm_arch_free_vm(kvm);
+	kvm_free_vm(kvm);
 	preempt_notifier_dec();
 	hardware_disable_all();
 	mmdrop(mm);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] kvm: Fixes for race conditions
From: Suzuki K Poulose @ 2017-04-24 10:10 UTC (permalink / raw)
  To: linux-arm-kernel


The two patches here fixes race conditions in the KVM hypervisor code
dealing with the shadow MMU.

The first one applies to core KVM code where mmu_notifier->ops.release()
could be called twice with one instance possibily accessing a free'd KVM
instance. Reported here :

 http://lkml.kernel.org/r/CAAeHK+x8udHKq9xa1zkTO6ax5E8Dk32HYWfaT05FMchL2cr48g at mail.gmail.com

The second patch is specific to arm/arm64 stage2 PGD, where there are issues
with modifications to the PGD pointer outside the mmu_lock, leading to crashes.
Reported here :
 http://lkml.kernel.org/r/febea966-3767-21ff-3c40-1a76d1399138 at suse.de


Suzuki K Poulose (2):
  kvm: Fix mmu_notifier release race
  kvm: arm/arm64: Fix race in resetting stage2 PGD

 arch/arm/kvm/mmu.c       | 14 +++++++-----
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 59 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 61 insertions(+), 13 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH] clk: meson: gxbb: fix build error without RESET_CONTROLLER
From: Tobias Regnery @ 2017-04-24 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

With CONFIG_RESET_CONTROLLER=n we see the following link error in the
meson gxbb clk driver:

drivers/built-in.o: In function 'gxbb_aoclkc_probe':
drivers/clk/meson/gxbb-aoclk.c:161: undefined reference to 'devm_reset_controller_register'

Fix this by selecting the reset controller subsystem.

Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
---
 drivers/clk/meson/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 19480bcc7046..2f29ee1a4d00 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -14,6 +14,7 @@ config COMMON_CLK_MESON8B
 config COMMON_CLK_GXBB
 	bool
 	depends on COMMON_CLK_AMLOGIC
+	select RESET_CONTROLLER
 	help
 	  Support for the clock controller on AmLogic S905 devices, aka gxbb.
 	  Say Y if you want peripherals and CPU frequency scaling to work.
-- 
2.11.0

^ permalink raw reply related

* [RFC] minimum gcc version for kernel: raise to gcc-4.3 or 4.6?
From: Arnd Bergmann @ 2017-04-24  9:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdWTiurUSacddYXgW69=QwOrZDV3Zt2DezJauPy8j-GMHw@mail.gmail.com>

On Sun, Apr 23, 2017 at 10:13 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Arnd,
>
> On Sat, Apr 22, 2017 at 5:30 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> Based on what I found so far, gcc-4 can be pretty much ruled out from
>> being the minimum version based on the number of failures I got.
>> It's much better than 3.4 but much worse than 4.1 or 4.2 which seem
>> fixable on MIPS and x86 at least, and may or may not work depending
>> on configuration. So the best two possible (but conflicting) answers I
>> have are
>>
>> a) There are known users on gcc-4.1, and we never break things that
>>     work for users, so gcc-4.1 (or possibly 4.0 if a user shows up) would
>>     be the minimum version.
>> b) gcc-4.1 and 4.2 have too many problems, so users are better off
>>     when we tell them to upgrade to something newer, and a minimum
>>     version of gcc-4.3 has fewer surprises. We should remove all
>>     workarounds for pre-gcc-4.3 compilers and just force a build error
>>     message.
>
> If there's no real good reason (brokenness) to deprecate gcc-4.1, I would not
> do it.I guess most people using old compilers know what they're doing.

What I'm trying to find out first is whether "people regularly using 10+
year old compilers for the latest kernels" is a strict subset of "people in
Geert's household".

Given that none of the three architectures I looked at (arm, mips, x86)
has successfully built any defconfig for a few years, it's quite possible
that it's just you ;-) The other architectures that were around 10 years
ago (so they might have someone who still has old toolchain binaries)
and that still exist today are alpha, cris, frv, ia64, m32r, parisc, powerpc,
s390, sh, sparc and xtensa. The first six are similar to m68k in that the
hardware is mostly obsolete and the ports are kept around to support
those old systems that might also use ancient toolchains, or new
toolchains may be unmaintained or buggy, which could be a reason
to keep 4.1 supported or at least try to find out if 4.1 (or even any other
version) still works at all.

> My main motivation for keep on using gcc-4.1 is that it gives many warnings
> that were disabled in later gcc versions.  I do look at all new warnings, and
> send patches when they are real bugs, or are trivial to silence.

What kind of warnings do you see that disappeared with later versions?
Do you know what caused them to disappear in later versions (different
optimization decisions, warning getting disabled by default but still available
for turning on manually, ...)? Do you know if the disabled warnings are
still there in gcc-4.3 (I can try it out if you give me examples)?

> Lately the usefulness has been decreasing, as you've been too aggressively
> killing compiler warnings with recent gcc versions (which became better) ;-)
> Hence if I detected a new warning with a point or an rc release, it usually
> means the code was never in nex (ugh)t, or the maintainer ignored your patch.
>
> I could easily switch to v4.9 from kernel.org crosstool, though, but then I
> would loose all those warnings.

If gcc-4.3 produces similarly useful warnings, we could think about integrating
gcc-4.3 into kernelci.org build reports as an option, and fix up all
the existing
warnings we get with that. I wouldn't want to do that with gcc-4.1 though as
the older versions have relatively random behavior.

One particular feature I'd like to use that requires a newer compiler is being
able to control warnings per function liker glibc does, using e.g.
 '_Pragma("GCC diagnostic disable \"-Woverride-init\"")'. Once we have that,
we could turn on a couple of additional warnings that are generally useful
but also warn about code that intentionally does something that would
trigger the warning.

> BTW, below is the diff I use to avoid an ICE.
> After that, it builds and (test)boots fine on ARAnyM ;-)

I guess this means that even your builds require extra patches and you
can't strictly build a defconfig and expect that to work ;-)

      Arnd

^ permalink raw reply

* [PATCH V2] clk: hi6220: Add the hi655x's pmic clock
From: Daniel Lezcano @ 2017-04-24  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170424093154.ffo7zsr66a2yjy74@dell>

On Mon, Apr 24, 2017 at 10:31:54AM +0100, Lee Jones wrote:
> On Sat, 08 Apr 2017, Daniel Lezcano wrote:
> 
> > The hi655x multi function device is a PMIC providing regulators.
> > 
> > The PMIC also provides a clock for the WiFi and the Bluetooth, let's implement
> > this clock in order to add it in the hi655x MFD and allow proper wireless
> > initialization.
> > 
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> > 
> > Changelog:
> > 
> >  V2:
> >     - Added COMPILE_TEST option, compiled on x86
> >     - Removed useless parenthesis
> >     - Used of_clk_hw_simple_get() instead of deref dance
> >     - Do bailout if the clock-names is not specified
> >     - Rollback on error
> >     - Folded mfd line change and binding
> >     - Added #clock-cells binding documentation
> >     - Added #clock-cells in the DT
> > 
> >  V1: initial post
> > ---
> 
> ??
> 
> > ---
> 
> I'm unsure if this as been mentioned before, but bundling;
> driver & architecture changes and documentation into a single patch is
> very seldom a good idea.  In this case, you should split this into at
> least 3, perhaps 4 patches.
> 
> >  .../devicetree/bindings/mfd/hisilicon,hi655x.txt   |   6 +
> 
> Patch 1
> 
> >  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   1 +
> 
> Patch 2
> 
> >  drivers/clk/Kconfig                                |   8 ++
> >  drivers/clk/Makefile                               |   1 +
> >  drivers/clk/clk-hi655x.c                           | 140 +++++++++++++++++++++
> 
> Patch 3
> 
> >  drivers/mfd/hi655x-pmic.c                          |   3 +-
> 
> Patch 4
> 
> [...]

Yep. Will do that next time.

Thanks.

  -- Daniel

^ permalink raw reply

* [PATCH v3] kvm: arm/arm64: Fix locking for kvm_free_stage2_pgd
From: Suzuki K Poulose @ 2017-04-24  9:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <febea966-3767-21ff-3c40-1a76d1399138@suse.de>

On Sat, Apr 22, 2017 at 02:28:44AM +0200, Alexander Graf wrote:
> 
> 
> On 04.04.17 12:35, Suzuki K Poulose wrote:
> > Hi Christoffer,
> > 
> > On 04/04/17 11:13, Christoffer Dall wrote:
> > > Hi Suzuki,
> > > 
> > > On Mon, Apr 03, 2017 at 03:12:43PM +0100, Suzuki K Poulose wrote:
> > > > In kvm_free_stage2_pgd() we don't hold the kvm->mmu_lock while calling
> > > > unmap_stage2_range() on the entire memory range for the guest. This
> > > > could
> > > > cause problems with other callers (e.g, munmap on a memslot) trying to
> > > > unmap a range. And since we have to unmap the entire Guest memory range
> > > > holding a spinlock, make sure we yield the lock if necessary, after we
> > > > unmap each PUD range.
> > > > 
> > > > Fixes: commit d5d8184d35c9 ("KVM: ARM: Memory virtualization setup")
> > > > Cc: stable at vger.kernel.org # v3.10+
> > > > Cc: Paolo Bonzini <pbonzin@redhat.com>
> > > > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > > > Cc: Christoffer Dall <christoffer.dall@linaro.org>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > [ Avoid vCPU starvation and lockup detector warnings ]
> > > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > 
> > > 
> > > This unfortunately fails to build on 32-bit ARM, and I also think we
> > > intended to check against S2_PGDIR_SIZE, not S2_PUD_SIZE.
> > 
> > Sorry about that, I didn't test the patch with arm32. I am fine the
> > patch below. And I agree that the name change does make things more
> > readable. See below for a hunk that I posted to the kbuild report.
> > 
> > > 
> > > How about adding this to your patch (which includes a rename of
> > > S2_PGD_SIZE which is horribly confusing as it indicates the size of the
> > > first level stage-2 table itself, where S2_PGDIR_SIZE indicates the size
> > > of address space mapped by a single entry in the same table):
> > > 
> > > diff --git a/arch/arm/include/asm/stage2_pgtable.h
> > > b/arch/arm/include/asm/stage2_pgtable.h
> > > index 460d616..c997f2d 100644
> > > --- a/arch/arm/include/asm/stage2_pgtable.h
> > > +++ b/arch/arm/include/asm/stage2_pgtable.h
> > > @@ -35,10 +35,13 @@
> > > 
> > >  #define stage2_pud_huge(pud)            pud_huge(pud)
> > > 
> > > +#define S2_PGDIR_SIZE                PGDIR_SIZE
> > > +#define S2_PGDIR_MASK                PGDIR_MASK
> > > +
> > >  /* Open coded p*d_addr_end that can deal with 64bit addresses */
> > >  static inline phys_addr_t stage2_pgd_addr_end(phys_addr_t addr,
> > > phys_addr_t end)
> > >  {
> > > -    phys_addr_t boundary = (addr + PGDIR_SIZE) & PGDIR_MASK;
> > > +    phys_addr_t boundary = (addr + S2_PGDIR_SIZE) & S2_PGDIR_MASK;
> > > 
> > >      return (boundary - 1 < end - 1) ? boundary : end;
> > >  }
> > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> > > index db94f3a..6e79a4c 100644
> > > --- a/arch/arm/kvm/mmu.c
> > > +++ b/arch/arm/kvm/mmu.c
> > > @@ -41,7 +41,7 @@ static unsigned long hyp_idmap_start;
> > >  static unsigned long hyp_idmap_end;
> > >  static phys_addr_t hyp_idmap_vector;
> > > 
> > > -#define S2_PGD_SIZE    (PTRS_PER_S2_PGD * sizeof(pgd_t))
> > > +#define S2_PGD_TABLE_SIZE    (PTRS_PER_S2_PGD * sizeof(pgd_t))
> > >  #define hyp_pgd_order get_order(PTRS_PER_PGD * sizeof(pgd_t))
> > > 
> > >  #define KVM_S2PTE_FLAG_IS_IOMAP        (1UL << 0)
> > > @@ -299,7 +299,7 @@ static void unmap_stage2_range(struct kvm *kvm,
> > > phys_addr_t start, u64 size)
> > >           * If the range is too large, release the kvm->mmu_lock
> > >           * to prevent starvation and lockup detector warnings.
> > >           */
> > > -        if (size > S2_PUD_SIZE)
> > > +        if (size > S2_PGDIR_SIZE)
> > >              cond_resched_lock(&kvm->mmu_lock);
> > >          next = stage2_pgd_addr_end(addr, end);
> > >          if (!stage2_pgd_none(*pgd))
> > > @@ -747,7 +747,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm)
> > >      }
> > > 
> > >      /* Allocate the HW PGD, making sure that each page gets its own
> > > refcount */
> > > -    pgd = alloc_pages_exact(S2_PGD_SIZE, GFP_KERNEL | __GFP_ZERO);
> > > +    pgd = alloc_pages_exact(S2_PGD_TABLE_SIZE, GFP_KERNEL | __GFP_ZERO);
> > >      if (!pgd)
> > >          return -ENOMEM;
> > > 
> > > @@ -843,7 +843,7 @@ void kvm_free_stage2_pgd(struct kvm *kvm)
> > >      spin_unlock(&kvm->mmu_lock);
> > > 
> > >      /* Free the HW pgd, one page at a time */
> > > -    free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
> > > +    free_pages_exact(kvm->arch.pgd, S2_PGD_TABLE_SIZE);
> > >      kvm->arch.pgd = NULL;
> > >  }
> > > 
> > 
> > Btw, I have a different hunk to solve the problem, posted to the kbuild
> > report. I will post it here for the sake of capturing the discussion in
> > one place. The following hunk on top of the patch, changes the lock
> > release after we process one PGDIR entry. As for the first time
> > we enter the loop we haven't done much with the lock held, hence it may
> > make
> > sense to do it after the first round and we have more work to do.
> > 
> > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> > index db94f3a..582a972 100644
> > --- a/arch/arm/kvm/mmu.c
> > +++ b/arch/arm/kvm/mmu.c
> > @@ -295,15 +295,15 @@ static void unmap_stage2_range(struct kvm *kvm,
> > phys_addr_t start, u64 size)
> >         assert_spin_locked(&kvm->mmu_lock);
> >         pgd = kvm->arch.pgd + stage2_pgd_index(addr);
> >         do {
> > +               next = stage2_pgd_addr_end(addr, end);
> > +               if (!stage2_pgd_none(*pgd))
> 
> Just as heads up, I had this version applied to my tree by accident (commit
> 8b3405e345b5a098101b0c31b264c812bba045d9 from Christoffer's queue) and ran
> into a NULL pointer dereference:
> 
> [223090.242280] Unable to handle kernel NULL pointer dereference at virtual
> address 00000040
> [223090.262330] PC is at unmap_stage2_range+0x8c/0x428
> [223090.262332] LR is at kvm_unmap_hva_handler+0x2c/0x3c
> [223090.262531] Call trace:
> [223090.262533] [<ffff0000080adb78>] unmap_stage2_range+0x8c/0x428
> [223090.262535] [<ffff0000080adf40>] kvm_unmap_hva_handler+0x2c/0x3c
> [223090.262537] [<ffff0000080ace2c>] handle_hva_to_gpa+0xb0/0x104
> [223090.262539] [<ffff0000080af988>] kvm_unmap_hva+0x5c/0xbc
> [223090.262543] [<ffff0000080a2478>]
> kvm_mmu_notifier_invalidate_page+0x50/0x8c
> [223090.262547] [<ffff0000082274f8>]
> __mmu_notifier_invalidate_page+0x5c/0x84
> [223090.262551] [<ffff00000820b700>] try_to_unmap_one+0x1d0/0x4a0
> [223090.262553] [<ffff00000820c5c8>] rmap_walk+0x1cc/0x2e0
> [223090.262555] [<ffff00000820c90c>] try_to_unmap+0x74/0xa4
> [223090.262557] [<ffff000008230ce4>] migrate_pages+0x31c/0x5ac
> [223090.262561] [<ffff0000081f869c>] compact_zone+0x3fc/0x7ac
> [223090.262563] [<ffff0000081f8ae0>] compact_zone_order+0x94/0xb0
> [223090.262564] [<ffff0000081f91c0>] try_to_compact_pages+0x108/0x290
> [223090.262569] [<ffff0000081d5108>] __alloc_pages_direct_compact+0x70/0x1ac
> [223090.262571] [<ffff0000081d64a0>] __alloc_pages_nodemask+0x434/0x9f4
> [223090.262572] [<ffff0000082256f0>] alloc_pages_vma+0x230/0x254
> [223090.262574] [<ffff000008235e5c>] do_huge_pmd_anonymous_page+0x114/0x538
> [223090.262576] [<ffff000008201bec>] handle_mm_fault+0xd40/0x17a4
> [223090.262577] [<ffff0000081fb324>] __get_user_pages+0x12c/0x36c
> [223090.262578] [<ffff0000081fb804>] get_user_pages_unlocked+0xa4/0x1b8
> [223090.262579] [<ffff0000080a3ce8>] __gfn_to_pfn_memslot+0x280/0x31c
> [223090.262580] [<ffff0000080a3dd0>] gfn_to_pfn_prot+0x4c/0x5c
> [223090.262582] [<ffff0000080af3f8>] kvm_handle_guest_abort+0x240/0x774
> [223090.262584] [<ffff0000080b2bac>] handle_exit+0x11c/0x1ac
> [223090.262586] [<ffff0000080ab99c>] kvm_arch_vcpu_ioctl_run+0x31c/0x648
> [223090.262587] [<ffff0000080a1d78>] kvm_vcpu_ioctl+0x378/0x768
> [223090.262590] [<ffff00000825df5c>] do_vfs_ioctl+0x324/0x5a4
> [223090.262591] [<ffff00000825e26c>] SyS_ioctl+0x90/0xa4
> [223090.262595] [<ffff000008085d84>] el0_svc_naked+0x38/0x3c
> 
> 0xffff0000080adb78 is in unmap_stage2_range (../arch/arm/kvm/mmu.c:260).
> 255		pud_t *pud, *start_pud;
> 256
> 257		start_pud = pud = stage2_pud_offset(pgd, addr);
> 258		do {
> 259			next = stage2_pud_addr_end(addr, end);
> 260			if (!stage2_pud_none(*pud)) {
> 261				if (stage2_pud_huge(*pud)) {
> 262					pud_t old_pud = *pud;
> 263
> 264					stage2_pud_clear(pud);
> 
> 
> So please beware that for some reason pud may become invalid after
> rescheduling.

Alex,

Thanks for the report. The patch below should fix this one.

---8>---

kvm: arm/arm64: Fix race in resetting stage2 PGD

In kvm_free_stage2_pgd() we check the stage2 PGD before holding
the lock and proceed to take the lock if it is valid. And we unmap
the page tables, followed by releasing the lock. We reset the PGD
only after dropping this lock, which could cause a race condition
where another thread waiting on the lock could potentially see that
the PGD is still valid and proceed to perform a stage2 operation.

This patch moves the stage2 PGD manipulation under the lock.

Reported-by: Alexander Graf <agraf@suse.de>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm/kvm/mmu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 582a972..9c4026d 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -835,16 +835,18 @@ void stage2_unmap_vm(struct kvm *kvm)
  */
 void kvm_free_stage2_pgd(struct kvm *kvm)
 {
-	if (kvm->arch.pgd == NULL)
-		return;
+	void *pgd = NULL;
 
 	spin_lock(&kvm->mmu_lock);
-	unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
+	if (kvm->arch.pgd) {
+		unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE);
+		pgd = kvm->arch.pgd;
+		kvm->arch.pgd = NULL;
+	}
 	spin_unlock(&kvm->mmu_lock);
-
 	/* Free the HW pgd, one page at a time */
-	free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE);
-	kvm->arch.pgd = NULL;
+	if (pgd)
+		free_pages_exact(pgd, S2_PGD_SIZE);
 }
 
 static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox