| Message ID | 1472132255-23470-8-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 ? ~Andrew
Hi Andrew, On 25/08/2016 09:58, Andrew Cooper 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 ? Do you mean for ARM? If so, alternatives is not supported by ARM32 so it should not be selected. Cheers,
On 25/08/16 15:02, Julien Grall wrote: > Hi Andrew, > > On 25/08/2016 09:58, Andrew Cooper 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 ? > > Do you mean for ARM? If so, alternatives is not supported by ARM32 so > it should not be selected. Perhaps we need a "selects ALTERNATIVES if X86" then, but the x86 implementation of livepatching does strictly need alternatives support, and should be represented appropriately. ~Andrew
>>> On 25.08.16 at 15:37, <konrad.wilk@oracle.com> wrote: > @@ -22,6 +24,12 @@ config X86 > select NUMA > select VGA > > +config ALTERNATIVE > + bool > + > +config HAS_EX_TABLE > + bool These need to move out of arch/x86/, and the first one's name is too generic (and should probably also start with HAS_). > --- a/xen/common/livepatch.c > +++ b/xen/common/livepatch.c > @@ -3,6 +3,7 @@ > * > */ > > +#include <xen/config.h> > #include <xen/cpu.h> > #include <xen/elf.h> > #include <xen/err.h> No new explicit inclusions of xen/config.h please. Jan
>>> 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. Jan
On Thu, Aug 25, 2016 at 08:54:51AM -0600, Jan Beulich wrote: > >>> On 25.08.16 at 15:37, <konrad.wilk@oracle.com> wrote: > > @@ -22,6 +24,12 @@ config X86 > > select NUMA > > select VGA > > > > +config ALTERNATIVE > > + bool > > + > > +config HAS_EX_TABLE > > + bool > > These need to move out of arch/x86/, and the first one's name is > too generic (and should probably also start with HAS_). <nods> Done. ARM64 is going to look a bit funny as it has ALTERNATIVE sprinkled and now it will also have an HAS_ALTERNATIVE. > > > --- a/xen/common/livepatch.c > > +++ b/xen/common/livepatch.c > > @@ -3,6 +3,7 @@ > > * > > */ > > > > +#include <xen/config.h> > > #include <xen/cpu.h> > > #include <xen/elf.h> > > #include <xen/err.h> > > No new explicit inclusions of xen/config.h please. /me scratches his head. I need the 'generated/autoconf.h' for its generated names. Oh, I can just do '#include <xen/kconfig.h>"! > > Jan >
>>> On 06.09.16 at 22:16, <konrad.wilk@oracle.com> wrote: > On Thu, Aug 25, 2016 at 08:54:51AM -0600, Jan Beulich wrote: >> >>> On 25.08.16 at 15:37, <konrad.wilk@oracle.com> wrote: >> > --- a/xen/common/livepatch.c >> > +++ b/xen/common/livepatch.c >> > @@ -3,6 +3,7 @@ >> > * >> > */ >> > >> > +#include <xen/config.h> >> > #include <xen/cpu.h> >> > #include <xen/elf.h> >> > #include <xen/err.h> >> >> No new explicit inclusions of xen/config.h please. > > /me scratches his head. > > I need the 'generated/autoconf.h' for its generated names. > > Oh, I can just do '#include <xen/kconfig.h>"! I don't understand. The make rules force the inclusion of xen/config.h by every translation unit (see xen/Rules.mk). Jan
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 265fd79..daef864 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -5,10 +5,12 @@ config X86 def_bool y select ACPI select ACPI_LEGACY_TABLES_LOOKUP + select ALTERNATIVE select COMPAT select CORE_PARKING select HAS_CPUFREQ select HAS_EHCI + select HAS_EX_TABLE select HAS_GDBSX select HAS_IOPORTS select HAS_KEXEC @@ -22,6 +24,12 @@ config X86 select NUMA select VGA +config ALTERNATIVE + bool + +config HAS_EX_TABLE + bool + config ARCH_DEFCONFIG string default "arch/x86/configs/x86_64_defconfig" diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index b771c7d..7e36d97 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -3,6 +3,7 @@ * */ +#include <xen/config.h> #include <xen/cpu.h> #include <xen/elf.h> #include <xen/err.h> @@ -684,7 +685,7 @@ static int prepare_payload(struct payload *payload, sizeof(*region->frame[i].bugs); } -#ifndef CONFIG_ARM +#ifdef CONFIG_ALTERNATIVE sec = livepatch_elf_sec_by_name(elf, ".altinstructions"); if ( sec ) { @@ -715,7 +716,9 @@ static int prepare_payload(struct payload *payload, } apply_alternatives(start, end); } +#endif +#ifdef CONFIG_HAS_EX_TABLE sec = livepatch_elf_sec_by_name(elf, ".ex_table"); if ( sec ) {
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> --- 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> Cc: Doug Goldstein <cardoe@cardoe.com> v2: First submission --- xen/arch/x86/Kconfig | 8 ++++++++ xen/common/livepatch.c | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-)