| Message ID | 1471216074-3007-5-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Konrad, On 15/08/16 00:07, Konrad Rzeszutek Wilk wrote: > Which is only used by Livepatch code. The purpose behind > this call is to modify the page table entries flags. > > Specifically the .ro and .nx flags. The current mechanism > puts cache attributes in the flags and the .ro and .nx are > locked down and assumed to be .ro=0, nx=1. > > Livepatch needs .nx=0 and also .ro to be set to 1. > > We introduce a new 'flags' where various bits determine > whether .ro and .nx bits are set or cleared. We can't use > an enum as the function prototype would diverge from x86. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > -- > Cc: Ross Lagerwall <ross.lagerwall@citrix.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > [Added as he wrote the x86 modify_xen_mappings version] > > RFC: First submission. > v1: Add #defines to make it simpler to understand the bit-shifting. > --- > xen/arch/arm/mm.c | 21 ++++++++++++++++++--- > xen/include/asm-arm/page.h | 11 +++++++++++ > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 4e256c2..520c0e0 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -836,6 +836,7 @@ static int create_xen_table(lpae_t *entry) > enum xenmap_operation { > INSERT, > REMOVE, > + MODIFY, > RESERVE > }; > > @@ -881,14 +882,22 @@ static int create_xen_entries(enum xenmap_operation op, > pte.pt.table = 1; > write_pte(&third[third_table_offset(addr)], pte); > break; > + case MODIFY: > case REMOVE: > if ( !third[third_table_offset(addr)].pt.valid ) > { > - printk("create_xen_entries: trying to remove a non-existing mapping addr=%lx\n", > - addr); > + printk("create_xen_entries: trying to %s a non-existing mapping addr=%lx\n", > + op == REMOVE ? "remove" : "modify", addr); > return -EINVAL; > } > - pte.bits = 0; > + if ( op == REMOVE ) > + pte.bits = 0; > + else > + { > + pte = third[third_table_offset(addr)]; > + pte.pt.ro = PTE_RO_MASK(ai); > + pte.pt.xn = PTE_NX_MASK(ai); I would like to see some sanity check for read-only and execute-never bit. For instance, ro = 0 and xn = 0 would lead to a crash later on because Xen is enabling SCTLR_EL2.WXN. I think the other combinations should be fine. > + } > write_pte(&third[third_table_offset(addr)], pte); > break; > default: > @@ -922,6 +931,12 @@ void destroy_xen_mappings(unsigned long v, unsigned long e) > create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0); > } > > +void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) create_xen_entries may fail, don't you want to report the error to the caller? > +{ > + ASSERT((flags & (PTE_NX | PTE_RO)) == flags); > + create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags); > +} > + > enum mg { mg_clear, mg_ro, mg_rw, mg_rx }; > static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) > { > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h > index 05d9f82..2f66740 100644 > --- a/xen/include/asm-arm/page.h > +++ b/xen/include/asm-arm/page.h > @@ -66,6 +66,17 @@ > #define PAGE_HYPERVISOR_WC (DEV_WC) > > /* > + * Defines for changing the PTE .ro and .nx bits. This is only to be > + * used with modify_xen_mappings. > + */ > +#define _PTE_NX_BIT 0U > +#define _PTE_RO_BIT 1U > +#define PTE_NX (1U << _PTE_NX_BIT) > +#define PTE_RO (1U << _PTE_RO_BIT) > +#define PTE_NX_MASK(x) (((x) >> _PTE_NX_BIT) & 0x1U) > +#define PTE_RO_MASK(x) (((x) >> _PTE_RO_BIT) & 0x1U) > + > +/* > * Stage 2 Memory Type. > * > * These are valid in the MemAttr[3:0] field of an LPAE stage 2 page > Regards,
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 4e256c2..520c0e0 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -836,6 +836,7 @@ static int create_xen_table(lpae_t *entry) enum xenmap_operation { INSERT, REMOVE, + MODIFY, RESERVE }; @@ -881,14 +882,22 @@ static int create_xen_entries(enum xenmap_operation op, pte.pt.table = 1; write_pte(&third[third_table_offset(addr)], pte); break; + case MODIFY: case REMOVE: if ( !third[third_table_offset(addr)].pt.valid ) { - printk("create_xen_entries: trying to remove a non-existing mapping addr=%lx\n", - addr); + printk("create_xen_entries: trying to %s a non-existing mapping addr=%lx\n", + op == REMOVE ? "remove" : "modify", addr); return -EINVAL; } - pte.bits = 0; + if ( op == REMOVE ) + pte.bits = 0; + else + { + pte = third[third_table_offset(addr)]; + pte.pt.ro = PTE_RO_MASK(ai); + pte.pt.xn = PTE_NX_MASK(ai); + } write_pte(&third[third_table_offset(addr)], pte); break; default: @@ -922,6 +931,12 @@ void destroy_xen_mappings(unsigned long v, unsigned long e) create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0); } +void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags) +{ + ASSERT((flags & (PTE_NX | PTE_RO)) == flags); + create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags); +} + enum mg { mg_clear, mg_ro, mg_rw, mg_rx }; static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) { diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h index 05d9f82..2f66740 100644 --- a/xen/include/asm-arm/page.h +++ b/xen/include/asm-arm/page.h @@ -66,6 +66,17 @@ #define PAGE_HYPERVISOR_WC (DEV_WC) /* + * Defines for changing the PTE .ro and .nx bits. This is only to be + * used with modify_xen_mappings. + */ +#define _PTE_NX_BIT 0U +#define _PTE_RO_BIT 1U +#define PTE_NX (1U << _PTE_NX_BIT) +#define PTE_RO (1U << _PTE_RO_BIT) +#define PTE_NX_MASK(x) (((x) >> _PTE_NX_BIT) & 0x1U) +#define PTE_RO_MASK(x) (((x) >> _PTE_RO_BIT) & 0x1U) + +/* * Stage 2 Memory Type. * * These are valid in the MemAttr[3:0] field of an LPAE stage 2 page
Which is only used by Livepatch code. The purpose behind this call is to modify the page table entries flags. Specifically the .ro and .nx flags. The current mechanism puts cache attributes in the flags and the .ro and .nx are locked down and assumed to be .ro=0, nx=1. Livepatch needs .nx=0 and also .ro to be set to 1. We introduce a new 'flags' where various bits determine whether .ro and .nx bits are set or cleared. We can't use an enum as the function prototype would diverge from x86. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> -- Cc: Ross Lagerwall <ross.lagerwall@citrix.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien.grall@arm.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> [Added as he wrote the x86 modify_xen_mappings version] RFC: First submission. v1: Add #defines to make it simpler to understand the bit-shifting. --- xen/arch/arm/mm.c | 21 ++++++++++++++++++--- xen/include/asm-arm/page.h | 11 +++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-)