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.

  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/484bee87-00a2-9121-6a99-77949b5fbf5d%40oth-regensburg.de.

Reply via email to