On 8/14/19 5:50 PM, Jan Kiszka wrote:
> On 14.08.19 17:27, Ralf Ramsauer wrote:
>>
>>
>> On 8/14/19 5:18 PM, Jan Kiszka wrote:
>>> On 14.08.19 17:16, Ralf Ramsauer wrote:
>>>>
>>>>
>>>> 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).
>>>
>>> #if VERSION < 5.3
>>> RESOLVE_EXTERNAL_SYMBOL(lapic_timer_frequency);
>>> #else
>>> RESOLVE_EXTERNAL_SYMBOL(lapic_timer_period);
>>> #endif
>>
>> Huh? How should this work?
>>
>> Independent of the kernel version, we have to set
>> lapic_timer_period_sym. The VERSION < 5.3 case will try to set
>> lapic_timer_frequency_sym, which doesn't exist and isn't used.
>
> Ah, oh. Would conditionally redefining lapic_timer_period_sym to
> lapic_timer_frequency_sym help us?
Not really, then we run into the 2-stage preprocessor issue again. But
this might do the job (only compile-time tested):
diff --git a/driver/main.c b/driver/main.c
index b3896609..beff73d6 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -102,6 +102,8 @@ static typeof(ioremap_page_range)
#ifdef CONFIG_X86
#if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0)
#define lapic_timer_period lapic_timer_frequency
+static typeof(lapic_timer_period) *lapic_timer_frequency_sym
+ __attribute__((alias("lapic_timer_period_sym")));
#endif
static typeof(lapic_timer_period) *lapic_timer_period_sym;
#endif
@@ -900,8 +902,12 @@ static int __init jailhouse_init(void)
RESOLVE_EXTERNAL_SYMBOL(ioremap_page_range);
#ifdef CONFIG_X86
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0)
+ RESOLVE_EXTERNAL_SYMBOL(lapic_timer_frequency);
+#else
RESOLVE_EXTERNAL_SYMBOL(lapic_timer_period);
-#endif
+#endif /* LINUX_VERSION_CODE */
+#endif /* CONFIG_X86 */
#ifdef CONFIG_ARM
RESOLVE_EXTERNAL_SYMBOL(__boot_cpu_mode);
#endif
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/aae3503f-d41e-6d74-76b2-166c1f02a51a%40oth-regensburg.de.
