[ Josh, Seth and Vojtech added as klp maintainers. See http://lkml.kernel.org/g/1456402305-1612-1-git-send-email-bsingharora@xxxxxxxxx for the original mail and the thread. ] On Tue, 1 Mar 2016, Balbir Singh wrote: > >> +#ifndef _ASM_POWERPC64_LIVEPATCH_H > >> +#define _ASM_POWERPC64_LIVEPATCH_H > >> + > >> +#include <linux/module.h> > >> +#include <linux/ftrace.h> > >> + > >> +#ifdef CONFIG_LIVEPATCH > >> +static inline int klp_check_compiler_support(void) > >> +{ > >> +#if !defined(_CALL_ELF) || _CALL_ELF != 2 || !defined(CC_USING_MPROFILE_KERNEL) > >> + return 1; > >> +#endif > >> + return 0; > >> +} > > I don't understand why we need that. If your compiler is not supported then you > > can't even compile the live patch code. I guess we have to implement it to keep > > the livepatch core happy, but it should just return 1. > It's been around from the previous patches, we can definitely refactor it See for example arch/s390/include/asm/livepatch.h for reference how the file could basically look like. > >> +static inline int klp_matchaddr(struct ftrace_ops *ops, unsigned long ip, > >> + int remove, int reset) > >> +{ > >> + int offsets[] = {8, 16}; > > Because of the two versions of mprofile-kernel (2 or 3 instruction sequence) > > and the presence or absense of the global entry point, the full set of offsets > > is: 4, 8, 12, 16. > Petr wanted to do a version where we lookup things in ftrace(). Steve suggested using ftrace_location(). We can refactor these bits. Yes, make it more general please. > > > >> + int i; > >> + int ret = 1; > >> + > >> + for (i = 0; i < ARRAY_SIZE(offsets); i++) { > >> + ret = ftrace_set_filter_ip(ops, ip+offsets[i], remove, reset); > >> + if (!ret) > >> + break; > >> + } > >> + return ret; > >> +} > >> + > >> +extern int klp_write_module_reloc(struct module *mod, unsigned long type, > >> + unsigned long loc, unsigned long value); > > That could be static inline for the moment, all it does is return ENOSYS. > Done! > >> + > >> +static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip) > >> +{ > >> + regs->nip = ip; > >> +} > > ptrace already defines instruction_pointer_set() to do this. But I guess that's > > a cleanup for later. > > > > > >> +#else > >> +#error Live patching support is disabled; check CONFIG_LIVEPATCH > >> +#endif > > I don't think you need that here in the header, the kconfig logic already > > handles this for us. > Yes We need an error there, but change it to #error Include linux/livepatch.h, not asm/livepatch.h please. See 383bf44d1a8b ("livepatch: change the error message in asm/livepatch.h header files"). I cannot comment on powerpc asm code much, so that is it from me. Thanks, Miroslav -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
