On 7/26/23 12:19, Simon Horman wrote:
On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:
Introduce a STM32 firewall framework that offers to firewall consumers different firewall services such as the ability to check their access rights against their firewall controller(s).
The STM32 firewall framework offers a generic API for STM32 firewall controllers that is defined in their drivers to best fit the specificity of each firewall.
There are various types of firewalls: -Peripheral firewalls that filter accesses to peripherals -Memory firewalls that filter accesses to memories or memory regions -No type for undefined type of firewall
Signed-off-by: Gatien Chevallier gatien.chevallier@foss.st.com
...
diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
...
+int stm32_firewall_populate_bus(struct stm32_firewall_controller *firewall_controller) +{
- struct stm32_firewall *firewalls;
- struct device_node *child;
- struct device *parent;
- unsigned int i;
- int len;
- int err;
- parent = firewall_controller->dev;
- dev_dbg(parent, "Populating %s system bus\n", dev_name(firewall_controller->dev));
- for_each_available_child_of_node(dev_of_node(parent), child) {
/* The feature-domains property is mandatory for firewall bus devices */len = of_count_phandle_with_args(child, "feature-domains", "#feature-domain-cells");if (len <= 0)Coccinelle says that, due to breaking out of a for_each_available_child_of_node() loop, a call to of_node_put() is needed here
Hi Simon,
Thank you, I already sent a V3 correcting the patch ordering issue. I will implement this for V4.
return -EINVAL;firewalls = kcalloc(len, sizeof(*firewalls), GFP_KERNEL);if (!firewalls)And here.
ditto
return -ENOMEM;err = stm32_firewall_get_firewall(child, firewalls, (unsigned int)len);if (err) {kfree(firewalls);And here.
ditto
return err;}for (i = 0; i < len; i++) {if (firewall_controller->grant_access(firewall_controller,firewalls[i].firewall_id)) {/** Peripheral access not allowed or not defined.* Mark the node as populated so platform bus won't probe it*/of_node_set_flag(child, OF_POPULATED);dev_err(parent, "%s: Device driver will not be probed\n",child->full_name);}}kfree(firewalls);- }
- return 0;
+} +EXPORT_SYMBOL_GPL(stm32_firewall_populate_bus);
diff --git a/drivers/bus/stm32_firewall.h b/drivers/bus/stm32_firewall.h
...
+/**
- struct stm32_firewall_controller - Information on firewall controller supplying services
- @name Name of the firewall controller
kernel-doc complains that name and the other fields of struct stm32_firewall_controller are not documented. I believe this is because a ':' is needed after the name of the parameter (in this case 'name').
- @name: Name of the firewall controller
Likewise, elsewhere.
I will implement it in V4, thank you.
- @dev Device reference of the firewall controller
- @mmio Base address of the firewall controller
- @entry List entry of the firewall controller list
- @type Type of firewall
- @max_entries Number of entries covered by the firewall
- @grant_access Callback used to grant access for a device access against a
firewall controller
- @release_access Callback used to release resources taken by a device when access was
granted
- @grant_memory_range_access Callback used to grant access for a device to a given memory region
- */
+struct stm32_firewall_controller {
- const char *name;
- struct device *dev;
- void __iomem *mmio;
- struct list_head entry;
- unsigned int type;
- unsigned int max_entries;
- int (*grant_access)(struct stm32_firewall_controller *ctrl, u32 id);
- void (*release_access)(struct stm32_firewall_controller *ctrl, u32 id);
- int (*grant_memory_range_access)(struct stm32_firewall_controller *ctrl, phys_addr_t paddr,
size_t size);+};
+/**
- int stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall
kernel-doc seems unhappy about the presence of 'int' on this line.
- stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall
Likewise, elsewhere.
Yes, I will remove the type in V4.
framework
- @firewall_controller Firewall controller to register
- Returns 0 in case of success or -ENODEV if no controller was given.
- */
+int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller);
...
diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h new file mode 100644 index 000000000000..9bdc4060154c --- /dev/null +++ b/include/linux/bus/stm32_firewall_device.h @@ -0,0 +1,140 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/*
- Copyright (C) 2023, STMicroelectronics - All Rights Reserved
- */
+#ifndef STM32_FIREWALL_DEVICE_H +#define STM32_FIREWALL_DEVICE_H
+#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/types.h>
+#define STM32_FIREWALL_MAX_EXTRA_ARGS 5
+/* Opaque reference to stm32_firewall_controller */ +struct stm32_firewall_controller;
+/**
- stm32_firewall - Information on a device's firewall. Each device can have more than one firewall.
kernel-doc seems unhappy about the absence of struct on this line.
- struct stm32_firewall - Information on a device's firewall. Each device can have more than one firewall.
Yes, I will add the struct in V4.
- @firewall_ctrl Pointer referencing a firewall controller of the device. It is
opaque so a device cannot manipulate the controller's ops or access
the controller's data
- @extra_args Extra arguments that are implementation dependent
- @entry Name of the firewall entry
- @extra_args_size Number of extra arguments
- @firewall_id Firewall ID associated the device for this firewall controller
- */
+struct stm32_firewall {
- struct stm32_firewall_controller *firewall_ctrl;
- u32 extra_args[STM32_FIREWALL_MAX_EXTRA_ARGS];
- const char *entry;
- size_t extra_args_size;
- u32 firewall_id;
+};
...
Best regards, Gatien