| Message ID | 20171201131321.918-3-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 1 Dec 2017 14:13:17 +0100 Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > + > +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, > + u64 *cur_tsc) > +{ > + *cur_tsc = rdtsc(); > + > + return cur_tsc; Why do return and setting by reference. Looks like an ugly API.
On 01/12/2017 18:29, Stephen Hemminger wrote: >> +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, >> + u64 *cur_tsc) >> +{ >> + *cur_tsc = rdtsc(); >> + >> + return cur_tsc; > Why do return and setting by reference. Looks like an ugly API. This is the fallback implementation for !CONFIG_HYPERV_TSCPAGE, which explains why it's ugly, but why is it needed at all (or it could just BUG())? Thanks, Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 01/12/2017 18:29, Stephen Hemminger wrote: >>> +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, >>> + u64 *cur_tsc) >>> +{ >>> + *cur_tsc = rdtsc(); >>> + >>> + return cur_tsc; >> Why do return and setting by reference. Looks like an ugly API. > > This is the fallback implementation for !CONFIG_HYPERV_TSCPAGE, which > explains why it's ugly, but why is it needed at all (or it could just > BUG())? > It is not needed indeed, the intention was just to avoid '#if IS_ENABLED(CONFIG_HYPERV)' in kvm code. I can replace it with BUG() or return U64_MAX;
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h index 5400add2885b..2611d2c49f27 100644 --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -323,9 +323,10 @@ static inline void hyperv_setup_mmu_ops(void) {} #ifdef CONFIG_HYPERV_TSCPAGE struct ms_hyperv_tsc_page *hv_get_tsc_page(void); -static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, + u64 *cur_tsc) { - u64 scale, offset, cur_tsc; + u64 scale, offset; u32 sequence; /* @@ -356,7 +357,7 @@ static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) scale = READ_ONCE(tsc_pg->tsc_scale); offset = READ_ONCE(tsc_pg->tsc_offset); - cur_tsc = rdtsc_ordered(); + *cur_tsc = rdtsc_ordered(); /* * Make sure we read sequence after we read all other values @@ -366,7 +367,14 @@ static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence); - return mul_u64_u64_shr(cur_tsc, scale, 64) + offset; + return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset; +} + +static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) +{ + u64 cur_tsc; + + return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc); } #else @@ -374,5 +382,13 @@ static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) { return NULL; } + +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, + u64 *cur_tsc) +{ + *cur_tsc = rdtsc(); + + return cur_tsc; +} #endif #endif
This is going to be used from KVM code were we need to get both TSC and TSC page value. When Hyper-V code is compiled out just return rdtsc(), this will allow us to avoid ugly ifdefs in non-Hyper-V code. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/include/asm/mshyperv.h | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)