Implement reference counting to enable inclusive MPU regions. An inclusive region is defined as a region encapsulated inside a previously allocated region sharing the same permissions.
Reference counts are incremented and decremented in xen_mpumap_update_entry. A region will be destroyed if the reference count is 0 upon calling destroy_xen_mappings and if the full region range is specified. Additionally XEN_MPUMAP_ENTRY_SHIFT and XEN_MPUMAP_ENTRY_SHIFT_ZERO are no longer hardcoded and defined inside asm-offsets.c. Signed-off-by: Harry Ramsey <[email protected]> Reviewed-by: Michal Orzel <[email protected]> --- Changes in v3: - Return early when checking memory attribute condtions - Replace references with refcount Changes in v2: - Improve clarity with regards to MPU inclusive regions - Fix code format issues --- xen/arch/arm/arm32/asm-offsets.c | 2 + xen/arch/arm/arm64/asm-offsets.c | 2 + xen/arch/arm/include/asm/arm32/mpu.h | 2 + xen/arch/arm/include/asm/arm64/mpu.h | 2 + xen/arch/arm/include/asm/mpu/regions.inc | 11 +++- xen/arch/arm/mpu/mm.c | 73 +++++++++++++++++++----- 6 files changed, 76 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c index c203ce269d..951f8d03f3 100644 --- a/xen/arch/arm/arm32/asm-offsets.c +++ b/xen/arch/arm/arm32/asm-offsets.c @@ -79,6 +79,8 @@ void __dummy__(void) #ifdef CONFIG_MPU DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask)); DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap)); + DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t))); + DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t)); BLANK(); #endif } diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c index 320289b281..38a3894a3b 100644 --- a/xen/arch/arm/arm64/asm-offsets.c +++ b/xen/arch/arm/arm64/asm-offsets.c @@ -73,6 +73,8 @@ void __dummy__(void) #ifdef CONFIG_MPU DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask)); DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap)); + DEFINE(XEN_MPUMAP_ENTRY_SHIFT, ilog2(sizeof(pr_t))); + DEFINE(XEN_MPUMAP_ENTRY_ZERO_OFFSET, sizeof(prbar_t) + sizeof(prlar_t)); BLANK(); #endif } diff --git a/xen/arch/arm/include/asm/arm32/mpu.h b/xen/arch/arm/include/asm/arm32/mpu.h index 0a6930b3a0..137022d922 100644 --- a/xen/arch/arm/include/asm/arm32/mpu.h +++ b/xen/arch/arm/include/asm/arm32/mpu.h @@ -39,6 +39,8 @@ typedef union { typedef struct { prbar_t prbar; prlar_t prlar; + uint8_t refcount; + uint8_t pad[7]; /* Pad structure to 16 Bytes */ } pr_t; #endif /* __ASSEMBLY__ */ diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h index f0ce344e78..17f62ccaf6 100644 --- a/xen/arch/arm/include/asm/arm64/mpu.h +++ b/xen/arch/arm/include/asm/arm64/mpu.h @@ -38,6 +38,8 @@ typedef union { typedef struct { prbar_t prbar; prlar_t prlar; + uint8_t refcount; + uint8_t pad[15]; /* Pad structure to 32 Bytes */ } pr_t; #endif /* __ASSEMBLY__ */ diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/include/asm/mpu/regions.inc index 23fead3b21..0cdbb17bc3 100644 --- a/xen/arch/arm/include/asm/mpu/regions.inc +++ b/xen/arch/arm/include/asm/mpu/regions.inc @@ -14,14 +14,12 @@ #define PRLAR_ELx_EN 0x1 #ifdef CONFIG_ARM_64 -#define XEN_MPUMAP_ENTRY_SHIFT 0x4 /* 16 byte structure */ .macro store_pair reg1, reg2, dst stp \reg1, \reg2, [\dst] .endm #else -#define XEN_MPUMAP_ENTRY_SHIFT 0x3 /* 8 byte structure */ .macro store_pair reg1, reg2, dst strd \reg1, \reg2, [\dst] @@ -97,6 +95,15 @@ 3: + /* Clear the rest of the xen_mpumap entry. */ +#ifdef CONFIG_ARM_64 + stp xzr, xzr, [\base, #XEN_MPUMAP_ENTRY_ZERO_OFFSET] +#else + mov \prbar, #0 + mov \prlar, #0 + strd \prbar, \prlar, [\base, #XEN_MPUMAP_ENTRY_ZERO_OFFSET] +#endif + add \sel, \sel, #1 1: diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c index b80edcf1ca..29dd8c4622 100644 --- a/xen/arch/arm/mpu/mm.c +++ b/xen/arch/arm/mpu/mm.c @@ -106,6 +106,7 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags) region = (pr_t) { .prbar = prbar, .prlar = prlar, + .refcount = 0, }; /* Set base address and limit address. */ @@ -170,6 +171,35 @@ int mpumap_contains_region(pr_t *table, uint8_t nr_regions, paddr_t base, return MPUMAP_REGION_NOTFOUND; } +static bool is_mm_attr_match(pr_t *region, unsigned int attributes) +{ + if ( region->prbar.reg.ro != PAGE_RO_MASK(attributes) ) + { + printk(XENLOG_WARNING + "Mismatched Access Permission attributes (%#x instead of %#x)\n", + region->prbar.reg.ro, PAGE_RO_MASK(attributes)); + return false; + } + + if ( region->prbar.reg.xn != PAGE_XN_MASK(attributes) ) + { + printk(XENLOG_WARNING + "Mismatched Execute Never attributes (%#x instead of %#x)\n", + region->prbar.reg.xn, PAGE_XN_MASK(attributes)); + return false; + } + + if ( region->prlar.reg.ai != PAGE_AI_MASK(attributes) ) + { + printk(XENLOG_WARNING + "Mismatched Memory Attribute Index (%#x instead of %#x)\n", + region->prlar.reg.ai, PAGE_AI_MASK(attributes)); + return false; + } + + return true; +} + /* Map a frame table to cover physical addresses ps through pe */ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe) { @@ -284,22 +314,26 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, flags_has_page_present = flags & _PAGE_PRESENT; - /* Currently we don't support modifying an existing entry. */ + /* + * Currently, we only support removing/modifying a *WHOLE* MPU memory + * region. Part-region removal/modification is not supported as in the worst + * case it will leave two/three fragments behind. + */ if ( flags_has_page_present && (rc >= MPUMAP_REGION_FOUND) ) { - printk("Modifying an existing entry is not supported\n"); - return -EINVAL; - } + if ( !is_mm_attr_match(&xen_mpumap[idx], flags) ) + { + printk("Modifying an existing entry is not supported\n"); + return -EINVAL; + } - /* - * Currently, we only support removing/modifying a *WHOLE* MPU memory - * region. Part-region removal/modification is not supported as in the worst - * case it will leave two/three fragments behind. - */ - if ( rc == MPUMAP_REGION_INCLUSIVE ) - { - printk("Part-region removal/modification is not supported\n"); - return -EINVAL; + /* Check for overflow of refcount before incrementing. */ + if ( xen_mpumap[idx].refcount == 0xFF ) + { + printk("Cannot allocate region as it would cause refcount overflow\n"); + return -ENOENT; + } + xen_mpumap[idx].refcount += 1; } /* We are inserting a mapping => Create new region. */ @@ -323,7 +357,18 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit, return -EINVAL; } - disable_mpu_region_from_index(idx); + if ( xen_mpumap[idx].refcount == 0 ) + { + if ( MPUMAP_REGION_FOUND == rc ) + disable_mpu_region_from_index(idx); + else + { + printk("Cannot remove a partial region\n"); + return -EINVAL; + } + } + else + xen_mpumap[idx].refcount -= 1; } return 0; -- 2.43.0
