* [GIT PULL 2/5] Broadcom defconfig-arm64 changes for 4.18
From: Florian Fainelli @ 2018-05-11 21:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511214339.24139-1-f.fainelli@gmail.com>
The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:
Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)
are available in the git repository at:
https://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.18/defconfig-arm64
for you to fetch changes up to dce4c94eedbd35cd63aa04ee9b022de6d23ee587:
Merge tag 'bcm2835-defconfig-64-next-2018-04-30' into defconfig-arm64/next (2018-05-01 11:54:16 -0700)
----------------------------------------------------------------
This pull request contains Broadcom ARM64-based SoCs defconfig changes
for 4.18, please pull the following:
- Stefan provides a set of updates targeting the Raspberry Pi 3 B+
platform: LAN7515 USB Ethernet driver, Cypress CYW43455 Bluetooth when
using the Pi 3 B+ in AArch64 boot mode.
----------------------------------------------------------------
Florian Fainelli (1):
Merge tag 'bcm2835-defconfig-64-next-2018-04-30' into defconfig-arm64/next
Stefan Wahren (1):
arm64: defconfig: Enable LAN and BT support for RPi 3 B+
arch/arm64/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)
^ permalink raw reply
* [GIT PULL 1/5] Broadcom defconfig changes for 4.18
From: Florian Fainelli @ 2018-05-11 21:43 UTC (permalink / raw)
To: linux-arm-kernel
The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:
Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)
are available in the git repository at:
https://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.18/defconfig
for you to fetch changes up to f8928381021121123e9f4f783606c51cd37ab5d9:
Merge tag 'bcm2835-defconfig-next-2018-04-30' into defconfig/next (2018-05-01 11:53:24 -0700)
----------------------------------------------------------------
This pull request contains Broadcom ARM-based SoCs defconfig changes for
4.18, please pull the following:
- Stefan provides a set of updates targeting the Raspberry Pi 3 B+
platform: LAN7515 USB Ethernet driver, Cypress CYW43455 Bluetooth and
he also enables the VCHIQ driver to help with continous testing on
kernelci.org.
----------------------------------------------------------------
Florian Fainelli (1):
Merge tag 'bcm2835-defconfig-next-2018-04-30' into defconfig/next
Stefan Wahren (3):
ARM: bcm2835_defconfig: Enable LAN78XX driver
ARM: bcm2835_defconfig: Enable VCHIQ driver
ARM: multi_v7_defconfig: Enable LAN and BT for RPi 3 B+
arch/arm/configs/bcm2835_defconfig | 2 ++
arch/arm/configs/multi_v7_defconfig | 4 ++++
2 files changed, 6 insertions(+)
^ permalink raw reply
* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Evan Green @ 2018-05-11 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-2-georgi.djakov@linaro.org>
Hi Georgi,
On Fri, Mar 9, 2018 at 1:12 PM Georgi Djakov <georgi.djakov@linaro.org>
wrote:
> This patch introduce a new API to get requirements and configure the
> interconnect buses across the entire chipset to fit with the current
> demand.
> The API is using a consumer/provider-based model, where the providers are
> the interconnect buses and the consumers could be various drivers.
> The consumers request interconnect resources (path) between endpoints and
> set the desired constraints on this data flow path. The providers receive
> requests from consumers and aggregate these requests for all master-slave
> pairs on that path. Then the providers configure each participating in the
> topology node according to the requested data flow path, physical links
and
> constraints. The topology could be complicated and multi-tiered and is SoC
> specific.
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> Documentation/interconnect/interconnect.rst | 96 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/interconnect/Kconfig | 10 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/core.c | 489
++++++++++++++++++++++++++++
> include/linux/interconnect-provider.h | 109 +++++++
> include/linux/interconnect.h | 40 +++
> 8 files changed, 748 insertions(+)
> create mode 100644 Documentation/interconnect/interconnect.rst
> create mode 100644 drivers/interconnect/Kconfig
> create mode 100644 drivers/interconnect/Makefile
> create mode 100644 drivers/interconnect/core.c
> create mode 100644 include/linux/interconnect-provider.h
> create mode 100644 include/linux/interconnect.h
...
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> new file mode 100644
> index 000000000000..6306e258b9b9
> --- /dev/null
> +++ b/drivers/interconnect/core.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework core driver
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_IDR(icc_idr);
> +static LIST_HEAD(icc_provider_list);
> +static DEFINE_MUTEX(icc_provider_list_mutex);
> +static DEFINE_MUTEX(icc_path_mutex);
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + *
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @avg_bw: an integer describing the average bandwidth in kbps
> + * @peak_bw: an integer describing the peak bandwidth in kbps
> + */
> +struct icc_req {
> + struct hlist_node req_node;
> + struct icc_node *node;
> + u32 avg_bw;
> + u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> + size_t num_nodes;
> + struct icc_req reqs[0];
> +};
> +
> +static struct icc_node *node_find(const int id)
> +{
> + struct icc_node *node;
> +
> + node = idr_find(&icc_idr, id);
> +
> + return node;
> +}
> +
> +static struct icc_path *path_allocate(struct icc_node *node, ssize_t
num_nodes)
> +{
So node is really the destination, correct? Then we use ->reverse to walk
backwards num_nodes steps towards the source. It might increase readability
to call the parameter dest, then assign that to a local called node for
traversal.
> + struct icc_path *path;
> + size_t i;
> +
> + path = kzalloc(sizeof(*path) + num_nodes * sizeof(*path->reqs),
> + GFP_KERNEL);
> + if (!path)
> + return ERR_PTR(-ENOMEM);
> +
> + path->num_nodes = num_nodes;
> +
> + for (i = 0; i < num_nodes; i++) {
> + hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> +
> + path->reqs[i].node = node;
> + /* reference to previous node was saved during path
traversal */
> + node = node->reverse;
> + }
> +
> + return path;
> +}
> +
> +static struct icc_path *path_find(struct icc_node *src, struct icc_node
*dst)
> +{
> + struct icc_node *node = NULL;
> + struct list_head traverse_list;
> + struct list_head edge_list;
> + struct list_head tmp_list;
> + size_t i, number = 0;
> + bool found = false;
> +
> + INIT_LIST_HEAD(&traverse_list);
> + INIT_LIST_HEAD(&edge_list);
> + INIT_LIST_HEAD(&tmp_list);
tmp_list is really the list of nodes you've already visited and need to
remember to reset is_traversed for. Maybe calling this done_list or
visited_list would be more descriptive.
> +
> + list_add_tail(&src->search_list, &traverse_list);
For added paranoia, you could set src->reverse to NULL so that somebody
elsewhere who had a bug in their back-traversal would fall off the end,
rather than into some previous scrapped path.
> +
> + do {
> + list_for_each_entry(node, &traverse_list, search_list) {
> + if (node == dst) {
> + found = true;
> + list_add(&node->search_list, &tmp_list);
> + break;
> + }
> + for (i = 0; i < node->num_links; i++) {
> + struct icc_node *tmp = node->links[i];
> +
> + if (!tmp)
> + return ERR_PTR(-ENOENT);
You just bail out here, but never clean up the nodes is_traversed, which
will ruin later searches. Maybe a goto towards the common cleanup path?
> +
> + if (tmp->is_traversed)
> + continue;
> +
> + tmp->is_traversed = true;
> + tmp->reverse = node;
> + list_add_tail(&tmp->search_list,
&edge_list);
> + }
> + }
> + if (found)
> + break;
> +
> + list_splice_init(&traverse_list, &tmp_list);
> + list_splice_init(&edge_list, &traverse_list);
> +
> + /* count the number of nodes */
> + number++;
Depth might be a better name for this, since this really counts the hops
away from the source, rather than the number of nodes you've processed.
> +
> + } while (!list_empty(&traverse_list));
> +
> + /* reset the traversed state */
> + list_for_each_entry(node, &tmp_list, search_list)
> + node->is_traversed = false;
> +
> + if (found)
> + return path_allocate(dst, number);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int path_init(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users++;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + return 0;
> +}
This function cannot fail, nor do you check its return value, so you should
change the return type to void.
I'm wondering if the locking here is a little sketchy. I was in the process
of typing a suggestion that you call path_init from within path_find, since
it seemed weird to have this gray zone of a path without its reference
counts, when I noticed the locks.
I can't evaluate fully, since the implementation seems to be missing
icc_node_remove, a critical function in terms of evaluating the locks. You
have an icc_del_provider, but its warning of if (provider->users) is pretty
weak, since without node removal provider->users could easily be
incremented after the provider lock is released. It also leaks all of its
nodes, since there's no way to remove them.
Here's my suggestion as far as the locking goes:
* To add or remove links/nodes from the graph, you're going to need to hold
a global lock to avoid colliding with traversals. You've already got an
icc_path_mutex, so that would work.
* icc_link_create needs to hold the global icc_path_mutex, since it's
messing with arrays and connections used in path traversal, and doesn't
need to hold the provider lock, since it's not changing anything there.
* The presumably upcoming icc_link_destroy, or its parent icc_node_destroy,
also needs to hold the global lock. node_destroy may also need the provider
lock in symmetry with icc_node_add.
* Provider->users will be protected under the global icc_path_mutex, rather
than the provider lock. Then move path_init into path_find, or inline it
into path_allocate.
* Once you do that, provider->lock is now only protecting its node list.
For now, it's probably more efficient to roll the protection of
provider->nodes under the global lock as well, and remove the lock from the
provider altogether. If you anticipate other functions in the future that
will require a lock in the provider, then it might make sense to keep the
lock, or maybe just add it later with that new functionality.
> +
> +static void node_aggregate(struct icc_node *node)
> +{
> + struct icc_req *r;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + hlist_for_each_entry(r, &node->req_list, req_node) {
> + /* sum(averages) and max(peaks) */
> + agg_avg += r->avg_bw;
> + agg_peak = max(agg_peak, r->peak_bw);
> + }
> +
> + node->avg_bw = agg_avg;
> + node->peak_bw = agg_peak;
> +}
> +
> +static void provider_aggregate(struct icc_provider *provider, u32
*avg_bw,
> + u32 *peak_bw)
> +{
> + struct icc_node *n;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + /* aggregate for the interconnect provider */
> + list_for_each_entry(n, &provider->nodes, node_list) {
> + /* sum the average and max the peak */
> + agg_avg += n->avg_bw;
> + agg_peak = max(agg_peak, n->peak_bw);
> + }
> +
> + *avg_bw = agg_avg;
> + *peak_bw = agg_peak;
> +}
> +
> +static int constraints_apply(struct icc_path *path)
> +{
Nit: maybe name it apply_constraints, since constraints_apply sounds like a
query (do the constraints apply?).
> + struct icc_node *next, *prev = NULL;
> + int i;
> +
> + for (i = 0; i < path->num_nodes; i++, prev = next) {
> + struct icc_provider *provider;
> + u32 avg_bw = 0;
> + u32 peak_bw = 0;
> + int ret;
> +
> + next = path->reqs[i].node;
> + /*
> + * Both endpoints should be valid master-slave pairs of
the
> + * same interconnect provider that will be configured.
> + */
> + if (!next || !prev)
> + continue;
> +
> + if (next->provider != prev->provider)
> + continue;
next should never be null, right? So you could shorten this to if (!prev ||
(next->provider != prev->provider))
> +
> + provider = next->provider;
> + mutex_lock(&provider->lock);
> +
> + /* aggregate requests for the provider */
> + provider_aggregate(provider, &avg_bw, &peak_bw);
> +
> + if (provider->set) {
> + /* set the constraints */
> + ret = provider->set(prev, next, avg_bw, peak_bw);
> + }
> +
> + mutex_unlock(&provider->lock);
> +
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * icc_set() - set constraints on an interconnect path between two
endpoints
> + * @path: reference to the path returned by icc_get()
> + * @avg_bw: average bandwidth in kbps
> + * @peak_bw: peak bandwidth in kbps
> + *
> + * This function is used by an interconnect consumer to express its own
needs
> + * in term of bandwidth and QoS for a previously requested path between
two
"in terms of" rather than "in term of", and not really QoS yet, right?
> + * endpoints. The requests are aggregated and each node is updated
accordingly.
> + *
> + * Returns 0 on success, or an approproate error code otherwise.
appropriate
> + */
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path)
> + return 0;
Can we ditch this null check? My understanding is it's generally preferred
to skip this if it's only there to avoid developer errors.
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&icc_path_mutex);
> +
> + /* update the consumer request for this path */
> + path->reqs[i].avg_bw = avg_bw;
> + path->reqs[i].peak_bw = peak_bw;
> +
> + /* aggregate requests for this node */
> + node_aggregate(node);
> +
> + mutex_unlock(&icc_path_mutex);
> + }
> +
> + ret = constraints_apply(path);
> + if (ret)
> + pr_err("interconnect: error applying constraints (%d)",
ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(icc_set);
> +
> +/**
> + * icc_get() - return a handle for path between two endpoints
> + * @src_id: source device port id
> + * @dst_id: destination device port id
> + *
> + * This function will search for a path between two endpoints and return
an
> + * icc_path handle on success. Use icc_put() to release
> + * constraints when the they are not needed anymore.
> + *
> + * Return: icc_path pointer on success, or ERR_PTR() on error
> + */
> +struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + struct icc_node *src, *dst;
> + struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> + src = node_find(src_id);
> + if (!src)
> + goto out;
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + goto out;
> +
> + mutex_lock(&icc_path_mutex);
> + path = path_find(src, dst);
> + mutex_unlock(&icc_path_mutex);
> + if (IS_ERR(path))
> + goto out;
> +
> + path_init(path);
> +
> +out:
> + return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
> +/**
> + * icc_put() - release the reference to the icc_path
> + * @path: interconnect path
> + *
> + * Use this function to release the constraints on a path when the path
is
> + * no longer needed. The constraints will be re-aggregated.
> + */
> +void icc_put(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path || WARN_ON_ONCE(IS_ERR(path)))
> + return;
> +
> + ret = icc_set(path, 0, 0);
> + if (ret)
> + pr_err("%s: error (%d)\n", __func__, ret);
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> + hlist_del(&path->reqs[i].req_node);
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users--;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + kfree(path);
> +}
> +EXPORT_SYMBOL_GPL(icc_put);
> +
> +/**
> + * icc_node_create() - create a node
> + * @id: node id
> + *
> + * Return: icc_node pointer on success, or ERR_PTR() on error
> + */
> +struct icc_node *icc_node_create(int id)
> +{
> + struct icc_node *node;
> +
> + /* check if node already exists */
> + node = node_find(id);
> + if (node)
> + return node;
This is probably going to do more harm than good once icc_node_delete comes
in, since it almost certainly indicates a programmer error or ID collision,
and will likely result in a double free. We should probably fail with
EEXIST instead.
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return ERR_PTR(-ENOMEM);
> +
> + id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> + if (WARN(id < 0, "couldn't get idr"))
> + return ERR_PTR(id);
> +
> + node->id = id;
> +
> + return node;
> +}
> +EXPORT_SYMBOL_GPL(icc_node_create);
> +
> +/**
> + * icc_link_create() - create a link between two nodes
> + * @src_id: source node id
> + * @dst_id: destination node id
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + struct icc_node *dst;
> + struct icc_node **new;
> + int ret = 0;
> +
> + if (IS_ERR_OR_NULL(node))
> + return PTR_ERR(node);
Remove this.
> +
> + mutex_lock(&node->provider->lock);
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + dst = icc_node_create(dst_id);
icc_node_create can fail, you should fail here if it does.
> +
> + new = krealloc(node->links,
> + (node->num_links + 1) * sizeof(*node->links),
> + GFP_KERNEL);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + node->links = new;
> + node->links[node->num_links++] = dst;
> +
> +out:
> + mutex_unlock(&node->provider->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_link_create);
> +
> +/**
> + * icc_add_node() - add an interconnect node to interconnect provider
> + * @node: pointer to the interconnect node
> + * @provider: pointer to the interconnect provider
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + if (WARN_ON(!node))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider))
> + return -EINVAL;
Remove these.
> +
> + node->provider = provider;
> +
> + mutex_lock(&provider->lock);
> + list_add_tail(&node->node_list, &provider->nodes);
> + mutex_unlock(&provider->lock);
> +
> + return 0;
> +}
icc_node_add should be exported, right? I see it being used in msm8916.c.
You should make sure that "make allmodconfig" still builds with your
changes.
I think you should add a safety check in icc_link_create to ensure that the
node has a provider before adding any links. If some consumer made a
mistake and added links before adding the node to the provider, path
traversal would use the uninitialized or NULL provider pointer. I was
thinking about this while noticing that you assign node->provider before
acquiring the lock.
> +
> +/**
> + * icc_add_provider() - add a new interconnect provider
> + * @icc_provider: the interconnect provider that will be added into
topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_add_provider(struct icc_provider *provider)
> +{
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
Remove this one. The one below is okay.
> + if (WARN_ON(!provider->set))
> + return -EINVAL;
> +
> + mutex_init(&provider->lock);
> + INIT_LIST_HEAD(&provider->nodes);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_add(&provider->provider_list, &icc_provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + dev_dbg(provider->dev, "interconnect provider added to
topology\n");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_add_provider);
> +
> +/**
> + * icc_del_provider() - delete previously added interconnect provider
> + * @icc_provider: the interconnect provider that will be removed from
topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_del_provider(struct icc_provider *provider)
> +{
> + mutex_lock(&provider->lock);
> + if (provider->users) {
> + pr_warn("interconnect provider still has %d users\n",
> + provider->users);
> + }
> + mutex_unlock(&provider->lock);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_del(&provider->provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_del_provider);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org");
> +MODULE_DESCRIPTION("Interconnect Driver Core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect-provider.h
b/include/linux/interconnect-provider.h
> new file mode 100644
> index 000000000000..779b5b5b1306
> --- /dev/null
> +++ b/include/linux/interconnect-provider.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_PROVIDER_H
> +#define _LINUX_INTERCONNECT_PROVIDER_H
> +
> +#include <linux/interconnect.h>
> +
> +struct icc_node;
> +
> +/**
> + * struct icc_provider - interconnect provider (controller) entity that
might
> + * provide multiple interconnect controls
> + *
> + * @provider_list: list of the registered interconnect providers
> + * @nodes: internal list of the interconnect provider nodes
> + * @set: pointer to device specific set operation function
> + * @dev: the device this interconnect provider belongs to
> + * @lock: lock to provide consistency during aggregation/update of
constraints
> + * @users: count of active users
> + * @data: pointer to private data
> + */
> +struct icc_provider {
> + struct list_head provider_list;
> + struct list_head nodes;
> + int (*set)(struct icc_node *src, struct icc_node *dst,
> + u32 avg_bw, u32 peak_bw);
> + struct device *dev;
> + struct mutex lock;
> + int users;
> + void *data;
> +};
> +
> +/**
> + * struct icc_node - entity that is part of the interconnect topology
> + *
> + * @id: platform specific node id
> + * @name: node name used in debugfs
> + * @links: a list of targets where we can go next when traversing
> + * @num_links: number of links to other interconnect nodes
> + * @provider: points to the interconnect provider of this node
> + * @node_list: list of interconnect nodes associated with @provider
> + * @search_list: list used when walking the nodes graph
> + * @reverse: pointer to previous node when walking the nodes graph
> + * @is_traversed: flag that is used when walking the nodes graph
> + * @req_list: a list of QoS constraint requests associated with this node
> + * @avg_bw: aggregated value of average bandwidth
> + * @peak_bw: aggregated value of peak bandwidth
> + * @data: pointer to private data
> + */
> +struct icc_node {
> + int id;
Why int here? Are you expecting negative numbers? Maybe u32 instead? Or
even better, maybe a typedef u32 icc_id? Ooh yeah, that way we know when
parameters and such are passed around that they refer to this.
> + const char *name;
> + struct icc_node **links;
> + size_t num_links;
> +
> + struct icc_provider *provider;
> + struct list_head node_list;
So the difference between node_list and links is that node_list nodes live
inside this node, whereas links point at other peers?
Oh no, I get it now after reading the .c file: node_list is the list entry
in the parent provider's "nodes" list. The comment description could be
clearer about that.
-Evan
^ permalink raw reply
* [PATCH v4 4/7] interconnect: qcom: Add RPM communication
From: Evan Green @ 2018-05-11 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-5-georgi.djakov@linaro.org>
On Fri, Mar 9, 2018 at 1:11 PM Georgi Djakov <georgi.djakov@linaro.org>
wrote:
> On some Qualcomm SoCs, there is a remote processor, which controls some of
> the Network-On-Chip interconnect resources. Other CPUs express their needs
> by communicating with this processor. Add a driver to handle comminication
> with this remote processor.
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> .../devicetree/bindings/interconnect/qcom-smd.txt | 31 ++++++++
> drivers/interconnect/qcom/Makefile | 1 +
> drivers/interconnect/qcom/smd-rpm.c | 90
++++++++++++++++++++++
> drivers/interconnect/qcom/smd-rpm.h | 15 ++++
> 4 files changed, 137 insertions(+)
> create mode 100644
Documentation/devicetree/bindings/interconnect/qcom-smd.txt
> create mode 100644 drivers/interconnect/qcom/Makefile
> create mode 100644 drivers/interconnect/qcom/smd-rpm.c
> create mode 100644 drivers/interconnect/qcom/smd-rpm.h
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom-smd.txt
b/Documentation/devicetree/bindings/interconnect/qcom-smd.txt
> new file mode 100644
> index 000000000000..14e83ed7019b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom-smd.txt
> @@ -0,0 +1,31 @@
> +Qualcomm SMD-RPM interconnect driver binding
> +------------------------------------------------
> +The RPM is a dedicated hardware engine for managing the shared
> +SoC resources in order to keep the lowest power profile. It
> +communicates with other hardware subsystems via shared memory
> +and accepts requests for various resources.
You never say what RPM or SMD stands for. RPM is Resource Power Manager,
right? But I'm not in the know about SMD. Can you define these somewhere?
-Evan
^ permalink raw reply
* [PATCH v4 5/7] interconnect: qcom: Add msm8916 interconnect provider driver
From: Evan Green @ 2018-05-11 21:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-6-georgi.djakov@linaro.org>
Hi Georgi,
On Fri, Mar 9, 2018 at 1:11 PM Georgi Djakov <georgi.djakov@linaro.org>
wrote:
> Add driver for the Qualcomm interconnect buses found in msm8916 based
> platforms.
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> drivers/interconnect/Kconfig | 5 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/qcom/Kconfig | 11 +
> drivers/interconnect/qcom/Makefile | 2 +
> drivers/interconnect/qcom/msm8916.c | 482
++++++++++++++++++++++++++++++++++++
> include/linux/interconnect/qcom.h | 350 ++++++++++++++++++++++++++
> 6 files changed, 851 insertions(+)
> create mode 100644 drivers/interconnect/qcom/Kconfig
> create mode 100644 drivers/interconnect/qcom/msm8916.c
> create mode 100644 include/linux/interconnect/qcom.h
...
> diff --git a/drivers/interconnect/qcom/msm8916.c
b/drivers/interconnect/qcom/msm8916.c
> new file mode 100644
> index 000000000000..d5b54f8261c8
> --- /dev/null
> +++ b/drivers/interconnect/qcom/msm8916.c
...
> +static int qnoc_probe(struct platform_device *pdev)
> +{
> + const struct qcom_icc_desc *desc;
> + struct qcom_icc_node **qnodes;
> + struct qcom_icc_provider *qp;
> + struct resource *res;
> + struct icc_provider *provider;
> + size_t num_nodes, i;
> + int ret;
> +
> + /* wait for RPM */
> + if (!qcom_icc_rpm_smd_available())
> + return -EPROBE_DEFER;
> +
> + desc = of_device_get_match_data(&pdev->dev);
> + if (!desc)
> + return -EINVAL;
> +
> + qnodes = desc->nodes;
> + num_nodes = desc->num_nodes;
> +
> + qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> + if (!qp)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + qp->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(qp->base))
> + return PTR_ERR(qp->base);
> +
> + qp->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
> + if (IS_ERR(qp->bus_clk))
> + return PTR_ERR(qp->bus_clk);
> +
> + qp->bus_a_clk = devm_clk_get(&pdev->dev, "bus_a_clk");
> + if (IS_ERR(qp->bus_a_clk))
> + return PTR_ERR(qp->bus_a_clk);
> +
> + provider = &qp->provider;
> + provider->dev = &pdev->dev;
> + provider->set = &qcom_icc_set;
> + INIT_LIST_HEAD(&provider->nodes);
> + provider->data = qp;
> +
> + ret = icc_add_provider(provider);
> + if (ret) {
> + dev_err(&pdev->dev, "error adding interconnect
provider\n");
> + return ret;
> + }
> +
> + for (i = 0; i < num_nodes; i++) {
> + struct icc_node *node;
> + int ret;
> + size_t j;
> +
> + node = icc_node_create(qnodes[i]->id);
> + if (IS_ERR(node)) {
> + ret = PTR_ERR(node);
> + goto err;
> + }
> +
> + node->name = qnodes[i]->name;
> + node->data = qnodes[i];
> + icc_node_add(node, provider);
> +
> + dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> + qnodes[i]->name, node->id);
> +
> + /* populate links */
> + for (j = 0; j < qnodes[i]->num_links; j++)
> + if (qnodes[i]->links[j])
> + icc_link_create(node,
qnodes[i]->links[j]);
> +
> + ret = qcom_icc_init(node);
> + if (ret)
> + dev_err(&pdev->dev, "%s init error (%d)\n",
node->name,
> + ret);
Don't you want to call qcom_icc_init before icc_link_create? As soon as
icc_link_create is called, the node is connected to the graph, and
qcom_icc_set can be called.
> + }
> +
> + platform_set_drvdata(pdev, provider);
> +
> + return ret;
> +err:
> + icc_del_provider(provider);
> + return ret;
> +}
> +
> +static int qnoc_remove(struct platform_device *pdev)
> +{
> + struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> + icc_del_provider(provider);
Presumably in the framework nodes and links ought to get cleaned up
somewhere too, right? As it is now, the devm code frees provider when this
device is removed, even though provider is still very connected in the
graph via the nodes and links.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qnoc_of_match[] = {
> + { .compatible = "qcom,msm8916-pnoc", .data = &msm8916_pnoc },
> + { .compatible = "qcom,msm8916-snoc", .data = &msm8916_snoc },
> + { .compatible = "qcom,msm8916-bimc", .data = &msm8916_bimc },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, qnoc_of_match);
> +
> +static struct platform_driver qnoc_driver = {
> + .probe = qnoc_probe,
> + .remove = qnoc_remove,
> + .driver = {
> + .name = "qnoc-msm8916",
> + .of_match_table = qnoc_of_match,
> + },
> +};
> +module_platform_driver(qnoc_driver);
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org>");
> +MODULE_DESCRIPTION("Qualcomm msm8916 NoC driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect/qcom.h
b/include/linux/interconnect/qcom.h
> new file mode 100644
> index 000000000000..2cd378d2f575
> --- /dev/null
> +++ b/include/linux/interconnect/qcom.h
> @@ -0,0 +1,350 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Qualcomm interconnect IDs
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef __QCOM_INTERCONNECT_IDS_H
> +#define __QCOM_INTERCONNECT_IDS_H
> +
> +#define FAB_BIMC 0
> +#define FAB_SYS_NOC 1024
> +#define FAB_MMSS_NOC 2048
> +#define FAB_OCMEM_NOC 3072
> +#define FAB_PERIPH_NOC 4096
> +#define FAB_CONFIG_NOC 5120
> +#define FAB_OCMEM_VNOC 6144
Looks like you're still following that downstream convention of NoCs being
divisible by 1024, masters in 1-512, and slaves in 512-1023, then I don't
know about the 10000s. This was documented somewhere downstream, can you
add that documentation somewhere in here?
-Evan
^ permalink raw reply
* [PATCH v4 7/7] interconnect: Allow endpoints translation via DT
From: Evan Green @ 2018-05-11 21:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-8-georgi.djakov@linaro.org>
On Fri, Mar 9, 2018 at 1:10 PM Georgi Djakov <georgi.djakov@linaro.org>
wrote:
> Currently we support only platform data for specifying the interconnect
> endpoints. As now the endpoints are hard-coded into the consumer driver
> this may leed to complications when a single driver is used by multiple
Nit: s/leed/lead/
-Evan
^ permalink raw reply
* [PATCH] iommu/arm-smmu-v3: Set GBPA to abort all transactions
From: Nate Watterson @ 2018-05-11 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <265a41d4-ec3c-6697-0340-6830807ea10f@arm.com>
Hi Mark,
On 4/12/2018 7:56 AM, Marc Zyngier wrote:
> On 12/04/18 11:17, Robin Murphy wrote:
>> On 11/04/18 17:54, Marc Zyngier wrote:
>>> Hi Sammer,
>>>
>>> On 11/04/18 16:58, Goel, Sameer wrote:
>>>>
>>>>
>>>> On 3/28/2018 9:00 AM, Marc Zyngier wrote:
>>>>> On 2018-03-28 15:39, Timur Tabi wrote:
>>>>>> From: Sameer Goel <sgoel@codeaurora.org>
>>>>>>
>>>>>> Set SMMU_GBPA to abort all incoming translations during the SMMU reset
>>>>>> when SMMUEN==0.
>>>>>>
>>>>>> This prevents a race condition where a stray DMA from the crashed primary
>>>>>> kernel can try to access an IOVA address as an invalid PA when SMMU is
>>>>>> disabled during reset in the crash kernel.
>>>>>>
>>>>>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
>>>>>> ---
>>>>>> ?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++
>>>>>> ?1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>>>>>> index 3f2f1fc68b52..c04a89310c59 100644
>>>>>> --- a/drivers/iommu/arm-smmu-v3.c
>>>>>> +++ b/drivers/iommu/arm-smmu-v3.c
>>>>>> @@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct
>>>>>> arm_smmu_device *smmu, bool bypass)
>>>>>> ???? if (reg & CR0_SMMUEN)
>>>>>> ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
>>>>>>
>>>>>> +??? /*
>>>>>> +???? * Abort all incoming translations. This can happen in a kdump case
>>>>>> +???? * where SMMU is initialized when a prior DMA is pending. Just
>>>>>> +???? * disabling the SMMU in this case might result in writes to invalid
>>>>>> +???? * PAs.
>>>>>> +???? */
>>>>>> +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT);
>>>>>> +??? if (ret) {
>>>>>> +??????? dev_err(smmu->dev, "GBPA not responding to update\n");
>>>>>> +??????? return ret;
>>>>>> +??? }
>>>>>> +
>>>>>> ???? ret = arm_smmu_device_disable(smmu);
>>>>>> ???? if (ret)
>>>>>> ???????? return ret;
>>>>>
>>>>> A tangential question: can we reliably detect that the SMMU already
>>>>> has valid mappings, which would indicate that we're in a pretty bad
>>>>> shape already by the time we set that bit? For all we know, memory
>>>>> could have been corrupted long before we hit this point, and this
>>>>> patch barely narrows the window of opportunity.
>>>>
>>>> :) Yes that is correct. This only covers the kdump scenario. Trying
>>>> to get some reliability when booting up the crash kernel. The system
>>>> is already in a bad state. I don't think that this will happen in a
>>>> normal scenario. But please point me to the GICv3 change and I'll
>>>> have a look.
>>>
>>> See this:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/irqchip-4.17&id=6eb486b66a3094cdcd68dc39c9df3a29d6a51dd5#n3407
>>
>> The nearest equivalent to that is probably the top-level SMMUEN check
>> that we already have (see the diff context above). To go beyond that
>> you'd have to chase the old stream table pointer and scan the whole
>> thing looking for valid contexts, then potentially walk page tables
>> within those contexts to check for live translations if you really
>> wanted to be sure. That would be a hell of a lot of work to do in the
>> boot path.
> Yeah, feels a bit too involved for sanity. I'd simply suggest you taint
> the kernel if you find the SMMU enabled, as you're already on shaky ground.
Finding the SMMU already enabled does not necessarily indicate that
anything catastrophic has occurred.
For instance, to support OSes without an SMMUv3 driver, boot FW may have
enabled the SMMU and installed 1-to-1 mappings for DDR and MSI target
addr(s) to compensate for a MSI-capable master whose default DMA attrs
needed tweaking (ex: non-coherent -> coherent).
If such a configuration warrants tainting the kernel, then we should
similarly check GBPA for attr overrides and taint the kernel if any are
found there.
>
> Thanks,
>
> M.
>
--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 2/2] arm64: dts: renesas: r8a77980: disable EtherAVB
From: Sergei Shtylyov @ 2018-05-11 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5844eef7-0f5e-6366-4d3d-a3a658e7f988@cogentembedded.com>
When adding the R8A77980 EtherAVB device I failed to notice that it does
not have the usual "status" property disabling the described devices in
anticipation that the board device trees enable the devices according to
their needs. This causes the EtherAVB driver to successfully probe despite
e.g. the needed pins not having been configured -- luckily, "eth<n>" device
can't be opened anyway...
Fixes: bf6f90832f81 ("arm64: dts: renesas: r8a77980: add EtherAVB support")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
arch/arm64/boot/dts/renesas/r8a77980.dtsi | 1 +
1 file changed, 1 insertion(+)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
===================================================================
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -244,6 +244,7 @@
phy-mode = "rgmii";
#address-cells = <1>;
#size-cells = <0>;
+ status = "disabled";
};
scif0: serial at e6e60000 {
^ permalink raw reply
* [PATCH 1/2] arm64: dts: renesas: r8a77970: disable EtherAVB
From: Sergei Shtylyov @ 2018-05-11 20:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5844eef7-0f5e-6366-4d3d-a3a658e7f988@cogentembedded.com>
When adding the R8A77970 EtherAVB device I failed to notice that it does
not have the usual "status" property disabling the described devices in
anticipation that the board device trees enable the devices according to
their needs. This causes the EtherAVB driver to successfully probe despite
e.g. the needed pins not having been configured -- luckily, "eth<n>" device
can't be opened anyway...
Fixes: bea2ab136eaa ("arm64: dts: renesas: r8a77970: add EtherAVB support")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
arch/arm64/boot/dts/renesas/r8a77970.dtsi | 1 +
1 file changed, 1 insertion(+)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
===================================================================
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
@@ -473,6 +473,7 @@
iommus = <&ipmmu_rt 3>;
#address-cells = <1>;
#size-cells = <0>;
+ status = "disabled";
};
scif0: serial at e6e60000 {
^ permalink raw reply
* [PATCH 0/2] Disable EtherAVB by default on R8A779{7|8}0 SoCs
From: Sergei Shtylyov @ 2018-05-11 20:18 UTC (permalink / raw)
To: linux-arm-kernel
Hello!
Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's
'renesas-devel-20180511-v4.17-rc4' tag. I'm fixing the issue in the EtherAVB
device nodes in the R8A779{7|8}0 device trees that missed the "status" prop,
usually disabling the SoC devices in anticipation that the board device trees
enable the devices according to their needs. There should be no issues with
the current R8A779{7|8}0 board device trees, as all of them use EtherAVB
anyway, so I'm sending the patches generated against the 'devel' branch...
[1/2] arm64: dts: renesas: r8a77970: disable EtherAVB
[2/2] arm64: dts: renesas: r8a77980: disable EtherAVB
MBR, Sergei
^ permalink raw reply
* [PATCH 00/12] introduce support for early platform drivers
From: Rob Herring @ 2018-05-11 20:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511162028.20616-1-brgl@bgdev.pl>
On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> This series is a follow-up to the RFC[1] posted a couple days ago.
>
> NOTE: this series applies on top of my recent patches[2] that move the previous
> implementation of early platform devices to arch/sh.
>
> Problem:
>
> Certain class of devices, such as timers, certain clock drivers and irq chip
> drivers need to be probed early in the boot sequence. The currently preferred
> approach is using one of the OF_DECLARE() macros. This however does not create
> a platform device which has many drawbacks - such as not being able to use
> devres routines, dev_ log functions or no way of deferring the init OF function
> if some other resources are missing.
I skimmed though this and it doesn't look horrible (how's that for
positive feedback? ;) ). But before going into the details, I think
first there needs to be agreement this is the right direction.
The question does remain though as to whether this class of devices
should be platform drivers. They can't be modules. They can't be
hotplugged. Can they be runtime-pm enabled? So the advantage is ...
I assume that the clock maintainers had some reason to move clocks to
be platform drivers. It's just not clear to me what that was.
> For drivers that use both platform drivers and OF_DECLARE the situation is even
> more complicated as the code needs to take into account that there can possibly
> be no struct device present. For a specific use case that we're having problems
> with, please refer to the recent DaVinci common-clock conversion patches and
> the nasty workaround that this problem implies[3].
So devm_kzalloc will work with this solution? Why did we need
devm_kzalloc in the first place? The clocks can never be removed and
cleaning up on error paths is kind of pointless. The system would be
hosed, right?
> We also used to have an early platform drivers implementation but they were not
> integrated with the linux device model at all - they merely used the same data
> structures. The users could not use devres, defer probe and the early devices
> never became actual platform devices later on.
>
> Proposed solution:
>
> This series aims at solving this problem by (re-)introducing the concept of
> early platform drivers and devices - this time however in a way that seamlessly
> integrates with the existing platform drivers and also offers device-tree
> support.
>
> The idea is to provide a way for users to probe devices early, while already
> being able to use devres, devices resources and properties and also deferred
> probing.
>
> New structures are introduced: the early platform driver contains the
> early_probe callback which has the same signature as regular platform_device
> probe. This callback is called early on. The user can have both the early and
> regular probe speficied or only one of them and they both receive the same
> platform device object as argument. Any device data allocated early will be
> carried over to the normal probe.
>
> The architecture code is responsible for calling early_platform_start() in
> which the early drivers will be registered and devices populated from DT.
Can we really do this in one spot for different devices (clk, timers,
irq). The sequence is all very carefully crafted. Platform specific
hooks is another thing to consider.
> Once the device and kobject mechanisms are ready, all early drivers and devices
> will be converted into real platform drivers and devices. Also: if any of the
> early platform registration functions will be called once early initialization
> is done, these functions will work like regular platform_device/driver ones.
This could leave devices in a weird state. They've successfully probed
early, but then are on the deferred list for normal probe for example.
Rob
^ permalink raw reply
* [linux-next PATCH 4/4] ARM: keystone: k2g: enable micrel and dp83867 phys
From: Murali Karicheri @ 2018-05-11 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526066952-5230-1-git-send-email-m-karicheri2@ti.com>
Enable micrel and dp83867 phys for K2G Boards in keystone_defconfig
to support Network driver on these boards (K2G ICE and GP EVM).
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
arch/arm/configs/keystone_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig
index 2536c23..85cf2fc 100644
--- a/arch/arm/configs/keystone_defconfig
+++ b/arch/arm/configs/keystone_defconfig
@@ -235,3 +235,5 @@ CONFIG_INPUT_GPIO_DECODER=m
CONFIG_GPIO_PCA953X=m
CONFIG_LEDS_TRIGGER_ACTIVITY=y
CONFIG_LEDS_TRIGGER_CPU=y
+CONFIG_MICREL_PHY=y
+CONFIG_DP83867_PHY=y
--
1.9.1
^ permalink raw reply related
* [linux-next PATCH 3/4] ARM: dts: keystone-k2g-ice: Enable netcp network driver
From: Murali Karicheri @ 2018-05-11 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526066952-5230-1-git-send-email-m-karicheri2@ti.com>
This patch adds dt bindings to enable netcp network driver on K2G
ICE boards. This consists of enabling bindings for NSS qmss, pktdma,
2u ethss, mdio, pinmux and netcp devices as well as DP83867 phy.
EVM hardware spec recommends to add 0.25 nsec delay in the tx direction
and 2.25 nsec delay in the rx direction for internal delay in the clock
path to be on the safer side.
The board straps RX_DV/RX_CTRL pin of on board DP83867 phy in mode 1.
The phy data manual disallows this. Add ti,dp83867-rxctrl-strap-quirk
in the phy node to allow software to enable the workaround suggested for
this incorrect strap setting. This ensures proper operation of this PHY.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
arch/arm/boot/dts/keystone-k2g-ice.dts | 59 ++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/arch/arm/boot/dts/keystone-k2g-ice.dts b/arch/arm/boot/dts/keystone-k2g-ice.dts
index d820ed2..2a2d38c 100644
--- a/arch/arm/boot/dts/keystone-k2g-ice.dts
+++ b/arch/arm/boot/dts/keystone-k2g-ice.dts
@@ -7,6 +7,7 @@
/dts-v1/;
#include "keystone-k2g.dtsi"
+#include <dt-bindings/net/ti-dp83867.h>
/ {
compatible = "ti,k2g-ice", "ti,k2g", "ti,keystone";
@@ -281,6 +282,30 @@
K2G_CORE_IOPAD(0x11bc) (BUFFER_CLASS_B | PIN_PULLUP | MUX_MODE3) /* spi2_scsn1.gpio0_102 */
>;
};
+
+ emac_pins: pinmux_emac_pins {
+ pinctrl-single,pins = <
+ K2G_CORE_IOPAD(0x113C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD1.RGMII_RXD1 */
+ K2G_CORE_IOPAD(0x1138) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD2.RGMII_RXD2 */
+ K2G_CORE_IOPAD(0x1134) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD3.RGMII_RXD3 */
+ K2G_CORE_IOPAD(0x1140) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD0.RGMII_RXD0 */
+ K2G_CORE_IOPAD(0x1178) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD0.RGMII_TXD0 */
+ K2G_CORE_IOPAD(0x1174) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD1.RGMII_TXD1 */
+ K2G_CORE_IOPAD(0x1170) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD2.RGMII_TXD2 */
+ K2G_CORE_IOPAD(0x116C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD3.RGMII_TXD3 */
+ K2G_CORE_IOPAD(0x1154) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXCLK.RGMII_TXC */
+ K2G_CORE_IOPAD(0x117C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXEN.RGMII_TXCTL */
+ K2G_CORE_IOPAD(0x1120) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXCLK.RGMII_RXC */
+ K2G_CORE_IOPAD(0x1144) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXDV.RGMII_RXCTL */
+ >;
+ };
+
+ mdio_pins: pinmux_mdio_pins {
+ pinctrl-single,pins = <
+ K2G_CORE_IOPAD(0x118C) (BUFFER_CLASS_B | PULL_DISABLE | MUX_MODE0) /* MDIO_CLK.MDIO_CLK */
+ K2G_CORE_IOPAD(0x1188) (BUFFER_CLASS_B | PULL_DISABLE | MUX_MODE0) /* MDIO_DATA.MDIO_DATA */
+ >;
+ };
};
&uart0 {
@@ -386,3 +411,37 @@
vcc-supply = <&vdd_3v3>;
};
};
+
+&qmss {
+ status = "okay";
+};
+
+&knav_dmas {
+ status = "okay";
+};
+
+&netcp {
+ pinctrl-names = "default";
+ pinctrl-0 = <&emac_pins>;
+ status = "okay";
+};
+
+&mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mdio_pins>;
+ status = "okay";
+ ethphy0: ethernet-phy at 0 {
+ reg = <0>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+ ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
+ ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
+ ti,min-output-impedance;
+ ti,dp83867-rxctrl-strap-quirk;
+ };
+};
+
+&gbe0 {
+ phy-handle = <ðphy0>;
+ phy-mode = "rgmii-id";
+ status = "okay";
+};
--
1.9.1
^ permalink raw reply related
* [linux-next PATCH 2/4] ARM: dts: keystone-k2g-evm: Enable netcp network driver
From: Murali Karicheri @ 2018-05-11 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526066952-5230-1-git-send-email-m-karicheri2@ti.com>
Add dt bindings to enable netcp network driver on K2G GP EVM. This
consists of enabling bindings for nss qmss, pktdma, 2u ethss,
mdio, pinmux and netcp devices.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
arch/arm/boot/dts/keystone-k2g-evm.dts | 53 ++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/arch/arm/boot/dts/keystone-k2g-evm.dts b/arch/arm/boot/dts/keystone-k2g-evm.dts
index 154fdd7..f028a00 100644
--- a/arch/arm/boot/dts/keystone-k2g-evm.dts
+++ b/arch/arm/boot/dts/keystone-k2g-evm.dts
@@ -128,6 +128,30 @@
K2G_CORE_IOPAD(0x1228) (BUFFER_CLASS_B | PIN_PULLDOWN | MUX_MODE1) /* qspicsn3.dcan1rx */
>;
};
+
+ emac_pins: pinmux_emac_pins {
+ pinctrl-single,pins = <
+ K2G_CORE_IOPAD(0x113C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD1.RGMII_RXD1 */
+ K2G_CORE_IOPAD(0x1138) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD2.RGMII_RXD2 */
+ K2G_CORE_IOPAD(0x1134) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD3.RGMII_RXD3 */
+ K2G_CORE_IOPAD(0x1140) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXD0.RGMII_RXD0 */
+ K2G_CORE_IOPAD(0x1178) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD0.RGMII_TXD0 */
+ K2G_CORE_IOPAD(0x1174) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD1.RGMII_TXD1 */
+ K2G_CORE_IOPAD(0x1170) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD2.RGMII_TXD2 */
+ K2G_CORE_IOPAD(0x116C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXD3.RGMII_TXD3 */
+ K2G_CORE_IOPAD(0x1154) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXCLK.RGMII_TXC */
+ K2G_CORE_IOPAD(0x117C) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_TXEN.RGMII_TXCTL */
+ K2G_CORE_IOPAD(0x1120) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXCLK.RGMII_RXC */
+ K2G_CORE_IOPAD(0x1144) (BUFFER_CLASS_D | PULL_DISABLE | MUX_MODE1) /* MII_RXDV.RGMII_RXCTL */
+ >;
+ };
+
+ mdio_pins: pinmux_mdio_pins {
+ pinctrl-single,pins = <
+ K2G_CORE_IOPAD(0x118C) (BUFFER_CLASS_B | PULL_DISABLE | MUX_MODE0) /* MDIO_CLK.MDIO_CLK */
+ K2G_CORE_IOPAD(0x1188) (BUFFER_CLASS_B | PULL_DISABLE | MUX_MODE0) /* MDIO_DATA.MDIO_DATA */
+ >;
+ };
};
&uart0 {
@@ -294,3 +318,32 @@
pinctrl-0 = <&dcan1_pins>;
status = "okay";
};
+
+&qmss {
+ status = "okay";
+};
+
+&knav_dmas {
+ status = "okay";
+};
+
+&mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mdio_pins>;
+ status = "okay";
+ ethphy0: ethernet-phy at 0 {
+ reg = <0>;
+ };
+};
+
+&gbe0 {
+ phy-handle = <ðphy0>;
+ phy-mode = "rgmii-id";
+ status = "okay";
+};
+
+&netcp {
+ pinctrl-names = "default";
+ pinctrl-0 = <&emac_pins>;
+ status = "okay";
+};
--
1.9.1
^ permalink raw reply related
* [linux-next PATCH 1/4] ARM: dts: k2g: add dt bindings to support network driver
From: Murali Karicheri @ 2018-05-11 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526066952-5230-1-git-send-email-m-karicheri2@ti.com>
This patch add dt bindings to support network driver based on
Network Sub System (NSS) found on k2g SoC. This consists of
bindings for netcp node, nss qmss , pktdma, cpsw 2u version
of ethss and mdio.
In order to support transitioning between non-promiscuous
and promiscuous modes in K2G's ethernet switch, netcp ALE
needs to be enabled. Therefore this patch enables K2G's
netcp ALE for that purpose.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/keystone-k2g-netcp.dtsi | 147 ++++++++++++++++++++++++++++++
arch/arm/boot/dts/keystone-k2g.dtsi | 13 +++
2 files changed, 160 insertions(+)
create mode 100644 arch/arm/boot/dts/keystone-k2g-netcp.dtsi
diff --git a/arch/arm/boot/dts/keystone-k2g-netcp.dtsi b/arch/arm/boot/dts/keystone-k2g-netcp.dtsi
new file mode 100644
index 0000000..d0e6a9a
--- /dev/null
+++ b/arch/arm/boot/dts/keystone-k2g-netcp.dtsi
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for K2G Netcp driver
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+qmss: qmss at 4020000 {
+ compatible = "ti,66ak2g-navss-qm";
+ dma-coherent;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ power-domains = <&k2g_pds 0x0018>;
+ clocks = <&k2g_clks 0x0018 0>;
+ clock-names = "nss_vclk";
+ ranges;
+ queue-range = <0 0x80>;
+ linkram0 = <0x4020000 0x7ff>;
+ status = "disabled";
+
+ qmgrs {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ qmgr0 {
+ managed-queues = <0 0x80>;
+ reg = <0x4100000 0x800>,
+ <0x4040000 0x100>,
+ <0x4080000 0x800>,
+ <0x40c0000 0x800>;
+ reg-names = "peek", "config",
+ "region", "push";
+ };
+
+ };
+ queue-pools {
+ qpend {
+ qpend-0 {
+ qrange = <77 8>;
+ interrupts =<0 308 0xf04 0 309 0xf04 0 310 0xf04
+ 0 311 0xf04 0 312 0xf04 0 313 0xf04
+ 0 314 0xf04 0 315 0xf04>;
+ qalloc-by-id;
+ };
+ };
+ general-purpose {
+ gp-0 {
+ qrange = <112 8>;
+ };
+ netcp-tx {
+ qrange = <5 8>;
+ qalloc-by-id;
+ };
+ };
+ };
+
+ descriptor-regions {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ region-12 {
+ id = <12>;
+ region-spec = <1023 128>; /* num_desc desc_size */
+ link-index = <0x400>;
+ };
+ };
+}; /* qmss */
+
+knav_dmas: knav_dmas at 0 {
+ compatible = "ti,keystone-navigator-dma";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+ power-domains = <&k2g_pds 0x0018>;
+ clocks = <&k2g_clks 0x0018 0>;
+ clock-names = "nss_vclk";
+ ranges;
+ ti,navigator-cloud-address = <0x40c0000 0x40c0000 0x40c0000 0x40c0000>;
+
+ dma_gbe: dma_gbe at 0 {
+ reg = <0x4010000 0x100>,
+ <0x4011000 0x2a0>, /* 21 Tx channels */
+ <0x4012000 0x400>, /* 32 Rx channels */
+ <0x4010100 0x80>,
+ <0x4013000 0x400>; /* 32 Rx flows */
+ reg-names = "global", "txchan", "rxchan",
+ "txsched", "rxflow";
+ };
+
+};
+
+netcp: netcp at 4000000 {
+ reg = <0x2620110 0x8>;
+ reg-names = "efuse";
+ compatible = "ti,netcp-1.0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+ power-domains = <&k2g_pds 0x0018>;
+ clocks = <&k2g_clks 0x0018 3>, <&k2g_clks 0x0018 8>;
+ clock-names = "ethss_clk", "cpts";
+
+ /* NetCP address range */
+ ranges = <0 0x4000000 0x1000000>;
+
+ dma-coherent;
+
+ ti,navigator-dmas = <&dma_gbe 0>, <&dma_gbe 5>;
+ ti,navigator-dma-names = "netrx0", "nettx";
+
+ netcp-devices {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ gbe: gbe at 200000 {
+ label = "netcp-gbe";
+ compatible = "ti,netcp-gbe-2";
+ reg = <0x200000 0x20>, <0x220000 0x20000>;
+ enable-ale;
+ tx-queue = <5>;
+ tx-channel = "nettx";
+ cpts-rftclk-sel = <0>;
+ cpts-ext-ts-inputs = <8>;
+
+ interfaces {
+ gbe0: interface-0 {
+ slave-port = <0>;
+ link-interface = <5>;
+ };
+ };
+ };
+ };
+
+ netcp-interfaces {
+ interface-0 {
+ rx-channel = "netrx0";
+ rx-pool = <512 12>;
+ tx-pool = <511 12>;
+ rx-queue-depth = <128 128 0 0>;
+ rx-buffer-size = <1518 4096 0 0>;
+ rx-queue = <77>;
+ tx-completion-queue = <78>;
+ efuse-mac = <1>;
+ netcp-gbe = <&gbe0>;
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/keystone-k2g.dtsi b/arch/arm/boot/dts/keystone-k2g.dtsi
index da78c00..a8324a7 100644
--- a/arch/arm/boot/dts/keystone-k2g.dtsi
+++ b/arch/arm/boot/dts/keystone-k2g.dtsi
@@ -609,5 +609,18 @@
reg = <0x21010000 0x200>;
interrupts = <GIC_SPI 123 IRQ_TYPE_EDGE_RISING>;
};
+
+ mdio: mdio at 4200f00 {
+ compatible = "ti,keystone_mdio", "ti,davinci_mdio";
+ reg = <0x04200f00 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&k2g_clks 0x0018 3>;
+ clock-names = "fck";
+ power-domains = <&k2g_pds 0x0018>;
+ status = "disabled";
+ bus_freq = <2500000>;
+ };
+ #include "keystone-k2g-netcp.dtsi"
};
};
--
1.9.1
^ permalink raw reply related
* [linux-next PATCH 0/4] Enable network driver on K2G ICE and GP EVMs
From: Murali Karicheri @ 2018-05-11 19:29 UTC (permalink / raw)
To: linux-arm-kernel
Now that NetCP driver patches for K2G SoC is merged to linux-next master
this series add patches to enable network driver on K2G ICE and GP EVMs.
Thanks
Applied the patches on top of latest linux-next master, built kernel and
booted up on both EVMs. The logs are below
K2G GP EVM: https://pastebin.ubuntu.com/p/ycZDnZXYPx/
K2G ICE EVM: https://pastebin.ubuntu.com/p/bdCpzgdrXr/
Murali Karicheri (4):
ARM: dts: k2g: add dt bindings to support network driver
ARM: dts: keystone-k2g-evm: Enable netcp network driver
ARM: dts: keystone-k2g-ice: Enable netcp network driver
ARM: keystone: k2g: enable micrel and dp83867 phys
arch/arm/boot/dts/keystone-k2g-evm.dts | 53 +++++++++++
arch/arm/boot/dts/keystone-k2g-ice.dts | 59 ++++++++++++
arch/arm/boot/dts/keystone-k2g-netcp.dtsi | 147 ++++++++++++++++++++++++++++++
arch/arm/boot/dts/keystone-k2g.dtsi | 13 +++
arch/arm/configs/keystone_defconfig | 2 +
5 files changed, 274 insertions(+)
create mode 100644 arch/arm/boot/dts/keystone-k2g-netcp.dtsi
--
1.9.1
^ permalink raw reply
* [PATCH v2 40/40] iommu/arm-smmu-v3: Add support for PCI PASID
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Enable PASID for PCI devices that support it. Unlike PRI, we can't enable
PASID lazily in iommu_sva_device_init(), because it has to be enabled
before ATS, and because we have to allocate substream tables early.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 54 +++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 0edbb8d19579..ac6e69f25893 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2542,6 +2542,52 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
return sid < limit;
}
+static int arm_smmu_enable_pasid(struct arm_smmu_master_data *master)
+{
+ int ret;
+ int features;
+ u8 pasid_bits;
+ int num_pasids;
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return -ENOSYS;
+
+ pdev = to_pci_dev(master->dev);
+
+ features = pci_pasid_features(pdev);
+ if (features < 0)
+ return -ENOSYS;
+
+ num_pasids = pci_max_pasids(pdev);
+ if (num_pasids <= 0)
+ return -ENOSYS;
+
+ pasid_bits = min_t(u8, ilog2(num_pasids), master->smmu->ssid_bits);
+
+ dev_dbg(&pdev->dev, "device supports %#x PASID bits [%s%s]\n", pasid_bits,
+ (features & PCI_PASID_CAP_EXEC) ? "x" : "",
+ (features & PCI_PASID_CAP_PRIV) ? "p" : "");
+
+ ret = pci_enable_pasid(pdev, features);
+ return ret ? ret : pasid_bits;
+}
+
+static void arm_smmu_disable_pasid(struct arm_smmu_master_data *master)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return;
+
+ pdev = to_pci_dev(master->dev);
+
+ if (!pdev->pasid_enabled)
+ return;
+
+ pci_disable_pasid(pdev);
+}
+
static int arm_smmu_enable_ats(struct arm_smmu_master_data *master)
{
size_t stu;
@@ -2712,6 +2758,11 @@ static int arm_smmu_add_device(struct device *dev)
master->ste.can_stall = true;
}
+ /* PASID must be enabled before ATS */
+ ret = arm_smmu_enable_pasid(master);
+ if (ret > 0)
+ master->ssid_bits = ret;
+
arm_smmu_enable_ats(master);
ret = iommu_device_link(&smmu->iommu, dev);
@@ -2740,6 +2791,7 @@ static int arm_smmu_add_device(struct device *dev)
err_disable_ats:
arm_smmu_disable_ats(master);
+ arm_smmu_disable_pasid(master);
err_free_master:
kfree(master);
@@ -2769,7 +2821,9 @@ static void arm_smmu_remove_device(struct device *dev)
arm_smmu_remove_master(smmu, master);
iommu_device_unlink(&smmu->iommu, dev);
arm_smmu_disable_pri(master);
+ /* PASID must be disabled after ATS */
arm_smmu_disable_ats(master);
+ arm_smmu_disable_pasid(master);
kfree(master);
iommu_fwspec_free(dev);
}
--
2.17.0
^ permalink raw reply related
* [PATCH v2 39/40] iommu/arm-smmu-v3: Add support for PRI
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
For PCI devices that support it, enable the PRI capability and handle PRI
Page Requests with the generic fault handler. It is enabled on demand by
iommu_sva_device_init().
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2:
* Terminate the page request and disable PRI if no handler is registered
* Enable and disable PRI in sva_device_init/shutdown, instead of
add/remove_device
---
drivers/iommu/arm-smmu-v3.c | 192 +++++++++++++++++++++++++++---------
1 file changed, 145 insertions(+), 47 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 6cb69ace371b..0edbb8d19579 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -248,6 +248,7 @@
#define STRTAB_STE_1_S1COR GENMASK_ULL(5, 4)
#define STRTAB_STE_1_S1CSH GENMASK_ULL(7, 6)
+#define STRTAB_STE_1_PPAR (1UL << 18)
#define STRTAB_STE_1_S1STALLD (1UL << 27)
#define STRTAB_STE_1_EATS GENMASK_ULL(29, 28)
@@ -309,6 +310,9 @@
#define CMDQ_PRI_0_SID GENMASK_ULL(63, 32)
#define CMDQ_PRI_1_GRPID GENMASK_ULL(8, 0)
#define CMDQ_PRI_1_RESP GENMASK_ULL(13, 12)
+#define CMDQ_PRI_1_RESP_FAILURE FIELD_PREP(CMDQ_PRI_1_RESP, 0UL)
+#define CMDQ_PRI_1_RESP_INVALID FIELD_PREP(CMDQ_PRI_1_RESP, 1UL)
+#define CMDQ_PRI_1_RESP_SUCCESS FIELD_PREP(CMDQ_PRI_1_RESP, 2UL)
#define CMDQ_RESUME_0_SID GENMASK_ULL(63, 32)
#define CMDQ_RESUME_0_ACTION_RETRY (1UL << 12)
@@ -383,12 +387,6 @@ module_param_named(disable_ats_check, disable_ats_check, bool, S_IRUGO);
MODULE_PARM_DESC(disable_ats_check,
"By default, the SMMU checks whether each incoming transaction marked as translated is allowed by the stream configuration. This option disables the check.");
-enum pri_resp {
- PRI_RESP_DENY = 0,
- PRI_RESP_FAIL = 1,
- PRI_RESP_SUCC = 2,
-};
-
enum arm_smmu_msi_index {
EVTQ_MSI_INDEX,
GERROR_MSI_INDEX,
@@ -471,7 +469,7 @@ struct arm_smmu_cmdq_ent {
u32 sid;
u32 ssid;
u16 grpid;
- enum pri_resp resp;
+ enum page_response_code resp;
} pri;
#define CMDQ_OP_RESUME 0x44
@@ -556,6 +554,7 @@ struct arm_smmu_strtab_ent {
struct arm_smmu_s2_cfg *s2_cfg;
bool can_stall;
+ bool prg_resp_needs_ssid;
};
struct arm_smmu_strtab_cfg {
@@ -907,14 +906,18 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
cmd[0] |= FIELD_PREP(CMDQ_PRI_0_SID, ent->pri.sid);
cmd[1] |= FIELD_PREP(CMDQ_PRI_1_GRPID, ent->pri.grpid);
switch (ent->pri.resp) {
- case PRI_RESP_DENY:
- case PRI_RESP_FAIL:
- case PRI_RESP_SUCC:
+ case IOMMU_PAGE_RESP_FAILURE:
+ cmd[1] |= CMDQ_PRI_1_RESP_FAILURE;
+ break;
+ case IOMMU_PAGE_RESP_INVALID:
+ cmd[1] |= CMDQ_PRI_1_RESP_INVALID;
+ break;
+ case IOMMU_PAGE_RESP_SUCCESS:
+ cmd[1] |= CMDQ_PRI_1_RESP_SUCCESS;
break;
default:
return -EINVAL;
}
- cmd[1] |= FIELD_PREP(CMDQ_PRI_1_RESP, ent->pri.resp);
break;
case CMDQ_OP_RESUME:
cmd[0] |= FIELD_PREP(CMDQ_RESUME_0_SID, ent->resume.sid);
@@ -1114,8 +1117,15 @@ static int arm_smmu_page_response(struct device *dev,
cmd.resume.sid = sid;
cmd.resume.stag = resp->page_req_group_id;
cmd.resume.resp = resp->resp_code;
+ } else if (master->can_fault) {
+ cmd.opcode = CMDQ_OP_PRI_RESP;
+ cmd.substream_valid = resp->pasid_present &&
+ master->ste.prg_resp_needs_ssid;
+ cmd.pri.sid = sid;
+ cmd.pri.ssid = resp->pasid;
+ cmd.pri.grpid = resp->page_req_group_id;
+ cmd.pri.resp = resp->resp_code;
} else {
- /* TODO: put PRI response here */
return -ENODEV;
}
@@ -1236,6 +1246,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
FIELD_PREP(STRTAB_STE_1_STRW, strw));
+ if (ste->prg_resp_needs_ssid)
+ dst[1] |= STRTAB_STE_1_PPAR;
+
if (smmu->features & ARM_SMMU_FEAT_STALLS &&
!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE) &&
!ste->can_stall)
@@ -1471,39 +1484,54 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
static void arm_smmu_handle_ppr(struct arm_smmu_device *smmu, u64 *evt)
{
- u32 sid, ssid;
- u16 grpid;
- bool ssv, last;
-
- sid = FIELD_GET(PRIQ_0_SID, evt[0]);
- ssv = FIELD_GET(PRIQ_0_SSID_V, evt[0]);
- ssid = ssv ? FIELD_GET(PRIQ_0_SSID, evt[0]) : 0;
- last = FIELD_GET(PRIQ_0_PRG_LAST, evt[0]);
- grpid = FIELD_GET(PRIQ_1_PRG_IDX, evt[1]);
-
- dev_info(smmu->dev, "unexpected PRI request received:\n");
- dev_info(smmu->dev,
- "\tsid 0x%08x.0x%05x: [%u%s] %sprivileged %s%s%s access at iova 0x%016llx\n",
- sid, ssid, grpid, last ? "L" : "",
- evt[0] & PRIQ_0_PERM_PRIV ? "" : "un",
- evt[0] & PRIQ_0_PERM_READ ? "R" : "",
- evt[0] & PRIQ_0_PERM_WRITE ? "W" : "",
- evt[0] & PRIQ_0_PERM_EXEC ? "X" : "",
- evt[1] & PRIQ_1_ADDR_MASK);
-
- if (last) {
- struct arm_smmu_cmdq_ent cmd = {
- .opcode = CMDQ_OP_PRI_RESP,
- .substream_valid = ssv,
- .pri = {
- .sid = sid,
- .ssid = ssid,
- .grpid = grpid,
- .resp = PRI_RESP_DENY,
- },
+ u32 sid = FIELD_PREP(PRIQ_0_SID, evt[0]);
+
+ struct arm_smmu_master_data *master;
+ struct iommu_fault_event fault = {
+ .type = IOMMU_FAULT_PAGE_REQ,
+ .last_req = FIELD_GET(PRIQ_0_PRG_LAST, evt[0]),
+ .pasid_valid = FIELD_GET(PRIQ_0_SSID_V, evt[0]),
+ .pasid = FIELD_GET(PRIQ_0_SSID, evt[0]),
+ .page_req_group_id = FIELD_GET(PRIQ_1_PRG_IDX, evt[1]),
+ .addr = evt[1] & PRIQ_1_ADDR_MASK,
+ };
+
+ if (evt[0] & PRIQ_0_PERM_READ)
+ fault.prot |= IOMMU_FAULT_READ;
+ if (evt[0] & PRIQ_0_PERM_WRITE)
+ fault.prot |= IOMMU_FAULT_WRITE;
+ if (evt[0] & PRIQ_0_PERM_EXEC)
+ fault.prot |= IOMMU_FAULT_EXEC;
+ if (evt[0] & PRIQ_0_PERM_PRIV)
+ fault.prot |= IOMMU_FAULT_PRIV;
+
+ /* Discard Stop PASID marker, it isn't used */
+ if (!(fault.prot & (IOMMU_FAULT_READ|IOMMU_FAULT_WRITE)) &&
+ fault.last_req)
+ return;
+
+ master = arm_smmu_find_master(smmu, sid);
+ if (WARN_ON(!master))
+ return;
+
+ if (iommu_report_device_fault(master->dev, &fault)) {
+ /*
+ * No handler registered, so subsequent faults won't produce
+ * better results. Try to disable PRI.
+ */
+ struct page_response_msg page_response = {
+ .addr = fault.addr,
+ .pasid = fault.pasid,
+ .pasid_present = fault.pasid_valid,
+ .page_req_group_id = fault.page_req_group_id,
+ .resp_code = IOMMU_PAGE_RESP_FAILURE,
};
- arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+ dev_warn(master->dev,
+ "PPR 0x%x:0x%llx 0x%x: nobody cared, disabling PRI\n",
+ fault.pasid_valid ? fault.pasid : 0, fault.addr,
+ fault.prot);
+ arm_smmu_page_response(master->dev, &page_response);
}
}
@@ -1529,6 +1557,11 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
}
if (queue_sync_prod(q) == -EOVERFLOW)
+ /*
+ * TODO: flush pending faults, since the SMMU might have
+ * auto-responded to the Last request of a pending
+ * group
+ */
dev_err(smmu->dev, "PRIQ overflow detected -- requests lost\n");
} while (!queue_empty(q));
@@ -1577,7 +1610,8 @@ static int arm_smmu_flush_queues(void *cookie, struct device *dev)
master = dev->iommu_fwspec->iommu_priv;
if (master->ste.can_stall)
arm_smmu_flush_queue(smmu, &smmu->evtq.q, "evtq");
- /* TODO: add support for PRI */
+ else if (master->can_fault)
+ arm_smmu_flush_queue(smmu, &smmu->priq.q, "priq");
return 0;
}
@@ -2301,6 +2335,59 @@ arm_smmu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
return ops->iova_to_phys(ops, iova);
}
+static int arm_smmu_enable_pri(struct arm_smmu_master_data *master)
+{
+ int ret, pos;
+ struct pci_dev *pdev;
+ /*
+ * TODO: find a good inflight PPR number. We should divide the PRI queue
+ * by the number of PRI-capable devices, but it's impossible to know
+ * about current and future (hotplugged) devices. So we're at risk of
+ * dropping PPRs (and leaking pending requests in the FQ).
+ */
+ size_t max_inflight_pprs = 16;
+ struct arm_smmu_device *smmu = master->smmu;
+
+ if (!(smmu->features & ARM_SMMU_FEAT_PRI) || !dev_is_pci(master->dev))
+ return -ENOSYS;
+
+ pdev = to_pci_dev(master->dev);
+
+ ret = pci_reset_pri(pdev);
+ if (ret)
+ return ret;
+
+ ret = pci_enable_pri(pdev, max_inflight_pprs);
+ if (ret) {
+ dev_err(master->dev, "cannot enable PRI: %d\n", ret);
+ return ret;
+ }
+
+ master->can_fault = true;
+ master->ste.prg_resp_needs_ssid = pci_prg_resp_requires_prefix(pdev);
+
+ dev_dbg(master->dev, "enabled PRI\n");
+
+ return 0;
+}
+
+static void arm_smmu_disable_pri(struct arm_smmu_master_data *master)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return;
+
+ pdev = to_pci_dev(master->dev);
+
+ if (!pdev->pri_enabled)
+ return;
+
+ pci_disable_pri(pdev);
+ dev_dbg(master->dev, "disabled PRI\n");
+ master->can_fault = false;
+}
+
static int arm_smmu_sva_init(struct device *dev, struct iommu_sva_param *param)
{
int ret;
@@ -2314,11 +2401,15 @@ static int arm_smmu_sva_init(struct device *dev, struct iommu_sva_param *param)
return -EINVAL;
if (param->features & IOMMU_SVA_FEAT_IOPF) {
- if (!master->can_fault)
- return -EINVAL;
+ arm_smmu_enable_pri(master);
+ if (!master->can_fault) {
+ ret = -ENODEV;
+ goto err_disable_pri;
+ }
+
ret = iopf_queue_add_device(master->smmu->iopf_queue, dev);
if (ret)
- return ret;
+ goto err_disable_pri;
}
if (!param->max_pasid)
@@ -2329,11 +2420,17 @@ static int arm_smmu_sva_init(struct device *dev, struct iommu_sva_param *param)
param->max_pasid = min(param->max_pasid, (1U << master->ssid_bits) - 1);
return 0;
+
+err_disable_pri:
+ arm_smmu_disable_pri(master);
+
+ return ret;
}
static void arm_smmu_sva_shutdown(struct device *dev,
struct iommu_sva_param *param)
{
+ arm_smmu_disable_pri(dev->iommu_fwspec->iommu_priv);
iopf_queue_remove_device(dev);
}
@@ -2671,6 +2768,7 @@ static void arm_smmu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
arm_smmu_remove_master(smmu, master);
iommu_device_unlink(&smmu->iommu, dev);
+ arm_smmu_disable_pri(master);
arm_smmu_disable_ats(master);
kfree(master);
iommu_fwspec_free(dev);
--
2.17.0
^ permalink raw reply related
* [PATCH v2 38/40] PCI: Make "PRG Response PASID Required" handling common
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
The PASID ECN to the PCIe spec added a bit in the PRI status register that
allows a Function to declare whether a PRG Response should contain the
PASID prefix or not.
Move the helper that accesses it from amd_iommu into the PCI subsystem,
renaming it to be consistent with the current PCI Express specification
(PRPR - PRG Response PASID Required).
Cc: bhelgaas at google.com
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/amd_iommu.c | 19 +------------------
drivers/pci/ats.c | 17 +++++++++++++++++
include/linux/pci-ats.h | 8 ++++++++
include/uapi/linux/pci_regs.h | 1 +
4 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8fb8c737fffe..ac0d97fa514f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2043,23 +2043,6 @@ static int pdev_iommuv2_enable(struct pci_dev *pdev)
return ret;
}
-/* FIXME: Move this to PCI code */
-#define PCI_PRI_TLP_OFF (1 << 15)
-
-static bool pci_pri_tlp_required(struct pci_dev *pdev)
-{
- u16 status;
- int pos;
-
- pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
- if (!pos)
- return false;
-
- pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
-
- return (status & PCI_PRI_TLP_OFF) ? true : false;
-}
-
/*
* If a device is not yet associated with a domain, this function
* assigns it visible for the hardware
@@ -2088,7 +2071,7 @@ static int attach_device(struct device *dev,
dev_data->ats.enabled = true;
dev_data->ats.qdep = pci_ats_queue_depth(pdev);
- dev_data->pri_tlp = pci_pri_tlp_required(pdev);
+ dev_data->pri_tlp = pci_prg_resp_requires_prefix(pdev);
}
} else if (amd_iommu_iotlb_sup &&
pci_enable_ats(pdev, PAGE_SHIFT) == 0) {
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 89305b569d3d..dc743e2fae7d 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -388,3 +388,20 @@ int pci_max_pasids(struct pci_dev *pdev)
}
EXPORT_SYMBOL_GPL(pci_max_pasids);
#endif /* CONFIG_PCI_PASID */
+
+#if defined(CONFIG_PCI_PASID) && defined(CONFIG_PCI_PRI)
+bool pci_prg_resp_requires_prefix(struct pci_dev *pdev)
+{
+ u16 status;
+ int pos;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
+ if (!pos)
+ return false;
+
+ pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
+
+ return !!(status & PCI_PRI_STATUS_PRPR);
+}
+EXPORT_SYMBOL_GPL(pci_prg_resp_requires_prefix);
+#endif /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
index 7c4b8e27268c..1825ca2c9bf4 100644
--- a/include/linux/pci-ats.h
+++ b/include/linux/pci-ats.h
@@ -68,5 +68,13 @@ static inline int pci_max_pasids(struct pci_dev *pdev)
#endif /* CONFIG_PCI_PASID */
+#if defined(CONFIG_PCI_PASID) && defined(CONFIG_PCI_PRI)
+bool pci_prg_resp_requires_prefix(struct pci_dev *pdev);
+#else
+static inline bool pci_prg_resp_requires_prefix(struct pci_dev *pdev)
+{
+ return false;
+}
+#endif /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */
#endif /* LINUX_PCI_ATS_H*/
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 103ba797a8f3..f9a11a13131c 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -871,6 +871,7 @@
#define PCI_PRI_STATUS_RF 0x001 /* Response Failure */
#define PCI_PRI_STATUS_UPRGI 0x002 /* Unexpected PRG index */
#define PCI_PRI_STATUS_STOPPED 0x100 /* PRI Stopped */
+#define PCI_PRI_STATUS_PRPR 0x8000 /* PRG Response requires PASID prefix */
#define PCI_PRI_MAX_REQ 0x08 /* PRI max reqs supported */
#define PCI_PRI_ALLOC_REQ 0x0c /* PRI max reqs allowed */
#define PCI_EXT_CAP_PRI_SIZEOF 16
--
2.17.0
^ permalink raw reply related
* [PATCH v2 37/40] iommu/arm-smmu-v3: Disable tagged pointers
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
The ARM architecture has a "Top Byte Ignore" (TBI) option that makes the
MMU mask out bits [63:56] of an address, allowing a userspace application
to store data in its pointers. This option is incompatible with PCI ATS.
If TBI is enabled in the SMMU and userspace triggers DMA transactions on
tagged pointers, the endpoint might create ATC entries for addresses that
include a tag. Software would then have to send ATC invalidation packets
for each 255 possible alias of an address, or just wipe the whole address
space. This is not a viable option, so disable TBI.
The impact of this change is unclear, since there are very few users of
tagged pointers, much less SVA. But the requirement introduced by this
patch doesn't seem excessive: a userspace application using both tagged
pointers and SVA should now sanitize addresses (clear the tag) before
using them for device DMA.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3-context.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3-context.c b/drivers/iommu/arm-smmu-v3-context.c
index bdc9bfd1f35d..22e7b80a7682 100644
--- a/drivers/iommu/arm-smmu-v3-context.c
+++ b/drivers/iommu/arm-smmu-v3-context.c
@@ -200,7 +200,6 @@ static u64 arm_smmu_cpu_tcr_to_cd(struct arm_smmu_context_cfg *cfg, u64 tcr)
val |= ARM_SMMU_TCR2CD(tcr, EPD0);
val |= ARM_SMMU_TCR2CD(tcr, EPD1);
val |= ARM_SMMU_TCR2CD(tcr, IPS);
- val |= ARM_SMMU_TCR2CD(tcr, TBI0);
if (cfg->hw_access)
val |= ARM_SMMU_TCR2CD(tcr, HA);
--
2.17.0
^ permalink raw reply related
* [PATCH v2 36/40] iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
The core calls us when an mm is modified. Perform the required ATC
invalidations.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 7034b0bdcbdf..6cb69ace371b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1735,6 +1735,15 @@ static int arm_smmu_atc_inv_master_all(struct arm_smmu_master_data *master,
return arm_smmu_atc_inv_master(master, &cmd);
}
+static int arm_smmu_atc_inv_master_range(struct arm_smmu_master_data *master,
+ int ssid, unsigned long iova, size_t size)
+{
+ struct arm_smmu_cmdq_ent cmd;
+
+ arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
+ return arm_smmu_atc_inv_master(master, &cmd);
+}
+
static size_t
arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
unsigned long iova, size_t size)
@@ -2389,11 +2398,12 @@ static void arm_smmu_mm_detach(struct iommu_domain *domain, struct device *dev,
struct arm_smmu_mm *smmu_mm = to_smmu_mm(io_mm);
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct iommu_pasid_table_ops *ops = smmu_domain->s1_cfg.ops;
+ struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
if (detach_domain)
ops->clear_entry(ops, io_mm->pasid, smmu_mm->cd);
- /* TODO: Invalidate ATC. */
+ arm_smmu_atc_inv_master_all(master, io_mm->pasid);
/* TODO: Invalidate all mappings if last and not DVM. */
}
@@ -2401,8 +2411,10 @@ static void arm_smmu_mm_invalidate(struct iommu_domain *domain,
struct device *dev, struct io_mm *io_mm,
unsigned long iova, size_t size)
{
+ struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
+
+ arm_smmu_atc_inv_master_range(master, io_mm->pasid, iova, size);
/*
- * TODO: Invalidate ATC.
* TODO: Invalidate mapping if not DVM
*/
}
--
2.17.0
^ permalink raw reply related
* [PATCH v2 35/40] iommu/arm-smmu-v3: Add support for PCI ATS
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
PCIe devices can implement their own TLB, named Address Translation Cache
(ATC). Enable Address Translation Service (ATS) for devices that support
it and send them invalidation requests whenever we invalidate the IOTLBs.
Range calculation
-----------------
The invalidation packet itself is a bit awkward: range must be naturally
aligned, which means that the start address is a multiple of the range
size. In addition, the size must be a power of two number of 4k pages. We
have a few options to enforce this constraint:
(1) Find the smallest naturally aligned region that covers the requested
range. This is simple to compute and only takes one ATC_INV, but it
will spill on lots of neighbouring ATC entries.
(2) Align the start address to the region size (rounded up to a power of
two), and send a second invalidation for the next range of the same
size. Still not great, but reduces spilling.
(3) Cover the range exactly with the smallest number of naturally aligned
regions. This would be interesting to implement but as for (2),
requires multiple ATC_INV.
As I suspect ATC invalidation packets will be a very scarce resource, I'll
go with option (1) for now, and only send one big invalidation. We can
move to (2), which is both easier to read and more gentle with the ATC,
once we've observed on real systems that we can send multiple smaller
Invalidation Requests for roughly the same price as a single big one.
Note that with io-pgtable, the unmap function is called for each page, so
this doesn't matter. The problem shows up when sharing page tables with
the MMU.
Timeout
-------
ATC invalidation is allowed to take up to 90 seconds, according to the
PCIe spec, so it is possible to hit the SMMU command queue timeout during
normal operations.
Some SMMU implementations will raise a CERROR_ATC_INV_SYNC when a CMD_SYNC
fails because of an ATC invalidation. Some will just abort the CMD_SYNC.
Others might let CMD_SYNC complete and have an asynchronous IMPDEF
mechanism to record the error. When we receive a CERROR_ATC_INV_SYNC, we
could retry sending all ATC_INV since last successful CMD_SYNC. When a
CMD_SYNC fails without CERROR_ATC_INV_SYNC, we could retry sending *all*
commands since last successful CMD_SYNC.
We cannot afford to wait 90 seconds in iommu_unmap, let alone MMU
notifiers. So we'd have to introduce a more clever system if this timeout
becomes a problem, like keeping hold of mappings and invalidating in the
background. Implementing safe delayed invalidations is a very complex
problem and deserves a series of its own. We'll assess whether more work
is needed to properly handle ATC invalidation timeouts once this code runs
on real hardware.
Misc
----
I didn't put ATC and TLB invalidations in the same functions for three
reasons:
* TLB invalidation by range is batched and committed with a single sync.
Batching ATC invalidation is inconvenient, endpoints limit the number of
inflight invalidations. We'd have to count the number of invalidations
queued and send a sync periodically. In addition, I suspect we always
need a sync between TLB and ATC invalidation for the same page.
* Doing ATC invalidation outside tlb_inv_range also allows to send less
requests, since TLB invalidations are done per page or block, while ATC
invalidations target IOVA ranges.
* TLB invalidation by context is performed when freeing the domain, at
which point there isn't any device attached anymore.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: display error if ats is supported but cannot be enabled
---
drivers/iommu/arm-smmu-v3.c | 225 +++++++++++++++++++++++++++++++++++-
1 file changed, 219 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 8a6a799ba04a..7034b0bdcbdf 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -39,6 +39,7 @@
#include <linux/of_iommu.h>
#include <linux/of_platform.h>
#include <linux/pci.h>
+#include <linux/pci-ats.h>
#include <linux/platform_device.h>
#include <linux/sched/mm.h>
@@ -103,6 +104,7 @@
#define IDR5_VAX_52_BIT 1
#define ARM_SMMU_CR0 0x20
+#define CR0_ATSCHK (1 << 4)
#define CR0_CMDQEN (1 << 3)
#define CR0_EVTQEN (1 << 2)
#define CR0_PRIQEN (1 << 1)
@@ -277,6 +279,7 @@
#define CMDQ_ERR_CERROR_NONE_IDX 0
#define CMDQ_ERR_CERROR_ILL_IDX 1
#define CMDQ_ERR_CERROR_ABT_IDX 2
+#define CMDQ_ERR_CERROR_ATC_INV_IDX 3
#define CMDQ_0_OP GENMASK_ULL(7, 0)
#define CMDQ_0_SSV (1UL << 11)
@@ -296,6 +299,12 @@
#define CMDQ_TLBI_1_VA_MASK GENMASK_ULL(63, 12)
#define CMDQ_TLBI_1_IPA_MASK GENMASK_ULL(51, 12)
+#define CMDQ_ATC_0_SSID GENMASK_ULL(31, 12)
+#define CMDQ_ATC_0_SID GENMASK_ULL(63, 32)
+#define CMDQ_ATC_0_GLOBAL (1UL << 9)
+#define CMDQ_ATC_1_SIZE GENMASK_ULL(5, 0)
+#define CMDQ_ATC_1_ADDR_MASK GENMASK_ULL(63, 12)
+
#define CMDQ_PRI_0_SSID GENMASK_ULL(31, 12)
#define CMDQ_PRI_0_SID GENMASK_ULL(63, 32)
#define CMDQ_PRI_1_GRPID GENMASK_ULL(8, 0)
@@ -369,6 +378,11 @@ module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
MODULE_PARM_DESC(disable_bypass,
"Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
+static bool disable_ats_check;
+module_param_named(disable_ats_check, disable_ats_check, bool, S_IRUGO);
+MODULE_PARM_DESC(disable_ats_check,
+ "By default, the SMMU checks whether each incoming transaction marked as translated is allowed by the stream configuration. This option disables the check.");
+
enum pri_resp {
PRI_RESP_DENY = 0,
PRI_RESP_FAIL = 1,
@@ -442,6 +456,16 @@ struct arm_smmu_cmdq_ent {
u64 addr;
} tlbi;
+ #define CMDQ_OP_ATC_INV 0x40
+ #define ATC_INV_SIZE_ALL 52
+ struct {
+ u32 sid;
+ u32 ssid;
+ u64 addr;
+ u8 size;
+ bool global;
+ } atc;
+
#define CMDQ_OP_PRI_RESP 0x41
struct {
u32 sid;
@@ -869,6 +893,14 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
case CMDQ_OP_TLBI_EL2_ASID:
cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_ASID, ent->tlbi.asid);
break;
+ case CMDQ_OP_ATC_INV:
+ cmd[0] |= FIELD_PREP(CMDQ_0_SSV, ent->substream_valid);
+ cmd[0] |= FIELD_PREP(CMDQ_ATC_0_GLOBAL, ent->atc.global);
+ cmd[0] |= FIELD_PREP(CMDQ_ATC_0_SSID, ent->atc.ssid);
+ cmd[0] |= FIELD_PREP(CMDQ_ATC_0_SID, ent->atc.sid);
+ cmd[1] |= FIELD_PREP(CMDQ_ATC_1_SIZE, ent->atc.size);
+ cmd[1] |= ent->atc.addr & CMDQ_ATC_1_ADDR_MASK;
+ break;
case CMDQ_OP_PRI_RESP:
cmd[0] |= FIELD_PREP(CMDQ_0_SSV, ent->substream_valid);
cmd[0] |= FIELD_PREP(CMDQ_PRI_0_SSID, ent->pri.ssid);
@@ -922,6 +954,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
[CMDQ_ERR_CERROR_NONE_IDX] = "No error",
[CMDQ_ERR_CERROR_ILL_IDX] = "Illegal command",
[CMDQ_ERR_CERROR_ABT_IDX] = "Abort on command fetch",
+ [CMDQ_ERR_CERROR_ATC_INV_IDX] = "ATC invalidate timeout",
};
int i;
@@ -941,6 +974,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
dev_err(smmu->dev, "retrying command fetch\n");
case CMDQ_ERR_CERROR_NONE_IDX:
return;
+ case CMDQ_ERR_CERROR_ATC_INV_IDX:
+ /*
+ * ATC Invalidation Completion timeout. CONS is still pointing
+ * at the CMD_SYNC. Attempt to complete other pending commands
+ * by repeating the CMD_SYNC, though we might well end up back
+ * here since the ATC invalidation may still be pending.
+ */
+ return;
case CMDQ_ERR_CERROR_ILL_IDX:
/* Fallthrough */
default:
@@ -1193,9 +1234,6 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
-#ifdef CONFIG_PCI_ATS
- FIELD_PREP(STRTAB_STE_1_EATS, STRTAB_STE_1_EATS_TRANS) |
-#endif
FIELD_PREP(STRTAB_STE_1_STRW, strw));
if (smmu->features & ARM_SMMU_FEAT_STALLS &&
@@ -1225,6 +1263,10 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S2_TRANS);
}
+ if (IS_ENABLED(CONFIG_PCI_ATS))
+ dst[1] |= cpu_to_le64(FIELD_PREP(STRTAB_STE_1_EATS,
+ STRTAB_STE_1_EATS_TRANS));
+
arm_smmu_sync_ste_for_sid(smmu, sid);
dst[0] = cpu_to_le64(val);
arm_smmu_sync_ste_for_sid(smmu, sid);
@@ -1613,6 +1655,104 @@ static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
return IRQ_WAKE_THREAD;
}
+/* ATS invalidation */
+static bool arm_smmu_master_has_ats(struct arm_smmu_master_data *master)
+{
+ return dev_is_pci(master->dev) && to_pci_dev(master->dev)->ats_enabled;
+}
+
+static void
+arm_smmu_atc_inv_to_cmd(int ssid, unsigned long iova, size_t size,
+ struct arm_smmu_cmdq_ent *cmd)
+{
+ size_t log2_span;
+ size_t span_mask;
+ /* ATC invalidates are always on 4096 bytes pages */
+ size_t inval_grain_shift = 12;
+ unsigned long page_start, page_end;
+
+ *cmd = (struct arm_smmu_cmdq_ent) {
+ .opcode = CMDQ_OP_ATC_INV,
+ .substream_valid = !!ssid,
+ .atc.ssid = ssid,
+ };
+
+ if (!size) {
+ cmd->atc.size = ATC_INV_SIZE_ALL;
+ return;
+ }
+
+ page_start = iova >> inval_grain_shift;
+ page_end = (iova + size - 1) >> inval_grain_shift;
+
+ /*
+ * Find the smallest power of two that covers the range. Most
+ * significant differing bit between start and end address indicates the
+ * required span, ie. fls(start ^ end). For example:
+ *
+ * We want to invalidate pages [8; 11]. This is already the ideal range:
+ * x = 0b1000 ^ 0b1011 = 0b11
+ * span = 1 << fls(x) = 4
+ *
+ * To invalidate pages [7; 10], we need to invalidate [0; 15]:
+ * x = 0b0111 ^ 0b1010 = 0b1101
+ * span = 1 << fls(x) = 16
+ */
+ log2_span = fls_long(page_start ^ page_end);
+ span_mask = (1ULL << log2_span) - 1;
+
+ page_start &= ~span_mask;
+
+ cmd->atc.addr = page_start << inval_grain_shift;
+ cmd->atc.size = log2_span;
+}
+
+static int arm_smmu_atc_inv_master(struct arm_smmu_master_data *master,
+ struct arm_smmu_cmdq_ent *cmd)
+{
+ int i;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ if (!arm_smmu_master_has_ats(master))
+ return 0;
+
+ for (i = 0; i < fwspec->num_ids; i++) {
+ cmd->atc.sid = fwspec->ids[i];
+ arm_smmu_cmdq_issue_cmd(master->smmu, cmd);
+ }
+
+ arm_smmu_cmdq_issue_sync(master->smmu);
+
+ return 0;
+}
+
+static int arm_smmu_atc_inv_master_all(struct arm_smmu_master_data *master,
+ int ssid)
+{
+ struct arm_smmu_cmdq_ent cmd;
+
+ arm_smmu_atc_inv_to_cmd(ssid, 0, 0, &cmd);
+ return arm_smmu_atc_inv_master(master, &cmd);
+}
+
+static size_t
+arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
+ unsigned long iova, size_t size)
+{
+ unsigned long flags;
+ struct arm_smmu_cmdq_ent cmd;
+ struct arm_smmu_master_data *master;
+
+ arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, list)
+ arm_smmu_atc_inv_master(master, &cmd);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+ return size;
+}
+
/* IO_PGTABLE API */
static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
{
@@ -2026,6 +2166,8 @@ static void arm_smmu_detach_dev(struct device *dev)
if (smmu_domain) {
__iommu_sva_unbind_dev_all(dev);
+ arm_smmu_atc_inv_master_all(master, 0);
+
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
list_del(&master->list);
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
@@ -2113,12 +2255,19 @@ static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
static size_t
arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
{
- struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+ int ret;
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
if (!ops)
return 0;
- return ops->unmap(ops, iova, size);
+ ret = ops->unmap(ops, iova, size);
+
+ if (ret && smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS)
+ ret = arm_smmu_atc_inv_domain(smmu_domain, 0, iova, size);
+
+ return ret;
}
static void arm_smmu_iotlb_sync(struct iommu_domain *domain)
@@ -2284,6 +2433,54 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
return sid < limit;
}
+static int arm_smmu_enable_ats(struct arm_smmu_master_data *master)
+{
+ size_t stu;
+ int ret, pos;
+ struct pci_dev *pdev;
+ struct arm_smmu_device *smmu = master->smmu;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ if (!(smmu->features & ARM_SMMU_FEAT_ATS) || !dev_is_pci(master->dev) ||
+ (fwspec->flags & IOMMU_FWSPEC_PCI_NO_ATS))
+ return -ENOSYS;
+
+ pdev = to_pci_dev(master->dev);
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS);
+ if (!pos)
+ return -ENOSYS;
+
+ /* Smallest Translation Unit: log2 of the smallest supported granule */
+ stu = __ffs(smmu->pgsize_bitmap);
+
+ ret = pci_enable_ats(pdev, stu);
+ if (ret) {
+ dev_err(&pdev->dev, "could not enable ATS: %d\n", ret);
+ return ret;
+ }
+
+ dev_dbg(&pdev->dev, "enabled ATS (STU=%zu, QDEP=%d)\n", stu,
+ pci_ats_queue_depth(pdev));
+
+ return 0;
+}
+
+static void arm_smmu_disable_ats(struct arm_smmu_master_data *master)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return;
+
+ pdev = to_pci_dev(master->dev);
+
+ if (!pdev->ats_enabled)
+ return;
+
+ pci_disable_ats(pdev);
+}
+
static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
struct arm_smmu_master_data *master)
{
@@ -2406,9 +2603,11 @@ static int arm_smmu_add_device(struct device *dev)
master->ste.can_stall = true;
}
+ arm_smmu_enable_ats(master);
+
ret = iommu_device_link(&smmu->iommu, dev);
if (ret)
- goto err_free_master;
+ goto err_disable_ats;
ret = arm_smmu_insert_master(smmu, master);
if (ret)
@@ -2430,6 +2629,9 @@ static int arm_smmu_add_device(struct device *dev)
err_unlink:
iommu_device_unlink(&smmu->iommu, dev);
+err_disable_ats:
+ arm_smmu_disable_ats(master);
+
err_free_master:
kfree(master);
fwspec->iommu_priv = NULL;
@@ -2457,6 +2659,7 @@ static void arm_smmu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
arm_smmu_remove_master(smmu, master);
iommu_device_unlink(&smmu->iommu, dev);
+ arm_smmu_disable_ats(master);
kfree(master);
iommu_fwspec_free(dev);
}
@@ -3069,6 +3272,16 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
}
}
+ if (smmu->features & ARM_SMMU_FEAT_ATS && !disable_ats_check) {
+ enables |= CR0_ATSCHK;
+ ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+ ARM_SMMU_CR0ACK);
+ if (ret) {
+ dev_err(smmu->dev, "failed to enable ATS check\n");
+ return ret;
+ }
+ }
+
ret = arm_smmu_setup_irqs(smmu);
if (ret) {
dev_err(smmu->dev, "failed to setup irqs\n");
--
2.17.0
^ permalink raw reply related
* [PATCH v2 34/40] ACPI/IORT: Check ATS capability in root complex nodes
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Root complex node in IORT has a bit telling whether it supports ATS or
not. Store this bit in the IOMMU fwspec when setting up a device, so it
can be accessed later by an IOMMU driver.
Use the negative version (NO_ATS) at the moment because it's not clear
if/how the bit needs to be integrated in other firmware descriptions. The
SMMU has a feature bit telling if it supports ATS, which might be
sufficient in most systems for deciding whether or not we should enable
the ATS capability in endpoints.
Cc: lorenzo.pieralisi at arm.com
Cc: hanjun.guo at linaro.org
Cc: sudeep.holla at arm.com
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/acpi/arm64/iort.c | 11 +++++++++++
include/linux/iommu.h | 4 ++++
2 files changed, 15 insertions(+)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7a3a541046ed..4f4907e58cc8 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1004,6 +1004,14 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
dev_dbg(dev, "dma_pfn_offset(%#08llx)\n", offset);
}
+static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node)
+{
+ struct acpi_iort_root_complex *pci_rc;
+
+ pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+ return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
+}
+
/**
* iort_iommu_configure - Set-up IOMMU configuration for a device.
*
@@ -1039,6 +1047,9 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
info.node = node;
err = pci_for_each_dma_alias(to_pci_dev(dev),
iort_pci_iommu_init, &info);
+
+ if (!err && !iort_pci_rc_supports_ats(node))
+ dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS;
} else {
int i = 0;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index bcce44455117..d7e2f54086e4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -655,12 +655,16 @@ struct iommu_fwspec {
const struct iommu_ops *ops;
struct fwnode_handle *iommu_fwnode;
void *iommu_priv;
+ u32 flags;
unsigned int num_ids;
unsigned int num_pasid_bits;
bool can_stall;
u32 ids[1];
};
+/* Firmware disabled ATS in the root complex */
+#define IOMMU_FWSPEC_PCI_NO_ATS (1 << 0)
+
int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
const struct iommu_ops *ops);
void iommu_fwspec_free(struct device *dev);
--
2.17.0
^ permalink raw reply related
* [PATCH v2 33/40] iommu/arm-smmu-v3: Add stall support for platform devices
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
The SMMU provides a Stall model for handling page faults in platform
devices. It is similar to PCI PRI, but doesn't require devices to have
their own translation cache. Instead, faulting transactions are parked and
the OS is given a chance to fix the page tables and retry the transaction.
Enable stall for devices that support it (opt-in by firmware). When an
event corresponds to a translation error, call the IOMMU fault handler. If
the fault is recoverable, it will call us back to terminate or continue
the stall.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 178 +++++++++++++++++++++++++++++++++++-
1 file changed, 173 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 0f2d8aa0deee..8a6a799ba04a 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -301,6 +301,11 @@
#define CMDQ_PRI_1_GRPID GENMASK_ULL(8, 0)
#define CMDQ_PRI_1_RESP GENMASK_ULL(13, 12)
+#define CMDQ_RESUME_0_SID GENMASK_ULL(63, 32)
+#define CMDQ_RESUME_0_ACTION_RETRY (1UL << 12)
+#define CMDQ_RESUME_0_ACTION_ABORT (1UL << 13)
+#define CMDQ_RESUME_1_STAG GENMASK_ULL(15, 0)
+
#define CMDQ_SYNC_0_CS GENMASK_ULL(13, 12)
#define CMDQ_SYNC_0_CS_NONE 0
#define CMDQ_SYNC_0_CS_IRQ 1
@@ -316,6 +321,25 @@
#define EVTQ_0_ID GENMASK_ULL(7, 0)
+#define EVT_ID_TRANSLATION_FAULT 0x10
+#define EVT_ID_ADDR_SIZE_FAULT 0x11
+#define EVT_ID_ACCESS_FAULT 0x12
+#define EVT_ID_PERMISSION_FAULT 0x13
+
+#define EVTQ_0_SSV (1UL << 11)
+#define EVTQ_0_SSID GENMASK_ULL(31, 12)
+#define EVTQ_0_SID GENMASK_ULL(63, 32)
+#define EVTQ_1_STAG GENMASK_ULL(15, 0)
+#define EVTQ_1_STALL (1UL << 31)
+#define EVTQ_1_PRIV (1UL << 33)
+#define EVTQ_1_EXEC (1UL << 34)
+#define EVTQ_1_READ (1UL << 35)
+#define EVTQ_1_S2 (1UL << 39)
+#define EVTQ_1_CLASS GENMASK_ULL(41, 40)
+#define EVTQ_1_TT_READ (1UL << 44)
+#define EVTQ_2_ADDR GENMASK_ULL(63, 0)
+#define EVTQ_3_IPA GENMASK_ULL(51, 12)
+
/* PRI queue */
#define PRIQ_ENT_DWORDS 2
#define PRIQ_MAX_SZ_SHIFT 8
@@ -426,6 +450,13 @@ struct arm_smmu_cmdq_ent {
enum pri_resp resp;
} pri;
+ #define CMDQ_OP_RESUME 0x44
+ struct {
+ u32 sid;
+ u16 stag;
+ enum page_response_code resp;
+ } resume;
+
#define CMDQ_OP_CMD_SYNC 0x46
struct {
u32 msidata;
@@ -499,6 +530,8 @@ struct arm_smmu_strtab_ent {
bool assigned;
struct arm_smmu_s1_cfg *s1_cfg;
struct arm_smmu_s2_cfg *s2_cfg;
+
+ bool can_stall;
};
struct arm_smmu_strtab_cfg {
@@ -851,6 +884,21 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
}
cmd[1] |= FIELD_PREP(CMDQ_PRI_1_RESP, ent->pri.resp);
break;
+ case CMDQ_OP_RESUME:
+ cmd[0] |= FIELD_PREP(CMDQ_RESUME_0_SID, ent->resume.sid);
+ cmd[1] |= FIELD_PREP(CMDQ_RESUME_1_STAG, ent->resume.stag);
+ switch (ent->resume.resp) {
+ case IOMMU_PAGE_RESP_INVALID:
+ case IOMMU_PAGE_RESP_FAILURE:
+ cmd[0] |= CMDQ_RESUME_0_ACTION_ABORT;
+ break;
+ case IOMMU_PAGE_RESP_SUCCESS:
+ cmd[0] |= CMDQ_RESUME_0_ACTION_RETRY;
+ break;
+ default:
+ return -EINVAL;
+ }
+ break;
case CMDQ_OP_CMD_SYNC:
if (ent->sync.msiaddr)
cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
@@ -1013,6 +1061,34 @@ static void arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
dev_err_ratelimited(smmu->dev, "CMD_SYNC timeout\n");
}
+static int arm_smmu_page_response(struct device *dev,
+ struct page_response_msg *resp)
+{
+ int sid = dev->iommu_fwspec->ids[0];
+ struct arm_smmu_cmdq_ent cmd = {0};
+ struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
+
+ if (master->ste.can_stall) {
+ cmd.opcode = CMDQ_OP_RESUME;
+ cmd.resume.sid = sid;
+ cmd.resume.stag = resp->page_req_group_id;
+ cmd.resume.resp = resp->resp_code;
+ } else {
+ /* TODO: put PRI response here */
+ return -ENODEV;
+ }
+
+ arm_smmu_cmdq_issue_cmd(master->smmu, &cmd);
+ /*
+ * Don't send a SYNC, it doesn't do anything for RESUME or PRI_RESP.
+ * RESUME consumption guarantees that the stalled transaction will be
+ * terminated... at some point in the future. PRI_RESP is fire and
+ * forget.
+ */
+
+ return 0;
+}
+
/* Stream table manipulation functions */
static void
arm_smmu_write_strtab_l1_desc(__le64 *dst, struct arm_smmu_strtab_l1_desc *desc)
@@ -1123,7 +1199,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
FIELD_PREP(STRTAB_STE_1_STRW, strw));
if (smmu->features & ARM_SMMU_FEAT_STALLS &&
- !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
+ !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE) &&
+ !ste->can_stall)
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
val |= (ste->s1_cfg->tables.base & STRTAB_STE_0_S1CTXPTR_MASK) |
@@ -1196,7 +1273,6 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
-__maybe_unused
static struct arm_smmu_master_data *
arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
{
@@ -1222,10 +1298,86 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
return master;
}
+static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
+{
+ int ret;
+ struct arm_smmu_master_data *master;
+ u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
+ u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
+
+ struct iommu_fault_event fault = {
+ .page_req_group_id = FIELD_GET(EVTQ_1_STAG, evt[1]),
+ .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
+ .last_req = true,
+ };
+
+ switch (type) {
+ case EVT_ID_TRANSLATION_FAULT:
+ case EVT_ID_ADDR_SIZE_FAULT:
+ case EVT_ID_ACCESS_FAULT:
+ fault.reason = IOMMU_FAULT_REASON_PTE_FETCH;
+ break;
+ case EVT_ID_PERMISSION_FAULT:
+ fault.reason = IOMMU_FAULT_REASON_PERMISSION;
+ break;
+ default:
+ /* TODO: report other unrecoverable faults. */
+ return -EFAULT;
+ }
+
+ /* Stage-2 is always pinned at the moment */
+ if (evt[1] & EVTQ_1_S2)
+ return -EFAULT;
+
+ master = arm_smmu_find_master(smmu, sid);
+ if (!master)
+ return -EINVAL;
+
+ /*
+ * The domain is valid until the fault returns, because detach() flushes
+ * the fault queue.
+ */
+ if (evt[1] & EVTQ_1_STALL)
+ fault.type = IOMMU_FAULT_PAGE_REQ;
+ else
+ fault.type = IOMMU_FAULT_DMA_UNRECOV;
+
+ if (evt[1] & EVTQ_1_READ)
+ fault.prot |= IOMMU_FAULT_READ;
+ else
+ fault.prot |= IOMMU_FAULT_WRITE;
+
+ if (evt[1] & EVTQ_1_EXEC)
+ fault.prot |= IOMMU_FAULT_EXEC;
+
+ if (evt[1] & EVTQ_1_PRIV)
+ fault.prot |= IOMMU_FAULT_PRIV;
+
+ if (evt[0] & EVTQ_0_SSV) {
+ fault.pasid_valid = true;
+ fault.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
+ }
+
+ ret = iommu_report_device_fault(master->dev, &fault);
+ if (ret && fault.type == IOMMU_FAULT_PAGE_REQ) {
+ /* Nobody cared, abort the access */
+ struct page_response_msg resp = {
+ .addr = fault.addr,
+ .pasid = fault.pasid,
+ .pasid_present = fault.pasid_valid,
+ .page_req_group_id = fault.page_req_group_id,
+ .resp_code = IOMMU_PAGE_RESP_FAILURE,
+ };
+ arm_smmu_page_response(master->dev, &resp);
+ }
+
+ return ret;
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
- int i;
+ int i, ret;
int num_handled = 0;
struct arm_smmu_device *smmu = dev;
struct arm_smmu_queue *q = &smmu->evtq.q;
@@ -1237,12 +1389,19 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
while (!queue_remove_raw(q, evt)) {
u8 id = FIELD_GET(EVTQ_0_ID, evt[0]);
+ spin_unlock(&q->wq.lock);
+ ret = arm_smmu_handle_evt(smmu, evt);
+ spin_lock(&q->wq.lock);
+
if (++num_handled == queue_size) {
q->batch++;
wake_up_all_locked(&q->wq);
num_handled = 0;
}
+ if (!ret)
+ continue;
+
dev_info(smmu->dev, "event 0x%02x received:\n", id);
for (i = 0; i < ARRAY_SIZE(evt); ++i)
dev_info(smmu->dev, "\t0x%016llx\n",
@@ -1374,7 +1533,9 @@ static int arm_smmu_flush_queues(void *cookie, struct device *dev)
if (dev) {
master = dev->iommu_fwspec->iommu_priv;
- /* TODO: add support for PRI and Stall */
+ if (master->ste.can_stall)
+ arm_smmu_flush_queue(smmu, &smmu->evtq.q, "evtq");
+ /* TODO: add support for PRI */
return 0;
}
@@ -1688,7 +1849,8 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
.order = master->ssid_bits,
.sync = &arm_smmu_ctx_sync,
.arm_smmu = {
- .stall = !!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE),
+ .stall = !!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE) ||
+ master->ste.can_stall,
.asid_bits = smmu->asid_bits,
.hw_access = !!(smmu->features & ARM_SMMU_FEAT_HA),
.hw_dirty = !!(smmu->features & ARM_SMMU_FEAT_HD),
@@ -2239,6 +2401,11 @@ static int arm_smmu_add_device(struct device *dev)
master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
+ if (fwspec->can_stall && smmu->features & ARM_SMMU_FEAT_STALLS) {
+ master->can_fault = true;
+ master->ste.can_stall = true;
+ }
+
ret = iommu_device_link(&smmu->iommu, dev);
if (ret)
goto err_free_master;
@@ -2403,6 +2570,7 @@ static struct iommu_ops arm_smmu_ops = {
.mm_attach = arm_smmu_mm_attach,
.mm_detach = arm_smmu_mm_detach,
.mm_invalidate = arm_smmu_mm_invalidate,
+ .page_response = arm_smmu_page_response,
.map = arm_smmu_map,
.unmap = arm_smmu_unmap,
.map_sg = default_iommu_map_sg,
--
2.17.0
^ permalink raw reply related
* [PATCH v2 32/40] iommu/arm-smmu-v3: Maintain a SID->device structure
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
When handling faults from the event or PRI queue, we need to find the
struct device associated to a SID. Add a rb_tree to keep track of SIDs.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 114 +++++++++++++++++++++++++++++++++++-
1 file changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d5f3875abfb9..0f2d8aa0deee 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -570,9 +570,18 @@ struct arm_smmu_device {
/* IOMMU core code handle */
struct iommu_device iommu;
+ struct rb_root streams;
+ struct mutex streams_mutex;
+
struct iopf_queue *iopf_queue;
};
+struct arm_smmu_stream {
+ u32 id;
+ struct arm_smmu_master_data *master;
+ struct rb_node node;
+};
+
/* SMMU private data for each master */
struct arm_smmu_master_data {
struct arm_smmu_device *smmu;
@@ -580,6 +589,7 @@ struct arm_smmu_master_data {
struct arm_smmu_domain *domain;
struct list_head list; /* domain->devices */
+ struct arm_smmu_stream *streams;
struct device *dev;
size_t ssid_bits;
@@ -1186,6 +1196,32 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
+__maybe_unused
+static struct arm_smmu_master_data *
+arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
+{
+ struct rb_node *node;
+ struct arm_smmu_stream *stream;
+ struct arm_smmu_master_data *master = NULL;
+
+ mutex_lock(&smmu->streams_mutex);
+ node = smmu->streams.rb_node;
+ while (node) {
+ stream = rb_entry(node, struct arm_smmu_stream, node);
+ if (stream->id < sid) {
+ node = node->rb_right;
+ } else if (stream->id > sid) {
+ node = node->rb_left;
+ } else {
+ master = stream->master;
+ break;
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return master;
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
@@ -2086,6 +2122,71 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
return sid < limit;
}
+static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_data *master)
+{
+ int i;
+ int ret = 0;
+ struct arm_smmu_stream *new_stream, *cur_stream;
+ struct rb_node **new_node, *parent_node = NULL;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ master->streams = kcalloc(fwspec->num_ids,
+ sizeof(struct arm_smmu_stream), GFP_KERNEL);
+ if (!master->streams)
+ return -ENOMEM;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < fwspec->num_ids && !ret; i++) {
+ new_stream = &master->streams[i];
+ new_stream->id = fwspec->ids[i];
+ new_stream->master = master;
+
+ new_node = &(smmu->streams.rb_node);
+ while (*new_node) {
+ cur_stream = rb_entry(*new_node, struct arm_smmu_stream,
+ node);
+ parent_node = *new_node;
+ if (cur_stream->id > new_stream->id) {
+ new_node = &((*new_node)->rb_left);
+ } else if (cur_stream->id < new_stream->id) {
+ new_node = &((*new_node)->rb_right);
+ } else {
+ dev_warn(master->dev,
+ "stream %u already in tree\n",
+ cur_stream->id);
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ if (!ret) {
+ rb_link_node(&new_stream->node, parent_node, new_node);
+ rb_insert_color(&new_stream->node, &smmu->streams);
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return ret;
+}
+
+static void arm_smmu_remove_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_data *master)
+{
+ int i;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ if (!master->streams)
+ return;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < fwspec->num_ids; i++)
+ rb_erase(&master->streams[i].node, &smmu->streams);
+ mutex_unlock(&smmu->streams_mutex);
+
+ kfree(master->streams);
+}
+
static struct iommu_ops arm_smmu_ops;
static int arm_smmu_add_device(struct device *dev)
@@ -2142,16 +2243,23 @@ static int arm_smmu_add_device(struct device *dev)
if (ret)
goto err_free_master;
+ ret = arm_smmu_insert_master(smmu, master);
+ if (ret)
+ goto err_unlink;
+
group = iommu_group_get_for_dev(dev);
if (IS_ERR(group)) {
ret = PTR_ERR(group);
- goto err_unlink;
+ goto err_remove_master;
}
iommu_group_put(group);
return 0;
+err_remove_master:
+ arm_smmu_remove_master(smmu, master);
+
err_unlink:
iommu_device_unlink(&smmu->iommu, dev);
@@ -2180,6 +2288,7 @@ static void arm_smmu_remove_device(struct device *dev)
if (master->ste.assigned)
arm_smmu_detach_dev(dev);
iommu_group_remove_device(dev);
+ arm_smmu_remove_master(smmu, master);
iommu_device_unlink(&smmu->iommu, dev);
kfree(master);
iommu_fwspec_free(dev);
@@ -2483,6 +2592,9 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
int ret;
atomic_set(&smmu->sync_nr, 0);
+ mutex_init(&smmu->streams_mutex);
+ smmu->streams = RB_ROOT;
+
ret = arm_smmu_init_queues(smmu);
if (ret)
return ret;
--
2.17.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox