| Message ID | 20230725164104.273965-1-gatien.chevallier@foss.st.com |
|---|---|
| Headers | show |
| Series | Introduce STM32 Firewall framework | expand |
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