[PATCH v2 0/2] Background firewall fix

Signed-off-by: Manorit Chawdhry m-chawdhry@ti.com --- Changes in v2: - Change the series fundamentally as the previous patch seemed to break linux boot - Link to v1: https://lore.kernel.org/r/20230418-b4-upstream-j721s2-bg-fwl-fix-v1-1-7d42d1...
--- Manorit Chawdhry (2): Revert "arm: mach-k3: common: don't reconfigure background firewalls" arm: mach-k3: common: reorder removal of firewalls
arch/arm/mach-k3/common.c | 58 ++++++++++++++++++++++++++++------------------- arch/arm/mach-k3/common.h | 9 +++++--- 2 files changed, 41 insertions(+), 26 deletions(-) --- base-commit: 2440719d258a97824395532cb4a775752b423f63 change-id: 20230418-b4-upstream-j721s2-bg-fwl-fix-2c5c340eb4a9
Best regards,

This reverts commit b8ebf24e7f4afb5093a31bdf122e1ed0781e5c3c.
This patch seems to be fundamentally wrong and requires a different way on how the background firewalls should be configured so revert the patch
Signed-off-by: Manorit Chawdhry m-chawdhry@ti.com --- arch/arm/mach-k3/common.c | 5 +---- arch/arm/mach-k3/common.h | 4 ---- 2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index 3c85caee579d..b40e930b615d 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -580,10 +580,7 @@ void remove_fwl_configs(struct fwl_data *fwl_data, size_t fwl_data_size)
fwl_ops->get_fwl_region(ti_sci, ®ion);
- /* Don't disable the background regions */ - if (region.control != 0 && - ((region.control & K3_BACKGROUND_FIREWALL_BIT) == - 0)) { + if (region.control != 0) { pr_debug("Attempting to disable firewall %5d (%25s)\n", region.fwl_id, fwl_data[i].name); region.control = 0; diff --git a/arch/arm/mach-k3/common.h b/arch/arm/mach-k3/common.h index e7e59f533b70..130f5021123a 100644 --- a/arch/arm/mach-k3/common.h +++ b/arch/arm/mach-k3/common.h @@ -9,10 +9,6 @@ #include <asm/armv7_mpu.h> #include <asm/hardware.h>
-#define J721E 0xbb64 -#define J7200 0xbb6d -#define K3_BACKGROUND_FIREWALL_BIT BIT(8) - struct fwl_data { const char *name; u16 fwl_id;

On Fri, May 05, 2023 at 03:53:59PM +0530, Manorit Chawdhry wrote:
This reverts commit b8ebf24e7f4afb5093a31bdf122e1ed0781e5c3c.
This patch seems to be fundamentally wrong and requires a different way on how the background firewalls should be configured so revert the patch
Signed-off-by: Manorit Chawdhry m-chawdhry@ti.com
Applied to u-boot/next, thanks!

K3 devices have some firewalls set up by ROM that we usually remove so that the development is easy in HS devices.
While removing the firewalls disabling a background region before disabling the foreground regions keeps the firewall in a state where all the transactions will be blacklisted until all the regions are disabled. This causes a race for some other entity trying to access that memory region before all the firewalls are disabled and causes an exception.
Since there is no guarantee on where the background regions lie based on ROM configurations or no guarantee if the background regions will allow all transactions across the memory spaces, iterate the loop twice removing the foregrounds first and then backgrounds.
Signed-off-by: Manorit Chawdhry m-chawdhry@ti.com --- arch/arm/mach-k3/common.c | 55 ++++++++++++++++++++++++++++++----------------- arch/arm/mach-k3/common.h | 7 ++++++ 2 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index b40e930b615d..0e045919dd01 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -563,36 +563,51 @@ void disable_linefill_optimization(void) } #endif
-void remove_fwl_configs(struct fwl_data *fwl_data, size_t fwl_data_size) +static void remove_fwl_regions(struct fwl_data fwl_data, size_t num_regions, + enum k3_firewall_region_type fwl_type) { - struct ti_sci_msg_fwl_region region; struct ti_sci_fwl_ops *fwl_ops; struct ti_sci_handle *ti_sci; - size_t i, j; + struct ti_sci_msg_fwl_region region; + size_t j;
ti_sci = get_ti_sci_handle(); fwl_ops = &ti_sci->ops.fwl_ops; - for (i = 0; i < fwl_data_size; i++) { - for (j = 0; j < fwl_data[i].regions; j++) { - region.fwl_id = fwl_data[i].fwl_id; - region.region = j; - region.n_permission_regs = 3; - - fwl_ops->get_fwl_region(ti_sci, ®ion); - - if (region.control != 0) { - pr_debug("Attempting to disable firewall %5d (%25s)\n", - region.fwl_id, fwl_data[i].name); - region.control = 0; - - if (fwl_ops->set_fwl_region(ti_sci, ®ion)) - pr_err("Could not disable firewall %5d (%25s)\n", - region.fwl_id, fwl_data[i].name); - } + + for (j = 0; j < fwl_data.regions; j++) { + region.fwl_id = fwl_data.fwl_id; + region.region = j; + region.n_permission_regs = 3; + + fwl_ops->get_fwl_region(ti_sci, ®ion); + + /* Don't disable the background regions */ + if (region.control != 0 && + ((region.control & K3_FIREWALL_BACKGROUND_BIT) == + fwl_type)) { + pr_debug("Attempting to disable firewall %5d (%25s)\n", + region.fwl_id, fwl_data.name); + region.control = 0; + + if (fwl_ops->set_fwl_region(ti_sci, ®ion)) + pr_err("Could not disable firewall %5d (%25s)\n", + region.fwl_id, fwl_data.name); } } }
+void remove_fwl_configs(struct fwl_data *fwl_data, size_t fwl_data_size) +{ + size_t i; + + for (i = 0; i < fwl_data_size; i++) { + remove_fwl_regions(fwl_data[i], fwl_data[i].regions, + K3_FIREWALL_REGION_FOREGROUND); + remove_fwl_regions(fwl_data[i], fwl_data[i].regions, + K3_FIREWALL_REGION_BACKGROUND); + } +} + void spl_enable_dcache(void) { #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) diff --git a/arch/arm/mach-k3/common.h b/arch/arm/mach-k3/common.h index 130f5021123a..a994c3df3e03 100644 --- a/arch/arm/mach-k3/common.h +++ b/arch/arm/mach-k3/common.h @@ -9,12 +9,19 @@ #include <asm/armv7_mpu.h> #include <asm/hardware.h>
+#define K3_FIREWALL_BACKGROUND_BIT BIT(8) + struct fwl_data { const char *name; u16 fwl_id; u16 regions; };
+enum k3_firewall_region_type { + K3_FIREWALL_REGION_FOREGROUND, + K3_FIREWALL_REGION_BACKGROUND +}; + enum k3_device_type { K3_DEVICE_TYPE_BAD, K3_DEVICE_TYPE_GP,

On Fri, May 05, 2023 at 03:54:00PM +0530, Manorit Chawdhry wrote:
K3 devices have some firewalls set up by ROM that we usually remove so that the development is easy in HS devices.
While removing the firewalls disabling a background region before disabling the foreground regions keeps the firewall in a state where all the transactions will be blacklisted until all the regions are disabled. This causes a race for some other entity trying to access that memory region before all the firewalls are disabled and causes an exception.
Since there is no guarantee on where the background regions lie based on ROM configurations or no guarantee if the background regions will allow all transactions across the memory spaces, iterate the loop twice removing the foregrounds first and then backgrounds.
Signed-off-by: Manorit Chawdhry m-chawdhry@ti.com
Applied to u-boot/next, thanks!
participants (2)
-
Manorit Chawdhry
-
Tom Rini