| Message ID | 20160906203640.GB7062@char.us.oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@citrix.com> 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@arm.com> > > >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > > > 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@oracle.com> > 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@oracle.com> > > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > 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 >
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; }