[PATCH 02/17] mm: don't export arch_add_memory

[Christoph Hellwig]

Only x86_64 and sh export this symbol, and it is not used by any
modular code.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Dan Williams 
---
 arch/sh/mm/init.c | 1 -
 arch/x86/mm/init_64.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index bf726af5f1a5..afc54d593a26 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -498,7 +498,6 @@ int arch_add_memory(int nid, u64 start, u64 size, bool 
want_memblock)
 
return ret;
 }
-EXPORT_SYMBOL_GPL(arch_add_memory);
 
 #ifdef CONFIG_NUMA
 int memory_add_physaddr_to_nid(u64 addr)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4a837289f2ad..8acdc35c2dfa 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -796,7 +796,6 @@ int arch_add_memory(int nid, u64 start, u64 size, bool 
want_memblock)
 
return add_pages(nid, start_pfn, nr_pages, want_memblock);
 }
-EXPORT_SYMBOL_GPL(arch_add_memory);
 
 #define PAGE_INUSE 0xFD
 
-- 
2.14.2

[PATCH 01/17] memremap: provide stubs for vmem_altmap_offset and vmem_altmap_free

[Christoph Hellwig]

Currently all calls to those functions are eliminated by the compiler when
CONFIG_ZONE_DEVICE is not set, but this soon won't be the case.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Dan Williams 
---
 include/linux/memremap.h | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 10d23c367048..d5a6736d9737 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -26,9 +26,6 @@ struct vmem_altmap {
unsigned long alloc;
 };
 
-unsigned long vmem_altmap_offset(struct vmem_altmap *altmap);
-void vmem_altmap_free(struct vmem_altmap *altmap, unsigned long nr_pfns);
-
 #ifdef CONFIG_ZONE_DEVICE
 struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start);
 #else
@@ -138,6 +135,9 @@ void *devm_memremap_pages(struct device *dev, struct 
resource *res,
struct percpu_ref *ref, struct vmem_altmap *altmap);
 struct dev_pagemap *find_dev_pagemap(resource_size_t phys);
 
+unsigned long vmem_altmap_offset(struct vmem_altmap *altmap);
+void vmem_altmap_free(struct vmem_altmap *altmap, unsigned long nr_pfns);
+
 static inline bool is_zone_device_page(const struct page *page);
 #else
 static inline void *devm_memremap_pages(struct device *dev,
@@ -157,7 +157,17 @@ static inline struct dev_pagemap 
*find_dev_pagemap(resource_size_t phys)
 {
return NULL;
 }
-#endif
+
+static inline unsigned long vmem_altmap_offset(struct vmem_altmap *altmap)
+{
+   return 0;
+}
+
+static inline void vmem_altmap_free(struct vmem_altmap *altmap,
+   unsigned long nr_pfns)
+{
+}
+#endif /* CONFIG_ZONE_DEVICE */
 
 #if defined(CONFIG_DEVICE_PRIVATE) || defined(CONFIG_DEVICE_PUBLIC)
 static inline bool is_device_private_page(const struct page *page)
-- 
2.14.2

Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries

[kbuild test robot]

Hi Ard,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5 next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/add-support-for-relative-references-in-special-sections/20171228-171634
config: s390-gcov_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390 

All errors (new ones prefixed by >>):

>> arch/s390/kernel/ebcdic.o:(.data+0x118): undefined reference to 
>> `__gcov_merge_add'
   arch/s390/kernel/ebcdic.o: In function `_GLOBAL__sub_I_00100_0__ascebc':
>> ebcdic.c:(.text.startup+0xe): undefined reference to `__gcov_init'
   arch/s390/kernel/ebcdic.o: In function `_GLOBAL__sub_D_00100_1__ascebc':
>> ebcdic.c:(.text.exit+0x8): undefined reference to `__gcov_exit'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

[Benjamin Herrenschmidt]

On Thu, 2017-12-28 at 17:22 -0600, Bjorn Helgaas wrote:
> Both paths end up calling the pci_error_handlers.error_detected()
> hook.
> 
> Drivers are not supposed to care what arch they're running on.  If the
> driver supplies an .error_detected() entry point, it's up to the PCI
> core and powerpc code to use it consistently across arches.  That
> means the same uevents (if any) should be emitted from both paths.
> 
> The best way would be to unify the call of .error_detected() so the
> AER path and the powerpc path do it via the same function.  The AER
> report_error_detected() and the powerpc eeh_report_error() do look
> fairly similar, so this seems possible in principle, but I'm not
> holding my breath.

Factoring these callers into a common function that can then do the
uevent for errors makes a lot of sense.

The "resume" path might be trickier, but even then, rather than calling
directly the driver op, it would be easy to have a little wrapper that
does it, which can then also do the uevent.

Ben.

Re: [PATCH v6 5/8] kernel: tracepoints: add support for relative references

[Ard Biesheuvel]

On 28 December 2017 at 15:42, Steven Rostedt  wrote:
> On Wed, 27 Dec 2017 08:50:30 +
> Ard Biesheuvel  wrote:
>
>> To avoid the need for relocating absolute references to tracepoint
>> structures at boot time when running relocatable kernels (which may
>> take a disproportionate amount of space), add the option to emit
>> these tables as relative references instead.
>>
>
> I gave this patch a quick skim over. It appears to not modify anything
> when CONFIG_HAVE_PREL32_RELOCATIONS is not defined. I haven't
> thoroughly reviewed it or tested it. But if it doesn't break anything,
> I'm fine giving you an ack.
>
> Acked-by: Steven Rostedt (VMware) 
>

Thank you Steven.

I should mention though (as you don't appear to recall) that an
earlier version of this patch triggered an issue for you

https://marc.info/?l=linux-arch=150584374820168=2

but I have never managed to reproduce it, neither at the time nor
currently with this v6.


ard@bezzzef:~/linux-2.6$ sudo tools/testing/selftests/ftrace/ftracetest
=== Ftrace unit tests ===
[1] Basic trace file check [PASS]
[2] Basic test for tracers [PASS]
[3] Basic trace clock test [PASS]
[4] Basic event tracing check [PASS]
[5] event tracing - enable/disable with event level files [PASS]
[6] event tracing - restricts events based on pid [PASS]
[7] event tracing - enable/disable with subsystem level files [PASS]
[8] event tracing - enable/disable with top level files [PASS]
[9] ftrace - function graph filters with stack tracer [PASS]
[10] ftrace - function graph filters [PASS]
[11] ftrace - test for function event triggers [PASS]
[12] ftrace - function glob filters [PASS]
[13] ftrace - function pid filters [PASS]
[14] ftrace - function profiler with function tracing [PASS]
[15] ftrace - test reading of set_ftrace_filter [PASS]
[16] ftrace - test for function traceon/off triggers [PASS]
[17] Test creation and deletion of trace instances while setting an event [PASS]
[18] Test creation and deletion of trace instances [PASS]
[19] Kprobe dynamic event - adding and removing [PASS]
[20] Kprobe dynamic event - busy event check [PASS]
[21] Kprobe dynamic event with arguments [PASS]
[22] Kprobes event arguments with types [PASS]
[23] Kprobe event auto/manual naming [PASS]
[24] Kprobe dynamic event with function tracer [PASS]
[25] Kprobe dynamic event - probing module [PASS]
[26] Kretprobe dynamic event with arguments [PASS]
[27] Kretprobe dynamic event with maxactive [PASS]
[28] Register/unregister many kprobe events [PASS]
[29] event trigger - test event enable/disable trigger [PASS]
[30] event trigger - test trigger filter [PASS]
[31] event trigger - test histogram modifiers [PASS]
[32] event trigger - test histogram trigger [PASS]
[33] event trigger - test multiple histogram triggers [PASS]
[34] event trigger - test snapshot-trigger [PASS]
[35] event trigger - test stacktrace-trigger [PASS]
[36] event trigger - test traceon/off trigger [PASS]
[37] (instance)  Basic test for tracers [PASS]
[38] (instance)  Basic trace clock test [PASS]
[39] (instance)  event tracing - enable/disable with event level files [PASS]
[40] (instance)  event tracing - restricts events based on pid [PASS]
[41] (instance)  event tracing - enable/disable with subsystem level
files [PASS]
[42] (instance)  ftrace - test for function event triggers [PASS]
[43] (instance)  ftrace - test for function traceon/off triggers [PASS]
[44] (instance)  event trigger - test event enable/disable trigger [PASS]
[45] (instance)  event trigger - test trigger filter [PASS]
[46] (instance)  event trigger - test histogram modifiers [PASS]
[47] (instance)  event trigger - test histogram trigger [PASS]
[48] (instance)  event trigger - test multiple histogram triggers [PASS]

# of passed:  48
# of failed:  0
# of unresolved:  0
# of untested:  0
# of unsupported:  0
# of xfailed:  0
# of undefined(test bug):  0

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

[Bjorn Helgaas]

On Wed, Dec 20, 2017 at 09:04:27PM -0600, Juan Alvarez wrote:
> On 12/19/17 12:27 AM, Benjamin Herrenschmidt wrote:
> 
> > On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:
> >> [+cc Keith, Gabriele, Dongdong]
> >>
> >> On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:
> >>> Devices can go offline when EEH is reported. This patch adds
> >>> a change to the kernel object and lets udev know of error.
> >>> When device resumes a change is also set reporting device as
> >>> online. Therefore, EEH events are better propagated to user
> >>> space for devices in powerpc arch.
> >> I'm on vacation and can't review this in detail, but I wonder if you
> >> can compare this with the uevents we emit for DPC, AER, and hotplug
> >> events (if any).  I hope we don't end up with userspace having to be
> >> aware of the differences between EEH, DPC, AER, etc.
> >>
> >>> From a very quick look, I only see a few uevents even mentioned in
> >> drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
> >> SR-IOV code.  I'm worried that we're missing some important uevents in
> >> the PCI core.  That's not an argument against what you're doing here;
> >> it just would be nice to fill in any missing pieces in the core also,
> >> and hopefully make them consistent with these EEH events.
> > We also need to be careful about what specific EEH activity we are
> > talking about, and if we bring into the picture things like DPDK, it
> > gets even more murky...
> >
> > The basic way EEH is supposed to work for recovery (minus all sort of
> > implementation nasties which hopefully Russell and Sam are trying to
> > cleanup and fix) is that either:
> >
> > - The driver of the device has recovery callbacks, in which
> > case the driver participates in the recovery process, the device
> > doesn't "go away" (though it shouldn't be accessed during that process
> > by other entities, userspace originated config space could be a problem
> > and needs to be blocked...). The recovery typically involves a reset of
> > the device but in sync with the driver.
> >
> > - The driver doesn't have the callbacks. In this case, we
> > simulate an unplug, reset the device, and replug.
> >
> > So it makes sense for the second case to emit the same uevents as a
> > normal PCI(e) hotplug.
> >
> > For the former case I'm less sure Do we really need userspace to be
> > notified ? If yes, what for precisely ?
> 
> In pSeries SR-IOV environment the management console might need to apply
> certain configuration changes to the PF driver after it has been recovered
> and before the VF drivers are allowed to resume their recovery path.
> I could not think of another way to notify user space of these events.
> I made this assumption because I saw there were no uevents added when 
> the device goes offline and come back online in EEH code. It was my 
> intention to make the event as generic as possible in EEH component,
> therefore, making this change independent of pSeries SR-IOV.

I don't know what your plan for this is, but we do have two different
paths that use the struct pci_error_handlers hooks that drivers may
supply.  There's this AER path that may be used on all arches:

  aer_isr
get_e_source  # remove from rpc->e_sources[] queue
aer_isr_one_error
  aer_process_err_devices
handle_error_source   # or aer_recover_work_func
  do_recovery # for uncorrectable (fatal/nonfatal) only
broadcast_error_message(dev, ..., report_error_detected)
  pci_walk_bus(..., report_error_detected)
report_error_detected
  dev->driver->err_handler->error_detected

And there's this powerpc path where you're adding a uevent:

  eeh_event_handler
eeh_handle_event
  eeh_handle_normal_event
eeh_pe_dev_traverse(pe, eeh_report_error, )
  eeh_report_error
driver->err_handler->error_detected(dev, pci_channel_io_frozen)
 +  kobject_uevent_env(>dev.kobj, KOBJ_CHANGE, envp);

Both paths end up calling the pci_error_handlers.error_detected()
hook.

Drivers are not supposed to care what arch they're running on.  If the
driver supplies an .error_detected() entry point, it's up to the PCI
core and powerpc code to use it consistently across arches.  That
means the same uevents (if any) should be emitted from both paths.

The best way would be to unify the call of .error_detected() so the
AER path and the powerpc path do it via the same function.  The AER
report_error_detected() and the powerpc eeh_report_error() do look
fairly similar, so this seems possible in principle, but I'm not
holding my breath.

Bjorn

Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references

[Steven Rostedt]

On Thu, 28 Dec 2017 16:26:07 +
Ard Biesheuvel  wrote:

> On 28 December 2017 at 16:19, Steven Rostedt  wrote:
> > On Wed, 27 Dec 2017 08:50:33 +
> > Ard Biesheuvel  wrote:
> >  
> >>  static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
> >>  {
> >> - return entry->code;
> >> + return (jump_label_t)>code + entry->code;  
> >
> > I'm paranoid about doing arithmetic on abstract types. What happens in
> > the future if jump_label_t becomes a pointer? You will get a different
> > result.
> >  
> 
> In general, I share your concern. In this case, however, jump_label_t
> is typedef'd three lines up and is never used anywhere else.

I would agree if this was in a .c file, but it's in a header file,
which causes me to be more paranoid.

> 
> > Could we switch these calculations to something like:
> >
> > return (jump_label_t)((long)>code + entry->code);
> >  
> 
> jump_label_t is local to this .h file, so it can be defined as u32 or
> u64 depending on the word size. I don't mind adding the extra cast,
> but I am not sure if your paranoia is justified in this particular
> case. Perhaps we should just use 'unsigned long' throughout?

Actually, that may be better. Have the return value be jump_label_t,
but the cast be "unsigned long". That way it should always work.

static inline jump_label_t jump_entry_code(...)
{
return (unsigned long)>code + entry->code;
}


-- Steve

Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references

[Ard Biesheuvel]

On 28 December 2017 at 16:19, Steven Rostedt  wrote:
> On Wed, 27 Dec 2017 08:50:33 +
> Ard Biesheuvel  wrote:
>
>>  static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
>>  {
>> - return entry->code;
>> + return (jump_label_t)>code + entry->code;
>
> I'm paranoid about doing arithmetic on abstract types. What happens in
> the future if jump_label_t becomes a pointer? You will get a different
> result.
>

In general, I share your concern. In this case, however, jump_label_t
is typedef'd three lines up and is never used anywhere else.

> Could we switch these calculations to something like:
>
> return (jump_label_t)((long)>code + entry->code);
>

jump_label_t is local to this .h file, so it can be defined as u32 or
u64 depending on the word size. I don't mind adding the extra cast,
but I am not sure if your paranoia is justified in this particular
case. Perhaps we should just use 'unsigned long' throughout?

>> +}
>> +
>> +static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
>> +{
>> + return (jump_label_t)>target + entry->target;
>>  }
>>
>>  static inline struct static_key *jump_entry_key(const struct jump_entry 
>> *entry)
>>  {
>> - return (struct static_key *)((unsigned long)entry->key & ~1UL);
>> + unsigned long key = (unsigned long)>key + entry->key;
>> +
>> + return (struct static_key *)(key & ~1UL);
>>  }
>>
>>  static inline bool jump_entry_is_branch(const struct jump_entry *entry)
>> @@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct 
>> jump_entry *entry)
>>   entry->code = 0;
>>  }
>>
>> -#define jump_label_swap  NULL
>> +void jump_label_swap(void *a, void *b, int size);
>>
>>  #else/* __ASSEMBLY__ */
>>
>> @@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct 
>> jump_entry *entry)
>>   .byte   STATIC_KEY_INIT_NOP
>>   .endif
>>   .pushsection __jump_table, "aw"
>> - _ASM_ALIGN
>> - _ASM_PTR.Lstatic_jump_\@, \target, \key
>> + .balign 4
>> + .long   .Lstatic_jump_\@ - ., \target - ., \key - .
>>   .popsection
>>  .endm
>>
>> @@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct 
>> jump_entry *entry)
>>  .Lstatic_jump_after_\@:
>>   .endif
>>   .pushsection __jump_table, "aw"
>> - _ASM_ALIGN
>> - _ASM_PTR.Lstatic_jump_\@, \target, \key + 1
>> + .balign 4
>> + .long   .Lstatic_jump_\@ - ., \target - ., \key - . + 1
>>   .popsection
>>  .endm
>>
>> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
>> index e56c95be2808..cc5034b42335 100644
>> --- a/arch/x86/kernel/jump_label.c
>> +++ b/arch/x86/kernel/jump_label.c
>> @@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry 
>> *entry,
>>* Jump label is enabled for the first time.
>>* So we expect a default_nop...
>>*/
>> - if (unlikely(memcmp((void *)entry->code, default_nop, 
>> 5)
>> -  != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + default_nop, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>
> You have the functions already made before this patch. Perhaps we
> should have a separate patch to use them (here and elsewhere) before
> you make the conversion to using relative references. It will help out
> in debugging and bisects. To know if the use of functions is an issue,
> or the conversion of relative references is an issue.
>
> I suggest splitting this into two patches.
>

Fair enough.


>> +__LINE__);
>>   } else {
>>   /*
>>* ...otherwise expect an ideal_nop. Otherwise
>>* something went horribly wrong.
>>*/
>> - if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
>> -  != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + ideal_nop, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>> +__LINE__);
>>   }
>>
>>   code.jump = 0xe9;
>> - code.offset = entry->target -
>> - (entry->code + JUMP_LABEL_NOP_SIZE);
>> + code.offset = jump_entry_target(entry) -
>> +   (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
>>   } else {
>>   /*
>>* We are 

Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references

[Steven Rostedt]

On Wed, 27 Dec 2017 08:50:33 +
Ard Biesheuvel  wrote:

>  static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
>  {
> - return entry->code;
> + return (jump_label_t)>code + entry->code;

I'm paranoid about doing arithmetic on abstract types. What happens in
the future if jump_label_t becomes a pointer? You will get a different
result.

Could we switch these calculations to something like:

return (jump_label_t)((long)>code + entry->code);

> +}
> +
> +static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
> +{
> + return (jump_label_t)>target + entry->target;
>  }
>  
>  static inline struct static_key *jump_entry_key(const struct jump_entry 
> *entry)
>  {
> - return (struct static_key *)((unsigned long)entry->key & ~1UL);
> + unsigned long key = (unsigned long)>key + entry->key;
> +
> + return (struct static_key *)(key & ~1UL);
>  }
>  
>  static inline bool jump_entry_is_branch(const struct jump_entry *entry)
> @@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct 
> jump_entry *entry)
>   entry->code = 0;
>  }
>  
> -#define jump_label_swap  NULL
> +void jump_label_swap(void *a, void *b, int size);
>  
>  #else/* __ASSEMBLY__ */
>  
> @@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct 
> jump_entry *entry)
>   .byte   STATIC_KEY_INIT_NOP
>   .endif
>   .pushsection __jump_table, "aw"
> - _ASM_ALIGN
> - _ASM_PTR.Lstatic_jump_\@, \target, \key
> + .balign 4
> + .long   .Lstatic_jump_\@ - ., \target - ., \key - .
>   .popsection
>  .endm
>  
> @@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct 
> jump_entry *entry)
>  .Lstatic_jump_after_\@:
>   .endif
>   .pushsection __jump_table, "aw"
> - _ASM_ALIGN
> - _ASM_PTR.Lstatic_jump_\@, \target, \key + 1
> + .balign 4
> + .long   .Lstatic_jump_\@ - ., \target - ., \key - . + 1
>   .popsection
>  .endm
>  
> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> index e56c95be2808..cc5034b42335 100644
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry 
> *entry,
>* Jump label is enabled for the first time.
>* So we expect a default_nop...
>*/
> - if (unlikely(memcmp((void *)entry->code, default_nop, 5)
> -  != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + default_nop, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),

You have the functions already made before this patch. Perhaps we
should have a separate patch to use them (here and elsewhere) before
you make the conversion to using relative references. It will help out
in debugging and bisects. To know if the use of functions is an issue,
or the conversion of relative references is an issue.

I suggest splitting this into two patches.

-- Steve


> +__LINE__);
>   } else {
>   /*
>* ...otherwise expect an ideal_nop. Otherwise
>* something went horribly wrong.
>*/
> - if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
> -  != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + ideal_nop, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),
> +__LINE__);
>   }
>  
>   code.jump = 0xe9;
> - code.offset = entry->target -
> - (entry->code + JUMP_LABEL_NOP_SIZE);
> + code.offset = jump_entry_target(entry) -
> +   (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
>   } else {
>   /*
>* We are disabling this jump label. If it is not what
> @@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry 
> *entry,
>* are converting the default nop to the ideal nop.
>*/
>   if (init) {
> - if (unlikely(memcmp((void *)entry->code, default_nop, 
> 5) != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + default_nop, 5) != 0))
> + 

Re: [PATCH v6 5/8] kernel: tracepoints: add support for relative references

[Steven Rostedt]

On Wed, 27 Dec 2017 08:50:30 +
Ard Biesheuvel  wrote:

> To avoid the need for relocating absolute references to tracepoint
> structures at boot time when running relocatable kernels (which may
> take a disproportionate amount of space), add the option to emit
> these tables as relative references instead.
> 

I gave this patch a quick skim over. It appears to not modify anything
when CONFIG_HAVE_PREL32_RELOCATIONS is not defined. I haven't
thoroughly reviewed it or tested it. But if it doesn't break anything,
I'm fine giving you an ack.

Acked-by: Steven Rostedt (VMware) 

--  Steve

Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries

[Ard Biesheuvel]

On 28 December 2017 at 12:05, Ingo Molnar  wrote:
>
> * Ard Biesheuvel  wrote:
>
>> Annoyingly, we need this because there is a single instance of a
>> special section that ends up in the EFI stub code: we build lib/sort.c
>> again as a EFI libstub object, and given that sort() is exported, we
>> end up with a ksymtab section in the EFI stub. The sort() thing has
>> caused issues before [0], so perhaps I should just clone sort.c into
>> drivers/firmware/efi/libstub and get rid of that hack.
>>
>> [0] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29f9007b3182ab3f328a31da13e6b1c9072f7a95
>
> If the root problem is early bootstrap code randomly using generic facility 
> that
> isn't __init, then we should definitely improve tooling to at least detect 
> these
> problems.
>
> As bootstrap code gets improved (KASLR, more complex decompression, etc. 
> etc.) we
> keep using new bits of generic facilities...
>
> So this should definitely not be hidden by open coding that function (which 
> has
> various other disadvantages as well), but should be turned from silent 
> breakage
> either into non-breakage (and do so not only for sort() but for other generic
> functions as well), or should be turned into a build failure.
>

We already have safeguards in place to ensure that the arm64 EFI stub
(which is essentially the same executable as the kernel proper) only
pulls in code that has been made available to it explicitly. That is
why sort.c is recompiled for the EFI stub, as well as all other C code
that is shared between the stub and the kernel. We also have a build
time check to ensure that the resulting code does not rely on absolute
symbol references, which will be invalid in the UEFI execution
context.

So the only problem is that unneeded ksymtab/kcrctab sections, which
affected ARM for obscure reasons; typically, they just take up some
space. On x86, the kaslr code deals with a similar issue by
#define'ing _LINUX_EXPORT_H before including linux/export.h, which
also gets rid of these sections, but I was a bit reluctant to copy
that pattern. Perhaps we should enhance linux/export.h for reasons
such as these by adding a macro that nops out EXPORT_SYMBOL()
declarations?

Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries

[Ingo Molnar]

* Ard Biesheuvel  wrote:

> Annoyingly, we need this because there is a single instance of a
> special section that ends up in the EFI stub code: we build lib/sort.c
> again as a EFI libstub object, and given that sort() is exported, we
> end up with a ksymtab section in the EFI stub. The sort() thing has
> caused issues before [0], so perhaps I should just clone sort.c into
> drivers/firmware/efi/libstub and get rid of that hack.
> 
> [0] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29f9007b3182ab3f328a31da13e6b1c9072f7a95

If the root problem is early bootstrap code randomly using generic facility 
that 
isn't __init, then we should definitely improve tooling to at least detect 
these 
problems.

As bootstrap code gets improved (KASLR, more complex decompression, etc. etc.) 
we 
keep using new bits of generic facilities...

So this should definitely not be hidden by open coding that function (which has 
various other disadvantages as well), but should be turned from silent breakage 
either into non-breakage (and do so not only for sort() but for other generic 
functions as well), or should be turned into a build failure.

Thanks,

Ingo