Yury,
this patch has been based on an earlier version of vdso and mainly adjusted to
match
the requirements of commit 601255ae3c98fdeeee3a8bb4696425e4f868b4f1.
As these are mainly style-changes, please feel free to revise and adjust as
needed.
Regards,
Philipp.
> On 01 Oct 2015, at 21:44, Yury Norov <yno...@caviumnetworks.com> wrote:
>
> On Tue, Sep 29, 2015 at 11:06:13PM -0500, Nathan Lynch wrote:
>> On 09/29/2015 05:14 PM, Yury Norov wrote:
>>> From: Philipp Tomsich <philipp.toms...@theobroma-systems.com>
>>>
>>> Adjusted to move the move data page before code pages in sync with
>>> commit 601255ae3c98fdeeee3a8bb4696425e4f868b4f1
>>
>> This commit message needs more information about how the ilp32 VDSO uses
>> the existing arm64 code. I had to really hunt through the Makefile to
>> figure out what's going on.
>>
>> The commit message should also identify the APIs that are supported.
>> The subject line mentions signal return, but gettimeofday, clock_gettime
>> and clock_getres are being added here too, and it is not obvious.
>>
>>
>>> Signed-off-by: Philipp Tomsich <philipp.toms...@theobroma-systems.com>
>>> Signed-off-by: Christoph Muellner <christoph.muell...@theobroma-systems.com>
>>> Signed-off-by: Yury Norov <yno...@caviumnetworks.com>
>>>
>>> create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
>>> create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
>>> copy arch/arm64/{include/asm/vdso.h => kernel/vdso-ilp32/vdso-ilp32.S} (56%)
>>> create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>>
>> How are you invoking git-format-patch? The copy detection in this case
>> is not conducive to review.
>>
>> It looks like the existing arm64 vdso Makefile has been copied to
>> vdso-ilp32/ and adjusted for paths and naming. While the gettimeofday
>> assembly implementation is reused, the build logic is duplicated. x86
>> produces VDSOs for multiple ABIs with a single Makefile; is a similar
>> approach not appropriate for arm64?
>>
>>
>>> diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>>> b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>>> new file mode 100644
>>> index 0000000..ac8029b
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>>> @@ -0,0 +1,98 @@
>>
>> [...]
>>
>>> +#include <linux/const.h>
>>> +#include <asm/page.h>
>>> +#include <asm/vdso.h>
>>> +
>>> +/*OUTPUT_FORMAT("elf32-littleaarch64", "elf32-bigaarch64",
>>> "elf32-littleaarch64")
>>> +OUTPUT_ARCH(aarch64)
>>> +*/
>>
>> If these lines aren't needed then omit them.
>>
>> [...]
>>
>>
>>> +/*
>>> + * This controls what symbols we export from the DSO.
>>> + */
>>> +VERSION
>>> +{
>>> + LINUX_2.6.39 {
>>> + global:
>>> + __kernel_rt_sigreturn;
>>> + __kernel_gettimeofday;
>>> + __kernel_clock_gettime;
>>> + __kernel_clock_getres;
>>> + local: *;
>>> + };
>>> +}
>>
>> Something that came up during review of arch/arm's VDSO code: consider
>> using version and names that match x86, i.e. LINUX_2.6, __vdso_gettimeofday.
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267940.html
>>
>> Using LINUX_2.6.39 for this code is nonsensical.
>>
>>
>>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>>> index b239b9b..bed6cf1 100644
>>> --- a/arch/arm64/kernel/vdso.c
>>> +++ b/arch/arm64/kernel/vdso.c
>>> @@ -40,6 +40,12 @@ extern char vdso_start, vdso_end;
>>> static unsigned long vdso_pages;
>>> static struct page **vdso_pagelist;
>>>
>>> +#ifdef CONFIG_ARM64_ILP32
>>> +extern char vdso_ilp32_start, vdso_ilp32_end;
>>> +static unsigned long vdso_ilp32_pages;
>>> +static struct page **vdso_ilp32_pagelist;
>>> +#endif
>>> +
>>> /*
>>> * The vDSO data page.
>>> */
>>> @@ -117,24 +123,29 @@ int aarch32_setup_vectors_page(struct linux_binprm
>>> *bprm, int uses_interp)
>>> }
>>> #endif /* CONFIG_AARCH32_EL0 */
>>>
>>> -static struct vm_special_mapping vdso_spec[2];
>>> -
>>> -static int __init vdso_init(void)
>>> +static inline int __init vdso_init_common(char *vdso_start, char *vdso_end,
>>
>> No inline please.
>>
>>
>>> + unsigned long *vdso_pagesp,
>>> + struct page ***vdso_pagelistp,
>>> + struct vm_special_mapping* vdso_spec)
>>> {
>>
>> [...]
>>
>>> int arch_setup_additional_pages(struct linux_binprm *bprm,
>>> int uses_interp)
>>> {
>>> struct mm_struct *mm = current->mm;
>>> unsigned long vdso_base, vdso_text_len, vdso_mapping_len;
>>> - void *ret;
>>> + void* ret;
>>
>> Gratuitous (and incorrect) style change.
>>
>>
>>> + unsigned long pages = vdso_pages;
>>> + struct vm_special_mapping* spec = vdso_spec;
>>
>> Incorrect style: *spec
>
> Hi Nathan,
>
> If Philipp Philipp Tomsich will not answer soon, I'll fix all this.
>
> BR,
> Yury.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
