* [PATCH v3] arm64: Improve kprobes test for atomic sequence
From: Masami Hiramatsu @ 2016-09-12 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D60AA7.6010304@linaro.org>
On Sun, 11 Sep 2016 21:53:43 -0400
David Long <dave.long@linaro.org> wrote:
> On 09/10/2016 01:48 AM, Masami Hiramatsu wrote:
> > On Fri, 9 Sep 2016 15:26:09 -0400
> > David Long <dave.long@linaro.org> wrote:
> >
> >> From: "David A. Long" <dave.long@linaro.org>
> >>
> >> Kprobes searches backwards a finite number of instructions to determine if
> >> there is an attempt to probe a load/store exclusive sequence. It stops when
> >> it hits the maximum number of instructions or a load or store exclusive.
> >> However this means it can run up past the beginning of the function and
> >> start looking at literal constants. This has been shown to cause a false
> >> positive and blocks insertion of the probe. To fix this, further limit the
> >> backwards search to stop if it hits a symbol address from kallsyms. The
> >> presumption is that this is the entry point to this code (particularly for
> >> the common case of placing probes at the beginning of functions).
> >>
> >> This also improves efficiency by not searching code that is not part of the
> >> function. There may be some possibility that the label might not denote the
> >> entry path to the probed instruction but the likelihood seems low and this
> >> is just another example of how the kprobes user really needs to be
> >> careful about what they are doing.
> >
> > Of course user should be careful, but also, in such case, kernel can reject
> > to probe it.
> >
>
> I'm not exactly sure what you mean. I'm just saying when everything
> goes right we still cannot promise perfection in detecting a probe
> within an atomic sequence. This patch will reject a probe that is after
> a ldx and has no intervening kallsyms label (and assuming it's within
> the defined maximum count of subsequent instructions).
>
Hmm, what I meant was the below code.
> >> + /*
> >> + * If there's a symbol defined in front of and near enough to
> >> + * the probe address assume it is the entry point to this
> >> + * code and use it to further limit how far back we search
> >> + * when determining if we're in an atomic sequence. If we could
> >> + * not find any symbol skip the atomic test altogether as we
> >> + * could otherwise end up searching irrelevant text/literals.
> >> + * KPROBES depends on KALLSYMS so this last case should never
> >> + * happen.
> >> + */
> >> + if (kallsyms_lookup_size_offset((unsigned long) addr, &size, &offset)) {
> >> + if (offset < (MAX_ATOMIC_CONTEXT_SIZE*sizeof(kprobe_opcode_t)))
> >> + scan_end = addr - (offset / sizeof(kprobe_opcode_t));
> >> + else
> >> + scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
> >
> > } else
> > return INSN_REJECTED;
> >
> > that is what I expected...
As you said above,
> >> + * KPROBES depends on KALLSYMS so this last case should never
> >> + * happen.
If it should never happen, it also would be better to reject it because
it is unexpected result.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Sebastian Frias @ 2016-09-12 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANCZdfri+T59FA4XTaH5JbW0BaJHt3w5eaSkpD8NrJn6nZrZbA@mail.gmail.com>
Hi Warner,
On 09/12/2016 04:26 PM, Warner Losh wrote:
> On Mon, Sep 12, 2016 at 8:01 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> Since the question seems understood, do you have an example of other SoC's
>>> doing something similar?
>>
>> I do not have an example. I know that others are using DT for data
>> beyond what Linux or another OS requires, but it's my understanding that
>> that is typically in a separate DTB.
>
> Just to clarify: FreeBSD uses, for the most part, the DTB's that the
> 'vendor' ships, which is quite often the same ones included in Linux.
> There's some exceptions where the bindings weren't really hardware
> independent, or where the abstraction model was really Linux specific
> (for things like the HDMI stack).
>
> However, with the advent of overlays, one would think that a vendor
> could easily include an overlay with the DTB data for the devices they
> don't wish to, or cannot for other reasons release. It seems like the
> perfect mechanism to comply with the rules about inclusion of nodes in
> the DTS. Vendors are free to document these nodes and don't require
> the Linux kernel include them in the Documents directory to do so.
> There have been recent efforts to move this documentation to a third
> party to maintain.
This is very interesting, do you have a more concrete example of such
usage?
The overlay technique could be a solution, but so is forking and
distributing a non-documented DT. That's why I'd put this solution a
little bit lower than just exposing the entire HW description through
the DT.
Best regards,
Sebastian
^ permalink raw reply
* [PATCH 2/3] ARM64: dts: amlogic: Add basic support for Amlogic S905X
From: Andreas Färber @ 2016-09-12 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160903082227.30559-3-narmstrong@baylibre.com>
Hi,
Am 03.09.2016 um 10:22 schrieb Neil Armstrong:
> From: Carlo Caione <carlo@endlessm.com>
>
> This patch introduces the basic support for the Amlogic S905X (Meson
> GXL) and for the Amlogic evaluation board P212.
> No documentation has been released yet for this SoC, so for now only the
> bare minimum has been added in the DT.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Andreas F?rber <afaerber@suse.de>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> Documentation/devicetree/bindings/arm/amlogic.txt | 5 ++
> arch/arm64/boot/dts/amlogic/Makefile | 1 +
> .../boot/dts/amlogic/meson-gxl-s905x-p212.dts | 70 ++++++++++++++++++++++
> arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi | 48 +++++++++++++++
> arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 48 +++++++++++++++
> 5 files changed, 172 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>
> diff --git a/Documentation/devicetree/bindings/arm/amlogic.txt b/Documentation/devicetree/bindings/arm/amlogic.txt
> index fcc6f6c..702c911 100644
> --- a/Documentation/devicetree/bindings/arm/amlogic.txt
> +++ b/Documentation/devicetree/bindings/arm/amlogic.txt
> @@ -17,6 +17,10 @@ Boards with the Amlogic Meson GXBaby SoC shall have the following properties:
> Required root node property:
> compatible: "amlogic,meson-gxbb";
>
> +Boards with the Amlogic Meson GXL SoC shall have the following properties:
> + Required root node property:
> + compatible: "amlogic,meson-gxl-s905x", "amlogic,meson-gxl";
Can we please use "amlogic,s905x", "amlogic,meson-gxl"? No need to
complicate the name. Also affects .dtsi and .dts below.
> +
> Board compatible values:
> - "geniatech,atv1200" (Meson6)
> - "minix,neo-x8" (Meson8)
> @@ -28,3 +32,4 @@ Board compatible values:
> - "hardkernel,odroid-c2" (Meson gxbb)
> - "amlogic,p200" (Meson gxbb)
> - "amlogic,p201" (Meson gxbb)
> + - "amlogic,p212" (Meson gxl s905x)
[...]
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
> new file mode 100644
> index 0000000..e0df975
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (c) 2016 Endless Computers, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + * a) This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + * b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-gxl-s905x.dtsi"
> +
> +/ {
> + compatible = "amlogic,p212", "amlogic,meson-gxl-s905x", "amlogic,meson-gxl";
> + model = "Amlogic Meson GXL (S905X) P212 Development Board";
Is that its official name? No objection, just wondering whether we need
both GXL and S905X in the name, and then again the SoC name at all if
the P212 is unique. Haven't compared the previous Pxxx ones.
> +
> + aliases {
> + serial0 = &uart_AO;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory at 0 {
> + device_type = "memory";
> + reg = <0x0 0x0 0x0 0x80000000>;
> + };
> +};
> +
> +/* This UART is brought out to the DB9 connector */
> +&uart_AO {
> + status = "okay";
> +};
> +
Trailing white line - please watch out for that, git-am will complain.
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> new file mode 100644
> index 0000000..b9e94f1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (c) 2016 Endless Computers, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + * a) This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + * b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "meson-gxl.dtsi"
> +
> +/ {
> + compatible = "amlogic,meson-gxl", "amlogic,meson-gxl-s905x";
This needs to be reversed.
> +};
[snip]
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply
* [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-12 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912102136.04c0728d@ipc1.ka-ro>
On Mon, Sep 12, 2016 at 3:21 AM, Lothar Wa?mann <LW@karo-electronics.de> wrote:
> Hi,
>
> On Fri, 9 Sep 2016 14:37:12 -0500 Leo Li wrote:
>> On Fri, Sep 9, 2016 at 11:51 AM, Stefan Agner <stefan@agner.ch> wrote:
>> > On 2016-09-08 16:57, Leo Li wrote:
> [...]
>> >> people fix problem that they don't really care by deliberately enlarge
>> >> the problem. That's why we don't panic() on any error we found. For
>> >> those who do care about the bus recovery, they can get the information
>> >> from the console.
>> >
>> > IMHO, it is just stupid to ignore errors and then let the developer
>> > later on trace back what the initial issue was. Error out early is a
>> > common sense software design principle...
>> >
>> > I am not asking for a panic(), I am just suggesting to only ignore
>> > pinctrl if it returns -ENODEV, the case you care are about.
>>
>> It was just an analogy for enlarging the problem for getting
>> attention. But you probably thought that it was not enlarging the
>> problem as you think pinctrl is required by the driver instead of an
>> optional thing.
>>
> The I2C bus recovery is a feature which is not used during normal
> operation and you won't find that it's not working unless something
> unusual happens (probably only in a productive environment). It may even
> be hard to trigger a condition to test the feature.
> Thus IMO it is vital that the driver complains loudly if something is
> missing to make this feature work if requested.
I agree. A message is already added in this patch to mention that bus
recovery is not supported due to pinctrl when there is a problem.
Regards,
Leo
^ permalink raw reply
* [PATCH] arm64: mm: move zero page from .bss to right before swapper_pg_dir
From: Mark Rutland @ 2016-09-12 16:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9R+jrc5w8FgrKKnm+vk82ahLHy-NYV4JzULHjPbNy02w@mail.gmail.com>
On Mon, Sep 12, 2016 at 04:54:58PM +0100, Ard Biesheuvel wrote:
> On 12 September 2016 at 16:40, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Sep 12, 2016 at 04:12:19PM +0100, Ard Biesheuvel wrote:
> >> On 12 September 2016 at 15:59, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Mon, Sep 12, 2016 at 03:16:24PM +0100, Ard Biesheuvel wrote:
> >> >> BSS_SECTION(0, 0, 0)
> >> >>
> >> >> - . = ALIGN(PAGE_SIZE);
> >> >> + . = ALIGN(SEGMENT_ALIGN);
> >> >> + __robss_start = .;
> >> >> idmap_pg_dir = .;
> >> >> - . += IDMAP_DIR_SIZE;
> >> >> + . = ALIGN(. + IDMAP_DIR_SIZE + PAGE_SIZE, SEGMENT_ALIGN);
> >> >> + __robss_end = .;
> >> >
> >> > Is it really worth aligning this beyond PAGE_SIZE?
> >> >
> >> > We shouldn't be poking these very often, the padding is always larger
> >> > than the number of used pages, and the swapper dir is relegated to page
> >> > mappings regardless.
> >>
> >> The segment alignment is intended to take advantage of PTE_CONT
> >> mappings (support for which still hasn't landed, afaict). I don't care
> >> deeply either way ...
> >
> > I understood that; my concern was that there was little gain relative to
> > the cost of the padding:
> >
> > * With the above .robss will contain 5 pages that we care about, but
> > could be padded to 16 or 512 pages (assuming 4K pages with or without
> > DEBUG_RODATA_ALIGN). I think we can put those pages to better use.
>
> Yes, I realised that. DEBUG_RODATA_ALIGN generally wastes a lot of
> memory on padding, and this case is no different.
>
> > * We don't frequently need to poke the idmap, so in practice I suspect
> > TLB pressure for it doesn't matter too much.
>
> Does that apply to empty_zero_page as well?
Good question -- I'm not sure how often we access it in practice from a
kernel VA.
> > * As we don't align _end, swapper (which we're more likely to access
> > frequently) is mapped with a non-contiguous mapping regardless.
>
> Indeed. However, we could defer the r/o mapping of this segment to
> mark_rodata_ro(), which allows us to move other stuff in there as
> well, such as bm_pmd/bm_pud (from fixmap), and actually, anything that
> would qualify for __ro_after_init but is not statically initialized to
> non-zero value.
True. That could be be good.
Thanks,
Mark.
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Sebastian Frias @ 2016-09-12 16:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D6D2A9.3010006@laposte.net>
On 09/12/2016 06:07 PM, Sebastian Frias wrote:
> Hi Mark,
>
> On 09/12/2016 04:01 PM, Mark Rutland wrote:
>>> 3rd parties could choose to write a driver (as opposed to use say, a user-mode
>>> library) if it fits their programming model better, if they think they would
>>> have better performance, or other reasons.
>>
>> A vendor can always choose to "add value" in this manner. The general
>> expectation of *some* driver being upstreamed remains.
>
> Yes, that's the idea.
Just to clarify, what I meant is that, using the DT as the authoritative
source of HW description is a way to "add value" to everybody, because both,
3rd-parties and the open-source community get the same information.
This creates the conditions for drivers to exist, with the expectation that
eventually said drivers would be upstreamed.
^ permalink raw reply
* [PATCH v2 1/7] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
From: Catalin Marinas @ 2016-09-12 16:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912150958.GC14165@leverpostej>
On Mon, Sep 12, 2016 at 04:09:59PM +0100, Mark Rutland wrote:
> On Mon, Sep 12, 2016 at 03:52:19PM +0100, Catalin Marinas wrote:
> > On Mon, Sep 05, 2016 at 04:38:28PM +0100, Mark Rutland wrote:
> > > On Fri, Sep 02, 2016 at 04:02:07PM +0100, Catalin Marinas wrote:
> > > > /*
> > > > + * User access enabling/disabling.
> > > > + */
> > > > +#define uaccess_disable(alt) \
> > > > +do { \
> > > > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
> > > > + CONFIG_ARM64_PAN)); \
> > > > +} while (0)
> > > > +
> > > > +#define uaccess_enable(alt) \
> > > > +do { \
> > > > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > > > + CONFIG_ARM64_PAN)); \
> > > > +} while (0)
> > >
> > > Passing the alternative down is somewhat confusing. e.g. in the futex
> > > case it looks like we're only doing something when PAN is present,
> > > whereas we'll manipulate TTBR0 in the absence of PAN.
> >
> > I agree it's confusing (I got it wrong first time as well and used the
> > wrong alternative for futex).
> >
> > > If I've understood correctly, we need this to distinguish regular
> > > load/store uaccess sequences (eg. the futex code) from potentially
> > > patched unprivileged load/store sequences (e.g. {get,put}_user) when
> > > poking PSTATE.PAN.
> > >
> > > So perhaps we could ahve something like:
> > >
> > > * privileged_uaccess_{enable,disable}()
> > > Which toggle TTBR0, or PAN (always).
> > > These would handle cases like the futex/swp code.
> > >
> > > * (unprivileged_)uaccess_{enable,disable}()
> > > Which toggle TTBR0, or PAN (in the absence of UAO).
> > > These would handle cases like the {get,put}_user sequences.
> > >
> > > Though perhaps that is just as confusing. ;)
> >
> > I find it more confusing.
>
> Fair enough. :)
>
> > In the non-UAO case, get_user etc. would
> > normally have to use privileged_uaccess_enable() since ldr is not
> > replaced with ldtr. Maybe uaccess_enable_for_exclusives() but it doesn't
> > look any better.
>
> I strongly prefer uaccess_enable_exclusives(), or something of that sort
> to both of the above. ;)
I think we would need a few more uaccess_enable_* variants (cache
maintenance, Xen) which makes this impractical.
We can consider the PAN_NOT_UAO the special case and if we assume that
UAO also implies PAN (ARMv8.2), we can define uaccess_enable_not_uao()
for the get_user etc. cases. We would use uaccess_enable() for the rest.
--
Catalin
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Timur Tabi @ 2016-09-12 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D6D2A9.3010006@laposte.net>
Sebastian Frias wrote:
>> You're not changing the code, but you are creating a binding.
>> Bindings
>>> are intended to be stable (i.e. a working DTB from today should
>>> continue to work in future), and thus there are ramifications.
> Ok, but who is responsible for such guarantee?
The developers who create the driver and modify it. Only upstream
contributions count.
> How is it enforced and verified?
The upstream reviewers verify it, and if they see a problem, they reject
the patch.
> Exactly, that's why to I'm having trouble to understand why there is
> so much insistence on "getting the DT 100% right", since a HW change
> could imply that what made 100% sense yesterday, does not today.
I'm not sure what you're getting at. Everything is best-effort. The
binding for a given device is supposed to accurately, but minimally,
describe the hardware so that the driver can program the device
properly. Specific properties are added to the binding to handle
specific cases. If a binding includes a description of properties that
are not used by the driver, the developer is typically asked to remove
those.
So I don't understand the "100% right" comment.
> Could you be more precise on those two issues? Namely: "the effort"
> and the "lack of benefit for the community"?
1) The effort = the effort by upstream developers to review the binding.
2) lack of benefit = proprietary and out-of-tree drivers do not benefit
the community.
> I can understand the effort it takes to review a binding and some
> driver, but if there's no driver, why would it matter if the DT
> binding is 100% right?
Why bother creating a binding if there's no driver to use it?
> Hence, why would it take more effort?
> Furthermore, if there's no driver, there's no backward compatibility
> to guarantee. Shouldn't it require less effort?
If a developer creates a binding without a driver, then it's very hard
to know if that binding is correct. That's like creating a recipe
without attempting to make the food. How do you know it actually tastes
good? No one would ever ask a chef to create a "technically correct"
recipe without cooking it first and tasting it.
> Also, what sort of "benefits" does the community expects or
> requires? Because the idea behind the proposal is to put the HW
> description in DT, so basically the HW would be documented. Maybe
> there wouldn't be a driver right away, but the HW description would
> allow for drivers to exist.
I'm not sure how many times we need to repeat this, but the idea that
the DT binding documentation can be used as official documentation for
the hardware is absurd. What's wrong with the ACTUAL hardware
documentation provided by the manufacturer in PDF format? The DT
binding has no value without an actual driver.
^ permalink raw reply
* [PATCH v10 1/5] Documentation: bindings: add dt documentation for dfi controller
From: Rob Herring @ 2016-09-12 16:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473051971-5500-2-git-send-email-hl@rock-chips.com>
On Mon, Sep 05, 2016 at 01:06:07PM +0800, Lin Huang wrote:
> This patch adds the documentation for rockchip dfi devfreq-event driver.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes in v10:
> - None
>
> Changes in v9:
> - reorder compatible and reg
>
> Changes in v8:
> - delete a unuse blank line
>
> Changes in v7:
> - None
>
> Changes in v6:
> - None
>
> Changes in v5:
> - None
>
> Changes in v4:
> - None
>
> Changes in v3:
> - None
>
> Changes in v2:
> - None
>
> Changes in v1:
> - None
>
> .../bindings/devfreq/event/rockchip-dfi.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt
>
> diff --git a/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt b/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt
> new file mode 100644
> index 0000000..f223313
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt
> @@ -0,0 +1,19 @@
> +
> +* Rockchip rk3399 DFI device
> +
> +Required properties:
> +- compatible: Must be "rockchip,rk3399-dfi".
> +- reg: physical base address of each DFI and length of memory mapped region
> +- rockchip,pmu: phandle to the syscon managing the "pmu general register files"
> +- clocks: phandles for clock specified in "clock-names" property
> +- clock-names : the name of clock used by the DFI, must be "pclk_ddr_mon";
> +
> +Example:
> + dfi: dfi at 0xff630000 {
Drop the '0x'. With that,
Acked-by: Rob Herring <robh@kernel.org>
> + compatible = "rockchip,rk3399-dfi";
> + reg = <0x00 0xff630000 0x00 0x4000>;
> + rockchip,pmu = <&pmugrf>;
> + clocks = <&cru PCLK_DDR_MON>;
> + clock-names = "pclk_ddr_mon";
> + status = "disabled";
> + };
> --
> 2.6.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Lina Iyer @ 2016-09-12 16:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87sht59mcw.fsf@arm.com>
On Mon, Sep 12 2016 at 09:19 -0600, Brendan Jackman wrote:
>
>Hi Lina,
>
>Sorry for the delay here, Sudeep and I were both been on holiday last week.
>
>On Fri, Sep 02 2016 at 21:16, Lina Iyer wrote:
>> On Fri, Sep 02 2016 at 07:21 -0700, Sudeep Holla wrote:
>[...]
>>>This version is *not very descriptive*. Also the discussion we had on v3
>>>version has not yet concluded IMO. So can I take that we agreed on what
>>>was proposed there or not ?
>>>
>> Sorry, this example is not very descriptive. Pls. check the 8916 dtsi
>> for the new changes in the following patches. Let me know if that makes
>> sense.
>
>The not-yet-concluded discussion Sudeep is referring to is at [1].
>
>In that thread we initially proposed the idea of, instead of splitting
>state phandles between cpu-idle-states and domain-idle-states, putting
>CPUs in their own domains and using domain-idle-states for _all_
>phandles, deprecating cpu-idle-states. I've brought this up in other
>threads [2] but discussion keeps petering out, and neither this example
>nor the 8916 dtsi in this patch series reflect the idea.
>
Brendan, while your idea is good and will work for CPUs, I do not expect
other domains and possibly CPU domains on some architectures to follow
this model. There is nothing that prevents you from doing this today,
you can specify domains around CPUs in your devicetree and CPU PM will
handle the hierarchy. I don't think its fair to force it on all SoCs
using CPU domains. This patchset does not restrict you from organizing
the idle states the way you want it. This revision of the series, clubs
CPU and domain idle states under idle-states umbrella. So part of your
requirement is also satisfied.
You can follow up the series with your new additions, I don't see a
conflict with this change.
Thanks,
Lina
>It would be great if we could go back to the thread at [1] where Sudeep
>has posted examples and come to a clear consensus on the binding design
>before reviewing implementation patches. Ideally with input from Ulf,
>Rob and Kevin.
>
>[1] https://patchwork.kernel.org/patch/9264507
>[2] http://www.spinics.net/lists/devicetree/msg141024.html
>>
>> Thanks,
>> Lina
>>
>>>We could have better example above *really* based on the discussions we
>>>had so far. This example always makes me think it's well crafted to
>>>avoid any sort of discussions. We need to consider different use-cases
>>>e.g. what about CPU level states ?
>>>
>>>IMO, we need to discuss this DT binding in detail and arrive at someq
>>>conclusion before you take all the troubles to respin the series.
>>>Also it's better to keep the DT binding separate until we have some
>>>conclusion instead of posting the implementation for each version.
>>>That's just my opinion(I would be least bothered about implementation
>>>until I know it will be accepted before I can peek into the code, others
>>>may differ.
>>>
>>>--
>>>Regards,
>>>Sudeep
>
>Cheers,
>Brendan
^ permalink raw reply
* [PATCH v3] arm64: mm: move zero page from .bss to right before swapper_pg_dir
From: Ard Biesheuvel @ 2016-09-12 16:15 UTC (permalink / raw)
To: linux-arm-kernel
Move the statically allocated zero page from the .bss section to right
before swapper_pg_dir. This allows us to refer to its physical address
by simply reading TTBR1_EL1 (which always points to swapper_pg_dir and
always has its ASID field cleared), and subtracting PAGE_SIZE.
To protect the zero page from inadvertent modification, carve out a
segment that covers it as well as idmap_pg_dir[], and mark it read-only
in both the primary and the linear mappings of the kernel.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v3: rebase onto for-next/core
add clarifying comments around cpu_set_reserved_ttbr0()
reduce alignment of r/o region covering idmap_pg_dir and empty_zero_page
to PAGE_SIZE
rename vmlinux_tail vm_struct to vmlinux_swapper
v2: make empty_zero_page[] read-only
make idmap_pg_dir[] read-only as well
fix issue in v1 with cpu_reserved_ttbr0()
arch/arm64/include/asm/mmu_context.h | 18 +++++--
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 9 ++++
arch/arm64/mm/mmu.c | 56 ++++++++++++--------
4 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index a50185375f09..c934449504a2 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -41,12 +41,16 @@ static inline void contextidr_thread_switch(struct task_struct *next)
/*
* Set TTBR0 to empty_zero_page. No translations will be possible via TTBR0.
+ * In some cases (e.g. where cpu_replace_ttbr1 is used), TTBR1 may not
+ * point at swapper_pg_dir, and this helper cannot be used.
*/
static inline void cpu_set_reserved_ttbr0(void)
{
- unsigned long ttbr = virt_to_phys(empty_zero_page);
-
- write_sysreg(ttbr, ttbr0_el1);
+ /*
+ * The zero page is located right before swapper_pg_dir, whose
+ * physical address we can easily fetch from TTBR1_EL1.
+ */
+ write_sysreg(read_sysreg(ttbr1_el1) - PAGE_SIZE, ttbr0_el1);
isb();
}
@@ -99,7 +103,9 @@ static inline void cpu_uninstall_idmap(void)
{
struct mm_struct *mm = current->active_mm;
- cpu_set_reserved_ttbr0();
+ /* see cpu_set_reserved_ttbr0 */
+ write_sysreg(virt_to_phys(empty_zero_page), ttbr0_el1);
+ isb();
local_flush_tlb_all();
cpu_set_default_tcr_t0sz();
@@ -109,7 +115,9 @@ static inline void cpu_uninstall_idmap(void)
static inline void cpu_install_idmap(void)
{
- cpu_set_reserved_ttbr0();
+ /* see cpu_set_reserved_ttbr0 */
+ write_sysreg(virt_to_phys(empty_zero_page), ttbr0_el1);
+ isb();
local_flush_tlb_all();
cpu_set_idmap_tcr_t0sz();
diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
index 4e7e7067afdb..44e94e234ba0 100644
--- a/arch/arm64/include/asm/sections.h
+++ b/arch/arm64/include/asm/sections.h
@@ -26,5 +26,6 @@ extern char __hyp_text_start[], __hyp_text_end[];
extern char __idmap_text_start[], __idmap_text_end[];
extern char __irqentry_text_start[], __irqentry_text_end[];
extern char __mmuoff_data_start[], __mmuoff_data_end[];
+extern char __robss_start[], __robss_end[];
#endif /* __ASM_SECTIONS_H */
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 5ce9b2929e0d..0f0675645f47 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -210,8 +210,17 @@ SECTIONS
BSS_SECTION(0, 0, 0)
. = ALIGN(PAGE_SIZE);
+ __robss_start = .;
idmap_pg_dir = .;
. += IDMAP_DIR_SIZE;
+ /*
+ * Put the zero page right before swapper_pg_dir so we can
+ * easily obtain its physical address by subtracting PAGE_SIZE
+ * from the contents of TTBR1_EL1.
+ */
+ empty_zero_page = .;
+ . += PAGE_SIZE;
+ __robss_end = .;
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 05615a3fdc6f..d2be62ff1ad3 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -52,7 +52,6 @@ EXPORT_SYMBOL(kimage_voffset);
* Empty_zero_page is a special page that is used for zero-initialized data
* and COW.
*/
-unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
EXPORT_SYMBOL(empty_zero_page);
static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
@@ -319,16 +318,18 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
{
- unsigned long kernel_start = __pa(_text);
- unsigned long kernel_end = __pa(__init_begin);
+ unsigned long text_start = __pa(_text);
+ unsigned long text_end = __pa(__init_begin);
+ unsigned long robss_start = __pa(__robss_start);
+ unsigned long robss_end = __pa(__robss_end);
/*
* Take care not to create a writable alias for the
- * read-only text and rodata sections of the kernel image.
+ * read-only text/rodata/robss sections of the kernel image.
*/
- /* No overlap with the kernel text/rodata */
- if (end < kernel_start || start >= kernel_end) {
+ /* No overlap with the kernel text/rodata/robss */
+ if (end < text_start || start >= robss_end) {
__create_pgd_mapping(pgd, start, __phys_to_virt(start),
end - start, PAGE_KERNEL,
early_pgtable_alloc,
@@ -340,27 +341,32 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
* This block overlaps the kernel text/rodata mappings.
* Map the portion(s) which don't overlap.
*/
- if (start < kernel_start)
- __create_pgd_mapping(pgd, start,
- __phys_to_virt(start),
- kernel_start - start, PAGE_KERNEL,
+ if (start < text_start)
+ __create_pgd_mapping(pgd, start, __phys_to_virt(start),
+ text_start - start, PAGE_KERNEL,
early_pgtable_alloc,
!debug_pagealloc_enabled());
- if (kernel_end < end)
- __create_pgd_mapping(pgd, kernel_end,
- __phys_to_virt(kernel_end),
- end - kernel_end, PAGE_KERNEL,
+ if (robss_end < end)
+ __create_pgd_mapping(pgd, robss_end, __phys_to_virt(robss_end),
+ end - robss_end, PAGE_KERNEL,
early_pgtable_alloc,
!debug_pagealloc_enabled());
/*
- * Map the linear alias of the [_text, __init_begin) interval as
- * read-only/non-executable. This makes the contents of the
- * region accessible to subsystems such as hibernate, but
- * protects it from inadvertent modification or execution.
+ * Map the linear alias of the intervals [_text, __init_begin) and
+ * [robss_start, robss_end) as read-only/non-executable. This makes
+ * the contents of these regions accessible to subsystems such
+ * as hibernate, but protects them from inadvertent modification or
+ * execution.
*/
- __create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
- kernel_end - kernel_start, PAGE_KERNEL_RO,
+ __create_pgd_mapping(pgd, text_start, __phys_to_virt(text_start),
+ text_end - text_start, PAGE_KERNEL_RO,
+ early_pgtable_alloc, !debug_pagealloc_enabled());
+ __create_pgd_mapping(pgd, text_end, __phys_to_virt(text_end),
+ robss_start - text_end, PAGE_KERNEL,
+ early_pgtable_alloc, !debug_pagealloc_enabled());
+ __create_pgd_mapping(pgd, robss_start, __phys_to_virt(robss_start),
+ robss_end - robss_start, PAGE_KERNEL_RO,
early_pgtable_alloc, !debug_pagealloc_enabled());
}
@@ -424,13 +430,19 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
*/
static void __init map_kernel(pgd_t *pgd)
{
- static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init, vmlinux_data;
+ static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init,
+ vmlinux_data, vmlinux_robss, vmlinux_swapper;
map_kernel_segment(pgd, _text, _etext, PAGE_KERNEL_EXEC, &vmlinux_text);
map_kernel_segment(pgd, __start_rodata, __init_begin, PAGE_KERNEL, &vmlinux_rodata);
map_kernel_segment(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC,
&vmlinux_init);
- map_kernel_segment(pgd, _data, _end, PAGE_KERNEL, &vmlinux_data);
+ map_kernel_segment(pgd, _data, __robss_start, PAGE_KERNEL,
+ &vmlinux_data);
+ map_kernel_segment(pgd, __robss_start, __robss_end, PAGE_KERNEL_RO,
+ &vmlinux_robss);
+ map_kernel_segment(pgd, __robss_end, _end, PAGE_KERNEL,
+ &vmlinux_swapper);
if (!pgd_val(*pgd_offset_raw(pgd, FIXADDR_START))) {
/*
--
2.7.4
^ permalink raw reply related
* [PATCH v7 22/22] iommu/dma: Avoid PCI host bridge windows
From: Robin Murphy @ 2016-09-12 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
With our DMA ops enabled for PCI devices, we should avoid allocating
IOVAs which a host bridge might misinterpret as peer-to-peer DMA and
lead to faults, corruption or other badness. To be safe, punch out holes
for all of the relevant host bridge's windows when initialising a DMA
domain for a PCI device.
CC: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Inki Dae <inki.dae@samsung.com>
Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Squash in the previous drm/exynos fixup
- If need be, this one can probably wait
---
arch/arm64/mm/dma-mapping.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_iommu.h | 2 +-
drivers/iommu/dma-iommu.c | 25 ++++++++++++++++++++++++-
include/linux/dma-iommu.h | 3 ++-
4 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c4284c432ae8..610d8e53011e 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -827,7 +827,7 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
* then the IOMMU core will have already configured a group for this
* device, and allocated the default domain for that group.
*/
- if (!domain || iommu_dma_init_domain(domain, dma_base, size)) {
+ if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
dev_name(dev));
return false;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.h b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
index c8de4913fdbe..87f6b5672e11 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
@@ -66,7 +66,7 @@ static inline int __exynos_iommu_create_mapping(struct exynos_drm_private *priv,
if (ret)
goto free_domain;
- ret = iommu_dma_init_domain(domain, start, size);
+ ret = iommu_dma_init_domain(domain, start, size, NULL);
if (ret)
goto put_cookie;
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 4329d18080cf..c5ab8667e6f2 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -27,6 +27,7 @@
#include <linux/iova.h>
#include <linux/irq.h>
#include <linux/mm.h>
+#include <linux/pci.h>
#include <linux/scatterlist.h>
#include <linux/vmalloc.h>
@@ -103,18 +104,38 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
}
EXPORT_SYMBOL(iommu_put_dma_cookie);
+static void iova_reserve_pci_windows(struct pci_dev *dev,
+ struct iova_domain *iovad)
+{
+ struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
+ struct resource_entry *window;
+ unsigned long lo, hi;
+
+ resource_list_for_each_entry(window, &bridge->windows) {
+ if (resource_type(window->res) != IORESOURCE_MEM &&
+ resource_type(window->res) != IORESOURCE_IO)
+ continue;
+
+ lo = iova_pfn(iovad, window->res->start - window->offset);
+ hi = iova_pfn(iovad, window->res->end - window->offset);
+ reserve_iova(iovad, lo, hi);
+ }
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
* @base: IOVA at which the mappable address space starts
* @size: Size of IOVA space
+ * @dev: Device the domain is being initialised for
*
* @base and @size should be exact multiples of IOMMU page granularity to
* avoid rounding surprises. If necessary, we reserve the page at address 0
* to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but
* any change which could make prior IOVAs invalid will fail.
*/
-int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size)
+int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
+ u64 size, struct device *dev)
{
struct iova_domain *iovad = cookie_iovad(domain);
unsigned long order, base_pfn, end_pfn;
@@ -152,6 +173,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size
iovad->dma_32bit_pfn = end_pfn;
} else {
init_iova_domain(iovad, 1UL << order, base_pfn, end_pfn);
+ if (dev && dev_is_pci(dev))
+ iova_reserve_pci_windows(to_pci_dev(dev), iovad);
}
return 0;
}
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 5ee806e41b5c..32c589062bd9 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -30,7 +30,8 @@ int iommu_get_dma_cookie(struct iommu_domain *domain);
void iommu_put_dma_cookie(struct iommu_domain *domain);
/* Setup call for arch DMA mapping code */
-int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size);
+int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
+ u64 size, struct device *dev);
/* General helpers for DMA-API <-> IOMMU-API interaction */
int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 21/22] iommu/dma: Add support for mapping MSIs
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
When an MSI doorbell is located downstream of an IOMMU, attaching
devices to a DMA ops domain and switching on translation leads to a rude
shock when their attempt to write to the physical address returned by
the irqchip driver faults (or worse, writes into some already-mapped
buffer) and no interrupt is forthcoming.
Address this by adding a hook for relevant irqchip drivers to call from
their compose_msi_msg() callback, to swizzle the physical address with
an appropriatly-mapped IOVA for any device attached to one of our DMA
ops domains.
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 136 ++++++++++++++++++++++++++++++++++-----
drivers/irqchip/irq-gic-v2m.c | 3 +
drivers/irqchip/irq-gic-v3-its.c | 3 +
include/linux/dma-iommu.h | 9 +++
4 files changed, 136 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 00c8a08d56e7..4329d18080cf 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -25,10 +25,28 @@
#include <linux/huge_mm.h>
#include <linux/iommu.h>
#include <linux/iova.h>
+#include <linux/irq.h>
#include <linux/mm.h>
#include <linux/scatterlist.h>
#include <linux/vmalloc.h>
+struct iommu_dma_msi_page {
+ struct list_head list;
+ dma_addr_t iova;
+ phys_addr_t phys;
+};
+
+struct iommu_dma_cookie {
+ struct iova_domain iovad;
+ struct list_head msi_page_list;
+ spinlock_t msi_lock;
+};
+
+static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
+{
+ return &((struct iommu_dma_cookie *)domain->iova_cookie)->iovad;
+}
+
int iommu_dma_init(void)
{
return iova_cache_get();
@@ -43,15 +61,19 @@ int iommu_dma_init(void)
*/
int iommu_get_dma_cookie(struct iommu_domain *domain)
{
- struct iova_domain *iovad;
+ struct iommu_dma_cookie *cookie;
if (domain->iova_cookie)
return -EEXIST;
- iovad = kzalloc(sizeof(*iovad), GFP_KERNEL);
- domain->iova_cookie = iovad;
+ cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
+ if (!cookie)
+ return -ENOMEM;
- return iovad ? 0 : -ENOMEM;
+ spin_lock_init(&cookie->msi_lock);
+ INIT_LIST_HEAD(&cookie->msi_page_list);
+ domain->iova_cookie = cookie;
+ return 0;
}
EXPORT_SYMBOL(iommu_get_dma_cookie);
@@ -63,14 +85,20 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
*/
void iommu_put_dma_cookie(struct iommu_domain *domain)
{
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi, *tmp;
- if (!iovad)
+ if (!cookie)
return;
- if (iovad->granule)
- put_iova_domain(iovad);
- kfree(iovad);
+ if (cookie->iovad.granule)
+ put_iova_domain(&cookie->iovad);
+
+ list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
+ list_del(&msi->list);
+ kfree(msi);
+ }
+ kfree(cookie);
domain->iova_cookie = NULL;
}
EXPORT_SYMBOL(iommu_put_dma_cookie);
@@ -88,7 +116,7 @@ EXPORT_SYMBOL(iommu_put_dma_cookie);
*/
int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size)
{
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
unsigned long order, base_pfn, end_pfn;
if (!iovad)
@@ -155,7 +183,7 @@ int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
dma_addr_t dma_limit)
{
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
unsigned long shift = iova_shift(iovad);
unsigned long length = iova_align(iovad, size) >> shift;
@@ -171,7 +199,7 @@ static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
/* The IOVA allocator knows what we mapped, so just unmap whatever that was */
static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
{
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
unsigned long shift = iova_shift(iovad);
unsigned long pfn = dma_addr >> shift;
struct iova *iova = find_iova(iovad, pfn);
@@ -294,7 +322,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
void (*flush_page)(struct device *, const void *, phys_addr_t))
{
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
struct iova *iova;
struct page **pages;
struct sg_table sgt;
@@ -386,7 +414,7 @@ dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
{
dma_addr_t dma_addr;
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
phys_addr_t phys = page_to_phys(page) + offset;
size_t iova_off = iova_offset(iovad, phys);
size_t len = iova_align(iovad, size + iova_off);
@@ -495,7 +523,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
int nents, int prot)
{
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
- struct iova_domain *iovad = domain->iova_cookie;
+ struct iova_domain *iovad = cookie_iovad(domain);
struct iova *iova;
struct scatterlist *s, *prev = NULL;
dma_addr_t dma_addr;
@@ -587,3 +615,81 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
{
return dma_addr == DMA_ERROR_CODE;
}
+
+static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
+ phys_addr_t msi_addr, struct iommu_domain *domain)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi_page;
+ struct iova_domain *iovad = &cookie->iovad;
+ struct iova *iova;
+ int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+ msi_addr &= ~(phys_addr_t)iova_mask(iovad);
+ list_for_each_entry(msi_page, &cookie->msi_page_list, list)
+ if (msi_page->phys == msi_addr)
+ return msi_page;
+
+ msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
+ if (!msi_page)
+ return NULL;
+
+ iova = __alloc_iova(domain, iovad->granule, dma_get_mask(dev));
+ if (!iova)
+ goto out_free_page;
+
+ msi_page->phys = msi_addr;
+ msi_page->iova = iova_dma_addr(iovad, iova);
+ if (iommu_map(domain, msi_page->iova, msi_addr, iovad->granule, prot))
+ goto out_free_iova;
+
+ INIT_LIST_HEAD(&msi_page->list);
+ list_add(&msi_page->list, &cookie->msi_page_list);
+ return msi_page;
+
+out_free_iova:
+ __free_iova(iovad, iova);
+out_free_page:
+ kfree(msi_page);
+ return NULL;
+}
+
+void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
+{
+ struct device *dev = msi_desc_to_dev(irq_get_msi_desc(irq));
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+ struct iommu_dma_cookie *cookie;
+ struct iommu_dma_msi_page *msi_page;
+ phys_addr_t msi_addr = (u64)msg->address_hi << 32 | msg->address_lo;
+ unsigned long flags;
+
+ if (!domain || !domain->iova_cookie)
+ return;
+
+ cookie = domain->iova_cookie;
+
+ /*
+ * We disable IRQs to rule out a possible inversion against
+ * irq_desc_lock if, say, someone tries to retarget the affinity
+ * of an MSI from within an IPI handler.
+ */
+ spin_lock_irqsave(&cookie->msi_lock, flags);
+ msi_page = iommu_dma_get_msi_page(dev, msi_addr, domain);
+ spin_unlock_irqrestore(&cookie->msi_lock, flags);
+
+ if (WARN_ON(!msi_page)) {
+ /*
+ * We're called from a void callback, so the best we can do is
+ * 'fail' by filling the message with obviously bogus values.
+ * Since we got this far due to an IOMMU being present, it's
+ * not like the existing address would have worked anyway...
+ */
+ msg->address_hi = ~0U;
+ msg->address_lo = ~0U;
+ msg->data = ~0U;
+ } else {
+ msg->address_hi = upper_32_bits(msi_page->iova);
+ msg->address_lo &= iova_mask(&cookie->iovad);
+ msg->address_lo += lower_32_bits(msi_page->iova);
+ }
+}
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 35eb7ac5d21f..863e073c6f7f 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -16,6 +16,7 @@
#define pr_fmt(fmt) "GICv2m: " fmt
#include <linux/acpi.h>
+#include <linux/dma-iommu.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
@@ -108,6 +109,8 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
if (v2m->flags & GICV2M_NEEDS_SPI_OFFSET)
msg->data -= v2m->spi_offset;
+
+ iommu_dma_map_msi_msg(data->irq, msg);
}
static struct irq_chip gicv2m_irq_chip = {
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 36b9c28a5c91..98ff669d5962 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -18,6 +18,7 @@
#include <linux/bitmap.h>
#include <linux/cpu.h>
#include <linux/delay.h>
+#include <linux/dma-iommu.h>
#include <linux/interrupt.h>
#include <linux/log2.h>
#include <linux/mm.h>
@@ -655,6 +656,8 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
msg->address_lo = addr & ((1UL << 32) - 1);
msg->address_hi = addr >> 32;
msg->data = its_get_event_id(d);
+
+ iommu_dma_map_msi_msg(d->irq, msg);
}
static struct irq_chip its_irq_chip = {
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 81c5c8d167ad..5ee806e41b5c 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -21,6 +21,7 @@
#ifdef CONFIG_IOMMU_DMA
#include <linux/iommu.h>
+#include <linux/msi.h>
int iommu_dma_init(void);
@@ -62,9 +63,13 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
int iommu_dma_supported(struct device *dev, u64 mask);
int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
+/* The DMA API isn't _quite_ the whole story, though... */
+void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
+
#else
struct iommu_domain;
+struct msi_msg;
static inline int iommu_dma_init(void)
{
@@ -80,6 +85,10 @@ static inline void iommu_put_dma_cookie(struct iommu_domain *domain)
{
}
+static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
+{
+}
+
#endif /* CONFIG_IOMMU_DMA */
#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 20/22] iommu/arm-smmu: Set domain geometry
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
For non-aperture-based IOMMUs, the domain geometry seems to have become
the de-facto way of indicating the input address space size. That is
quite a useful thing from the users' perspective, so let's do the same.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 2 ++
drivers/iommu/arm-smmu.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 0c45c1e02e04..15c01c3cd540 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1551,6 +1551,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
return -ENOMEM;
domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
+ domain->geometry.aperture_end = (1UL << ias) - 1;
+ domain->geometry.force_aperture = true;
smmu_domain->pgtbl_ops = pgtbl_ops;
ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f143dbc549b9..fa892d25004d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -940,6 +940,8 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
/* Update the domain's page sizes to reflect the page table format */
domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
+ domain->geometry.aperture_end = (1UL << ias) - 1;
+ domain->geometry.force_aperture = true;
/* Initialise the context bank with our page table cfg */
arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 19/22] iommu/arm-smmu: Wire up generic configuration support
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
With everything else now in place, fill in an of_xlate callback and the
appropriate registration to plumb into the generic configuration
machinery, and watch everything just work.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Convert to updated fwspec mechanism
---
drivers/iommu/arm-smmu.c | 168 ++++++++++++++++++++++++++++++-----------------
1 file changed, 108 insertions(+), 60 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c5ee5a51c7ec..f143dbc549b9 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -43,6 +43,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -418,6 +419,8 @@ struct arm_smmu_option_prop {
static atomic_t cavium_smmu_context_count = ATOMIC_INIT(0);
+static bool using_legacy_binding, using_generic_binding;
+
static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
{ 0, NULL},
@@ -817,12 +820,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
if (smmu_domain->smmu)
goto out_unlock;
- /* We're bypassing these SIDs, so don't allocate an actual context */
- if (domain->type == IOMMU_DOMAIN_DMA) {
- smmu_domain->smmu = smmu;
- goto out_unlock;
- }
-
/*
* Mapping the requested stage onto what we support is surprisingly
* complicated, mainly because the spec allows S1+S2 SMMUs without
@@ -981,7 +978,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
void __iomem *cb_base;
int irq;
- if (!smmu || domain->type == IOMMU_DOMAIN_DMA)
+ if (!smmu)
return;
/*
@@ -1015,8 +1012,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
if (!smmu_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&smmu_domain->domain)) {
+ if (type == IOMMU_DOMAIN_DMA && (using_legacy_binding ||
+ iommu_get_dma_cookie(&smmu_domain->domain))) {
kfree(smmu_domain);
return NULL;
}
@@ -1133,19 +1130,22 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
mutex_lock(&smmu->stream_map_mutex);
/* Figure out a viable stream map entry allocation */
for_each_cfg_sme(fwspec, i, idx) {
+ u16 sid = fwspec->ids[i];
+ u16 mask = fwspec->ids[i] >> SMR_MASK_SHIFT;
+
if (idx != INVALID_SMENDX) {
ret = -EEXIST;
goto out_err;
}
- ret = arm_smmu_find_sme(smmu, fwspec->ids[i], 0);
+ ret = arm_smmu_find_sme(smmu, sid, mask);
if (ret < 0)
goto out_err;
idx = ret;
if (smrs && smmu->s2crs[idx].count == 0) {
- smrs[idx].id = fwspec->ids[i];
- smrs[idx].mask = 0; /* We don't currently share SMRs */
+ smrs[idx].id = sid;
+ smrs[idx].mask = mask;
smrs[idx].valid = true;
}
smmu->s2crs[idx].count++;
@@ -1203,15 +1203,6 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
u8 cbndx = smmu_domain->cfg.cbndx;
int i, idx;
- /*
- * FIXME: This won't be needed once we have IOMMU-backed DMA ops
- * for all devices behind the SMMU. Note that we need to take
- * care configuring SMRs for devices both a platform_device and
- * and a PCI device (i.e. a PCI host controller)
- */
- if (smmu_domain->domain.type == IOMMU_DOMAIN_DMA)
- type = S2CR_TYPE_BYPASS;
-
for_each_cfg_sme(fwspec, i, idx) {
if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
continue;
@@ -1373,25 +1364,50 @@ static bool arm_smmu_capable(enum iommu_cap cap)
}
}
+static int arm_smmu_match_node(struct device *dev, void *data)
+{
+ return dev->of_node == data;
+}
+
+static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
+{
+ struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
+ np, arm_smmu_match_node);
+ put_device(dev);
+ return dev ? dev_get_drvdata(dev) : NULL;
+}
+
static int arm_smmu_add_device(struct device *dev)
{
struct arm_smmu_device *smmu;
struct arm_smmu_master_cfg *cfg;
- struct iommu_fwspec *fwspec;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
int i, ret;
- ret = arm_smmu_register_legacy_master(dev, &smmu);
- fwspec = dev->iommu_fwspec;
- if (ret)
- goto out_free;
+ if (using_legacy_binding) {
+ ret = arm_smmu_register_legacy_master(dev, &smmu);
+ fwspec = dev->iommu_fwspec;
+ if (ret)
+ goto out_free;
+ } else if (fwspec) {
+ smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
+ } else {
+ return -ENODEV;
+ }
ret = -EINVAL;
for (i = 0; i < fwspec->num_ids; i++) {
u16 sid = fwspec->ids[i];
+ u16 mask = fwspec->ids[i] >> SMR_MASK_SHIFT;
if (sid & ~smmu->streamid_mask) {
dev_err(dev, "stream ID 0x%x out of range for SMMU (0x%x)\n",
- sid, cfg->smmu->streamid_mask);
+ sid, smmu->streamid_mask);
+ goto out_free;
+ }
+ if (mask & ~smmu->smr_mask_mask) {
+ dev_err(dev, "SMR mask 0x%x out of range for SMMU (0x%x)\n",
+ sid, smmu->smr_mask_mask);
goto out_free;
}
}
@@ -1503,6 +1519,19 @@ out_unlock:
return ret;
}
+static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+{
+ u32 fwid = 0;
+
+ if (args->args_count > 0)
+ fwid |= (u16)args->args[0];
+
+ if (args->args_count > 1)
+ fwid |= (u16)args->args[1] << SMR_MASK_SHIFT;
+
+ return iommu_fwspec_add_ids(dev, &fwid, 1);
+}
+
static struct iommu_ops arm_smmu_ops = {
.capable = arm_smmu_capable,
.domain_alloc = arm_smmu_domain_alloc,
@@ -1517,6 +1546,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.domain_get_attr = arm_smmu_domain_get_attr,
.domain_set_attr = arm_smmu_domain_set_attr,
+ .of_xlate = arm_smmu_of_xlate,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};
@@ -1870,6 +1900,18 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
int num_irqs, i, err;
+ bool legacy_binding;
+
+ legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL);
+ if (legacy_binding && !using_generic_binding) {
+ pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n");
+ using_legacy_binding = true;
+ } else if (!legacy_binding && !using_legacy_binding) {
+ using_generic_binding = true;
+ } else {
+ dev_err(dev, "not probing due to mismatched DT properties\n");
+ return -ENODEV;
+ }
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
@@ -1954,6 +1996,20 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
+
+ /* Oh, for a proper bus abstraction */
+ if (!iommu_present(&platform_bus_type))
+ bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
+#ifdef CONFIG_ARM_AMBA
+ if (!iommu_present(&amba_bustype))
+ bus_set_iommu(&amba_bustype, &arm_smmu_ops);
+#endif
+#ifdef CONFIG_PCI
+ if (!iommu_present(&pci_bus_type)) {
+ pci_request_acs();
+ bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
+ }
+#endif
return 0;
}
@@ -1983,41 +2039,14 @@ static struct platform_driver arm_smmu_driver = {
static int __init arm_smmu_init(void)
{
- struct device_node *np;
- int ret;
+ static bool registered;
+ int ret = 0;
- /*
- * Play nice with systems that don't have an ARM SMMU by checking that
- * an ARM SMMU exists in the system before proceeding with the driver
- * and IOMMU bus operation registration.
- */
- np = of_find_matching_node(NULL, arm_smmu_of_match);
- if (!np)
- return 0;
-
- of_node_put(np);
-
- ret = platform_driver_register(&arm_smmu_driver);
- if (ret)
- return ret;
-
- /* Oh, for a proper bus abstraction */
- if (!iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
-
-#ifdef CONFIG_ARM_AMBA
- if (!iommu_present(&amba_bustype))
- bus_set_iommu(&amba_bustype, &arm_smmu_ops);
-#endif
-
-#ifdef CONFIG_PCI
- if (!iommu_present(&pci_bus_type)) {
- pci_request_acs();
- bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
+ if (!registered) {
+ ret = platform_driver_register(&arm_smmu_driver);
+ registered = !ret;
}
-#endif
-
- return 0;
+ return ret;
}
static void __exit arm_smmu_exit(void)
@@ -2028,6 +2057,25 @@ static void __exit arm_smmu_exit(void)
subsys_initcall(arm_smmu_init);
module_exit(arm_smmu_exit);
+static int __init arm_smmu_of_init(struct device_node *np)
+{
+ int ret = arm_smmu_init();
+
+ if (ret)
+ return ret;
+
+ if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
+ return -ENODEV;
+
+ return 0;
+}
+IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", arm_smmu_of_init);
+IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", arm_smmu_of_init);
+
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
MODULE_LICENSE("GPL v2");
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 18/22] Docs: dt: document ARM SMMU generic binding usage
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Document how the generic "iommus" binding should be used to describe ARM
SMMU stream IDs instead of the old "mmu-masters" binding.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Remove missed reference to old "mmu-masters" example
---
.../devicetree/bindings/iommu/arm,smmu.txt | 63 ++++++++++++++++------
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 19fe6f2c83f6..e862d1485205 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -35,12 +35,16 @@ conditions.
interrupt per context bank. In the case of a single,
combined interrupt, it must be listed multiple times.
-- mmu-masters : A list of phandles to device nodes representing bus
- masters for which the SMMU can provide a translation
- and their corresponding StreamIDs (see example below).
- Each device node linked from this list must have a
- "#stream-id-cells" property, indicating the number of
- StreamIDs associated with it.
+- #iommu-cells : See Documentation/devicetree/bindings/iommu/iommu.txt
+ for details. With a value of 1, each "iommus" entry
+ represents a distinct stream ID emitted by that device
+ into the relevant SMMU.
+
+ SMMUs with stream matching support and complex masters
+ may use a value of 2, where the second cell represents
+ an SMR mask to combine with the ID in the first cell.
+ Care must be taken to ensure the set of matched IDs
+ does not result in conflicts.
** System MMU optional properties:
@@ -56,9 +60,20 @@ conditions.
aliases of secure registers have to be used during
SMMU configuration.
-Example:
+** Deprecated properties:
- smmu {
+- mmu-masters (deprecated in favour of the generic "iommus" binding) :
+ A list of phandles to device nodes representing bus
+ masters for which the SMMU can provide a translation
+ and their corresponding Stream IDs. Each device node
+ linked from this list must have a "#stream-id-cells"
+ property, indicating the number of Stream ID
+ arguments associated with its phandle.
+
+** Examples:
+
+ /* SMMU with stream matching or stream indexing */
+ smmu1: iommu {
compatible = "arm,smmu-v1";
reg = <0xba5e0000 0x10000>;
#global-interrupts = <2>;
@@ -68,11 +83,29 @@ Example:
<0 35 4>,
<0 36 4>,
<0 37 4>;
-
- /*
- * Two DMA controllers, the first with two StreamIDs (0xd01d
- * and 0xd01e) and the second with only one (0xd11c).
- */
- mmu-masters = <&dma0 0xd01d 0xd01e>,
- <&dma1 0xd11c>;
+ #iommu-cells = <1>;
+ };
+
+ /* device with two stream IDs, 0 and 7 */
+ master1 {
+ iommus = <&smmu1 0>,
+ <&smmu1 7>;
+ };
+
+
+ /* SMMU with stream matching */
+ smmu2: iommu {
+ ...
+ #iommu-cells = <2>;
+ };
+
+ /* device with stream IDs 0 and 7 */
+ master2 {
+ iommus = <&smmu2 0 0>,
+ <&smmu2 7 0>;
+ };
+
+ /* device with stream IDs 1, 17, 33 and 49 */
+ master3 {
+ iommus = <&smmu2 1 0x30>;
};
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 17/22] iommu/arm-smmu: Convert to iommu_fwspec
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
In the final step of preparation for full generic configuration support,
swap our fixed-size master_cfg for the generic iommu_fwspec. For the
legacy DT bindings, the driver simply gets to act as its own 'firmware'.
Farewell, arbitrary MAX_MASTER_STREAMIDS!
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Convert to updated fwspec mechanism
---
drivers/iommu/arm-smmu.c | 140 ++++++++++++++++++++++++++---------------------
1 file changed, 78 insertions(+), 62 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 0e7dbf59335d..c5ee5a51c7ec 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -42,6 +42,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
+#include <linux/of_iommu.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -51,9 +52,6 @@
#include "io-pgtable.h"
-/* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS 128
-
/* Maximum number of context banks per SMMU */
#define ARM_SMMU_MAX_CBS 128
@@ -321,13 +319,13 @@ struct arm_smmu_smr {
struct arm_smmu_master_cfg {
struct arm_smmu_device *smmu;
- int num_streamids;
- u16 streamids[MAX_MASTER_STREAMIDS];
- s16 smendx[MAX_MASTER_STREAMIDS];
+ s16 smendx[];
};
#define INVALID_SMENDX -1
-#define for_each_cfg_sme(cfg, i, idx) \
- for (i = 0; idx = cfg->smendx[i], i < cfg->num_streamids; ++i)
+#define __fwspec_cfg(fw) ((struct arm_smmu_master_cfg *)fw->iommu_priv)
+#define fwspec_smmu(fw) (__fwspec_cfg(fw)->smmu)
+#define for_each_cfg_sme(fw, i, idx) \
+ for (i = 0; idx = __fwspec_cfg(fw)->smendx[i], i < fw->num_ids; ++i)
struct arm_smmu_device {
struct device *dev;
@@ -480,14 +478,16 @@ static int __find_legacy_master_phandle(struct device *dev, void *data)
}
static struct platform_driver arm_smmu_driver;
+static struct iommu_ops arm_smmu_ops;
-static int arm_smmu_register_legacy_master(struct device *dev)
+static int arm_smmu_register_legacy_master(struct device *dev,
+ struct arm_smmu_device **smmu)
{
- struct arm_smmu_device *smmu;
- struct arm_smmu_master_cfg *cfg;
+ struct device *smmu_dev;
struct device_node *np;
struct of_phandle_iterator it;
void *data = ⁢
+ u32 *sids;
__be32 pci_sid;
int err;
@@ -500,20 +500,13 @@ static int arm_smmu_register_legacy_master(struct device *dev)
it.node = np;
err = driver_for_each_device(&arm_smmu_driver.driver, NULL, &data,
__find_legacy_master_phandle);
+ smmu_dev = data;
of_node_put(np);
if (err == 0)
return -ENODEV;
if (err < 0)
return err;
- smmu = dev_get_drvdata(data);
-
- if (it.cur_count > MAX_MASTER_STREAMIDS) {
- dev_err(smmu->dev,
- "reached maximum number (%d) of stream IDs for master device %s\n",
- MAX_MASTER_STREAMIDS, dev_name(dev));
- return -ENOSPC;
- }
if (dev_is_pci(dev)) {
/* "mmu-masters" assumes Stream ID == Requester ID */
pci_for_each_dma_alias(to_pci_dev(dev), __arm_smmu_get_pci_sid,
@@ -522,17 +515,20 @@ static int arm_smmu_register_legacy_master(struct device *dev)
it.cur_count = 1;
}
- cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
- if (!cfg)
+ err = iommu_fwspec_init(dev, &smmu_dev->of_node->fwnode,
+ &arm_smmu_ops);
+ if (err)
+ return err;
+
+ sids = kcalloc(it.cur_count, sizeof(*sids), GFP_KERNEL);
+ if (!sids)
return -ENOMEM;
- cfg->smmu = smmu;
- dev->archdata.iommu = cfg;
-
- while (it.cur_count--)
- cfg->streamids[cfg->num_streamids++] = be32_to_cpup(it.cur++);
-
- return 0;
+ *smmu = dev_get_drvdata(smmu_dev);
+ of_phandle_iterator_args(&it, sids, it.cur_count);
+ err = iommu_fwspec_add_ids(dev, sids, it.cur_count);
+ kfree(sids);
+ return err;
}
static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
@@ -1127,7 +1123,8 @@ static bool arm_smmu_free_sme(struct arm_smmu_device *smmu, int idx)
static int arm_smmu_master_alloc_smes(struct device *dev)
{
- struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct arm_smmu_master_cfg *cfg = fwspec->iommu_priv;
struct arm_smmu_device *smmu = cfg->smmu;
struct arm_smmu_smr *smrs = smmu->smrs;
struct iommu_group *group;
@@ -1135,19 +1132,19 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
mutex_lock(&smmu->stream_map_mutex);
/* Figure out a viable stream map entry allocation */
- for_each_cfg_sme(cfg, i, idx) {
+ for_each_cfg_sme(fwspec, i, idx) {
if (idx != INVALID_SMENDX) {
ret = -EEXIST;
goto out_err;
}
- ret = arm_smmu_find_sme(smmu, cfg->streamids[i], 0);
+ ret = arm_smmu_find_sme(smmu, fwspec->ids[i], 0);
if (ret < 0)
goto out_err;
idx = ret;
if (smrs && smmu->s2crs[idx].count == 0) {
- smrs[idx].id = cfg->streamids[i];
+ smrs[idx].id = fwspec->ids[i];
smrs[idx].mask = 0; /* We don't currently share SMRs */
smrs[idx].valid = true;
}
@@ -1165,7 +1162,7 @@ static int arm_smmu_master_alloc_smes(struct device *dev)
iommu_group_put(group);
/* It worked! Now, poke the actual hardware */
- for_each_cfg_sme(cfg, i, idx) {
+ for_each_cfg_sme(fwspec, i, idx) {
arm_smmu_write_sme(smmu, idx);
smmu->s2crs[idx].group = group;
}
@@ -1182,13 +1179,14 @@ out_err:
return ret;
}
-static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
+static void arm_smmu_master_free_smes(struct iommu_fwspec *fwspec)
{
- struct arm_smmu_device *smmu = cfg->smmu;
+ struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
+ struct arm_smmu_master_cfg *cfg = fwspec->iommu_priv;
int i, idx;
mutex_lock(&smmu->stream_map_mutex);
- for_each_cfg_sme(cfg, i, idx) {
+ for_each_cfg_sme(fwspec, i, idx) {
if (arm_smmu_free_sme(smmu, idx))
arm_smmu_write_sme(smmu, idx);
cfg->smendx[i] = INVALID_SMENDX;
@@ -1197,7 +1195,7 @@ static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
}
static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
- struct arm_smmu_master_cfg *cfg)
+ struct iommu_fwspec *fwspec)
{
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s2cr *s2cr = smmu->s2crs;
@@ -1214,7 +1212,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
if (smmu_domain->domain.type == IOMMU_DOMAIN_DMA)
type = S2CR_TYPE_BYPASS;
- for_each_cfg_sme(cfg, i, idx) {
+ for_each_cfg_sme(fwspec, i, idx) {
if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
continue;
@@ -1229,16 +1227,18 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct arm_smmu_device *smmu;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
- struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
- if (!cfg) {
+ if (!fwspec || fwspec->ops != &arm_smmu_ops) {
dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
return -ENXIO;
}
+ smmu = fwspec_smmu(fwspec);
/* Ensure that the domain is finalised */
- ret = arm_smmu_init_domain_context(domain, cfg->smmu);
+ ret = arm_smmu_init_domain_context(domain, smmu);
if (ret < 0)
return ret;
@@ -1246,15 +1246,15 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
* Sanity check the domain. We don't support domains across
* different SMMUs.
*/
- if (smmu_domain->smmu != cfg->smmu) {
+ if (smmu_domain->smmu != smmu) {
dev_err(dev,
"cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n",
- dev_name(smmu_domain->smmu->dev), dev_name(cfg->smmu->dev));
+ dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
return -EINVAL;
}
/* Looks ok, so add the device to the domain */
- return arm_smmu_domain_add_master(smmu_domain, cfg);
+ return arm_smmu_domain_add_master(smmu_domain, fwspec);
}
static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1375,57 +1375,72 @@ static bool arm_smmu_capable(enum iommu_cap cap)
static int arm_smmu_add_device(struct device *dev)
{
+ struct arm_smmu_device *smmu;
struct arm_smmu_master_cfg *cfg;
+ struct iommu_fwspec *fwspec;
int i, ret;
- ret = arm_smmu_register_legacy_master(dev);
- cfg = dev->archdata.iommu;
+ ret = arm_smmu_register_legacy_master(dev, &smmu);
+ fwspec = dev->iommu_fwspec;
if (ret)
goto out_free;
ret = -EINVAL;
- for (i = 0; i < cfg->num_streamids; i++) {
- u16 sid = cfg->streamids[i];
+ for (i = 0; i < fwspec->num_ids; i++) {
+ u16 sid = fwspec->ids[i];
- if (sid & ~cfg->smmu->streamid_mask) {
+ if (sid & ~smmu->streamid_mask) {
dev_err(dev, "stream ID 0x%x out of range for SMMU (0x%x)\n",
sid, cfg->smmu->streamid_mask);
goto out_free;
}
- cfg->smendx[i] = INVALID_SMENDX;
}
+ ret = -ENOMEM;
+ cfg = kzalloc(offsetof(struct arm_smmu_master_cfg, smendx[i]),
+ GFP_KERNEL);
+ if (!cfg)
+ goto out_free;
+
+ cfg->smmu = smmu;
+ fwspec->iommu_priv = cfg;
+ while (i--)
+ cfg->smendx[i] = INVALID_SMENDX;
+
ret = arm_smmu_master_alloc_smes(dev);
- if (!ret)
- return ret;
+ if (ret)
+ goto out_free;
+
+ return 0;
out_free:
- kfree(cfg);
- dev->archdata.iommu = NULL;
+ if (fwspec)
+ kfree(fwspec->iommu_priv);
+ iommu_fwspec_free(dev);
return ret;
}
static void arm_smmu_remove_device(struct device *dev)
{
- struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
- if (!cfg)
+ if (!fwspec || fwspec->ops != &arm_smmu_ops)
return;
- arm_smmu_master_free_smes(cfg);
+ arm_smmu_master_free_smes(fwspec);
iommu_group_remove_device(dev);
- kfree(cfg);
- dev->archdata.iommu = NULL;
+ kfree(fwspec->iommu_priv);
+ iommu_fwspec_free(dev);
}
static struct iommu_group *arm_smmu_device_group(struct device *dev)
{
- struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
- struct arm_smmu_device *smmu = cfg->smmu;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
struct iommu_group *group = NULL;
int i, idx;
- for_each_cfg_sme(cfg, i, idx) {
+ for_each_cfg_sme(fwspec, i, idx) {
if (group && smmu->s2crs[idx].group &&
group != smmu->s2crs[idx].group)
return ERR_PTR(-EINVAL);
@@ -1936,6 +1951,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
}
+ of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
return 0;
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 16/22] iommu/arm-smmu: Intelligent SMR allocation
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Stream Match Registers are one of the more awkward parts of the SMMUv2
architecture; there are typically never enough to assign one to each
stream ID in the system, and configuring them such that a single ID
matches multiple entries is catastrophically bad - at best, every
transaction raises a global fault; at worst, they go *somewhere*.
To address the former issue, we can mask ID bits such that a single
register may be used to match multiple IDs belonging to the same device
or group, but doing so also heightens the risk of the latter problem
(which can be nasty to debug).
Tackle both problems at once by replacing the simple bitmap allocator
with something much cleverer. Now that we have convenient in-memory
representations of the stream mapping table, it becomes straightforward
to properly validate new SMR entries against the current state, opening
the door to arbitrary masking and SMR sharing.
Another feature which falls out of this is that with IDs shared by
separate devices being automatically accounted for, simply associating a
group pointer with the S2CR offers appropriate group allocation almost
for free, so hook that up in the process.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Simplify SMR matching logic, and document it properly
- Explicify INVALID_SMENDX check
---
drivers/iommu/arm-smmu.c | 201 ++++++++++++++++++++++++++++-------------------
1 file changed, 121 insertions(+), 80 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7a0edef34a22..0e7dbf59335d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -302,6 +302,8 @@ enum arm_smmu_implementation {
};
struct arm_smmu_s2cr {
+ struct iommu_group *group;
+ int count;
enum arm_smmu_s2cr_type type;
enum arm_smmu_s2cr_privcfg privcfg;
u8 cbndx;
@@ -363,6 +365,7 @@ struct arm_smmu_device {
u16 smr_mask_mask;
struct arm_smmu_smr *smrs;
struct arm_smmu_s2cr *s2crs;
+ struct mutex stream_map_mutex;
unsigned long va_size;
unsigned long ipa_size;
@@ -1041,23 +1044,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
kfree(smmu_domain);
}
-static int arm_smmu_alloc_smr(struct arm_smmu_device *smmu)
-{
- int i;
-
- for (i = 0; i < smmu->num_mapping_groups; i++)
- if (!cmpxchg(&smmu->smrs[i].valid, false, true))
- return i;
-
- return INVALID_SMENDX;
-}
-
-static void arm_smmu_free_smr(struct arm_smmu_device *smmu, int idx)
-{
- writel_relaxed(~SMR_VALID, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
- WRITE_ONCE(smmu->smrs[idx].valid, false);
-}
-
static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
{
struct arm_smmu_smr *smr = smmu->smrs + idx;
@@ -1085,49 +1071,115 @@ static void arm_smmu_write_sme(struct arm_smmu_device *smmu, int idx)
arm_smmu_write_smr(smmu, idx);
}
-static int arm_smmu_master_alloc_smes(struct arm_smmu_device *smmu,
- struct arm_smmu_master_cfg *cfg)
+static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask)
{
struct arm_smmu_smr *smrs = smmu->smrs;
- int i, idx;
+ int i, free_idx = -ENOSPC;
- /* Allocate the SMRs on the SMMU */
- for_each_cfg_sme(cfg, i, idx) {
- if (idx != INVALID_SMENDX)
- return -EEXIST;
+ /* Stream indexing is blissfully easy */
+ if (!smrs)
+ return id;
- /* ...except on stream indexing hardware, of course */
- if (!smrs) {
- cfg->smendx[i] = cfg->streamids[i];
+ /* Validating SMRs is... less so */
+ for (i = 0; i < smmu->num_mapping_groups; ++i) {
+ if (!smrs[i].valid) {
+ /*
+ * Note the first free entry we come across, which
+ * we'll claim in the end if nothing else matches.
+ */
+ if (free_idx < 0)
+ free_idx = i;
continue;
}
-
- idx = arm_smmu_alloc_smr(smmu);
- if (idx < 0) {
- dev_err(smmu->dev, "failed to allocate free SMR\n");
- goto err_free_smrs;
- }
- cfg->smendx[i] = idx;
-
- smrs[idx].id = cfg->streamids[i];
- smrs[idx].mask = 0; /* We don't currently share SMRs */
+ /*
+ * If the new entry is _entirely_ matched by an existing entry,
+ * then reuse that, with the guarantee that there also cannot
+ * be any subsequent conflicting entries. In normal use we'd
+ * expect simply identical entries for this case, but there's
+ * no harm in accommodating the generalisation.
+ */
+ if ((mask & smrs[i].mask) == mask &&
+ !((id ^ smrs[i].id) & ~smrs[i].mask))
+ return i;
+ /*
+ * If the new entry has any other overlap with an existing one,
+ * though, then there always exists at least one stream ID
+ * which would cause a conflict, and we can't allow that risk.
+ */
+ if (!((id ^ smrs[i].id) & ~(smrs[i].mask | mask)))
+ return -EINVAL;
}
- if (!smrs)
- return 0;
+ return free_idx;
+}
+
+static bool arm_smmu_free_sme(struct arm_smmu_device *smmu, int idx)
+{
+ if (--smmu->s2crs[idx].count)
+ return false;
+
+ smmu->s2crs[idx] = s2cr_init_val;
+ if (smmu->smrs)
+ smmu->smrs[idx].valid = false;
+
+ return true;
+}
+
+static int arm_smmu_master_alloc_smes(struct device *dev)
+{
+ struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
+ struct arm_smmu_device *smmu = cfg->smmu;
+ struct arm_smmu_smr *smrs = smmu->smrs;
+ struct iommu_group *group;
+ int i, idx, ret;
+
+ mutex_lock(&smmu->stream_map_mutex);
+ /* Figure out a viable stream map entry allocation */
+ for_each_cfg_sme(cfg, i, idx) {
+ if (idx != INVALID_SMENDX) {
+ ret = -EEXIST;
+ goto out_err;
+ }
+
+ ret = arm_smmu_find_sme(smmu, cfg->streamids[i], 0);
+ if (ret < 0)
+ goto out_err;
+
+ idx = ret;
+ if (smrs && smmu->s2crs[idx].count == 0) {
+ smrs[idx].id = cfg->streamids[i];
+ smrs[idx].mask = 0; /* We don't currently share SMRs */
+ smrs[idx].valid = true;
+ }
+ smmu->s2crs[idx].count++;
+ cfg->smendx[i] = (s16)idx;
+ }
+
+ group = iommu_group_get_for_dev(dev);
+ if (!group)
+ group = ERR_PTR(-ENOMEM);
+ if (IS_ERR(group)) {
+ ret = PTR_ERR(group);
+ goto out_err;
+ }
+ iommu_group_put(group);
/* It worked! Now, poke the actual hardware */
- for_each_cfg_sme(cfg, i, idx)
- arm_smmu_write_smr(smmu, idx);
+ for_each_cfg_sme(cfg, i, idx) {
+ arm_smmu_write_sme(smmu, idx);
+ smmu->s2crs[idx].group = group;
+ }
+ mutex_unlock(&smmu->stream_map_mutex);
return 0;
-err_free_smrs:
+out_err:
while (i--) {
- arm_smmu_free_smr(smmu, cfg->smendx[i]);
+ arm_smmu_free_sme(smmu, cfg->smendx[i]);
cfg->smendx[i] = INVALID_SMENDX;
}
- return -ENOSPC;
+ mutex_unlock(&smmu->stream_map_mutex);
+ return ret;
}
static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
@@ -1135,43 +1187,23 @@ static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
struct arm_smmu_device *smmu = cfg->smmu;
int i, idx;
- /*
- * We *must* clear the S2CR first, because freeing the SMR means
- * that it can be re-allocated immediately.
- */
+ mutex_lock(&smmu->stream_map_mutex);
for_each_cfg_sme(cfg, i, idx) {
- /* An IOMMU group is torn down by the first device to be removed */
- if (idx == INVALID_SMENDX)
- return;
-
- smmu->s2crs[idx] = s2cr_init_val;
- arm_smmu_write_s2cr(smmu, idx);
- }
- /* Sync S2CR updates before touching anything else */
- __iowmb();
-
- /* Invalidate the SMRs before freeing back to the allocator */
- for_each_cfg_sme(cfg, i, idx) {
- if (smmu->smrs)
- arm_smmu_free_smr(smmu, idx);
-
+ if (arm_smmu_free_sme(smmu, idx))
+ arm_smmu_write_sme(smmu, idx);
cfg->smendx[i] = INVALID_SMENDX;
}
+ mutex_unlock(&smmu->stream_map_mutex);
}
static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_master_cfg *cfg)
{
- int i, idx, ret = 0;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s2cr *s2cr = smmu->s2crs;
enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
u8 cbndx = smmu_domain->cfg.cbndx;
-
- if (cfg->smendx[0] == INVALID_SMENDX)
- ret = arm_smmu_master_alloc_smes(smmu, cfg);
- if (ret)
- return ret;
+ int i, idx;
/*
* FIXME: This won't be needed once we have IOMMU-backed DMA ops
@@ -1183,9 +1215,8 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
type = S2CR_TYPE_BYPASS;
for_each_cfg_sme(cfg, i, idx) {
- /* Devices in an IOMMU group may already be configured */
if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
- break;
+ continue;
s2cr[idx].type = type;
s2cr[idx].privcfg = S2CR_PRIVCFG_UNPRIV;
@@ -1345,7 +1376,6 @@ static bool arm_smmu_capable(enum iommu_cap cap)
static int arm_smmu_add_device(struct device *dev)
{
struct arm_smmu_master_cfg *cfg;
- struct iommu_group *group;
int i, ret;
ret = arm_smmu_register_legacy_master(dev);
@@ -1365,13 +1395,9 @@ static int arm_smmu_add_device(struct device *dev)
cfg->smendx[i] = INVALID_SMENDX;
}
- group = iommu_group_get_for_dev(dev);
- if (IS_ERR(group)) {
- ret = PTR_ERR(group);
- goto out_free;
- }
- iommu_group_put(group);
- return 0;
+ ret = arm_smmu_master_alloc_smes(dev);
+ if (!ret)
+ return ret;
out_free:
kfree(cfg);
@@ -1394,7 +1420,21 @@ static void arm_smmu_remove_device(struct device *dev)
static struct iommu_group *arm_smmu_device_group(struct device *dev)
{
- struct iommu_group *group;
+ struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
+ struct arm_smmu_device *smmu = cfg->smmu;
+ struct iommu_group *group = NULL;
+ int i, idx;
+
+ for_each_cfg_sme(cfg, i, idx) {
+ if (group && smmu->s2crs[idx].group &&
+ group != smmu->s2crs[idx].group)
+ return ERR_PTR(-EINVAL);
+
+ group = smmu->s2crs[idx].group;
+ }
+
+ if (group)
+ return group;
if (dev_is_pci(dev))
group = pci_device_group(dev);
@@ -1677,6 +1717,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
smmu->s2crs[i] = s2cr_init_val;
smmu->num_mapping_groups = size;
+ mutex_init(&smmu->stream_map_mutex);
if (smmu->version < ARM_SMMU_V2 || !(id & ID0_PTFS_NO_AARCH32)) {
smmu->features |= ARM_SMMU_FEAT_FMT_AARCH32_L;
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 15/22] iommu/arm-smmu: Add a stream map entry iterator
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
We iterate over the SMEs associated with a master config quite a lot in
various places, and are about to do so even more. Let's wrap the idiom
in a handy iterator macro before the repetition gets out of hand.
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 3962add0c33b..7a0edef34a22 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -324,6 +324,8 @@ struct arm_smmu_master_cfg {
s16 smendx[MAX_MASTER_STREAMIDS];
};
#define INVALID_SMENDX -1
+#define for_each_cfg_sme(cfg, i, idx) \
+ for (i = 0; idx = cfg->smendx[i], i < cfg->num_streamids; ++i)
struct arm_smmu_device {
struct device *dev;
@@ -1090,8 +1092,8 @@ static int arm_smmu_master_alloc_smes(struct arm_smmu_device *smmu,
int i, idx;
/* Allocate the SMRs on the SMMU */
- for (i = 0; i < cfg->num_streamids; ++i) {
- if (cfg->smendx[i] != INVALID_SMENDX)
+ for_each_cfg_sme(cfg, i, idx) {
+ if (idx != INVALID_SMENDX)
return -EEXIST;
/* ...except on stream indexing hardware, of course */
@@ -1115,8 +1117,8 @@ static int arm_smmu_master_alloc_smes(struct arm_smmu_device *smmu,
return 0;
/* It worked! Now, poke the actual hardware */
- for (i = 0; i < cfg->num_streamids; ++i)
- arm_smmu_write_smr(smmu, cfg->smendx[i]);
+ for_each_cfg_sme(cfg, i, idx)
+ arm_smmu_write_smr(smmu, idx);
return 0;
@@ -1131,15 +1133,13 @@ err_free_smrs:
static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
{
struct arm_smmu_device *smmu = cfg->smmu;
- int i;
+ int i, idx;
/*
* We *must* clear the S2CR first, because freeing the SMR means
* that it can be re-allocated immediately.
*/
- for (i = 0; i < cfg->num_streamids; ++i) {
- int idx = cfg->smendx[i];
-
+ for_each_cfg_sme(cfg, i, idx) {
/* An IOMMU group is torn down by the first device to be removed */
if (idx == INVALID_SMENDX)
return;
@@ -1151,9 +1151,9 @@ static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
__iowmb();
/* Invalidate the SMRs before freeing back to the allocator */
- for (i = 0; i < cfg->num_streamids; ++i) {
+ for_each_cfg_sme(cfg, i, idx) {
if (smmu->smrs)
- arm_smmu_free_smr(smmu, cfg->smendx[i]);
+ arm_smmu_free_smr(smmu, idx);
cfg->smendx[i] = INVALID_SMENDX;
}
@@ -1162,7 +1162,7 @@ static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_master_cfg *cfg)
{
- int i, ret = 0;
+ int i, idx, ret = 0;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s2cr *s2cr = smmu->s2crs;
enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
@@ -1182,9 +1182,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
if (smmu_domain->domain.type == IOMMU_DOMAIN_DMA)
type = S2CR_TYPE_BYPASS;
- for (i = 0; i < cfg->num_streamids; ++i) {
- int idx = cfg->smendx[i];
-
+ for_each_cfg_sme(cfg, i, idx) {
/* Devices in an IOMMU group may already be configured */
if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
break;
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 14/22] iommu/arm-smmu: Streamline SMMU data lookups
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Simplify things somewhat by stashing our arm_smmu_device instance in
drvdata, so that it's readily available to our driver model callbacks.
Then we can excise the private list entirely, since the driver core
already has a perfectly good list of SMMU devices we can use in the one
instance we actually need to. Finally, make a further modest code saving
with the relatively new of_device_get_match_data() helper.
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu.c | 44 +++++++++++---------------------------------
1 file changed, 11 insertions(+), 33 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 0a628f2c9297..3962add0c33b 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -41,6 +41,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -370,8 +371,6 @@ struct arm_smmu_device {
u32 num_context_irqs;
unsigned int *irqs;
- struct list_head list;
-
u32 cavium_id_base; /* Specific to Cavium */
};
@@ -409,9 +408,6 @@ struct arm_smmu_domain {
struct iommu_domain domain;
};
-static DEFINE_SPINLOCK(arm_smmu_devices_lock);
-static LIST_HEAD(arm_smmu_devices);
-
struct arm_smmu_option_prop {
u32 opt;
const char *prop;
@@ -478,6 +474,8 @@ static int __find_legacy_master_phandle(struct device *dev, void *data)
return err;
}
+static struct platform_driver arm_smmu_driver;
+
static int arm_smmu_register_legacy_master(struct device *dev)
{
struct arm_smmu_device *smmu;
@@ -495,19 +493,16 @@ static int arm_smmu_register_legacy_master(struct device *dev)
}
it.node = np;
- spin_lock(&arm_smmu_devices_lock);
- list_for_each_entry(smmu, &arm_smmu_devices, list) {
- err = __find_legacy_master_phandle(smmu->dev, &data);
- if (err)
- break;
- }
- spin_unlock(&arm_smmu_devices_lock);
+ err = driver_for_each_device(&arm_smmu_driver.driver, NULL, &data,
+ __find_legacy_master_phandle);
of_node_put(np);
if (err == 0)
return -ENODEV;
if (err < 0)
return err;
+ smmu = dev_get_drvdata(data);
+
if (it.cur_count > MAX_MASTER_STREAMIDS) {
dev_err(smmu->dev,
"reached maximum number (%d) of stream IDs for master device %s\n",
@@ -1816,7 +1811,6 @@ MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
static int arm_smmu_device_dt_probe(struct platform_device *pdev)
{
- const struct of_device_id *of_id;
const struct arm_smmu_match_data *data;
struct resource *res;
struct arm_smmu_device *smmu;
@@ -1830,8 +1824,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
smmu->dev = dev;
- of_id = of_match_node(arm_smmu_of_match, dev->of_node);
- data = of_id->data;
+ data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
@@ -1904,35 +1897,20 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
}
- INIT_LIST_HEAD(&smmu->list);
- spin_lock(&arm_smmu_devices_lock);
- list_add(&smmu->list, &arm_smmu_devices);
- spin_unlock(&arm_smmu_devices_lock);
-
+ platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
return 0;
}
static int arm_smmu_device_remove(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
- struct arm_smmu_device *curr, *smmu = NULL;
-
- spin_lock(&arm_smmu_devices_lock);
- list_for_each_entry(curr, &arm_smmu_devices, list) {
- if (curr->dev == dev) {
- smmu = curr;
- list_del(&smmu->list);
- break;
- }
- }
- spin_unlock(&arm_smmu_devices_lock);
+ struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
if (!smmu)
return -ENODEV;
if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
- dev_err(dev, "removing device with active domains!\n");
+ dev_err(&pdev->dev, "removing device with active domains!\n");
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 13/22] iommu/arm-smmu: Refactor mmu-masters handling
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
To be able to support the generic bindings and handle of_xlate() calls,
we need to be able to associate SMMUs and stream IDs directly with
devices *before* allocating IOMMU groups. Furthermore, to support real
default domains with multi-device groups we also have to handle domain
attach on a per-device basis, as the "whole group at a time" assumption
fails to properly handle subsequent devices added to a group after the
first has already triggered default domain creation and attachment.
To that end, use the now-vacant dev->archdata.iommu field for easy
config and SMMU instance lookup, and unify config management by chopping
down the platform-device-specific tree and probing the "mmu-masters"
property on-demand instead. This may add a bit of one-off overhead to
initially adding a new device, but we're about to deprecate that binding
in favour of the inherently-more-efficient generic ones anyway.
For the sake of simplicity, this patch does temporarily regress the case
of aliasing PCI devices by losing the duplicate stream ID detection that
the previous per-group config had. Stay tuned, because we'll be back to
fix that in a better and more general way momentarily...
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu.c | 382 +++++++++++++----------------------------------
1 file changed, 107 insertions(+), 275 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69b6cab65421..0a628f2c9297 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -317,18 +317,13 @@ struct arm_smmu_smr {
};
struct arm_smmu_master_cfg {
+ struct arm_smmu_device *smmu;
int num_streamids;
u16 streamids[MAX_MASTER_STREAMIDS];
s16 smendx[MAX_MASTER_STREAMIDS];
};
#define INVALID_SMENDX -1
-struct arm_smmu_master {
- struct device_node *of_node;
- struct rb_node node;
- struct arm_smmu_master_cfg cfg;
-};
-
struct arm_smmu_device {
struct device *dev;
@@ -376,7 +371,6 @@ struct arm_smmu_device {
unsigned int *irqs;
struct list_head list;
- struct rb_root masters;
u32 cavium_id_base; /* Specific to Cavium */
};
@@ -415,12 +409,6 @@ struct arm_smmu_domain {
struct iommu_domain domain;
};
-struct arm_smmu_phandle_args {
- struct device_node *np;
- int args_count;
- uint32_t args[MAX_MASTER_STREAMIDS];
-};
-
static DEFINE_SPINLOCK(arm_smmu_devices_lock);
static LIST_HEAD(arm_smmu_devices);
@@ -462,132 +450,89 @@ static struct device_node *dev_get_dev_node(struct device *dev)
while (!pci_is_root_bus(bus))
bus = bus->parent;
- return bus->bridge->parent->of_node;
+ return of_node_get(bus->bridge->parent->of_node);
}
- return dev->of_node;
+ return of_node_get(dev->of_node);
}
-static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
- struct device_node *dev_node)
+static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data)
{
- struct rb_node *node = smmu->masters.rb_node;
-
- while (node) {
- struct arm_smmu_master *master;
-
- master = container_of(node, struct arm_smmu_master, node);
-
- if (dev_node < master->of_node)
- node = node->rb_left;
- else if (dev_node > master->of_node)
- node = node->rb_right;
- else
- return master;
- }
-
- return NULL;
+ *((__be32 *)data) = cpu_to_be32(alias);
+ return 0; /* Continue walking */
}
-static struct arm_smmu_master_cfg *
-find_smmu_master_cfg(struct device *dev)
+static int __find_legacy_master_phandle(struct device *dev, void *data)
{
- struct arm_smmu_master_cfg *cfg = NULL;
- struct iommu_group *group = iommu_group_get(dev);
+ struct of_phandle_iterator *it = *(void **)data;
+ struct device_node *np = it->node;
+ int err;
- if (group) {
- cfg = iommu_group_get_iommudata(group);
- iommu_group_put(group);
- }
-
- return cfg;
-}
-
-static int insert_smmu_master(struct arm_smmu_device *smmu,
- struct arm_smmu_master *master)
-{
- struct rb_node **new, *parent;
-
- new = &smmu->masters.rb_node;
- parent = NULL;
- while (*new) {
- struct arm_smmu_master *this
- = container_of(*new, struct arm_smmu_master, node);
-
- parent = *new;
- if (master->of_node < this->of_node)
- new = &((*new)->rb_left);
- else if (master->of_node > this->of_node)
- new = &((*new)->rb_right);
- else
- return -EEXIST;
- }
-
- rb_link_node(&master->node, parent, new);
- rb_insert_color(&master->node, &smmu->masters);
- return 0;
-}
-
-static int register_smmu_master(struct arm_smmu_device *smmu,
- struct device *dev,
- struct arm_smmu_phandle_args *masterspec)
-{
- int i;
- struct arm_smmu_master *master;
-
- master = find_smmu_master(smmu, masterspec->np);
- if (master) {
- dev_err(dev,
- "rejecting multiple registrations for master device %s\n",
- masterspec->np->name);
- return -EBUSY;
- }
-
- if (masterspec->args_count > MAX_MASTER_STREAMIDS) {
- dev_err(dev,
- "reached maximum number (%d) of stream IDs for master device %s\n",
- MAX_MASTER_STREAMIDS, masterspec->np->name);
- return -ENOSPC;
- }
-
- master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
- if (!master)
- return -ENOMEM;
-
- master->of_node = masterspec->np;
- master->cfg.num_streamids = masterspec->args_count;
-
- for (i = 0; i < master->cfg.num_streamids; ++i) {
- u16 streamid = masterspec->args[i];
-
- if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
- (streamid >= smmu->num_mapping_groups)) {
- dev_err(dev,
- "stream ID for master device %s greater than maximum allowed (%d)\n",
- masterspec->np->name, smmu->num_mapping_groups);
- return -ERANGE;
+ of_for_each_phandle(it, err, dev->of_node, "mmu-masters",
+ "#stream-id-cells", 0)
+ if (it->node == np) {
+ *(void **)data = dev;
+ return 1;
}
- master->cfg.streamids[i] = streamid;
- master->cfg.smendx[i] = INVALID_SMENDX;
- }
- return insert_smmu_master(smmu, master);
+ it->node = np;
+ return err;
}
-static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
+static int arm_smmu_register_legacy_master(struct device *dev)
{
struct arm_smmu_device *smmu;
- struct arm_smmu_master *master = NULL;
- struct device_node *dev_node = dev_get_dev_node(dev);
+ struct arm_smmu_master_cfg *cfg;
+ struct device_node *np;
+ struct of_phandle_iterator it;
+ void *data = ⁢
+ __be32 pci_sid;
+ int err;
+ np = dev_get_dev_node(dev);
+ if (!np || !of_find_property(np, "#stream-id-cells", NULL)) {
+ of_node_put(np);
+ return -ENODEV;
+ }
+
+ it.node = np;
spin_lock(&arm_smmu_devices_lock);
list_for_each_entry(smmu, &arm_smmu_devices, list) {
- master = find_smmu_master(smmu, dev_node);
- if (master)
+ err = __find_legacy_master_phandle(smmu->dev, &data);
+ if (err)
break;
}
spin_unlock(&arm_smmu_devices_lock);
+ of_node_put(np);
+ if (err == 0)
+ return -ENODEV;
+ if (err < 0)
+ return err;
- return master ? smmu : NULL;
+ if (it.cur_count > MAX_MASTER_STREAMIDS) {
+ dev_err(smmu->dev,
+ "reached maximum number (%d) of stream IDs for master device %s\n",
+ MAX_MASTER_STREAMIDS, dev_name(dev));
+ return -ENOSPC;
+ }
+ if (dev_is_pci(dev)) {
+ /* "mmu-masters" assumes Stream ID == Requester ID */
+ pci_for_each_dma_alias(to_pci_dev(dev), __arm_smmu_get_pci_sid,
+ &pci_sid);
+ it.cur = &pci_sid;
+ it.cur_count = 1;
+ }
+
+ cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return -ENOMEM;
+
+ cfg->smmu = smmu;
+ dev->archdata.iommu = cfg;
+
+ while (it.cur_count--)
+ cfg->streamids[cfg->num_streamids++] = be32_to_cpup(it.cur++);
+
+ return 0;
}
static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
@@ -1119,8 +1064,7 @@ static void arm_smmu_free_smr(struct arm_smmu_device *smmu, int idx)
static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
{
struct arm_smmu_smr *smr = smmu->smrs + idx;
- u32 reg = (smr->id & smmu->streamid_mask) << SMR_ID_SHIFT |
- (smr->mask & smmu->smr_mask_mask) << SMR_MASK_SHIFT;
+ u32 reg = smr->id << SMR_ID_SHIFT | smr->mask << SMR_MASK_SHIFT;
if (smr->valid)
reg |= SMR_VALID;
@@ -1189,9 +1133,9 @@ err_free_smrs:
return -ENOSPC;
}
-static void arm_smmu_master_free_smes(struct arm_smmu_device *smmu,
- struct arm_smmu_master_cfg *cfg)
+static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
{
+ struct arm_smmu_device *smmu = cfg->smmu;
int i;
/*
@@ -1262,17 +1206,15 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
- struct arm_smmu_device *smmu;
- struct arm_smmu_master_cfg *cfg;
+ struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
- smmu = find_smmu_for_device(dev);
- if (!smmu) {
+ if (!cfg) {
dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
return -ENXIO;
}
/* Ensure that the domain is finalised */
- ret = arm_smmu_init_domain_context(domain, smmu);
+ ret = arm_smmu_init_domain_context(domain, cfg->smmu);
if (ret < 0)
return ret;
@@ -1280,18 +1222,14 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
* Sanity check the domain. We don't support domains across
* different SMMUs.
*/
- if (smmu_domain->smmu != smmu) {
+ if (smmu_domain->smmu != cfg->smmu) {
dev_err(dev,
"cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n",
- dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
+ dev_name(smmu_domain->smmu->dev), dev_name(cfg->smmu->dev));
return -EINVAL;
}
/* Looks ok, so add the device to the domain */
- cfg = find_smmu_master_cfg(dev);
- if (!cfg)
- return -ENODEV;
-
return arm_smmu_domain_add_master(smmu_domain, cfg);
}
@@ -1411,120 +1349,65 @@ static bool arm_smmu_capable(enum iommu_cap cap)
}
}
-static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data)
-{
- *((u16 *)data) = alias;
- return 0; /* Continue walking */
-}
-
-static void __arm_smmu_release_pci_iommudata(void *data)
-{
- kfree(data);
-}
-
-static int arm_smmu_init_pci_device(struct pci_dev *pdev,
- struct iommu_group *group)
-{
- struct arm_smmu_master_cfg *cfg;
- u16 sid;
- int i;
-
- cfg = iommu_group_get_iommudata(group);
- if (!cfg) {
- cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
- if (!cfg)
- return -ENOMEM;
-
- iommu_group_set_iommudata(group, cfg,
- __arm_smmu_release_pci_iommudata);
- }
-
- if (cfg->num_streamids >= MAX_MASTER_STREAMIDS)
- return -ENOSPC;
-
- /*
- * Assume Stream ID == Requester ID for now.
- * We need a way to describe the ID mappings in FDT.
- */
- pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
- for (i = 0; i < cfg->num_streamids; ++i)
- if (cfg->streamids[i] == sid)
- break;
-
- /* Avoid duplicate SIDs, as this can lead to SMR conflicts */
- if (i == cfg->num_streamids) {
- cfg->streamids[i] = sid;
- cfg->smendx[i] = INVALID_SMENDX;
- cfg->num_streamids++;
- }
-
- return 0;
-}
-
-static int arm_smmu_init_platform_device(struct device *dev,
- struct iommu_group *group)
-{
- struct arm_smmu_device *smmu = find_smmu_for_device(dev);
- struct arm_smmu_master *master;
-
- if (!smmu)
- return -ENODEV;
-
- master = find_smmu_master(smmu, dev->of_node);
- if (!master)
- return -ENODEV;
-
- iommu_group_set_iommudata(group, &master->cfg, NULL);
-
- return 0;
-}
-
static int arm_smmu_add_device(struct device *dev)
{
+ struct arm_smmu_master_cfg *cfg;
struct iommu_group *group;
+ int i, ret;
+
+ ret = arm_smmu_register_legacy_master(dev);
+ cfg = dev->archdata.iommu;
+ if (ret)
+ goto out_free;
+
+ ret = -EINVAL;
+ for (i = 0; i < cfg->num_streamids; i++) {
+ u16 sid = cfg->streamids[i];
+
+ if (sid & ~cfg->smmu->streamid_mask) {
+ dev_err(dev, "stream ID 0x%x out of range for SMMU (0x%x)\n",
+ sid, cfg->smmu->streamid_mask);
+ goto out_free;
+ }
+ cfg->smendx[i] = INVALID_SMENDX;
+ }
group = iommu_group_get_for_dev(dev);
- if (IS_ERR(group))
- return PTR_ERR(group);
-
+ if (IS_ERR(group)) {
+ ret = PTR_ERR(group);
+ goto out_free;
+ }
iommu_group_put(group);
return 0;
+
+out_free:
+ kfree(cfg);
+ dev->archdata.iommu = NULL;
+ return ret;
}
static void arm_smmu_remove_device(struct device *dev)
{
- struct arm_smmu_device *smmu = find_smmu_for_device(dev);
- struct arm_smmu_master_cfg *cfg = find_smmu_master_cfg(dev);
+ struct arm_smmu_master_cfg *cfg = dev->archdata.iommu;
- if (smmu && cfg)
- arm_smmu_master_free_smes(smmu, cfg);
+ if (!cfg)
+ return;
+ arm_smmu_master_free_smes(cfg);
iommu_group_remove_device(dev);
+ kfree(cfg);
+ dev->archdata.iommu = NULL;
}
static struct iommu_group *arm_smmu_device_group(struct device *dev)
{
struct iommu_group *group;
- int ret;
if (dev_is_pci(dev))
group = pci_device_group(dev);
else
group = generic_device_group(dev);
- if (IS_ERR(group))
- return group;
-
- if (dev_is_pci(dev))
- ret = arm_smmu_init_pci_device(to_pci_dev(dev), group);
- else
- ret = arm_smmu_init_platform_device(dev, group);
-
- if (ret) {
- iommu_group_put(group);
- group = ERR_PTR(ret);
- }
-
return group;
}
@@ -1938,9 +1821,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
struct resource *res;
struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
- struct rb_node *node;
- struct of_phandle_iterator it;
- struct arm_smmu_phandle_args *masterspec;
int num_irqs, i, err;
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -2001,37 +1881,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
if (err)
return err;
- i = 0;
- smmu->masters = RB_ROOT;
-
- err = -ENOMEM;
- /* No need to zero the memory for masterspec */
- masterspec = kmalloc(sizeof(*masterspec), GFP_KERNEL);
- if (!masterspec)
- goto out_put_masters;
-
- of_for_each_phandle(&it, err, dev->of_node,
- "mmu-masters", "#stream-id-cells", 0) {
- int count = of_phandle_iterator_args(&it, masterspec->args,
- MAX_MASTER_STREAMIDS);
- masterspec->np = of_node_get(it.node);
- masterspec->args_count = count;
-
- err = register_smmu_master(smmu, dev, masterspec);
- if (err) {
- dev_err(dev, "failed to add master %s\n",
- masterspec->np->name);
- kfree(masterspec);
- goto out_put_masters;
- }
-
- i++;
- }
-
- dev_notice(dev, "registered %d master devices\n", i);
-
- kfree(masterspec);
-
parse_driver_options(smmu);
if (smmu->version == ARM_SMMU_V2 &&
@@ -2039,8 +1888,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
dev_err(dev,
"found only %d context interrupt(s) but %d required\n",
smmu->num_context_irqs, smmu->num_context_banks);
- err = -ENODEV;
- goto out_put_masters;
+ return -ENODEV;
}
for (i = 0; i < smmu->num_global_irqs; ++i) {
@@ -2052,7 +1900,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
if (err) {
dev_err(dev, "failed to request global IRQ %d (%u)\n",
i, smmu->irqs[i]);
- goto out_put_masters;
+ return err;
}
}
@@ -2063,22 +1911,12 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
arm_smmu_device_reset(smmu);
return 0;
-
-out_put_masters:
- for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
- struct arm_smmu_master *master
- = container_of(node, struct arm_smmu_master, node);
- of_node_put(master->of_node);
- }
-
- return err;
}
static int arm_smmu_device_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct arm_smmu_device *curr, *smmu = NULL;
- struct rb_node *node;
spin_lock(&arm_smmu_devices_lock);
list_for_each_entry(curr, &arm_smmu_devices, list) {
@@ -2093,12 +1931,6 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
if (!smmu)
return -ENODEV;
- for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
- struct arm_smmu_master *master
- = container_of(node, struct arm_smmu_master, node);
- of_node_put(master->of_node);
- }
-
if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
dev_err(dev, "removing device with active domains!\n");
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 12/22] iommu/arm-smmu: Keep track of S2CR state
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Making S2CRs first-class citizens within the driver with a high-level
representation of their state offers a neat solution to a few problems:
Firstly, the information about which context a device's stream IDs are
associated with is already present by necessity in the S2CR. With that
state easily accessible we can refer directly to it and obviate the need
to track an IOMMU domain in each device's archdata (its earlier purpose
of enforcing correct attachment of multi-device groups now being handled
by the IOMMU core itself).
Secondly, the core API now deprecates explicit domain detach and expects
domain attach to move devices smoothly from one domain to another; for
SMMUv2, this notion maps directly to simply rewriting the S2CRs assigned
to the device. By giving the driver a suitable abstraction of those
S2CRs to work with, we can massively reduce the overhead of the current
heavy-handed "detach, free resources, reallocate resources, attach"
approach.
Thirdly, making the software state hardware-shaped and attached to the
SMMU instance once again makes suspend/resume of this register group
that much simpler to implement in future.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Make INVALID_SMENDX checks explicit
---
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++--------------------
1 file changed, 93 insertions(+), 66 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index dfe13780ba54..69b6cab65421 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -170,12 +170,20 @@
#define S2CR_CBNDX_MASK 0xff
#define S2CR_TYPE_SHIFT 16
#define S2CR_TYPE_MASK 0x3
-#define S2CR_TYPE_TRANS (0 << S2CR_TYPE_SHIFT)
-#define S2CR_TYPE_BYPASS (1 << S2CR_TYPE_SHIFT)
-#define S2CR_TYPE_FAULT (2 << S2CR_TYPE_SHIFT)
+enum arm_smmu_s2cr_type {
+ S2CR_TYPE_TRANS,
+ S2CR_TYPE_BYPASS,
+ S2CR_TYPE_FAULT,
+};
#define S2CR_PRIVCFG_SHIFT 24
-#define S2CR_PRIVCFG_UNPRIV (2 << S2CR_PRIVCFG_SHIFT)
+#define S2CR_PRIVCFG_MASK 0x3
+enum arm_smmu_s2cr_privcfg {
+ S2CR_PRIVCFG_DEFAULT,
+ S2CR_PRIVCFG_DIPAN,
+ S2CR_PRIVCFG_UNPRIV,
+ S2CR_PRIVCFG_PRIV,
+};
/* Context bank attribute registers */
#define ARM_SMMU_GR1_CBAR(n) (0x0 + ((n) << 2))
@@ -292,6 +300,16 @@ enum arm_smmu_implementation {
CAVIUM_SMMUV2,
};
+struct arm_smmu_s2cr {
+ enum arm_smmu_s2cr_type type;
+ enum arm_smmu_s2cr_privcfg privcfg;
+ u8 cbndx;
+};
+
+#define s2cr_init_val (struct arm_smmu_s2cr){ \
+ .type = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS, \
+}
+
struct arm_smmu_smr {
u16 mask;
u16 id;
@@ -346,6 +364,7 @@ struct arm_smmu_device {
u16 streamid_mask;
u16 smr_mask_mask;
struct arm_smmu_smr *smrs;
+ struct arm_smmu_s2cr *s2crs;
unsigned long va_size;
unsigned long ipa_size;
@@ -1108,6 +1127,23 @@ static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
}
+static void arm_smmu_write_s2cr(struct arm_smmu_device *smmu, int idx)
+{
+ struct arm_smmu_s2cr *s2cr = smmu->s2crs + idx;
+ u32 reg = (s2cr->type & S2CR_TYPE_MASK) << S2CR_TYPE_SHIFT |
+ (s2cr->cbndx & S2CR_CBNDX_MASK) << S2CR_CBNDX_SHIFT |
+ (s2cr->privcfg & S2CR_PRIVCFG_MASK) << S2CR_PRIVCFG_SHIFT;
+
+ writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_S2CR(idx));
+}
+
+static void arm_smmu_write_sme(struct arm_smmu_device *smmu, int idx)
+{
+ arm_smmu_write_s2cr(smmu, idx);
+ if (smmu->smrs)
+ arm_smmu_write_smr(smmu, idx);
+}
+
static int arm_smmu_master_alloc_smes(struct arm_smmu_device *smmu,
struct arm_smmu_master_cfg *cfg)
{
@@ -1158,6 +1194,23 @@ static void arm_smmu_master_free_smes(struct arm_smmu_device *smmu,
{
int i;
+ /*
+ * We *must* clear the S2CR first, because freeing the SMR means
+ * that it can be re-allocated immediately.
+ */
+ for (i = 0; i < cfg->num_streamids; ++i) {
+ int idx = cfg->smendx[i];
+
+ /* An IOMMU group is torn down by the first device to be removed */
+ if (idx == INVALID_SMENDX)
+ return;
+
+ smmu->s2crs[idx] = s2cr_init_val;
+ arm_smmu_write_s2cr(smmu, idx);
+ }
+ /* Sync S2CR updates before touching anything else */
+ __iowmb();
+
/* Invalidate the SMRs before freeing back to the allocator */
for (i = 0; i < cfg->num_streamids; ++i) {
if (smmu->smrs)
@@ -1170,9 +1223,16 @@ static void arm_smmu_master_free_smes(struct arm_smmu_device *smmu,
static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_master_cfg *cfg)
{
- int i, ret;
+ int i, ret = 0;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
+ struct arm_smmu_s2cr *s2cr = smmu->s2crs;
+ enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
+ u8 cbndx = smmu_domain->cfg.cbndx;
+
+ if (cfg->smendx[0] == INVALID_SMENDX)
+ ret = arm_smmu_master_alloc_smes(smmu, cfg);
+ if (ret)
+ return ret;
/*
* FIXME: This won't be needed once we have IOMMU-backed DMA ops
@@ -1181,58 +1241,21 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
* and a PCI device (i.e. a PCI host controller)
*/
if (smmu_domain->domain.type == IOMMU_DOMAIN_DMA)
- return 0;
+ type = S2CR_TYPE_BYPASS;
- /* Devices in an IOMMU group may already be configured */
- ret = arm_smmu_master_alloc_smes(smmu, cfg);
- if (ret)
- return ret == -EEXIST ? 0 : ret;
-
- for (i = 0; i < cfg->num_streamids; ++i) {
- u32 idx, s2cr;
-
- idx = cfg->smendx[i];
- s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
- (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
- writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
- }
-
- return 0;
-}
-
-static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
- struct arm_smmu_master_cfg *cfg)
-{
- int i;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
-
- /*
- * We *must* clear the S2CR first, because freeing the SMR means
- * that it can be re-allocated immediately.
- */
for (i = 0; i < cfg->num_streamids; ++i) {
int idx = cfg->smendx[i];
- u32 reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS;
- /* An IOMMU group is torn down by the first device to be removed */
- if (idx == INVALID_SMENDX)
- return;
+ /* Devices in an IOMMU group may already be configured */
+ if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
+ break;
- writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(idx));
+ s2cr[idx].type = type;
+ s2cr[idx].privcfg = S2CR_PRIVCFG_UNPRIV;
+ s2cr[idx].cbndx = cbndx;
+ arm_smmu_write_s2cr(smmu, idx);
}
-
- arm_smmu_master_free_smes(smmu, cfg);
-}
-
-static void arm_smmu_detach_dev(struct device *dev,
- struct arm_smmu_master_cfg *cfg)
-{
- struct iommu_domain *domain = dev->archdata.iommu;
- struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
-
- dev->archdata.iommu = NULL;
- arm_smmu_domain_remove_master(smmu_domain, cfg);
+ return 0;
}
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
@@ -1269,14 +1292,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (!cfg)
return -ENODEV;
- /* Detach the dev from its current domain */
- if (dev->archdata.iommu)
- arm_smmu_detach_dev(dev, cfg);
-
- ret = arm_smmu_domain_add_master(smmu_domain, cfg);
- if (!ret)
- dev->archdata.iommu = domain;
- return ret;
+ return arm_smmu_domain_add_master(smmu_domain, cfg);
}
static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1477,6 +1493,12 @@ static int arm_smmu_add_device(struct device *dev)
static void arm_smmu_remove_device(struct device *dev)
{
+ struct arm_smmu_device *smmu = find_smmu_for_device(dev);
+ struct arm_smmu_master_cfg *cfg = find_smmu_master_cfg(dev);
+
+ if (smmu && cfg)
+ arm_smmu_master_free_smes(smmu, cfg);
+
iommu_group_remove_device(dev);
}
@@ -1582,12 +1604,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
* Reset stream mapping groups: Initial values mark all SMRn as
* invalid and all S2CRn as bypass unless overridden.
*/
- reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS;
- for (i = 0; i < smmu->num_mapping_groups; ++i) {
- if (smmu->smrs)
- arm_smmu_write_smr(smmu, i);
- writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(i));
- }
+ for (i = 0; i < smmu->num_mapping_groups; ++i)
+ arm_smmu_write_sme(smmu, i);
/*
* Before clearing ARM_MMU500_ACTLR_CPRE, need to
@@ -1676,6 +1694,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
u32 id;
bool cttw_dt, cttw_reg;
+ int i;
dev_notice(smmu->dev, "probing hardware configuration...\n");
dev_notice(smmu->dev, "SMMUv%d with:\n",
@@ -1773,6 +1792,14 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
"\tstream matching with %lu register groups, mask 0x%x",
size, smmu->smr_mask_mask);
}
+ /* s2cr->type == 0 means translation, so initialise explicitly */
+ smmu->s2crs = devm_kmalloc_array(smmu->dev, size, sizeof(*smmu->s2crs),
+ GFP_KERNEL);
+ if (!smmu->s2crs)
+ return -ENOMEM;
+ for (i = 0; i < size; i++)
+ smmu->s2crs[i] = s2cr_init_val;
+
smmu->num_mapping_groups = size;
if (smmu->version < ARM_SMMU_V2 || !(id & ID0_PTFS_NO_AARCH32)) {
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 11/22] iommu/arm-smmu: Consolidate stream map entry state
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
In order to consider SMR masking, we really want to be able to validate
ID/mask pairs against existing SMR contents to prevent stream match
conflicts, which at best would cause transactions to fault unexpectedly,
and at worst lead to silent unpredictable behaviour. With our SMMU
instance data holding only an allocator bitmap, and the SMR values
themselves scattered across master configs hanging off devices which we
may have no way of finding, there's essentially no way short of digging
everything back out of the hardware. Similarly, the thought of power
management ops to support suspend/resume faces the exact same problem.
By massaging the software state into a closer shape to the underlying
hardware, everything comes together quite nicely; the allocator and the
high-level view of the data become a single centralised state which we
can easily keep track of, and to which any updates can be validated in
full before being synchronised to the hardware itself.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
- Make INVALID_SMENDX checks explicit
---
drivers/iommu/arm-smmu.c | 147 +++++++++++++++++++++++++++--------------------
1 file changed, 86 insertions(+), 61 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f86d7887f69a..dfe13780ba54 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -28,6 +28,7 @@
#define pr_fmt(fmt) "arm-smmu: " fmt
+#include <linux/atomic.h>
#include <linux/delay.h>
#include <linux/dma-iommu.h>
#include <linux/dma-mapping.h>
@@ -55,9 +56,6 @@
/* Maximum number of context banks per SMMU */
#define ARM_SMMU_MAX_CBS 128
-/* Maximum number of mapping groups per SMMU */
-#define ARM_SMMU_MAX_SMRS 128
-
/* SMMU global address space */
#define ARM_SMMU_GR0(smmu) ((smmu)->base)
#define ARM_SMMU_GR1(smmu) ((smmu)->base + (1 << (smmu)->pgshift))
@@ -295,16 +293,17 @@ enum arm_smmu_implementation {
};
struct arm_smmu_smr {
- u8 idx;
u16 mask;
u16 id;
+ bool valid;
};
struct arm_smmu_master_cfg {
int num_streamids;
u16 streamids[MAX_MASTER_STREAMIDS];
- struct arm_smmu_smr *smrs;
+ s16 smendx[MAX_MASTER_STREAMIDS];
};
+#define INVALID_SMENDX -1
struct arm_smmu_master {
struct device_node *of_node;
@@ -346,7 +345,7 @@ struct arm_smmu_device {
u32 num_mapping_groups;
u16 streamid_mask;
u16 smr_mask_mask;
- DECLARE_BITMAP(smr_map, ARM_SMMU_MAX_SMRS);
+ struct arm_smmu_smr *smrs;
unsigned long va_size;
unsigned long ipa_size;
@@ -550,6 +549,7 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
return -ERANGE;
}
master->cfg.streamids[i] = streamid;
+ master->cfg.smendx[i] = INVALID_SMENDX;
}
return insert_smmu_master(smmu, master);
}
@@ -1080,79 +1080,91 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
kfree(smmu_domain);
}
-static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
- struct arm_smmu_master_cfg *cfg)
+static int arm_smmu_alloc_smr(struct arm_smmu_device *smmu)
{
int i;
- struct arm_smmu_smr *smrs;
- void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
- if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH))
- return 0;
+ for (i = 0; i < smmu->num_mapping_groups; i++)
+ if (!cmpxchg(&smmu->smrs[i].valid, false, true))
+ return i;
- if (cfg->smrs)
- return -EEXIST;
+ return INVALID_SMENDX;
+}
- smrs = kmalloc_array(cfg->num_streamids, sizeof(*smrs), GFP_KERNEL);
- if (!smrs) {
- dev_err(smmu->dev, "failed to allocate %d SMRs\n",
- cfg->num_streamids);
- return -ENOMEM;
- }
+static void arm_smmu_free_smr(struct arm_smmu_device *smmu, int idx)
+{
+ writel_relaxed(~SMR_VALID, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
+ WRITE_ONCE(smmu->smrs[idx].valid, false);
+}
+
+static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
+{
+ struct arm_smmu_smr *smr = smmu->smrs + idx;
+ u32 reg = (smr->id & smmu->streamid_mask) << SMR_ID_SHIFT |
+ (smr->mask & smmu->smr_mask_mask) << SMR_MASK_SHIFT;
+
+ if (smr->valid)
+ reg |= SMR_VALID;
+ writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
+}
+
+static int arm_smmu_master_alloc_smes(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_cfg *cfg)
+{
+ struct arm_smmu_smr *smrs = smmu->smrs;
+ int i, idx;
/* Allocate the SMRs on the SMMU */
for (i = 0; i < cfg->num_streamids; ++i) {
- int idx = __arm_smmu_alloc_bitmap(smmu->smr_map, 0,
- smmu->num_mapping_groups);
+ if (cfg->smendx[i] != INVALID_SMENDX)
+ return -EEXIST;
+
+ /* ...except on stream indexing hardware, of course */
+ if (!smrs) {
+ cfg->smendx[i] = cfg->streamids[i];
+ continue;
+ }
+
+ idx = arm_smmu_alloc_smr(smmu);
if (idx < 0) {
dev_err(smmu->dev, "failed to allocate free SMR\n");
goto err_free_smrs;
}
+ cfg->smendx[i] = idx;
- smrs[i] = (struct arm_smmu_smr) {
- .idx = idx,
- .mask = 0, /* We don't currently share SMRs */
- .id = cfg->streamids[i],
- };
+ smrs[idx].id = cfg->streamids[i];
+ smrs[idx].mask = 0; /* We don't currently share SMRs */
}
+ if (!smrs)
+ return 0;
+
/* It worked! Now, poke the actual hardware */
- for (i = 0; i < cfg->num_streamids; ++i) {
- u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
- smrs[i].mask << SMR_MASK_SHIFT;
- writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_SMR(smrs[i].idx));
- }
+ for (i = 0; i < cfg->num_streamids; ++i)
+ arm_smmu_write_smr(smmu, cfg->smendx[i]);
- cfg->smrs = smrs;
return 0;
err_free_smrs:
- while (--i >= 0)
- __arm_smmu_free_bitmap(smmu->smr_map, smrs[i].idx);
- kfree(smrs);
+ while (i--) {
+ arm_smmu_free_smr(smmu, cfg->smendx[i]);
+ cfg->smendx[i] = INVALID_SMENDX;
+ }
return -ENOSPC;
}
-static void arm_smmu_master_free_smrs(struct arm_smmu_device *smmu,
+static void arm_smmu_master_free_smes(struct arm_smmu_device *smmu,
struct arm_smmu_master_cfg *cfg)
{
int i;
- void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
- struct arm_smmu_smr *smrs = cfg->smrs;
-
- if (!smrs)
- return;
/* Invalidate the SMRs before freeing back to the allocator */
for (i = 0; i < cfg->num_streamids; ++i) {
- u8 idx = smrs[i].idx;
+ if (smmu->smrs)
+ arm_smmu_free_smr(smmu, cfg->smendx[i]);
- writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(idx));
- __arm_smmu_free_bitmap(smmu->smr_map, idx);
+ cfg->smendx[i] = INVALID_SMENDX;
}
-
- cfg->smrs = NULL;
- kfree(smrs);
}
static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
@@ -1172,14 +1184,14 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
return 0;
/* Devices in an IOMMU group may already be configured */
- ret = arm_smmu_master_configure_smrs(smmu, cfg);
+ ret = arm_smmu_master_alloc_smes(smmu, cfg);
if (ret)
return ret == -EEXIST ? 0 : ret;
for (i = 0; i < cfg->num_streamids; ++i) {
u32 idx, s2cr;
- idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
+ idx = cfg->smendx[i];
s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
(smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
@@ -1195,22 +1207,22 @@ static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_device *smmu = smmu_domain->smmu;
void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
- /* An IOMMU group is torn down by the first device to be removed */
- if ((smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) && !cfg->smrs)
- return;
-
/*
* We *must* clear the S2CR first, because freeing the SMR means
* that it can be re-allocated immediately.
*/
for (i = 0; i < cfg->num_streamids; ++i) {
- u32 idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
+ int idx = cfg->smendx[i];
u32 reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS;
+ /* An IOMMU group is torn down by the first device to be removed */
+ if (idx == INVALID_SMENDX)
+ return;
+
writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(idx));
}
- arm_smmu_master_free_smrs(smmu, cfg);
+ arm_smmu_master_free_smes(smmu, cfg);
}
static void arm_smmu_detach_dev(struct device *dev,
@@ -1424,8 +1436,11 @@ static int arm_smmu_init_pci_device(struct pci_dev *pdev,
break;
/* Avoid duplicate SIDs, as this can lead to SMR conflicts */
- if (i == cfg->num_streamids)
- cfg->streamids[cfg->num_streamids++] = sid;
+ if (i == cfg->num_streamids) {
+ cfg->streamids[i] = sid;
+ cfg->smendx[i] = INVALID_SMENDX;
+ cfg->num_streamids++;
+ }
return 0;
}
@@ -1556,17 +1571,21 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
{
void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
void __iomem *cb_base;
- int i = 0;
+ int i;
u32 reg, major;
/* clear global FSR */
reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
- /* Mark all SMRn as invalid and all S2CRn as bypass unless overridden */
+ /*
+ * Reset stream mapping groups: Initial values mark all SMRn as
+ * invalid and all S2CRn as bypass unless overridden.
+ */
reg = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS;
for (i = 0; i < smmu->num_mapping_groups; ++i) {
- writel_relaxed(0, gr0_base + ARM_SMMU_GR0_SMR(i));
+ if (smmu->smrs)
+ arm_smmu_write_smr(smmu, i);
writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_S2CR(i));
}
@@ -1744,6 +1763,12 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
+ /* Zero-initialised to mark as invalid */
+ smmu->smrs = devm_kcalloc(smmu->dev, size, sizeof(*smmu->smrs),
+ GFP_KERNEL);
+ if (!smmu->smrs)
+ return -ENOMEM;
+
dev_notice(smmu->dev,
"\tstream matching with %lu register groups, mask 0x%x",
size, smmu->smr_mask_mask);
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 10/22] iommu/arm-smmu: Handle stream IDs more dynamically
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Rather than assuming fixed worst-case values for stream IDs and SMR
masks, keep track of whatever implemented bits the hardware actually
reports. This also obviates the slightly questionable validation of SMR
fields in isolation - rather than aborting the whole SMMU probe for a
hardware configuration which is still architecturally valid, we can
simply refuse masters later if they try to claim an unrepresentable ID
or mask (which almost certainly implies a DT error anyway).
Acked-by: Will Deacon <will.deacon@arm.com>
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4b1c87e947fd..f86d7887f69a 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -165,9 +165,7 @@
#define ARM_SMMU_GR0_SMR(n) (0x800 + ((n) << 2))
#define SMR_VALID (1 << 31)
#define SMR_MASK_SHIFT 16
-#define SMR_MASK_MASK 0x7fff
#define SMR_ID_SHIFT 0
-#define SMR_ID_MASK 0x7fff
#define ARM_SMMU_GR0_S2CR(n) (0xc00 + ((n) << 2))
#define S2CR_CBNDX_SHIFT 0
@@ -346,6 +344,8 @@ struct arm_smmu_device {
atomic_t irptndx;
u32 num_mapping_groups;
+ u16 streamid_mask;
+ u16 smr_mask_mask;
DECLARE_BITMAP(smr_map, ARM_SMMU_MAX_SMRS);
unsigned long va_size;
@@ -1715,39 +1715,40 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
dev_notice(smmu->dev,
"\t(IDR0.CTTW overridden by dma-coherent property)\n");
+ /* Max. number of entries we have for stream matching/indexing */
+ size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
+ smmu->streamid_mask = size - 1;
if (id & ID0_SMS) {
- u32 smr, sid, mask;
+ u32 smr;
smmu->features |= ARM_SMMU_FEAT_STREAM_MATCH;
- smmu->num_mapping_groups = (id >> ID0_NUMSMRG_SHIFT) &
- ID0_NUMSMRG_MASK;
- if (smmu->num_mapping_groups == 0) {
+ size = (id >> ID0_NUMSMRG_SHIFT) & ID0_NUMSMRG_MASK;
+ if (size == 0) {
dev_err(smmu->dev,
"stream-matching supported, but no SMRs present!\n");
return -ENODEV;
}
- smr = SMR_MASK_MASK << SMR_MASK_SHIFT;
- smr |= (SMR_ID_MASK << SMR_ID_SHIFT);
+ /*
+ * SMR.ID bits may not be preserved if the corresponding MASK
+ * bits are set, so check each one separately. We can reject
+ * masters later if they try to claim IDs outside these masks.
+ */
+ smr = smmu->streamid_mask << SMR_ID_SHIFT;
writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
+ smmu->streamid_mask = smr >> SMR_ID_SHIFT;
- mask = (smr >> SMR_MASK_SHIFT) & SMR_MASK_MASK;
- sid = (smr >> SMR_ID_SHIFT) & SMR_ID_MASK;
- if ((mask & sid) != sid) {
- dev_err(smmu->dev,
- "SMR mask bits (0x%x) insufficient for ID field (0x%x)\n",
- mask, sid);
- return -ENODEV;
- }
+ smr = smmu->streamid_mask << SMR_MASK_SHIFT;
+ writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
+ smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
+ smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
dev_notice(smmu->dev,
- "\tstream matching with %u register groups, mask 0x%x",
- smmu->num_mapping_groups, mask);
- } else {
- smmu->num_mapping_groups = (id >> ID0_NUMSIDB_SHIFT) &
- ID0_NUMSIDB_MASK;
+ "\tstream matching with %lu register groups, mask 0x%x",
+ size, smmu->smr_mask_mask);
}
+ smmu->num_mapping_groups = size;
if (smmu->version < ARM_SMMU_V2 || !(id & ID0_PTFS_NO_AARCH32)) {
smmu->features |= ARM_SMMU_FEAT_FMT_AARCH32_L;
--
2.8.1.dirty
^ permalink raw reply related
* [PATCH v7 09/22] iommu/arm-smmu: Set PRIVCFG in stage 1 STEs
From: Robin Murphy @ 2016-09-12 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Implement the SMMUv3 equivalent of d346180e70b9 ("iommu/arm-smmu: Treat
all device transactions as unprivileged"), so that once again those
pesky DMA controllers with their privileged instruction fetches don't
unexpectedly fault in stage 1 domains due to VMSAv8 rules.
Acked-by: Will Deacon <will.deacon@arm.com>
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 52860bcf80f2..0c45c1e02e04 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -267,6 +267,9 @@
#define STRTAB_STE_1_SHCFG_INCOMING 1UL
#define STRTAB_STE_1_SHCFG_SHIFT 44
+#define STRTAB_STE_1_PRIVCFG_UNPRIV 2UL
+#define STRTAB_STE_1_PRIVCFG_SHIFT 48
+
#define STRTAB_STE_2_S2VMID_SHIFT 0
#define STRTAB_STE_2_S2VMID_MASK 0xffffUL
#define STRTAB_STE_2_VTCR_SHIFT 32
@@ -1068,7 +1071,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
#ifdef CONFIG_PCI_ATS
STRTAB_STE_1_EATS_TRANS << STRTAB_STE_1_EATS_SHIFT |
#endif
- STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);
+ STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT |
+ STRTAB_STE_1_PRIVCFG_UNPRIV <<
+ STRTAB_STE_1_PRIVCFG_SHIFT);
if (smmu->features & ARM_SMMU_FEAT_STALLS)
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
--
2.8.1.dirty
^ 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