On 09/04/2025 09:26, Luca Fancellu wrote:
Hi Ayan,
Hi Luca,
The point of the code was to don’t issue an isb() every time we change the
selector,
of course the code would be easier otherwise, but do we want to do that?
Not sure if it is beneficial as you would need to use isb() from region16
onwards.
The isb() is issued only when changing the selector, so when you go from
reading/writing regions
from 0-15 to 16-31 for example, of course there is a case that if you
continuously write on region 15
and 16 you would have to always change the selector, but it’s the less impact
we could have.
armv8-R is even better since it’s able to address regions from 0 to 23 without
flushing the pipeline,
^— aarch32 :)
Can you point me to the document where you got this info ?
so I would say we should exploit this big advantage.
I’ll send shortly in this thread the code I would use and the code I was
thinking you could use.
Here the code I have in mind:
static void prepare_selector(uint8_t *sel)
{
uint8_t cur_sel = *sel;
/*
* {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.
*/
cur_sel &= 0xF0U;
if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
{
WRITE_SYSREG(cur_sel, PRSELR_EL2);
isb();
}
*sel = *sel & 0xFU;
}
void read_protection_region(pr_t *pr_read, uint8_t sel)
{
/*
* Before accessing EL2 MPU region register PRBAR_EL2/PRLAR_EL2,
* make sure PRSELR_EL2 is set, as it determines which MPU region
* is selected.
*/
prepare_selector(&sel);
switch ( sel )
{
GENERATE_READ_PR_REG_CASE(0, pr_read);
GENERATE_READ_PR_REG_CASE(1, pr_read);
GENERATE_READ_PR_REG_CASE(2, pr_read);
GENERATE_READ_PR_REG_CASE(3, pr_read);
GENERATE_READ_PR_REG_CASE(4, pr_read);
GENERATE_READ_PR_REG_CASE(5, pr_read);
GENERATE_READ_PR_REG_CASE(6, pr_read);
GENERATE_READ_PR_REG_CASE(7, pr_read);
GENERATE_READ_PR_REG_CASE(8, pr_read);
GENERATE_READ_PR_REG_CASE(9, pr_read);
GENERATE_READ_PR_REG_CASE(10, pr_read);
GENERATE_READ_PR_REG_CASE(11, pr_read);
GENERATE_READ_PR_REG_CASE(12, pr_read);
GENERATE_READ_PR_REG_CASE(13, pr_read);
GENERATE_READ_PR_REG_CASE(14, pr_read);
GENERATE_READ_PR_REG_CASE(15, pr_read);
default:
BUG(); /* Can't happen */
}
}
void write_protection_region(const pr_t *pr_write, uint8_t sel)
{
/*
* Before accessing EL2 MPU region register PRBAR_EL2/PRLAR_EL2,
* make sure PRSELR_EL2 is set, as it determines which MPU region
* is selected.
*/
prepare_selector(&sel);
switch ( sel )
{
GENERATE_WRITE_PR_REG_CASE(0, pr_write);
GENERATE_WRITE_PR_REG_CASE(1, pr_write);
GENERATE_WRITE_PR_REG_CASE(2, pr_write);
GENERATE_WRITE_PR_REG_CASE(3, pr_write);
GENERATE_WRITE_PR_REG_CASE(4, pr_write);
GENERATE_WRITE_PR_REG_CASE(5, pr_write);
GENERATE_WRITE_PR_REG_CASE(6, pr_write);
GENERATE_WRITE_PR_REG_CASE(7, pr_write);
GENERATE_WRITE_PR_REG_CASE(8, pr_write);
GENERATE_WRITE_PR_REG_CASE(9, pr_write);
GENERATE_WRITE_PR_REG_CASE(10, pr_write);
GENERATE_WRITE_PR_REG_CASE(11, pr_write);
GENERATE_WRITE_PR_REG_CASE(12, pr_write);
GENERATE_WRITE_PR_REG_CASE(13, pr_write);
GENERATE_WRITE_PR_REG_CASE(14, pr_write);
GENERATE_WRITE_PR_REG_CASE(15, pr_write);
default:
BUG(); /* Can't happen */
}
}
Till here I am fine if you think this is the correct approach for arm64.
Here the code I thought you could add for arm32:
static void prepare_selector(uint8_t *sel)
{
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.
*/
cur_sel &= 0xF0U;
if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
{
WRITE_SYSREG(cur_sel, PRSELR_EL2);
isb();
}
*sel = *sel & 0xFU;
#else
if ( cur_sel > 23 )
panic("Armv8-R AArch32 region selector exceeds maximum allowed
range!");
I am not sure about this. See my question before. However ...
#endif
}
From ARM DDI 0568A.c ID110520
E2.2.3 HPRBAR<n> - Provides access to the base addresses for the first
32 defined EL2 MPU regions.
E2.2.6 HPRLAR<n> - Provides access to the limit addresses for the first
32 defined EL2 MPU regions.
I understand that HPRSELR is not used when directly accessing the above
two registers. Thus, this function will be a nop for Arm32. Please let
me know if I am mistaken.
void read_protection_region(pr_t *pr_read, uint8_t sel)
{
/*
* Before accessing EL2 MPU region register PRBAR_EL2/PRLAR_EL2,
* make sure PRSELR_EL2 is set, as it determines which MPU region
* is selected.
*/
prepare_selector(&sel);
switch ( sel )
{
GENERATE_READ_PR_REG_CASE(0, pr_read);
GENERATE_READ_PR_REG_CASE(1, pr_read);
GENERATE_READ_PR_REG_CASE(2, pr_read);
GENERATE_READ_PR_REG_CASE(3, pr_read);
GENERATE_READ_PR_REG_CASE(4, pr_read);
GENERATE_READ_PR_REG_CASE(5, pr_read);
GENERATE_READ_PR_REG_CASE(6, pr_read);
GENERATE_READ_PR_REG_CASE(7, pr_read);
GENERATE_READ_PR_REG_CASE(8, pr_read);
GENERATE_READ_PR_REG_CASE(9, pr_read);
GENERATE_READ_PR_REG_CASE(10, pr_read);
GENERATE_READ_PR_REG_CASE(11, pr_read);
GENERATE_READ_PR_REG_CASE(12, pr_read);
GENERATE_READ_PR_REG_CASE(13, pr_read);
GENERATE_READ_PR_REG_CASE(14, pr_read);
GENERATE_READ_PR_REG_CASE(15, pr_read);
#ifdef CONFIG_ARM_32
GENERATE_READ_PR_REG_CASE(16, pr_read);
GENERATE_READ_PR_REG_CASE(17, pr_read);
GENERATE_READ_PR_REG_CASE(18, pr_read);
GENERATE_READ_PR_REG_CASE(19, pr_read);
GENERATE_READ_PR_REG_CASE(20, pr_read);
GENERATE_READ_PR_REG_CASE(21, pr_read);
GENERATE_READ_PR_REG_CASE(22, pr_read);
GENERATE_READ_PR_REG_CASE(23, pr_read);
At least 32 regions are directly accessible, thus thisn should go till
31 (0-31). Same ..
#endif
default:
BUG(); /* Can't happen */
}
}
void write_protection_region(const pr_t *pr_write, uint8_t sel)
{
/*
* Before accessing EL2 MPU region register PRBAR_EL2/PRLAR_EL2,
* make sure PRSELR_EL2 is set, as it determines which MPU region
* is selected.
*/
prepare_selector(&sel);
switch ( sel )
{
GENERATE_WRITE_PR_REG_CASE(0, pr_write);
GENERATE_WRITE_PR_REG_CASE(1, pr_write);
GENERATE_WRITE_PR_REG_CASE(2, pr_write);
GENERATE_WRITE_PR_REG_CASE(3, pr_write);
GENERATE_WRITE_PR_REG_CASE(4, pr_write);
GENERATE_WRITE_PR_REG_CASE(5, pr_write);
GENERATE_WRITE_PR_REG_CASE(6, pr_write);
GENERATE_WRITE_PR_REG_CASE(7, pr_write);
GENERATE_WRITE_PR_REG_CASE(8, pr_write);
GENERATE_WRITE_PR_REG_CASE(9, pr_write);
GENERATE_WRITE_PR_REG_CASE(10, pr_write);
GENERATE_WRITE_PR_REG_CASE(11, pr_write);
GENERATE_WRITE_PR_REG_CASE(12, pr_write);
GENERATE_WRITE_PR_REG_CASE(13, pr_write);
GENERATE_WRITE_PR_REG_CASE(14, pr_write);
GENERATE_WRITE_PR_REG_CASE(15, pr_write);
#ifdef CONFIG_ARM_32
GENERATE_WRITE_PR_REG_CASE(16, pr_write);
GENERATE_WRITE_PR_REG_CASE(17, pr_write);
GENERATE_WRITE_PR_REG_CASE(18, pr_write);
GENERATE_WRITE_PR_REG_CASE(19, pr_write);
GENERATE_WRITE_PR_REG_CASE(20, pr_write);
GENERATE_WRITE_PR_REG_CASE(21, pr_write);
GENERATE_WRITE_PR_REG_CASE(22, pr_write);
GENERATE_WRITE_PR_REG_CASE(23, pr_write);
for here.
#endif
default:
BUG(); /* Can't happen */
}
}
Please let me know your thoughts.
- Ayan
Cheers,
Luca
