Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Montag, den 03.02.2014, 14:04 -0800 schrieb Andy Lutomirski:
> On Mon, Feb 3, 2014 at 2:01 PM, Stefani Seibold wrote:
> > Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski:
> >> On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold
> >> wrote:
> >> > Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski:
> >> >> On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold
> >> >> wrote:
> >> >> > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski:
> >> >> >> On Sun, Feb 2, 2014 at 3:27 AM, wrote:
> >> >> >> > From: Stefani Seibold
> >> >> >> >
> >> >> >> > This patch add the time support for 32 bit a VDSO to a 32 bit
> >> >> >> > kernel.
> >> >> >>
> >> >> >> [...]
> >> >> >>
> >> >> >> Can you address the review comments from last time around? For
> >> >> >> example, this still seems to have redundant vvar and hpet mappings,
> >> >> >> it
> >> >> >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc.
> >> >> >>
> >> >> >
> >> >> > I will address the compat VDSO issue.
> >> >> >
> >> >> > But the VVAR macro will be not a part of this patch set. If you depend
> >> >> > on this, feel free to create one. From my point of view this is not
> >> >> > feasible without a macro hacking, because the address accessing the
> >> >> > vvar
> >> >> > area differs in kernel and VDSO user mode.
> >> >>
> >> >> Sorry, but "I will make the code messier for no apparent reason and I
> >> >> will not offer to fix it in the same series" gets my NAK.
> >> >>
> >> >> Hint: I'm talking about two or three lines of code in vvar.h.
> >> >>
> >> >
> >> > A hint back: if you threat me with a NAK for a requested code sequence
> >> > which currently no user, this is far away from professional. I am not
> >> > your trainee.
> >> >
> >> > BTW: If it is so easy, send me the two or three lines and i will merge
> >> > it ;-)
> >>
> >> Something to the effect of:
> >>
> >> #elif defined(BUILD_VDSO32)
> >> #define VVAR(name) (*vvar_ ## name)
> >> #endif
> >>
> >> Should do the trick.
> >
> > You are wrong...
> >
> > #ifdef BUILD_VDSO32
> >
> > #define DECLARE_VVAR(offset, type, name) \
> > extern type vvar_ ## name __attribute__((visibility("hidden")));
> >
> > #define VVAR(name) (vvar_ ## name)
> >
> > #else
> >
> > /* Base address of vvars. This is not ABI. */
> > #ifdef CONFIG_X86_64
> > #define VVAR_ADDRESS (-10*1024*1024 - 4096)
> > #else
> > extern char __vvar_page;
> >
> > #define VVAR_ADDRESS (&__vvar_page)
> > #endif
> >
> > This would do the trick!
> >
> > But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is
> > still a
> >
> > struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data
> > __attribute__((visibility("hidden")));
> > #define gtod (_vvar_vsyscall_gtod_data)
> >
> > needed, because vvar_vsyscall_gtod_data is the 32 bit version, which
> > would result in incorrect access of the struct members. So my code can't
> > use this VVAR macro.
>
> For 32-on-64, I must have read your code wrong. Are you sticking two
> copies of the same struct with different layout into the vvar page?
> If so, wouldn't it be better to only have the variant with a common
> layout and use it for all versions of the vdso?
>
No, only one. But for depend on 32/64 bit the layout differs.
We discuss this topic some days before, so please have a look at the
code and the previous posts.
- Stefani
--
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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Mon, Feb 3, 2014 at 2:01 PM, Stefani Seibold wrote:
> Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski:
>> On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold wrote:
>> > Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski:
>> >> On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold
>> >> wrote:
>> >> > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski:
>> >> >> On Sun, Feb 2, 2014 at 3:27 AM, wrote:
>> >> >> > From: Stefani Seibold
>> >> >> >
>> >> >> > This patch add the time support for 32 bit a VDSO to a 32 bit kernel.
>> >> >>
>> >> >> [...]
>> >> >>
>> >> >> Can you address the review comments from last time around? For
>> >> >> example, this still seems to have redundant vvar and hpet mappings, it
>> >> >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc.
>> >> >>
>> >> >
>> >> > I will address the compat VDSO issue.
>> >> >
>> >> > But the VVAR macro will be not a part of this patch set. If you depend
>> >> > on this, feel free to create one. From my point of view this is not
>> >> > feasible without a macro hacking, because the address accessing the vvar
>> >> > area differs in kernel and VDSO user mode.
>> >>
>> >> Sorry, but "I will make the code messier for no apparent reason and I
>> >> will not offer to fix it in the same series" gets my NAK.
>> >>
>> >> Hint: I'm talking about two or three lines of code in vvar.h.
>> >>
>> >
>> > A hint back: if you threat me with a NAK for a requested code sequence
>> > which currently no user, this is far away from professional. I am not
>> > your trainee.
>> >
>> > BTW: If it is so easy, send me the two or three lines and i will merge
>> > it ;-)
>>
>> Something to the effect of:
>>
>> #elif defined(BUILD_VDSO32)
>> #define VVAR(name) (*vvar_ ## name)
>> #endif
>>
>> Should do the trick.
>
> You are wrong...
>
> #ifdef BUILD_VDSO32
>
> #define DECLARE_VVAR(offset, type, name) \
> extern type vvar_ ## name __attribute__((visibility("hidden")));
>
> #define VVAR(name) (vvar_ ## name)
>
> #else
>
> /* Base address of vvars. This is not ABI. */
> #ifdef CONFIG_X86_64
> #define VVAR_ADDRESS (-10*1024*1024 - 4096)
> #else
> extern char __vvar_page;
>
> #define VVAR_ADDRESS (&__vvar_page)
> #endif
>
> This would do the trick!
>
> But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is
> still a
>
> struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data
> __attribute__((visibility("hidden")));
> #define gtod (_vvar_vsyscall_gtod_data)
>
> needed, because vvar_vsyscall_gtod_data is the 32 bit version, which
> would result in incorrect access of the struct members. So my code can't
> use this VVAR macro.
For 32-on-64, I must have read your code wrong. Are you sticking two
copies of the same struct with different layout into the vvar page?
If so, wouldn't it be better to only have the variant with a common
layout and use it for all versions of the vdso?
--Andy
--
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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski:
> On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold wrote:
> > Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski:
> >> On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold
> >> wrote:
> >> > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski:
> >> >> On Sun, Feb 2, 2014 at 3:27 AM, wrote:
> >> >> > From: Stefani Seibold
> >> >> >
> >> >> > This patch add the time support for 32 bit a VDSO to a 32 bit kernel.
> >> >>
> >> >> [...]
> >> >>
> >> >> Can you address the review comments from last time around? For
> >> >> example, this still seems to have redundant vvar and hpet mappings, it
> >> >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc.
> >> >>
> >> >
> >> > I will address the compat VDSO issue.
> >> >
> >> > But the VVAR macro will be not a part of this patch set. If you depend
> >> > on this, feel free to create one. From my point of view this is not
> >> > feasible without a macro hacking, because the address accessing the vvar
> >> > area differs in kernel and VDSO user mode.
> >>
> >> Sorry, but "I will make the code messier for no apparent reason and I
> >> will not offer to fix it in the same series" gets my NAK.
> >>
> >> Hint: I'm talking about two or three lines of code in vvar.h.
> >>
> >
> > A hint back: if you threat me with a NAK for a requested code sequence
> > which currently no user, this is far away from professional. I am not
> > your trainee.
> >
> > BTW: If it is so easy, send me the two or three lines and i will merge
> > it ;-)
>
> Something to the effect of:
>
> #elif defined(BUILD_VDSO32)
> #define VVAR(name) (*vvar_ ## name)
> #endif
>
> Should do the trick.
You are wrong...
#ifdef BUILD_VDSO32
#define DECLARE_VVAR(offset, type, name) \
extern type vvar_ ## name __attribute__((visibility("hidden")));
#define VVAR(name) (vvar_ ## name)
#else
/* Base address of vvars. This is not ABI. */
#ifdef CONFIG_X86_64
#define VVAR_ADDRESS (-10*1024*1024 - 4096)
#else
extern char __vvar_page;
#define VVAR_ADDRESS (&__vvar_page)
#endif
This would do the trick!
But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is
still a
struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data
__attribute__((visibility("hidden")));
#define gtod (_vvar_vsyscall_gtod_data)
needed, because vvar_vsyscall_gtod_data is the 32 bit version, which
would result in incorrect access of the struct members. So my code can't
use this VVAR macro.
>
> >
> >> >
> >> > I also see no redundant mapping. There are two modes, one is the map of
> >> > the kernel area the other maps the VDSO into the user space area. This
> >> > is exactly the behaviour of the origin VDSO implementation.
> >>
> >> No.
> >>
> >> In your series there are *three* mappings. There are:
> >>
> >> - The linear mapping that the kernel loader sets up (the writable
> >> mapping used in the kernel). This is implicit and, of course, fine.
> >> - There's the fixmap page, which aliases the normal kernel mapping at
> >> a fixed address with the user, ro, and nx attributes. The 64-bit vDSO
> >> uses that mapping. See vdso.h -- it's all arranged pretty clearly.
> >> Your code, for no discernible reason, sets up a fixmap entry on
> >> *32-bit* kernels.
> >> - The vma that you're setting up adjacent to the actual vdso text.
> >> This is what you are using.
> >>
> >> Please choose *one* user-readable mapping for the 32-bit vdso and
> >> stick with it. If the 64-bit vdso can use it to and userspace doesn't
> >> break, even better. But a pointless set of extra fixmap entries is
> >> not okay.
> >>
> >
> > Again: I wrote that there are two modes for a 32 bit kernel and
> > therefore there are two mappings at the same time. Since there are both
> > ways available in a 32 bit kernel via the vdso32= kernel parameter, both
> > must be supported.
> >
> > Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a
> > variable), the HPET and VVAR Page can only relative addressed. So this
> > pages must located before or after the VDSO.
> >
> > This is why i need to setup this pages into the fixmap area, this is the
> > compat mode "vdso32=2".
> >
> > For "vdso32=1" i need to map the VDSO Page together with the HPET and
> > VVAR into the user space.
> >
> > For compability reasons both mappings are required.
>
> Not at the same time, and I don't think you've guarded the
> map_vsyscall call with a check for compat mode.
>
I see no benefit to check the compat mode in map_vsyscall. The best way
is always map the VVAR page, because it is less error prone. In your
case tt would be also necessary not map the HPET page in non compat mode
too. So why bloat the code?
- Stefani
--
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
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold wrote: > Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: >> On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold wrote: >> > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: >> >> On Sun, Feb 2, 2014 at 3:27 AM, wrote: >> >> > From: Stefani Seibold >> >> > >> >> > This patch add the time support for 32 bit a VDSO to a 32 bit kernel. >> >> >> >> [...] >> >> >> >> Can you address the review comments from last time around? For >> >> example, this still seems to have redundant vvar and hpet mappings, it >> >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. >> >> >> > >> > I will address the compat VDSO issue. >> > >> > But the VVAR macro will be not a part of this patch set. If you depend >> > on this, feel free to create one. From my point of view this is not >> > feasible without a macro hacking, because the address accessing the vvar >> > area differs in kernel and VDSO user mode. >> >> Sorry, but "I will make the code messier for no apparent reason and I >> will not offer to fix it in the same series" gets my NAK. >> >> Hint: I'm talking about two or three lines of code in vvar.h. >> > > A hint back: if you threat me with a NAK for a requested code sequence > which currently no user, this is far away from professional. I am not > your trainee. > > BTW: If it is so easy, send me the two or three lines and i will merge > it ;-) Something to the effect of: #elif defined(BUILD_VDSO32) #define VVAR(name) (*vvar_ ## name) #endif Should do the trick. > >> > >> > I also see no redundant mapping. There are two modes, one is the map of >> > the kernel area the other maps the VDSO into the user space area. This >> > is exactly the behaviour of the origin VDSO implementation. >> >> No. >> >> In your series there are *three* mappings. There are: >> >> - The linear mapping that the kernel loader sets up (the writable >> mapping used in the kernel). This is implicit and, of course, fine. >> - There's the fixmap page, which aliases the normal kernel mapping at >> a fixed address with the user, ro, and nx attributes. The 64-bit vDSO >> uses that mapping. See vdso.h -- it's all arranged pretty clearly. >> Your code, for no discernible reason, sets up a fixmap entry on >> *32-bit* kernels. >> - The vma that you're setting up adjacent to the actual vdso text. >> This is what you are using. >> >> Please choose *one* user-readable mapping for the 32-bit vdso and >> stick with it. If the 64-bit vdso can use it to and userspace doesn't >> break, even better. But a pointless set of extra fixmap entries is >> not okay. >> > > Again: I wrote that there are two modes for a 32 bit kernel and > therefore there are two mappings at the same time. Since there are both > ways available in a 32 bit kernel via the vdso32= kernel parameter, both > must be supported. > > Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a > variable), the HPET and VVAR Page can only relative addressed. So this > pages must located before or after the VDSO. > > This is why i need to setup this pages into the fixmap area, this is the > compat mode "vdso32=2". > > For "vdso32=1" i need to map the VDSO Page together with the HPET and > VVAR into the user space. > > For compability reasons both mappings are required. Not at the same time, and I don't think you've guarded the map_vsyscall call with a check for compat mode. --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) Something to the effect of: #elif defined(BUILD_VDSO32) #define VVAR(name) (*vvar_ ## name) #endif Should do the trick. I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. No. In your series there are *three* mappings. There are: - The linear mapping that the kernel loader sets up (the writable mapping used in the kernel). This is implicit and, of course, fine. - There's the fixmap page, which aliases the normal kernel mapping at a fixed address with the user, ro, and nx attributes. The 64-bit vDSO uses that mapping. See vdso.h -- it's all arranged pretty clearly. Your code, for no discernible reason, sets up a fixmap entry on *32-bit* kernels. - The vma that you're setting up adjacent to the actual vdso text. This is what you are using. Please choose *one* user-readable mapping for the 32-bit vdso and stick with it. If the 64-bit vdso can use it to and userspace doesn't break, even better. But a pointless set of extra fixmap entries is not okay. Again: I wrote that there are two modes for a 32 bit kernel and therefore there are two mappings at the same time. Since there are both ways available in a 32 bit kernel via the vdso32= kernel parameter, both must be supported. Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a variable), the HPET and VVAR Page can only relative addressed. So this pages must located before or after the VDSO. This is why i need to setup this pages into the fixmap area, this is the compat mode vdso32=2. For vdso32=1 i need to map the VDSO Page together with the HPET and VVAR into the user space. For compability reasons both mappings are required. Not at the same time, and I don't think you've guarded the map_vsyscall call with a check for compat mode. --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) Something to the effect of: #elif defined(BUILD_VDSO32) #define VVAR(name) (*vvar_ ## name) #endif Should do the trick. You are wrong... #ifdef BUILD_VDSO32 #define DECLARE_VVAR(offset, type, name) \ extern type vvar_ ## name __attribute__((visibility(hidden))); #define VVAR(name) (vvar_ ## name) #else /* Base address of vvars. This is not ABI. */ #ifdef CONFIG_X86_64 #define VVAR_ADDRESS (-10*1024*1024 - 4096) #else extern char __vvar_page; #define VVAR_ADDRESS (__vvar_page) #endif This would do the trick! But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is still a struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data __attribute__((visibility(hidden))); #define gtod (arch_vvar_vsyscall_gtod_data) needed, because vvar_vsyscall_gtod_data is the 32 bit version, which would result in incorrect access of the struct members. So my code can't use this VVAR macro. I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. No. In your series there are *three* mappings. There are: - The linear mapping that the kernel loader sets up (the writable mapping used in the kernel). This is implicit and, of course, fine. - There's the fixmap page, which aliases the normal kernel mapping at a fixed address with the user, ro, and nx attributes. The 64-bit vDSO uses that mapping. See vdso.h -- it's all arranged pretty clearly. Your code, for no discernible reason, sets up a fixmap entry on *32-bit* kernels. - The vma that you're setting up adjacent to the actual vdso text. This is what you are using. Please choose *one* user-readable mapping for the 32-bit vdso and stick with it. If the 64-bit vdso can use it to and userspace doesn't break, even better. But a pointless set of extra fixmap entries is not okay. Again: I wrote that there are two modes for a 32 bit kernel and therefore there are two mappings at the same time. Since there are both ways available in a 32 bit kernel via the vdso32= kernel parameter, both must be supported. Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a variable), the HPET and VVAR Page can only relative addressed. So this pages must located before or after the VDSO. This is why i need to setup this pages into the fixmap area, this is the compat mode vdso32=2. For vdso32=1 i need to map the VDSO Page together with the HPET and VVAR into the user space. For compability reasons both mappings are required. Not at the same time, and I don't think you've guarded the map_vsyscall call with a check for compat mode. I see no benefit to check the compat mode in map_vsyscall. The best way is always map the VVAR page, because it is less error prone. In your case tt would be also necessary not map the HPET page in non compat mode too. So why bloat the code? - Stefani -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Mon, Feb 3, 2014 at 2:01 PM, Stefani Seibold stef...@seibold.net wrote: Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) Something to the effect of: #elif defined(BUILD_VDSO32) #define VVAR(name) (*vvar_ ## name) #endif Should do the trick. You are wrong... #ifdef BUILD_VDSO32 #define DECLARE_VVAR(offset, type, name) \ extern type vvar_ ## name __attribute__((visibility(hidden))); #define VVAR(name) (vvar_ ## name) #else /* Base address of vvars. This is not ABI. */ #ifdef CONFIG_X86_64 #define VVAR_ADDRESS (-10*1024*1024 - 4096) #else extern char __vvar_page; #define VVAR_ADDRESS (__vvar_page) #endif This would do the trick! But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is still a struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data __attribute__((visibility(hidden))); #define gtod (arch_vvar_vsyscall_gtod_data) needed, because vvar_vsyscall_gtod_data is the 32 bit version, which would result in incorrect access of the struct members. So my code can't use this VVAR macro. For 32-on-64, I must have read your code wrong. Are you sticking two copies of the same struct with different layout into the vvar page? If so, wouldn't it be better to only have the variant with a common layout and use it for all versions of the vdso? --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Montag, den 03.02.2014, 14:04 -0800 schrieb Andy Lutomirski: On Mon, Feb 3, 2014 at 2:01 PM, Stefani Seibold stef...@seibold.net wrote: Am Montag, den 03.02.2014, 08:36 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 11:44 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) Something to the effect of: #elif defined(BUILD_VDSO32) #define VVAR(name) (*vvar_ ## name) #endif Should do the trick. You are wrong... #ifdef BUILD_VDSO32 #define DECLARE_VVAR(offset, type, name) \ extern type vvar_ ## name __attribute__((visibility(hidden))); #define VVAR(name) (vvar_ ## name) #else /* Base address of vvars. This is not ABI. */ #ifdef CONFIG_X86_64 #define VVAR_ADDRESS (-10*1024*1024 - 4096) #else extern char __vvar_page; #define VVAR_ADDRESS (__vvar_page) #endif This would do the trick! But for 64 bit to 32 bit conversation layer in vclock_gettime.c there is still a struct arch_vsyscall_gtod_data arch_vvar_vsyscall_gtod_data __attribute__((visibility(hidden))); #define gtod (arch_vvar_vsyscall_gtod_data) needed, because vvar_vsyscall_gtod_data is the 32 bit version, which would result in incorrect access of the struct members. So my code can't use this VVAR macro. For 32-on-64, I must have read your code wrong. Are you sticking two copies of the same struct with different layout into the vvar page? If so, wouldn't it be better to only have the variant with a common layout and use it for all versions of the vdso? No, only one. But for depend on 32/64 bit the layout differs. We discuss this topic some days before, so please have a look at the code and the previous posts. - Stefani -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: > On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold wrote: > > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: > >> On Sun, Feb 2, 2014 at 3:27 AM, wrote: > >> > From: Stefani Seibold > >> > > >> > This patch add the time support for 32 bit a VDSO to a 32 bit kernel. > >> > >> [...] > >> > >> Can you address the review comments from last time around? For > >> example, this still seems to have redundant vvar and hpet mappings, it > >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. > >> > > > > I will address the compat VDSO issue. > > > > But the VVAR macro will be not a part of this patch set. If you depend > > on this, feel free to create one. From my point of view this is not > > feasible without a macro hacking, because the address accessing the vvar > > area differs in kernel and VDSO user mode. > > Sorry, but "I will make the code messier for no apparent reason and I > will not offer to fix it in the same series" gets my NAK. > > Hint: I'm talking about two or three lines of code in vvar.h. > A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) > > > > I also see no redundant mapping. There are two modes, one is the map of > > the kernel area the other maps the VDSO into the user space area. This > > is exactly the behaviour of the origin VDSO implementation. > > No. > > In your series there are *three* mappings. There are: > > - The linear mapping that the kernel loader sets up (the writable > mapping used in the kernel). This is implicit and, of course, fine. > - There's the fixmap page, which aliases the normal kernel mapping at > a fixed address with the user, ro, and nx attributes. The 64-bit vDSO > uses that mapping. See vdso.h -- it's all arranged pretty clearly. > Your code, for no discernible reason, sets up a fixmap entry on > *32-bit* kernels. > - The vma that you're setting up adjacent to the actual vdso text. > This is what you are using. > > Please choose *one* user-readable mapping for the 32-bit vdso and > stick with it. If the 64-bit vdso can use it to and userspace doesn't > break, even better. But a pointless set of extra fixmap entries is > not okay. > Again: I wrote that there are two modes for a 32 bit kernel and therefore there are two mappings at the same time. Since there are both ways available in a 32 bit kernel via the vdso32= kernel parameter, both must be supported. Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a variable), the HPET and VVAR Page can only relative addressed. So this pages must located before or after the VDSO. This is why i need to setup this pages into the fixmap area, this is the compat mode "vdso32=2". For "vdso32=1" i need to map the VDSO Page together with the HPET and VVAR into the user space. For compability reasons both mappings are required. There is only one binary for the VDSO page, regardless of the vdso= kernel parameter and this code can only do a relative addressing. A 64 bit kernel can do it in an other way, because there is a real fixmap area, so this special handling is not needed. - Stefani -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold wrote: > Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: >> On Sun, Feb 2, 2014 at 3:27 AM, wrote: >> > From: Stefani Seibold >> > >> > This patch add the time support for 32 bit a VDSO to a 32 bit kernel. >> >> [...] >> >> Can you address the review comments from last time around? For >> example, this still seems to have redundant vvar and hpet mappings, it >> doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. >> > > I will address the compat VDSO issue. > > But the VVAR macro will be not a part of this patch set. If you depend > on this, feel free to create one. From my point of view this is not > feasible without a macro hacking, because the address accessing the vvar > area differs in kernel and VDSO user mode. Sorry, but "I will make the code messier for no apparent reason and I will not offer to fix it in the same series" gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. > > I also see no redundant mapping. There are two modes, one is the map of > the kernel area the other maps the VDSO into the user space area. This > is exactly the behaviour of the origin VDSO implementation. No. In your series there are *three* mappings. There are: - The linear mapping that the kernel loader sets up (the writable mapping used in the kernel). This is implicit and, of course, fine. - There's the fixmap page, which aliases the normal kernel mapping at a fixed address with the user, ro, and nx attributes. The 64-bit vDSO uses that mapping. See vdso.h -- it's all arranged pretty clearly. Your code, for no discernible reason, sets up a fixmap entry on *32-bit* kernels. - The vma that you're setting up adjacent to the actual vdso text. This is what you are using. Please choose *one* user-readable mapping for the 32-bit vdso and stick with it. If the 64-bit vdso can use it to and userspace doesn't break, even better. But a pointless set of extra fixmap entries is not okay. hpa's right about the symbol versions, BTW -- those are ABI and will probably break (at least) every new-enough Go binary. --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: > On Sun, Feb 2, 2014 at 3:27 AM, wrote: > > From: Stefani Seibold > > > > This patch add the time support for 32 bit a VDSO to a 32 bit kernel. > > [...] > > Can you address the review comments from last time around? For > example, this still seems to have redundant vvar and hpet mappings, it > doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. > I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. > Also, the usual convention is to use [PATCH v2 3/8], etc, as subjects, > to keep separate versions separate. > Will do this. - Stefani -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 3:27 AM, wrote: > From: Stefani Seibold > > This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. Also, the usual convention is to use [PATCH v2 3/8], etc, as subjects, to keep separate versions separate. --Andy -- 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/
[PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
From: Stefani Seibold
This patch add the time support for 32 bit a VDSO to a 32 bit kernel.
For 32 bit programs running on a 32 bit kernel, the same mechanism is
used as for 64 bit programs running on a 64 bit kernel.
Signed-off-by: Stefani Seibold
---
arch/x86/include/asm/elf.h| 2 +-
arch/x86/include/asm/vdso.h | 3 ++
arch/x86/include/asm/vdso32.h | 10 ++
arch/x86/vdso/Makefile| 7 +
arch/x86/vdso/vclock_gettime.c| 59 +--
arch/x86/vdso/vdso-layout.lds.S | 25 +++
arch/x86/vdso/vdso32-setup.c | 47 ++--
arch/x86/vdso/vdso32/vclock_gettime.c | 16 ++
arch/x86/vdso/vdso32/vdso32.lds.S | 11 ++-
9 files changed, 167 insertions(+), 13 deletions(-)
create mode 100644 arch/x86/include/asm/vdso32.h
create mode 100644 arch/x86/vdso/vdso32/vclock_gettime.c
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 9c999c1..21bae90 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -289,7 +289,7 @@ do {
\
#else /* CONFIG_X86_32 */
-#define VDSO_HIGH_BASE 0xe000U /* CONFIG_COMPAT_VDSO address */
+#define VDSO_HIGH_BASE 0xc000U /* CONFIG_COMPAT_VDSO address */
/* 1GB for 64bit, 8MB for 32bit */
#define STACK_RND_MASK (test_thread_flag(TIF_ADDR32) ? 0x7ff : 0x3f)
diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index fddb53d..fe3cef9 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -2,6 +2,9 @@
#define _ASM_X86_VDSO_H
#if defined CONFIG_X86_32 || defined CONFIG_COMPAT
+
+#include
+
extern const char VDSO32_PRELINK[];
/*
diff --git a/arch/x86/include/asm/vdso32.h b/arch/x86/include/asm/vdso32.h
new file mode 100644
index 000..7dd2eb8
--- /dev/null
+++ b/arch/x86/include/asm/vdso32.h
@@ -0,0 +1,10 @@
+#ifndef _ASM_X86_VDSO32_H
+#define _ASM_X86_VDSO32_H
+
+#define VDSO_BASE_PAGE 0
+#define VDSO_VVAR_PAGE 1
+#define VDSO_HPET_PAGE 2
+#defineVDSO_PAGES 3
+#defineVDSO_OFFSET(x) ((x) * PAGE_SIZE)
+
+#endif
diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index fd14be1..1ff5b0a 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -145,8 +145,15 @@ KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS))
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_AFLAGS = $(KBUILD_AFLAGS_32)
$(vdso32-images:%=$(obj)/%.dbg): asflags-$(CONFIG_X86_64) += -m32
+KBUILD_CFLAGS_32 := $(filter-out -m64,$(KBUILD_CFLAGS))
+KBUILD_CFLAGS_32 := $(filter-out -mcmodel=kernel,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 := $(filter-out -fno-pic,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic
+$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
+
$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
$(obj)/vdso32/vdso32.lds \
+$(obj)/vdso32/vclock_gettime.o \
$(obj)/vdso32/note.o \
$(obj)/vdso32/%.o
$(call if_changed,vdso)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index bf969a0..42f641c 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -4,6 +4,9 @@
*
* Fast user context implementation of clock_gettime, gettimeofday, and time.
*
+ * 32 Bit compat layer by Stefani Seibold
+ * sponsored by Rohde & Schwarz GmbH & Co. KG Munich/Germany
+ *
* The code should have no internal unresolved relocations.
* Check with readelf after changing.
*/
@@ -24,6 +27,8 @@
#include
#include
+#ifndef BUILD_VDSO32
+
#define gtod ((vsyscall_gtod_data))
static notrace cycle_t vread_hpet(void)
@@ -47,6 +52,54 @@ notrace static long vdso_fallback_gtod(struct timeval *tv,
struct timezone *tz)
"0" (__NR_gettimeofday), "D" (tv), "S" (tz) : "memory");
return ret;
}
+#else
+
+struct vsyscall_gtod_data vvar_vsyscall_gtod_data
+ __attribute__((visibility("hidden")));
+
+u8 hpet_page
+ __attribute__((visibility("hidden")));
+
+#define gtod (_vsyscall_gtod_data)
+
+#ifdef CONFIG_HPET_TIMER
+static notrace cycle_t vread_hpet(void)
+{
+ return readl(_page + HPET_COUNTER);
+}
+#endif
+
+notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+{
+ long ret;
+
+ asm(
+ "push %%ebx \n"
+ "mov %2,%%ebx \n"
+ "call VDSO32_vsyscall \n"
+ "pop %%ebx \n"
+ : "=a" (ret)
+ : "0" (__NR_clock_gettime), "d" (clock), "c" (ts)
+ : "memory");
+ return ret;
+}
+
+notrace static long vdso_fallback_gtod(struct timeval *tv,
+ struct timezone *tz)
+{
+ long ret;
+
+ asm(
+ "push
[PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
From: Stefani Seibold stef...@seibold.net
This patch add the time support for 32 bit a VDSO to a 32 bit kernel.
For 32 bit programs running on a 32 bit kernel, the same mechanism is
used as for 64 bit programs running on a 64 bit kernel.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/include/asm/elf.h| 2 +-
arch/x86/include/asm/vdso.h | 3 ++
arch/x86/include/asm/vdso32.h | 10 ++
arch/x86/vdso/Makefile| 7 +
arch/x86/vdso/vclock_gettime.c| 59 +--
arch/x86/vdso/vdso-layout.lds.S | 25 +++
arch/x86/vdso/vdso32-setup.c | 47 ++--
arch/x86/vdso/vdso32/vclock_gettime.c | 16 ++
arch/x86/vdso/vdso32/vdso32.lds.S | 11 ++-
9 files changed, 167 insertions(+), 13 deletions(-)
create mode 100644 arch/x86/include/asm/vdso32.h
create mode 100644 arch/x86/vdso/vdso32/vclock_gettime.c
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 9c999c1..21bae90 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -289,7 +289,7 @@ do {
\
#else /* CONFIG_X86_32 */
-#define VDSO_HIGH_BASE 0xe000U /* CONFIG_COMPAT_VDSO address */
+#define VDSO_HIGH_BASE 0xc000U /* CONFIG_COMPAT_VDSO address */
/* 1GB for 64bit, 8MB for 32bit */
#define STACK_RND_MASK (test_thread_flag(TIF_ADDR32) ? 0x7ff : 0x3f)
diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index fddb53d..fe3cef9 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -2,6 +2,9 @@
#define _ASM_X86_VDSO_H
#if defined CONFIG_X86_32 || defined CONFIG_COMPAT
+
+#include asm/vdso32.h
+
extern const char VDSO32_PRELINK[];
/*
diff --git a/arch/x86/include/asm/vdso32.h b/arch/x86/include/asm/vdso32.h
new file mode 100644
index 000..7dd2eb8
--- /dev/null
+++ b/arch/x86/include/asm/vdso32.h
@@ -0,0 +1,10 @@
+#ifndef _ASM_X86_VDSO32_H
+#define _ASM_X86_VDSO32_H
+
+#define VDSO_BASE_PAGE 0
+#define VDSO_VVAR_PAGE 1
+#define VDSO_HPET_PAGE 2
+#defineVDSO_PAGES 3
+#defineVDSO_OFFSET(x) ((x) * PAGE_SIZE)
+
+#endif
diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index fd14be1..1ff5b0a 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -145,8 +145,15 @@ KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS))
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_AFLAGS = $(KBUILD_AFLAGS_32)
$(vdso32-images:%=$(obj)/%.dbg): asflags-$(CONFIG_X86_64) += -m32
+KBUILD_CFLAGS_32 := $(filter-out -m64,$(KBUILD_CFLAGS))
+KBUILD_CFLAGS_32 := $(filter-out -mcmodel=kernel,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 := $(filter-out -fno-pic,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic
+$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
+
$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
$(obj)/vdso32/vdso32.lds \
+$(obj)/vdso32/vclock_gettime.o \
$(obj)/vdso32/note.o \
$(obj)/vdso32/%.o
$(call if_changed,vdso)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index bf969a0..42f641c 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -4,6 +4,9 @@
*
* Fast user context implementation of clock_gettime, gettimeofday, and time.
*
+ * 32 Bit compat layer by Stefani Seibold stef...@seibold.net
+ * sponsored by Rohde Schwarz GmbH Co. KG Munich/Germany
+ *
* The code should have no internal unresolved relocations.
* Check with readelf after changing.
*/
@@ -24,6 +27,8 @@
#include asm/io.h
#include asm/pvclock.h
+#ifndef BUILD_VDSO32
+
#define gtod (VVAR(vsyscall_gtod_data))
static notrace cycle_t vread_hpet(void)
@@ -47,6 +52,54 @@ notrace static long vdso_fallback_gtod(struct timeval *tv,
struct timezone *tz)
0 (__NR_gettimeofday), D (tv), S (tz) : memory);
return ret;
}
+#else
+
+struct vsyscall_gtod_data vvar_vsyscall_gtod_data
+ __attribute__((visibility(hidden)));
+
+u8 hpet_page
+ __attribute__((visibility(hidden)));
+
+#define gtod (vvar_vsyscall_gtod_data)
+
+#ifdef CONFIG_HPET_TIMER
+static notrace cycle_t vread_hpet(void)
+{
+ return readl(hpet_page + HPET_COUNTER);
+}
+#endif
+
+notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+{
+ long ret;
+
+ asm(
+ push %%ebx \n
+ mov %2,%%ebx \n
+ call VDSO32_vsyscall \n
+ pop %%ebx \n
+ : =a (ret)
+ : 0 (__NR_clock_gettime), d (clock), c (ts)
+ : memory);
+ return ret;
+}
+
+notrace static long vdso_fallback_gtod(struct timeval *tv,
+ struct
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. Also, the usual convention is to use [PATCH v2 3/8], etc, as subjects, to keep separate versions separate. --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. Also, the usual convention is to use [PATCH v2 3/8], etc, as subjects, to keep separate versions separate. Will do this. - Stefani -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. No. In your series there are *three* mappings. There are: - The linear mapping that the kernel loader sets up (the writable mapping used in the kernel). This is implicit and, of course, fine. - There's the fixmap page, which aliases the normal kernel mapping at a fixed address with the user, ro, and nx attributes. The 64-bit vDSO uses that mapping. See vdso.h -- it's all arranged pretty clearly. Your code, for no discernible reason, sets up a fixmap entry on *32-bit* kernels. - The vma that you're setting up adjacent to the actual vdso text. This is what you are using. Please choose *one* user-readable mapping for the 32-bit vdso and stick with it. If the 64-bit vdso can use it to and userspace doesn't break, even better. But a pointless set of extra fixmap entries is not okay. hpa's right about the symbol versions, BTW -- those are ABI and will probably break (at least) every new-enough Go binary. --Andy -- 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/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
Am Sonntag, den 02.02.2014, 16:12 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 1:39 PM, Stefani Seibold stef...@seibold.net wrote: Am Sonntag, den 02.02.2014, 08:46 -0800 schrieb Andy Lutomirski: On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. I will address the compat VDSO issue. But the VVAR macro will be not a part of this patch set. If you depend on this, feel free to create one. From my point of view this is not feasible without a macro hacking, because the address accessing the vvar area differs in kernel and VDSO user mode. Sorry, but I will make the code messier for no apparent reason and I will not offer to fix it in the same series gets my NAK. Hint: I'm talking about two or three lines of code in vvar.h. A hint back: if you threat me with a NAK for a requested code sequence which currently no user, this is far away from professional. I am not your trainee. BTW: If it is so easy, send me the two or three lines and i will merge it ;-) I also see no redundant mapping. There are two modes, one is the map of the kernel area the other maps the VDSO into the user space area. This is exactly the behaviour of the origin VDSO implementation. No. In your series there are *three* mappings. There are: - The linear mapping that the kernel loader sets up (the writable mapping used in the kernel). This is implicit and, of course, fine. - There's the fixmap page, which aliases the normal kernel mapping at a fixed address with the user, ro, and nx attributes. The 64-bit vDSO uses that mapping. See vdso.h -- it's all arranged pretty clearly. Your code, for no discernible reason, sets up a fixmap entry on *32-bit* kernels. - The vma that you're setting up adjacent to the actual vdso text. This is what you are using. Please choose *one* user-readable mapping for the 32-bit vdso and stick with it. If the 64-bit vdso can use it to and userspace doesn't break, even better. But a pointless set of extra fixmap entries is not okay. Again: I wrote that there are two modes for a 32 bit kernel and therefore there are two mappings at the same time. Since there are both ways available in a 32 bit kernel via the vdso32= kernel parameter, both must be supported. Due the lack of a real fixmap for a 32 bit kernel (FIXADDR_TOP is a variable), the HPET and VVAR Page can only relative addressed. So this pages must located before or after the VDSO. This is why i need to setup this pages into the fixmap area, this is the compat mode vdso32=2. For vdso32=1 i need to map the VDSO Page together with the HPET and VVAR into the user space. For compability reasons both mappings are required. There is only one binary for the VDSO page, regardless of the vdso= kernel parameter and this code can only do a relative addressing. A 64 bit kernel can do it in an other way, because there is a real fixmap area, so this special handling is not needed. - Stefani -- 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/
