On 12/06/2025 10:35, Luca Fancellu wrote:
Hi Ayan,
Hi Luca,
On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.hal...@amd.com> wrote:
Define prepare_selector(), read_protection_region() and
write_protection_region() for arm32. Also, define
GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
Enable pr_of_addr() for arm32.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.hal...@amd.com>
---
Based on your v2
(https://patchwork.kernel.org/project/xen-devel/patch/20250606164854.1551148-4-ayan.kumar.hal...@amd.com/)
I was imaging something like this:
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 74e96ca57137..5d324b2d4ca5 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -87,20 +87,28 @@ static void __init __maybe_unused build_assertions(void)
*/
static void prepare_selector(uint8_t *sel)
{
-#ifdef CONFIG_ARM_64
uint8_t cur_sel = *sel;
+#ifdef CONFIG_ARM_64
/*
- * {read,write}_protection_region works using the direct access to the
0..15
- * regions, so in order to save the isb() overhead, change the PRSELR_EL2
- * only when needed, so when the upper 4 bits of the selector will change.
+ * {read,write}_protection_region works using the Arm64 direct access to
the
+ * 0..15 regions, so in order to save the isb() overhead, change the
+ * PRSELR_EL2 only when needed, so when the upper 4 bits of the selector
+ * will change.
*/
cur_sel &= 0xF0U;
+#else
+ /* Arm32 MPU can use direct access for 0-31 */
+ if ( cur_sel > 31 )
+ cur_sel = 0;
+#endif
if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
{
WRITE_SYSREG(cur_sel, PRSELR_EL2);
isb();
}
+
+#ifdef CONFIG_ARM_64
*sel = *sel & 0xFU;
#endif
}
@@ -144,6 +152,12 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
GENERATE_READ_PR_REG_CASE(29, pr_read);
GENERATE_READ_PR_REG_CASE(30, pr_read);
GENERATE_READ_PR_REG_CASE(31, pr_read);
+ case 32 ... 255:
+ {
+ pr->prbar.bits = READ_SYSREG(PRBAR_EL2);
+ pr->prlar.bits = READ_SYSREG(PRLAR_EL2);
+ break;
+ }
#endif
default:
BUG(); /* Can't happen */
@@ -190,6 +204,12 @@ void write_protection_region(const pr_t *pr_write, uint8_t
sel)
GENERATE_WRITE_PR_REG_CASE(29, pr_write);
GENERATE_WRITE_PR_REG_CASE(30, pr_write);
GENERATE_WRITE_PR_REG_CASE(31, pr_write);
+ case 32 ... 255:
+ {
+ WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2);
+ WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2);
+ break;
+ }
#endif
default:
BUG(); /* Can't happen */
Is it using too ifdefs in your opinion that would benefit the split you do in
v3?
Yes. However, I understand that this is subjective. I need your and
Michal/Julien to have an opinion here whether to go with the split
(which means some amount of code duplication) or introduce if-defs. I
will be happy to proceed as per your opinions.
- Ayan
