Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1576731 > unrolled thread
| Started by | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| First post | 2017-02-08 18:20 +0100 |
| Last post | 2017-02-08 18:40 +0100 |
| Articles | 6 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH RFC 0/2] x86/vdso: Add Hyper-V TSC page clocksource support Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-02-08 18:20 +0100
[PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-02-08 18:20 +0100
Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method Andy Lutomirski <luto@amacapital.net> - 2017-02-08 18:30 +0100
Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-02-08 18:30 +0100
Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method Thomas Gleixner <tglx@linutronix.de> - 2017-02-08 19:00 +0100
[PATCH RFC 1/2] hyperv: implement hv_get_tsc_page() Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-02-08 18:40 +0100
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-02-08 18:20 +0100 |
| Subject | [PATCH RFC 0/2] x86/vdso: Add Hyper-V TSC page clocksource support |
| Message-ID | <t8E4q-377-25@gated-at.bofh.it> |
Hi, Hyper-V TSC page clocksource is suitable for vDSO, however, the protocol defined by the hypervisor is different from VCLOCK_PVCLOCK. I implemented the required support re-using pvclock_page VVAR. Simple sysbench test shows the following results: Before: # time sysbench --test=memory --max-requests=500000 run ... real 1m22.618s user 0m50.193s sys 0m32.268s After: # time sysbench --test=memory --max-requests=500000 run ... real 0m50.218s user 0m50.171s sys 0m0.016s So it seems it is worth it. What do you think? Vitaly Kuznetsov (2): hyperv: implement hv_get_tsc_page() x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method arch/x86/entry/vdso/vclock_gettime.c | 48 ++++++++++++++++++++++++++++++++++++ arch/x86/entry/vdso/vma.c | 26 +++++++++++++------ arch/x86/hyperv/hv_init.c | 8 ++++++ arch/x86/include/asm/clocksource.h | 3 ++- arch/x86/include/asm/mshyperv.h | 8 ++++++ drivers/hv/Kconfig | 5 ++++ 6 files changed, 90 insertions(+), 8 deletions(-) -- 2.9.3
[toc] | [next] | [standalone]
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-02-08 18:20 +0100 |
| Subject | [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method |
| Message-ID | <t8E4q-377-35@gated-at.bofh.it> |
| In reply to | #1576731 |
Hyper-V TSC page clocksource is suitable for vDSO, however, the protocol
defined by the hypervisor is different from VCLOCK_PVCLOCK. Implement the
required support re-using pvclock_page VVAR as VCLOCK_PVCLOCK is mutually
exclusive with VCLOCK_HVCLOCK at run time.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/entry/vdso/vclock_gettime.c | 48 ++++++++++++++++++++++++++++++++++++
arch/x86/entry/vdso/vma.c | 26 +++++++++++++------
arch/x86/hyperv/hv_init.c | 3 +++
arch/x86/include/asm/clocksource.h | 3 ++-
4 files changed, 72 insertions(+), 8 deletions(-)
diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index 9d4d6e1..93e9dcd 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -17,6 +17,7 @@
#include <asm/unistd.h>
#include <asm/msr.h>
#include <asm/pvclock.h>
+#include <asm/mshyperv.h>
#include <linux/math64.h>
#include <linux/time.h>
#include <linux/kernel.h>
@@ -141,6 +142,49 @@ static notrace u64 vread_pvclock(int *mode)
return last;
}
#endif
+#ifdef CONFIG_HYPERV_CLOCK
+/* (a * b) >> 64 implementation */
+static inline u64 mul64x64_hi(u64 a, u64 b)
+{
+ u64 a_lo, a_hi, b_lo, b_hi, p1, p2;
+
+ a_lo = (u32)a;
+ a_hi = a >> 32;
+ b_lo = (u32)b;
+ b_hi = b >> 32;
+ p1 = a_lo * b_hi;
+ p2 = a_hi * b_lo;
+
+ return a_hi * b_hi + (p1 >> 32) + (p2 >> 32) +
+ ((((a_lo * b_lo) >> 32) + (u32)p1 + (u32)p2) >> 32);
+
+}
+
+static notrace u64 vread_hvclock(int *mode)
+{
+ const struct ms_hyperv_tsc_page *tsc_pg =
+ (const struct ms_hyperv_tsc_page *)&pvclock_page;
+ u64 sequence, scale, offset, current_tick, cur_tsc;
+
+ while (1) {
+ sequence = READ_ONCE(tsc_pg->tsc_sequence);
+ if (!sequence)
+ break;
+
+ scale = READ_ONCE(tsc_pg->tsc_scale);
+ offset = READ_ONCE(tsc_pg->tsc_offset);
+ rdtscll(cur_tsc);
+
+ current_tick = mul64x64_hi(cur_tsc, scale) + offset;
+
+ if (READ_ONCE(tsc_pg->tsc_sequence) == sequence)
+ return current_tick;
+ }
+
+ *mode = VCLOCK_NONE;
+ return 0;
+}
+#endif
notrace static u64 vread_tsc(void)
{
@@ -173,6 +217,10 @@ notrace static inline u64 vgetsns(int *mode)
else if (gtod->vclock_mode == VCLOCK_PVCLOCK)
cycles = vread_pvclock(mode);
#endif
+#ifdef CONFIG_HYPERV_CLOCK
+ else if (gtod->vclock_mode == VCLOCK_HVCLOCK)
+ cycles = vread_hvclock(mode);
+#endif
else
return 0;
v = (cycles - gtod->cycle_last) & gtod->mask;
diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 10820f6..4b9d90c 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -21,6 +21,7 @@
#include <asm/page.h>
#include <asm/desc.h>
#include <asm/cpufeature.h>
+#include <asm/mshyperv.h>
#if defined(CONFIG_X86_64)
unsigned int __read_mostly vdso64_enabled = 1;
@@ -112,13 +113,24 @@ static int vvar_fault(const struct vm_special_mapping *sm,
ret = vm_insert_pfn(vma, vmf->address,
__pa_symbol(&__vvar_page) >> PAGE_SHIFT);
} else if (sym_offset == image->sym_pvclock_page) {
- struct pvclock_vsyscall_time_info *pvti =
- pvclock_pvti_cpu0_va();
- if (pvti && vclock_was_used(VCLOCK_PVCLOCK)) {
- ret = vm_insert_pfn(
- vma,
- vmf->address,
- __pa(pvti) >> PAGE_SHIFT);
+ if (vclock_was_used(VCLOCK_PVCLOCK)) {
+ struct pvclock_vsyscall_time_info *pvti =
+ pvclock_pvti_cpu0_va();
+ if (pvti) {
+ ret = vm_insert_pfn(
+ vma,
+ vmf->address,
+ __pa(pvti) >> PAGE_SHIFT);
+ }
+ } else if (vclock_was_used(VCLOCK_HVCLOCK)) {
+ struct ms_hyperv_tsc_page *tsc_pg =
+ hv_get_tsc_page();
+ if (tsc_pg) {
+ ret = vm_insert_pfn(
+ vma,
+ vmf->address,
+ vmalloc_to_pfn(tsc_pg));
+ }
}
}
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index aa36049..3d534d2 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -157,6 +157,9 @@ void hyperv_init(void)
tsc_msr.guest_physical_address = vmalloc_to_pfn(tsc_pg);
wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
+
+ hyperv_cs_tsc.archdata.vclock_mode = VCLOCK_HVCLOCK;
+
clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
return;
}
diff --git a/arch/x86/include/asm/clocksource.h b/arch/x86/include/asm/clocksource.h
index eae33c7..47bea8c 100644
--- a/arch/x86/include/asm/clocksource.h
+++ b/arch/x86/include/asm/clocksource.h
@@ -6,7 +6,8 @@
#define VCLOCK_NONE 0 /* No vDSO clock available. */
#define VCLOCK_TSC 1 /* vDSO should use vread_tsc. */
#define VCLOCK_PVCLOCK 2 /* vDSO should use vread_pvclock. */
-#define VCLOCK_MAX 2
+#define VCLOCK_HVCLOCK 3 /* vDSO should use vread_hvclock. */
+#define VCLOCK_MAX 3
struct arch_clocksource_data {
int vclock_mode;
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-02-08 18:30 +0100 |
| Subject | Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method |
| Message-ID | <t8Ee5-3aO-9@gated-at.bofh.it> |
| In reply to | #1576735 |
On Wed, Feb 8, 2017 at 9:07 AM, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> Hyper-V TSC page clocksource is suitable for vDSO, however, the protocol
> defined by the hypervisor is different from VCLOCK_PVCLOCK. Implement the
> required support re-using pvclock_page VVAR as VCLOCK_PVCLOCK is mutually
> exclusive with VCLOCK_HVCLOCK at run time.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> arch/x86/entry/vdso/vclock_gettime.c | 48 ++++++++++++++++++++++++++++++++++++
> arch/x86/entry/vdso/vma.c | 26 +++++++++++++------
> arch/x86/hyperv/hv_init.c | 3 +++
> arch/x86/include/asm/clocksource.h | 3 ++-
> 4 files changed, 72 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
> index 9d4d6e1..93e9dcd 100644
> --- a/arch/x86/entry/vdso/vclock_gettime.c
> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> @@ -17,6 +17,7 @@
> #include <asm/unistd.h>
> #include <asm/msr.h>
> #include <asm/pvclock.h>
> +#include <asm/mshyperv.h>
> #include <linux/math64.h>
> #include <linux/time.h>
> #include <linux/kernel.h>
> @@ -141,6 +142,49 @@ static notrace u64 vread_pvclock(int *mode)
> return last;
> }
> #endif
> +#ifdef CONFIG_HYPERV_CLOCK
> +/* (a * b) >> 64 implementation */
> +static inline u64 mul64x64_hi(u64 a, u64 b)
> +{
> + u64 a_lo, a_hi, b_lo, b_hi, p1, p2;
> +
> + a_lo = (u32)a;
> + a_hi = a >> 32;
> + b_lo = (u32)b;
> + b_hi = b >> 32;
> + p1 = a_lo * b_hi;
> + p2 = a_hi * b_lo;
> +
> + return a_hi * b_hi + (p1 >> 32) + (p2 >> 32) +
> + ((((a_lo * b_lo) >> 32) + (u32)p1 + (u32)p2) >> 32);
> +
> +}
Unless GCC is waaay more clever than I think, this is hugely
suboptimal on 64-bit. x86 can do this in a single instruction, and
gcc can express it cleanly using __uint128_t. I wouldn't be terribly
surprised if the 32-bit generated code was fine, too.
> +
> +static notrace u64 vread_hvclock(int *mode)
> +{
> + const struct ms_hyperv_tsc_page *tsc_pg =
> + (const struct ms_hyperv_tsc_page *)&pvclock_page;
> + u64 sequence, scale, offset, current_tick, cur_tsc;
> +
> + while (1) {
> + sequence = READ_ONCE(tsc_pg->tsc_sequence);
> + if (!sequence)
> + break;
> +
> + scale = READ_ONCE(tsc_pg->tsc_scale);
> + offset = READ_ONCE(tsc_pg->tsc_offset);
> + rdtscll(cur_tsc);
> +
> + current_tick = mul64x64_hi(cur_tsc, scale) + offset;
> +
> + if (READ_ONCE(tsc_pg->tsc_sequence) == sequence)
> + return current_tick;
> + }
Can you explain better what's going on here? What protocol does the
hypervisor use to update this bizarro seqlock? What exactly does
sequence==0 mean?
> +
> + *mode = VCLOCK_NONE;
> + return 0;
> +}
> +#endif
>
> notrace static u64 vread_tsc(void)
> {
> @@ -173,6 +217,10 @@ notrace static inline u64 vgetsns(int *mode)
> else if (gtod->vclock_mode == VCLOCK_PVCLOCK)
> cycles = vread_pvclock(mode);
> #endif
> +#ifdef CONFIG_HYPERV_CLOCK
> + else if (gtod->vclock_mode == VCLOCK_HVCLOCK)
> + cycles = vread_hvclock(mode);
> +#endif
> else
> return 0;
> v = (cycles - gtod->cycle_last) & gtod->mask;
> diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
> index 10820f6..4b9d90c 100644
> --- a/arch/x86/entry/vdso/vma.c
> +++ b/arch/x86/entry/vdso/vma.c
> @@ -21,6 +21,7 @@
> #include <asm/page.h>
> #include <asm/desc.h>
> #include <asm/cpufeature.h>
> +#include <asm/mshyperv.h>
>
> #if defined(CONFIG_X86_64)
> unsigned int __read_mostly vdso64_enabled = 1;
> @@ -112,13 +113,24 @@ static int vvar_fault(const struct vm_special_mapping *sm,
> ret = vm_insert_pfn(vma, vmf->address,
> __pa_symbol(&__vvar_page) >> PAGE_SHIFT);
> } else if (sym_offset == image->sym_pvclock_page) {
> - struct pvclock_vsyscall_time_info *pvti =
> - pvclock_pvti_cpu0_va();
> - if (pvti && vclock_was_used(VCLOCK_PVCLOCK)) {
> - ret = vm_insert_pfn(
> - vma,
> - vmf->address,
> - __pa(pvti) >> PAGE_SHIFT);
> + if (vclock_was_used(VCLOCK_PVCLOCK)) {
> + struct pvclock_vsyscall_time_info *pvti =
> + pvclock_pvti_cpu0_va();
> + if (pvti) {
> + ret = vm_insert_pfn(
> + vma,
> + vmf->address,
> + __pa(pvti) >> PAGE_SHIFT);
> + }
> + } else if (vclock_was_used(VCLOCK_HVCLOCK)) {
> + struct ms_hyperv_tsc_page *tsc_pg =
> + hv_get_tsc_page();
> + if (tsc_pg) {
> + ret = vm_insert_pfn(
> + vma,
> + vmf->address,
> + vmalloc_to_pfn(tsc_pg));
> + }
This is IMO just too weird and fragile. Why not allocate another page
and avoid this strange aliasing?
--Andy
[toc] | [prev] | [next] | [standalone]
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-02-08 18:30 +0100 |
| Subject | Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method |
| Message-ID | <t8Ee6-3aO-31@gated-at.bofh.it> |
| In reply to | #1576742 |
Andy Lutomirski <luto@amacapital.net> writes:
> On Wed, Feb 8, 2017 at 9:07 AM, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>> Hyper-V TSC page clocksource is suitable for vDSO, however, the protocol
>> defined by the hypervisor is different from VCLOCK_PVCLOCK. Implement the
>> required support re-using pvclock_page VVAR as VCLOCK_PVCLOCK is mutually
>> exclusive with VCLOCK_HVCLOCK at run time.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> arch/x86/entry/vdso/vclock_gettime.c | 48 ++++++++++++++++++++++++++++++++++++
>> arch/x86/entry/vdso/vma.c | 26 +++++++++++++------
>> arch/x86/hyperv/hv_init.c | 3 +++
>> arch/x86/include/asm/clocksource.h | 3 ++-
>> 4 files changed, 72 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
>> index 9d4d6e1..93e9dcd 100644
>> --- a/arch/x86/entry/vdso/vclock_gettime.c
>> +++ b/arch/x86/entry/vdso/vclock_gettime.c
>> @@ -17,6 +17,7 @@
>> #include <asm/unistd.h>
>> #include <asm/msr.h>
>> #include <asm/pvclock.h>
>> +#include <asm/mshyperv.h>
>> #include <linux/math64.h>
>> #include <linux/time.h>
>> #include <linux/kernel.h>
>> @@ -141,6 +142,49 @@ static notrace u64 vread_pvclock(int *mode)
>> return last;
>> }
>> #endif
>> +#ifdef CONFIG_HYPERV_CLOCK
>> +/* (a * b) >> 64 implementation */
>> +static inline u64 mul64x64_hi(u64 a, u64 b)
>> +{
>> + u64 a_lo, a_hi, b_lo, b_hi, p1, p2;
>> +
>> + a_lo = (u32)a;
>> + a_hi = a >> 32;
>> + b_lo = (u32)b;
>> + b_hi = b >> 32;
>> + p1 = a_lo * b_hi;
>> + p2 = a_hi * b_lo;
>> +
>> + return a_hi * b_hi + (p1 >> 32) + (p2 >> 32) +
>> + ((((a_lo * b_lo) >> 32) + (u32)p1 + (u32)p2) >> 32);
>> +
>> +}
>
> Unless GCC is waaay more clever than I think, this is hugely
> suboptimal on 64-bit. x86 can do this in a single instruction, and
> gcc can express it cleanly using __uint128_t. I wouldn't be terribly
> surprised if the 32-bit generated code was fine, too.
>
Yes, this is a 'quick-and-dirty' mulq implementation suitable for 32
bit. I'll try making it better.
>> +
>> +static notrace u64 vread_hvclock(int *mode)
>> +{
>> + const struct ms_hyperv_tsc_page *tsc_pg =
>> + (const struct ms_hyperv_tsc_page *)&pvclock_page;
>> + u64 sequence, scale, offset, current_tick, cur_tsc;
>> +
>> + while (1) {
>> + sequence = READ_ONCE(tsc_pg->tsc_sequence);
>> + if (!sequence)
>> + break;
>> +
>> + scale = READ_ONCE(tsc_pg->tsc_scale);
>> + offset = READ_ONCE(tsc_pg->tsc_offset);
>> + rdtscll(cur_tsc);
>> +
>> + current_tick = mul64x64_hi(cur_tsc, scale) + offset;
>> +
>> + if (READ_ONCE(tsc_pg->tsc_sequence) == sequence)
>> + return current_tick;
>> + }
>
> Can you explain better what's going on here? What protocol does the
> hypervisor use to update this bizarro seqlock? What exactly does
> sequence==0 mean?
I basically copied the code from arch/x86/hyperv/hv_init.c (in
linux-next). The protocol is:
tsc_sequence == 0 means we should not be using this clocksource. This
happens during migration, for example.
When tsc_sequence != 0 we calculate
current_tick = rdtsc() * tsc_pg->tsc_scale + tsc_pg->tsc_offset
and then we check tsc_sequence again. If it changed it means that the
hypervisor was updating the data at this moment and we need to discard
the result and try again. If the sequence number is the same we're good
to go.
>
>> +
>> + *mode = VCLOCK_NONE;
>> + return 0;
>> +}
>> +#endif
>>
>> notrace static u64 vread_tsc(void)
>> {
>> @@ -173,6 +217,10 @@ notrace static inline u64 vgetsns(int *mode)
>> else if (gtod->vclock_mode == VCLOCK_PVCLOCK)
>> cycles = vread_pvclock(mode);
>> #endif
>> +#ifdef CONFIG_HYPERV_CLOCK
>> + else if (gtod->vclock_mode == VCLOCK_HVCLOCK)
>> + cycles = vread_hvclock(mode);
>> +#endif
>> else
>> return 0;
>> v = (cycles - gtod->cycle_last) & gtod->mask;
>> diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
>> index 10820f6..4b9d90c 100644
>> --- a/arch/x86/entry/vdso/vma.c
>> +++ b/arch/x86/entry/vdso/vma.c
>> @@ -21,6 +21,7 @@
>> #include <asm/page.h>
>> #include <asm/desc.h>
>> #include <asm/cpufeature.h>
>> +#include <asm/mshyperv.h>
>>
>> #if defined(CONFIG_X86_64)
>> unsigned int __read_mostly vdso64_enabled = 1;
>> @@ -112,13 +113,24 @@ static int vvar_fault(const struct vm_special_mapping *sm,
>> ret = vm_insert_pfn(vma, vmf->address,
>> __pa_symbol(&__vvar_page) >> PAGE_SHIFT);
>> } else if (sym_offset == image->sym_pvclock_page) {
>> - struct pvclock_vsyscall_time_info *pvti =
>> - pvclock_pvti_cpu0_va();
>> - if (pvti && vclock_was_used(VCLOCK_PVCLOCK)) {
>> - ret = vm_insert_pfn(
>> - vma,
>> - vmf->address,
>> - __pa(pvti) >> PAGE_SHIFT);
>> + if (vclock_was_used(VCLOCK_PVCLOCK)) {
>> + struct pvclock_vsyscall_time_info *pvti =
>> + pvclock_pvti_cpu0_va();
>> + if (pvti) {
>> + ret = vm_insert_pfn(
>> + vma,
>> + vmf->address,
>> + __pa(pvti) >> PAGE_SHIFT);
>> + }
>> + } else if (vclock_was_used(VCLOCK_HVCLOCK)) {
>> + struct ms_hyperv_tsc_page *tsc_pg =
>> + hv_get_tsc_page();
>> + if (tsc_pg) {
>> + ret = vm_insert_pfn(
>> + vma,
>> + vmf->address,
>> + vmalloc_to_pfn(tsc_pg));
>> + }
>
> This is IMO just too weird and fragile. Why not allocate another page
> and avoid this strange aliasing?
My idea was that as we can't be in both VCLOCK_PVCLOCK (KVM, Xen) and
VCLOCK_HVCLOCK (Hyper-V) modes simultaneously. But let's make it
different to avoid confusion.
--
Vitaly
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-02-08 19:00 +0100 |
| Subject | Re: [PATCH RFC 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method |
| Message-ID | <t8EH7-3ls-1@gated-at.bofh.it> |
| In reply to | #1576742 |
On Wed, 8 Feb 2017, Andy Lutomirski wrote:
> > +#ifdef CONFIG_HYPERV_CLOCK
> > +/* (a * b) >> 64 implementation */
> > +static inline u64 mul64x64_hi(u64 a, u64 b)
> > +{
> > + u64 a_lo, a_hi, b_lo, b_hi, p1, p2;
> > +
> > + a_lo = (u32)a;
> > + a_hi = a >> 32;
> > + b_lo = (u32)b;
> > + b_hi = b >> 32;
> > + p1 = a_lo * b_hi;
> > + p2 = a_hi * b_lo;
> > +
> > + return a_hi * b_hi + (p1 >> 32) + (p2 >> 32) +
> > + ((((a_lo * b_lo) >> 32) + (u32)p1 + (u32)p2) >> 32);
> > +
> > +}
>
> Unless GCC is waaay more clever than I think, this is hugely
> suboptimal on 64-bit. x86 can do this in a single instruction, and
> gcc can express it cleanly using __uint128_t. I wouldn't be terribly
> surprised if the 32-bit generated code was fine, too.
We already have that: mul_u64_u64_shr() which can be replaced by an arch
specific implementation. Nobody bothered to do that for x86 yet, but we
definitely don't want to open code it another time
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-02-08 18:40 +0100 |
| Subject | [PATCH RFC 1/2] hyperv: implement hv_get_tsc_page() |
| Message-ID | <t8EnO-3ey-51@gated-at.bofh.it> |
| In reply to | #1576731 |
To use Hyper-V TSC page clocksource from vDSO we need to make tsc_pg
available. Implement hv_get_tsc_page() and add CONFIG_HYPERV_CLOCK to
make #ifdef-s simple.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/hyperv/hv_init.c | 5 +++++
arch/x86/include/asm/mshyperv.h | 8 ++++++++
drivers/hv/Kconfig | 5 +++++
3 files changed, 18 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b371d0e..aa36049 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -31,6 +31,11 @@
static struct ms_hyperv_tsc_page *tsc_pg;
+struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
+{
+ return tsc_pg;
+}
+
static u64 read_hv_clock_tsc(struct clocksource *arg)
{
u64 current_tick;
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f8dc370..005d0bc 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -173,4 +173,12 @@ void hyperv_report_panic(struct pt_regs *regs);
bool hv_is_hypercall_page_setup(void);
void hyperv_cleanup(void);
#endif
+#ifdef CONFIG_HYPERV_CLOCK
+struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
+#else
+static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
+{
+ return NULL;
+}
+#endif
#endif
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 0403b51..3b45e69 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -3,10 +3,15 @@ menu "Microsoft Hyper-V guest support"
config HYPERV
tristate "Microsoft Hyper-V client drivers"
depends on X86 && ACPI && PCI && X86_LOCAL_APIC && HYPERVISOR_GUEST
+ select HYPERV_CLOCK
help
Select this option to run Linux as a Hyper-V client operating
system.
+config HYPERV_CLOCK
+ bool
+ depends on X86_64
+
config HYPERV_UTILS
tristate "Microsoft Hyper-V Utilities driver"
depends on HYPERV && CONNECTOR && NLS
--
2.9.3
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web