| Message ID | 20230113052914.3845596-27-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 |
Hi Penny, On 13/01/2023 05:28, Penny Zheng wrote: > 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. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > xen/arch/arm/include/asm/arm64/mpu.h | 20 ++++++ > xen/arch/arm/include/asm/arm64/sysregs.h | 3 + > xen/arch/arm/mm_mpu.c | 77 ++++++++++++++++++++++-- > 3 files changed, 95 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h > index 0044bbf05d..c1dea1c8e9 100644 > --- a/xen/arch/arm/include/asm/arm64/mpu.h > +++ b/xen/arch/arm/include/asm/arm64/mpu.h > @@ -16,6 +16,8 @@ > */ > #define ARM_MAX_MPU_MEMORY_REGIONS 255 > > +#define MPU_PRENR_BITS 32 > + > /* Access permission attributes. */ > /* Read/Write at EL2, No Access at EL1/EL0. */ > #define AP_RW_EL2 0x0 > @@ -132,6 +134,24 @@ typedef struct { > _pr->prlar.reg.en; \ > }) > > +/* > + * Access to get base address of MPU protection region(pr_t). > + * The base address shall be zero extended. > + */ > +#define pr_get_base(pr) ({ \ > + pr_t *_pr = pr; \ > + (uint64_t)_pr->prbar.reg.base << MPU_REGION_SHIFT; \ > +}) Can this be a static inline? > + > +/* > + * Access to get limit address of MPU protection region(pr_t). > + * The limit address shall be concatenated with 0x3f. > + */ > +#define pr_get_limit(pr) ({ \ > + pr_t *_pr = pr; \ > + (uint64_t)((_pr->prlar.reg.limit << MPU_REGION_SHIFT) | 0x3f); \ > +}) Same. > + > #endif /* __ASSEMBLY__ */ > > #endif /* __ARM64_MPU_H__ */ > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h > index aca9bca5b1..c46daf6f69 100644 > --- a/xen/arch/arm/include/asm/arm64/sysregs.h > +++ b/xen/arch/arm/include/asm/arm64/sysregs.h > @@ -505,6 +505,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_mpu.c b/xen/arch/arm/mm_mpu.c > index d2e19e836c..3a0d110b13 100644 > --- a/xen/arch/arm/mm_mpu.c > +++ b/xen/arch/arm/mm_mpu.c > @@ -385,6 +385,45 @@ static int mpumap_contain_region(pr_t *mpu, uint64_t nr_regions, > return MPUMAP_REGION_FAILED; > } > > +/* Disable or enable EL2 MPU memory region at index #index */ > +static void control_mpu_region_from_index(uint64_t index, bool enable) > +{ > + pr_t region; > + > + access_protection_region(true, ®ion, NULL, index); > + if ( (region_is_valid(®ion) && enable) || > + (!region_is_valid(®ion) && !enable) ) You could write: !(region_is_valid(®ion) ^ enable) > + { > + printk(XENLOG_WARNING > + "mpu: MPU memory region[%lu] 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); Don't you need an isb (or similar) to ensure this is visible before... > + } > + else > + { > + region.prlar.reg.en = enable ? 1 : 0; > + access_protection_region(false, NULL, (const pr_t*)®ion, index); > + } > +} > + > /* > * Update an entry at the index @idx. > * @base: base address > @@ -449,6 +488,30 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, > if ( system_state <= SYS_STATE_active ) > update_boot_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 > + * lead to two fragments in result after destroying. > + * 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); > + > + /* Clear the according MPU memory region entry.*/ > + memset(&xen_mpumap[idx], 0, sizeof(pr_t)); ... zeroing the entry? Also, you could use memzero() here. > + } > > return 0; > } > @@ -589,6 +652,15 @@ static void __init map_mpu_memory_section_on_boot(enum mpu_section_info type, > } > } > > +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); > + > + return xen_mpumap_update(s, e, 0); > +} > + > /* > * System RAM is statically partitioned into different functionality > * section in Device Tree, including static xenheap, guest memory > @@ -656,11 +728,6 @@ void *ioremap(paddr_t pa, size_t len) > return NULL; > } > > -int destroy_xen_mappings(unsigned long s, unsigned long e) > -{ > - return -ENOSYS; > -} > - > int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) > { > return -ENOSYS; Cheers,
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h index 0044bbf05d..c1dea1c8e9 100644 --- a/xen/arch/arm/include/asm/arm64/mpu.h +++ b/xen/arch/arm/include/asm/arm64/mpu.h @@ -16,6 +16,8 @@ */ #define ARM_MAX_MPU_MEMORY_REGIONS 255 +#define MPU_PRENR_BITS 32 + /* Access permission attributes. */ /* Read/Write at EL2, No Access at EL1/EL0. */ #define AP_RW_EL2 0x0 @@ -132,6 +134,24 @@ typedef struct { _pr->prlar.reg.en; \ }) +/* + * Access to get base address of MPU protection region(pr_t). + * The base address shall be zero extended. + */ +#define pr_get_base(pr) ({ \ + pr_t *_pr = pr; \ + (uint64_t)_pr->prbar.reg.base << MPU_REGION_SHIFT; \ +}) + +/* + * Access to get limit address of MPU protection region(pr_t). + * The limit address shall be concatenated with 0x3f. + */ +#define pr_get_limit(pr) ({ \ + pr_t *_pr = pr; \ + (uint64_t)((_pr->prlar.reg.limit << MPU_REGION_SHIFT) | 0x3f); \ +}) + #endif /* __ASSEMBLY__ */ #endif /* __ARM64_MPU_H__ */ diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index aca9bca5b1..c46daf6f69 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -505,6 +505,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_mpu.c b/xen/arch/arm/mm_mpu.c index d2e19e836c..3a0d110b13 100644 --- a/xen/arch/arm/mm_mpu.c +++ b/xen/arch/arm/mm_mpu.c @@ -385,6 +385,45 @@ static int mpumap_contain_region(pr_t *mpu, uint64_t nr_regions, return MPUMAP_REGION_FAILED; } +/* Disable or enable EL2 MPU memory region at index #index */ +static void control_mpu_region_from_index(uint64_t index, bool enable) +{ + pr_t region; + + access_protection_region(true, ®ion, NULL, index); + if ( (region_is_valid(®ion) && enable) || + (!region_is_valid(®ion) && !enable) ) + { + printk(XENLOG_WARNING + "mpu: MPU memory region[%lu] 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; + access_protection_region(false, NULL, (const pr_t*)®ion, index); + } +} + /* * Update an entry at the index @idx. * @base: base address @@ -449,6 +488,30 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, if ( system_state <= SYS_STATE_active ) update_boot_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 + * lead to two fragments in result after destroying. + * 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); + + /* Clear the according MPU memory region entry.*/ + memset(&xen_mpumap[idx], 0, sizeof(pr_t)); + } return 0; } @@ -589,6 +652,15 @@ static void __init map_mpu_memory_section_on_boot(enum mpu_section_info type, } } +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); + + return xen_mpumap_update(s, e, 0); +} + /* * System RAM is statically partitioned into different functionality * section in Device Tree, including static xenheap, guest memory @@ -656,11 +728,6 @@ void *ioremap(paddr_t pa, size_t len) return NULL; } -int destroy_xen_mappings(unsigned long s, unsigned long e) -{ - return -ENOSYS; -} - int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) { return -ENOSYS;