| Message ID | 1472132255-23470-6-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Konrad, On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote: > With livepatching the alternatives that should be patched are > outside the Xen hypervisor _start -> _end. As such having > to use an alternative VA is not neccessary. In fact we s/neccessary/necessary/ > can use the ones that the caller provided us with. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > > v2: First version > --- > xen/arch/arm/alternative.c | 40 +++++++++++++++++++++++++--------------- > 1 file changed, 25 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c > index aba06db..90a857a 100644 > --- a/xen/arch/arm/alternative.c > +++ b/xen/arch/arm/alternative.c > @@ -94,24 +94,30 @@ static int __apply_alternatives(const struct alt_region *region) > { > const struct alt_instr *alt; > const u32 *origptr, *replptr; > - u32 *writeptr, *writemap; > + u32 *writeptr, *writemap = NULL; > mfn_t text_mfn = _mfn(virt_to_mfn(_stext)); > unsigned int text_order = get_order_from_bytes(_end - _start); > > - printk(XENLOG_INFO "alternatives: Patching kernel code\n"); > - > - /* > - * The text section is read-only. So re-map Xen to be able to patch > - * the code. > - */ > - writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR, > - VMAP_DEFAULT); > - if ( !writemap ) > + if ( region->begin >= __alt_instructions && > + region->end <= __alt_instructions_end ) I think this is quite fragile because __alt_instructions is part of the init section. So after runtime, those regions could be reallocated for other purposes. I thought a bit more about the issue, each alt_instr contains a relative offset. So it would be possible to re-map the region in __apply_alternatives_multi_stop and passed the re-map the region to __apply_alternatives. Furthermore, I think it is safe to mandate apply_alternatives to work only read-write region. So we could simplify the hacky code I wrote to workaround the Write permissions implies execute-never. Let me try to write a patch to see if my solution works. Regards,
diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c index aba06db..90a857a 100644 --- a/xen/arch/arm/alternative.c +++ b/xen/arch/arm/alternative.c @@ -94,24 +94,30 @@ static int __apply_alternatives(const struct alt_region *region) { const struct alt_instr *alt; const u32 *origptr, *replptr; - u32 *writeptr, *writemap; + u32 *writeptr, *writemap = NULL; mfn_t text_mfn = _mfn(virt_to_mfn(_stext)); unsigned int text_order = get_order_from_bytes(_end - _start); - printk(XENLOG_INFO "alternatives: Patching kernel code\n"); - - /* - * The text section is read-only. So re-map Xen to be able to patch - * the code. - */ - writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR, - VMAP_DEFAULT); - if ( !writemap ) + if ( region->begin >= __alt_instructions && + region->end <= __alt_instructions_end ) { - printk(XENLOG_ERR "alternatives: Unable to map the text section (size %u)\n", - 1 << text_order); - return -ENOMEM; + printk(XENLOG_INFO "alternatives: Patching kernel code\n"); + /* + * The text section is read-only. So re-map Xen to be able to patch + * the code. + */ + writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR, + VMAP_DEFAULT); + if ( !writemap ) + { + printk(XENLOG_ERR "alternatives: Unable to map the text section (size %u)\n", + 1 << text_order); + return -ENOMEM; + } } + else + printk(XENLOG_INFO "alternatives: Patching %p -> %p\n", + region->begin, region->end); for ( alt = region->begin; alt < region->end; alt++ ) { @@ -124,8 +130,11 @@ static int __apply_alternatives(const struct alt_region *region) BUG_ON(alt->alt_len != alt->orig_len); origptr = ALT_ORIG_PTR(alt); - writeptr = origptr - (u32 *)_start + writemap; replptr = ALT_REPL_PTR(alt); + if ( writemap ) + writeptr = origptr - (u32 *)_start + writemap; + else + writeptr = (u32 *)origptr; nr_inst = alt->alt_len / sizeof(insn); @@ -143,7 +152,8 @@ static int __apply_alternatives(const struct alt_region *region) /* Nuke the instruction cache */ invalidate_icache(); - vunmap(writemap); + if ( writemap ) + vunmap(writemap); return 0; }
With livepatching the alternatives that should be patched are outside the Xen hypervisor _start -> _end. As such having to use an alternative VA is not neccessary. In fact we can use the ones that the caller provided us with. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien.grall@arm.com> v2: First version --- xen/arch/arm/alternative.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-)