Re: [Xen-devel] [PATCH v2 05/20] arm64/alternatives: Make it possible to patch outside of hypervisor.

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
>
> 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,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel