Re: [Xen-devel] [PATCH v2 07/20] arm/x86: Add ALTERNATIVE and HAS_EX_TABLE

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Sep 06, 2016 at 04:36:40PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 25, 2016 at 08:56:07AM -0600, Jan Beulich wrote:
> > >>> On 25.08.16 at 15:58, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote:
> > > 
> > >> x86 implements all of them by default - and we just
> > >> add two extra CONFIG_ variables to be declared in autoconf.h.
> > >>
> > >> ARM 64 only has alternative while ARM 32 has none of them.
> > >>
> > >> And while at it change the livepatch common code that
> > >> would benefit from this.
> > >>
> > >> Suggested-by: Julien Grall <julien.grall@xxxxxxx>
> > >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > 
> > > Surely livepatch should select alternatives ?
> > 
> > DYM depend on? It clearly shouldn't select them, as whether
> > they're present is determined by arch code.
> 
> And I am not sure if there is a need for Kconfig LIVEPATCH
> entry to depend on it?
> 
> Especially as the common/livepatch.c has #ifef CONFIG_ALTERNATIVE
> (and also CONFIG_HAS_EX_TABLE) - added by this patch - to deal with
> architectures that do not have support them.
> 
> Ssuch as ARM 32 or ARM64 without errata support built-in.

Err, ARM64 is always built with alternative, hence:

> 
> I spun out an seperate patch that would thrown an -EOPNOTSUPP
> if these sections are part of the payload and the hypervisor
> was built without the support, like this:
> 
> (This is to be on top of the patch discussed - or if folks
> prefer I can squash it in).
> 
> From 3a4953dd0f7d2411e5d638a82bfe57c0e16a22b3 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Tue, 6 Sep 2016 16:28:23 -0400
> Subject: [PATCH] livepatch: Reject payloads with .alternative or .ex_table if
>  support is not built-in.
> 
> ARM 64 can be built without alternative support (without the
> errata support) - and it would sad if the payload loaded
> had .alternative section but we did not support parsing it.

This would never happen.

So please ignore the patch below.
> 
> As this could lead to sad results instead of ignoring this
> lets error out during loading.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> v3: New submission.
> ---
>  xen/common/livepatch.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 73d4edb..342a5ec 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -685,10 +685,10 @@ static int prepare_payload(struct payload *payload,
>                                    sizeof(*region->frame[i].bugs);
>      }
>  
> -#ifdef CONFIG_HAS_ALTERNATIVE
>      sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
>      if ( sec )
>      {
> +#ifdef CONFIG_HAS_ALTERNATIVE
>          struct alt_instr *a, *start, *end;
>  
>          if ( sec->sec->sh_size % sizeof(*a) )
> @@ -715,13 +715,17 @@ static int prepare_payload(struct payload *payload,
>              }
>          }
>          apply_alternatives(start, end);
> -    }
> +#else
> +        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support alternative 
> patching!\n",
> +                elf->name);
> +        return -EOPNOTSUPP;
>  #endif
> +    }
>  
> -#ifdef CONFIG_HAS_EX_TABLE
>      sec = livepatch_elf_sec_by_name(elf, ".ex_table");
>      if ( sec )
>      {
> +#ifdef CONFIG_HAS_EX_TABLE
>          struct exception_table_entry *s, *e;
>  
>          if ( !sec->sec->sh_size ||
> @@ -740,8 +744,12 @@ static int prepare_payload(struct payload *payload,
>  
>          region->ex = s;
>          region->ex_end = e;
> -    }
> +#else
> +        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support .ex_table!\n",
> +                elf->name);
> +        return -EOPNOTSUPP;
>  #endif
> +    }
>  
>      return 0;
>  }
> -- 
> 2.4.11
> 

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