| Message ID | 20230626033443.2943270-35-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Add Armv8-R64 MPU support to Xen - Part#1 | expand |
On 26/06/2023 04:34, Penny Zheng wrote: > CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. > > > This commit expands xen_mpumap_update/xen_mpumap_update_entry to include > destroying an existing entry. > > We define a new helper "control_xen_mpumap_region_from_index" to enable/disable > the MPU region based on index. If region is within [0, 31], we could quickly > disable the MPU region through PRENR_EL2 which provides direct access to the > PRLAR_EL2.EN bits of EL2 MPU regions. > > Rignt now, we only support destroying a *WHOLE* MPU memory region, > part-region removing is not supported, as in worst case, it will > leave two fragments behind. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > v3: > - make pr_get_base()/pr_get_limit() static inline > - need an isb to ensure register write visible before zeroing the entry > --- > xen/arch/arm/include/asm/arm64/mpu.h | 2 + > xen/arch/arm/include/asm/arm64/sysregs.h | 3 + > xen/arch/arm/mm.c | 5 ++ > xen/arch/arm/mpu/mm.c | 74 ++++++++++++++++++++++++ > 4 files changed, 84 insertions(+) > > diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h > index 715ea69884..aee7947223 100644 > --- a/xen/arch/arm/include/asm/arm64/mpu.h > +++ b/xen/arch/arm/include/asm/arm64/mpu.h > @@ -25,6 +25,8 @@ > #define REGION_UART_SEL 0x07 > #define MPUIR_REGION_MASK ((_AC(1, UL) << 8) - 1) > > +#define MPU_PRENR_BITS 32 This is common to R52 and R82. Thus, you can put it in the common file (may be xen/arch/arm/include/asm/mpu/mm.h) > + > /* Access permission attributes. */ > /* Read/Write at EL2, No Access at EL1/EL0. */ > #define AP_RW_EL2 0x0 > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h > index c8a679afdd..96c025053b 100644 > --- a/xen/arch/arm/include/asm/arm64/sysregs.h > +++ b/xen/arch/arm/include/asm/arm64/sysregs.h > @@ -509,6 +509,9 @@ > /* MPU Type registers encode */ > #define MPUIR_EL2 S3_4_C0_C0_4 > > +/* MPU Protection Region Enable Register encode */ > +#define PRENR_EL2 S3_4_C6_C1_1 > + > #endif > > /* Access to system registers */ > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 8625066256..247d17cfa1 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -164,7 +164,12 @@ int destroy_xen_mappings(unsigned long s, unsigned long e) > ASSERT(IS_ALIGNED(s, PAGE_SIZE)); > ASSERT(IS_ALIGNED(e, PAGE_SIZE)); > ASSERT(s <= e); > +#ifndef CONFIG_HAS_MPU > return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, 0); > +#else > + return xen_mpumap_update(virt_to_maddr((void *)s), > + virt_to_maddr((void *)e), 0); > +#endif > } Refer my comment in previous patch. You can have two implementations of this function 1) xen/arch/arm/mmu/mm.c 2) xen/arch/arm/mpu/mm.h > > int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) > diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c > index 0a65b58dc4..a40055ae5e 100644 > --- a/xen/arch/arm/mpu/mm.c > +++ b/xen/arch/arm/mpu/mm.c > @@ -425,6 +425,59 @@ static int mpumap_contain_region(pr_t *table, uint8_t nr_regions, > return MPUMAP_REGION_FAILED; > } > > +/* Disable or enable EL2 MPU memory region at index #index */ > +static void control_mpu_region_from_index(uint8_t index, bool enable) > +{ > + pr_t region; > + > + read_protection_region(®ion, index); > + if ( !region_is_valid(®ion) ^ enable ) > + { > + printk(XENLOG_WARNING > + "mpu: MPU memory region[%u] is already %s\n", index, > + enable ? "enabled" : "disabled"); > + return; > + } > + > + /* > + * ARM64v8R provides PRENR_EL2 to have direct access to the > + * PRLAR_EL2.EN bits of EL2 MPU regions from 0 to 31. > + */ > + if ( index < MPU_PRENR_BITS ) > + { > + uint64_t orig, after; > + > + orig = READ_SYSREG(PRENR_EL2); > + if ( enable ) > + /* Set respective bit */ > + after = orig | (1UL << index); > + else > + /* Clear respective bit */ > + after = orig & (~(1UL << index)); > + WRITE_SYSREG(after, PRENR_EL2); > + } > + else > + { > + region.prlar.reg.en = enable ? 1 : 0; > + write_protection_region((const pr_t*)®ion, index); > + } > + /* Ensure the write before zeroing the entry */ dsb(); /* to ensure write completes */ > + isb(); > + > + /* Update according bitfield in xen_mpumap_mask */ > + spin_lock(&xen_mpumap_alloc_lock); > + > + if ( enable ) > + set_bit(index, xen_mpumap_mask); > + else > + { > + clear_bit(index, xen_mpumap_mask); > + memset(&xen_mpumap[index], 0, sizeof(pr_t)); > + } > + > + spin_unlock(&xen_mpumap_alloc_lock); > +} > + > /* > * Update an entry in Xen MPU memory region mapping table(xen_mpumap) at > * the index @idx. > @@ -461,6 +514,27 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, > > write_protection_region((const pr_t*)(&xen_mpumap[idx]), idx); > } > + else > + { > + /* > + * Currently, we only support destroying a *WHOLE* MPU memory region, > + * part-region removing is not supported, as in worst case, it will > + * leave two fragments behind. > + * part-region removing will be introduced only when actual usage > + * comes. > + */ > + if ( rc == MPUMAP_REGION_INCLUSIVE ) > + { > + region_printk("mpu: part-region removing is not supported\n"); > + return -EINVAL; > + } > + > + /* We are removing the region */ > + if ( rc != MPUMAP_REGION_FOUND ) > + return -EINVAL; > + > + control_mpu_region_from_index(idx, false); > + } > > return 0; > } > -- > 2.25.1 > > - Ayan
Hi Ayan On 2023/7/1 00:17, Ayan Kumar Halder wrote: > > On 26/06/2023 04:34, Penny Zheng wrote: >> CAUTION: This message has originated from an External Source. Please >> use proper judgment and caution when opening attachments, clicking >> links, or responding to this email. >> >> >> This commit expands xen_mpumap_update/xen_mpumap_update_entry to include >> destroying an existing entry. >> >> We define a new helper "control_xen_mpumap_region_from_index" to >> enable/disable >> the MPU region based on index. If region is within [0, 31], we could >> quickly >> disable the MPU region through PRENR_EL2 which provides direct access >> to the >> PRLAR_EL2.EN bits of EL2 MPU regions. >> >> Rignt now, we only support destroying a *WHOLE* MPU memory region, >> part-region removing is not supported, as in worst case, it will >> leave two fragments behind. >> >> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >> Signed-off-by: Wei Chen <wei.chen@arm.com> >> --- >> v3: >> - make pr_get_base()/pr_get_limit() static inline >> - need an isb to ensure register write visible before zeroing the entry >> --- >> xen/arch/arm/include/asm/arm64/mpu.h | 2 + >> xen/arch/arm/include/asm/arm64/sysregs.h | 3 + >> xen/arch/arm/mm.c | 5 ++ >> xen/arch/arm/mpu/mm.c | 74 ++++++++++++++++++++++++ >> 4 files changed, 84 insertions(+) >> >> diff --git a/xen/arch/arm/include/asm/arm64/mpu.h >> b/xen/arch/arm/include/asm/arm64/mpu.h >> index 715ea69884..aee7947223 100644 >> --- a/xen/arch/arm/include/asm/arm64/mpu.h >> +++ b/xen/arch/arm/include/asm/arm64/mpu.h >> @@ -25,6 +25,8 @@ >> #define REGION_UART_SEL 0x07 >> #define MPUIR_REGION_MASK ((_AC(1, UL) << 8) - 1) >> >> +#define MPU_PRENR_BITS 32 > > This is common to R52 and R82. > > Thus, you can put it in the common file (may be > xen/arch/arm/include/asm/mpu/mm.h) > Will do. >> + >> /* Access permission attributes. */ >> /* Read/Write at EL2, No Access at EL1/EL0. */ >> #define AP_RW_EL2 0x0 >> diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h >> b/xen/arch/arm/include/asm/arm64/sysregs.h >> index c8a679afdd..96c025053b 100644 >> --- a/xen/arch/arm/include/asm/arm64/sysregs.h >> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h >> @@ -509,6 +509,9 @@ >> /* MPU Type registers encode */ >> #define MPUIR_EL2 S3_4_C0_C0_4 >> >> +/* MPU Protection Region Enable Register encode */ >> +#define PRENR_EL2 S3_4_C6_C1_1 >> + >> #endif >> >> /* Access to system registers */ >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c >> index 8625066256..247d17cfa1 100644 >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -164,7 +164,12 @@ int destroy_xen_mappings(unsigned long s, >> unsigned long e) >> ASSERT(IS_ALIGNED(s, PAGE_SIZE)); >> ASSERT(IS_ALIGNED(e, PAGE_SIZE)); >> ASSERT(s <= e); >> +#ifndef CONFIG_HAS_MPU >> return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, 0); >> +#else >> + return xen_mpumap_update(virt_to_maddr((void *)s), >> + virt_to_maddr((void *)e), 0); >> +#endif >> } > > Refer my comment in previous patch. > > You can have two implementations of this function 1) > xen/arch/arm/mmu/mm.c 2) xen/arch/arm/mpu/mm.h > Refer my comment in previous patch. I prefer #ifdef in destroy_xen_mappings() >> >> int modify_xen_mappings(unsigned long s, unsigned long e, unsigned >> int flags) >> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c >> index 0a65b58dc4..a40055ae5e 100644 >> --- a/xen/arch/arm/mpu/mm.c >> +++ b/xen/arch/arm/mpu/mm.c >> @@ -425,6 +425,59 @@ static int mpumap_contain_region(pr_t *table, >> uint8_t nr_regions, >> return MPUMAP_REGION_FAILED; >> } >> >> +/* Disable or enable EL2 MPU memory region at index #index */ >> +static void control_mpu_region_from_index(uint8_t index, bool enable) >> +{ >> + pr_t region; >> + >> + read_protection_region(®ion, index); >> + if ( !region_is_valid(®ion) ^ enable ) >> + { >> + printk(XENLOG_WARNING >> + "mpu: MPU memory region[%u] is already %s\n", index, >> + enable ? "enabled" : "disabled"); >> + return; >> + } >> + >> + /* >> + * ARM64v8R provides PRENR_EL2 to have direct access to the >> + * PRLAR_EL2.EN bits of EL2 MPU regions from 0 to 31. >> + */ >> + if ( index < MPU_PRENR_BITS ) >> + { >> + uint64_t orig, after; >> + >> + orig = READ_SYSREG(PRENR_EL2); >> + if ( enable ) >> + /* Set respective bit */ >> + after = orig | (1UL << index); >> + else >> + /* Clear respective bit */ >> + after = orig & (~(1UL << index)); >> + WRITE_SYSREG(after, PRENR_EL2); >> + } >> + else >> + { >> + region.prlar.reg.en = enable ? 1 : 0; >> + write_protection_region((const pr_t*)®ion, index); >> + } >> + /* Ensure the write before zeroing the entry */ > dsb(); /* to ensure write completes */ >> + isb(); >> + >> + /* Update according bitfield in xen_mpumap_mask */ >> + spin_lock(&xen_mpumap_alloc_lock); >> + >> + if ( enable ) >> + set_bit(index, xen_mpumap_mask); >> + else >> + { >> + clear_bit(index, xen_mpumap_mask); >> + memset(&xen_mpumap[index], 0, sizeof(pr_t)); >> + } >> + >> + spin_unlock(&xen_mpumap_alloc_lock); >> +} >> + >> /* >> * Update an entry in Xen MPU memory region mapping >> table(xen_mpumap) at >> * the index @idx. >> @@ -461,6 +514,27 @@ static int xen_mpumap_update_entry(paddr_t base, >> paddr_t limit, >> >> write_protection_region((const pr_t*)(&xen_mpumap[idx]), idx); >> } >> + else >> + { >> + /* >> + * Currently, we only support destroying a *WHOLE* MPU memory >> region, >> + * part-region removing is not supported, as in worst case, >> it will >> + * leave two fragments behind. >> + * part-region removing will be introduced only when actual >> usage >> + * comes. >> + */ >> + if ( rc == MPUMAP_REGION_INCLUSIVE ) >> + { >> + region_printk("mpu: part-region removing is not >> supported\n"); >> + return -EINVAL; >> + } >> + >> + /* We are removing the region */ >> + if ( rc != MPUMAP_REGION_FOUND ) >> + return -EINVAL; >> + >> + control_mpu_region_from_index(idx, false); >> + } >> >> return 0; >> } >> -- >> 2.25.1 >> >> > - Ayan
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h index 715ea69884..aee7947223 100644 --- a/xen/arch/arm/include/asm/arm64/mpu.h +++ b/xen/arch/arm/include/asm/arm64/mpu.h @@ -25,6 +25,8 @@ #define REGION_UART_SEL 0x07 #define MPUIR_REGION_MASK ((_AC(1, UL) << 8) - 1) +#define MPU_PRENR_BITS 32 + /* Access permission attributes. */ /* Read/Write at EL2, No Access at EL1/EL0. */ #define AP_RW_EL2 0x0 diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index c8a679afdd..96c025053b 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -509,6 +509,9 @@ /* MPU Type registers encode */ #define MPUIR_EL2 S3_4_C0_C0_4 +/* MPU Protection Region Enable Register encode */ +#define PRENR_EL2 S3_4_C6_C1_1 + #endif /* Access to system registers */ diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 8625066256..247d17cfa1 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -164,7 +164,12 @@ int destroy_xen_mappings(unsigned long s, unsigned long e) ASSERT(IS_ALIGNED(s, PAGE_SIZE)); ASSERT(IS_ALIGNED(e, PAGE_SIZE)); ASSERT(s <= e); +#ifndef CONFIG_HAS_MPU return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, 0); +#else + return xen_mpumap_update(virt_to_maddr((void *)s), + virt_to_maddr((void *)e), 0); +#endif } int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c index 0a65b58dc4..a40055ae5e 100644 --- a/xen/arch/arm/mpu/mm.c +++ b/xen/arch/arm/mpu/mm.c @@ -425,6 +425,59 @@ static int mpumap_contain_region(pr_t *table, uint8_t nr_regions, return MPUMAP_REGION_FAILED; } +/* Disable or enable EL2 MPU memory region at index #index */ +static void control_mpu_region_from_index(uint8_t index, bool enable) +{ + pr_t region; + + read_protection_region(®ion, index); + if ( !region_is_valid(®ion) ^ enable ) + { + printk(XENLOG_WARNING + "mpu: MPU memory region[%u] is already %s\n", index, + enable ? "enabled" : "disabled"); + return; + } + + /* + * ARM64v8R provides PRENR_EL2 to have direct access to the + * PRLAR_EL2.EN bits of EL2 MPU regions from 0 to 31. + */ + if ( index < MPU_PRENR_BITS ) + { + uint64_t orig, after; + + orig = READ_SYSREG(PRENR_EL2); + if ( enable ) + /* Set respective bit */ + after = orig | (1UL << index); + else + /* Clear respective bit */ + after = orig & (~(1UL << index)); + WRITE_SYSREG(after, PRENR_EL2); + } + else + { + region.prlar.reg.en = enable ? 1 : 0; + write_protection_region((const pr_t*)®ion, index); + } + /* Ensure the write before zeroing the entry */ + isb(); + + /* Update according bitfield in xen_mpumap_mask */ + spin_lock(&xen_mpumap_alloc_lock); + + if ( enable ) + set_bit(index, xen_mpumap_mask); + else + { + clear_bit(index, xen_mpumap_mask); + memset(&xen_mpumap[index], 0, sizeof(pr_t)); + } + + spin_unlock(&xen_mpumap_alloc_lock); +} + /* * Update an entry in Xen MPU memory region mapping table(xen_mpumap) at * the index @idx. @@ -461,6 +514,27 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, write_protection_region((const pr_t*)(&xen_mpumap[idx]), idx); } + else + { + /* + * Currently, we only support destroying a *WHOLE* MPU memory region, + * part-region removing is not supported, as in worst case, it will + * leave two fragments behind. + * part-region removing will be introduced only when actual usage + * comes. + */ + if ( rc == MPUMAP_REGION_INCLUSIVE ) + { + region_printk("mpu: part-region removing is not supported\n"); + return -EINVAL; + } + + /* We are removing the region */ + if ( rc != MPUMAP_REGION_FOUND ) + return -EINVAL; + + control_mpu_region_from_index(idx, false); + } return 0; }