[RFC PATCH v3 07/11] arm64: compat: Add a 32-bit vDSO
flat view
STALE3533d
From: Nathan Lynch <hidden>
Date: 2016-12-07 18:51:01
Hi Kevin, Kevin Brodsky [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c new file mode 100644 index 000000000000..53c3d1f82b26 --- /dev/null +++ b/arch/arm64/kernel/vdso32/vgettimeofday.c
As I said at LPC last month, I'm not excited to have arch/arm's vgettimeofday.c copied into arch/arm64 and tweaked; I'd rather see this part of the implementation shared between arch/arm and arch/arm64 somehow, even if there's not an elegant way to do so. The situation which must be avoided is one where the arch/arm64 compat VDSO incompatibly diverges from the arch/arm VDSO. That becomes much less likely if there's only one copy of the userspace-exposed code to maintain. [...]
quoted hunk ↗ jump to hunk
+/* + * We use the hidden visibility to prevent the compiler from generating a GOT + * relocation. Not only is going through a GOT useless (the entry couldn't and + * musn't be overridden by another library), it does not even work: the linker + * cannot generate an absolute address to the data page. + * + * With the hidden visibility, the compiler simply generates a PC-relative + * relocation (R_ARM_REL32), and this is what we need. + */ +extern const struct vdso_data _vdso_data __attribute__((visibility("hidden"))); + +static inline const struct vdso_data *get_vdso_data(void) +{ + const struct vdso_data *ret; + /* + * This simply puts &_vdso_data into ret. The reason why we don't use + * `ret = &_vdso_data` is that the compiler tends to optimise this in a + * very suboptimal way: instead of keeping &_vdso_data in a register, + * it goes through a relocation almost every time _vdso_data must be + * accessed (even in subfunctions). This is both time and space + * consuming: each relocation uses a word in the code section, and it + * has to be loaded at runtime. + * + * This trick hides the assignment from the compiler. Since it cannot + * track where the pointer comes from, it will only use one relocation + * where get_vdso_data() is called, and then keep the result in a + * register. + */ + asm("mov %0, %1" : "=r"(ret) : "r"(&_vdso_data)); + return ret; +}
I think this is an instructive case: this code looks like an improvement over how the arch/arm VDSO accesses the data page. Enhancements like this (not to mention fixes) shouldn't require a patch-per-architecture; things inevitably get out of sync and it's more of a maintenance burden in the long term.