On 8/14/19 4:35 PM, Jan Kiszka wrote: > On 14.08.19 16:11, Ralf Ramsauer wrote: >> >> On 7/25/19 8:01 AM, Jan Kiszka wrote: >>> "lapic_timer_frequency heißt jetzt lapic_timer_period, sonst ändert sich >>> nix." >> >> Doch. >> >> We have a problem if KALLSYMS_ALL=y. Jailhouse will compile, but it >> fails loading the driver module with EINVAL, the symbol can not be >> resolved, see below. >> >>> >>> Signed-off-by: Jan Kiszka <[email protected]> >>> --- >>> driver/main.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/driver/main.c b/driver/main.c >>> index fe752753..b3896609 100644 >>> --- a/driver/main.c >>> +++ b/driver/main.c >>> @@ -100,7 +100,10 @@ static struct resource *hypervisor_mem_res; >>> static typeof(ioremap_page_range) *ioremap_page_range_sym; >>> #ifdef CONFIG_X86 >>> -static typeof(lapic_timer_frequency) *lapic_timer_frequency_sym; >>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0) >>> +#define lapic_timer_period lapic_timer_frequency >>> +#endif >>> +static typeof(lapic_timer_period) *lapic_timer_period_sym; >>> #endif >>> #ifdef CONFIG_ARM >>> static typeof(__boot_cpu_mode) *__boot_cpu_mode_sym; >>> @@ -550,7 +553,7 @@ static int jailhouse_cmd_enable(struct >>> jailhouse_system __user *arg) >>> config->platform_info.x86.tsc_khz = tsc_khz; >>> if (config->platform_info.x86.apic_khz == 0) >>> config->platform_info.x86.apic_khz = >>> - *lapic_timer_frequency_sym / (1000 / HZ); >>> + *lapic_timer_period_sym / (1000 / HZ); >>> #endif >>> err = jailhouse_cell_prepare_root(&config->root_cell); >>> @@ -897,7 +900,7 @@ static int __init jailhouse_init(void) >>> RESOLVE_EXTERNAL_SYMBOL(ioremap_page_range); >>> #ifdef CONFIG_X86 >>> - RESOLVE_EXTERNAL_SYMBOL(lapic_timer_frequency); >>> + RESOLVE_EXTERNAL_SYMBOL(lapic_timer_period); >> Here, lapic_timer_period won't be replaced with lapic_timer_frequency in >> the RESOLVE_EXTERNAL_SYMBOL macro: >> >> symbol##_sym = (void *)kallsyms_lookup_name(#symbol); \ >> > > Hmm, 2-stage pre-processor would be needed here. #if LINUX_VERSION_CODE > sees to be required then, what a pity.
Yep, I have that exact case: 4.19.x + KALLSYMS_ALL. > > Want to write a patch as you have the setup already? Well, my current setup is to disable KALLSYMS_ALL. :-) But I don't see how this could be patched, is there a way to tell the preprocessor to roll out #symbol before interpreting it as string. The only way that I see at the moment is something like the attached patch (only compile-time tested). Ralf > > Jan > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/54d5522f-16cb-0ebf-b93a-821d13da7ce8%40oth-regensburg.de.
diff --git a/driver/main.c b/driver/main.c index b3896609..6f926f18 100644 --- a/driver/main.c +++ b/driver/main.c @@ -102,6 +102,9 @@ static typeof(ioremap_page_range) *ioremap_page_range_sym; #ifdef CONFIG_X86 #if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0) #define lapic_timer_period lapic_timer_frequency +#define LAPIC_TIMER_PERIOD_NAME "lapic_timer_frequency" +#else /* Kernel >= 5.3.0 */ +#define LAPIC_TIMER_PERIOD_NAME "lapic_timer_period" #endif static typeof(lapic_timer_period) *lapic_timer_period_sym; #endif @@ -889,18 +892,21 @@ static int __init jailhouse_init(void) int err; #ifdef CONFIG_KALLSYMS_ALL -#define RESOLVE_EXTERNAL_SYMBOL(symbol) \ - symbol##_sym = (void *)kallsyms_lookup_name(#symbol); \ - if (!symbol##_sym) \ +#define _RESOLVE_EXTERNAL_SYMBOL(symbol, symbol_name) \ + symbol##_sym = (void *)kallsyms_lookup_name(symbol_name); \ + if (!symbol##_sym) \ return -EINVAL +#define RESOLVE_EXTERNAL_SYMBOL(symbol) \ + _RESOLVE_EXTERNAL_SYMBOL(symbol, #symbol) #else -#define RESOLVE_EXTERNAL_SYMBOL(symbol) \ +#define RESOLVE_EXTERNAL_SYMBOL(symbol) +#define _RESOLVE_EXTERNAL_SYMBOL(symbol, _) \ symbol##_sym = &symbol #endif RESOLVE_EXTERNAL_SYMBOL(ioremap_page_range); #ifdef CONFIG_X86 - RESOLVE_EXTERNAL_SYMBOL(lapic_timer_period); + _RESOLVE_EXTERNAL_SYMBOL(lapic_timer_period, LAPIC_TIMER_PERIOD_NAME); #endif #ifdef CONFIG_ARM RESOLVE_EXTERNAL_SYMBOL(__boot_cpu_mode);
