| Message ID | 20191220194749.19692-1-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Hyper-V clock source's offset should be signed | expand |
On 20/12/2019 19:47, Wei Liu wrote: > Fixes: 685d16bd5 (x86: implement Hyper-V clock source) > Signed-off-by: Wei Liu <liuwe@microsoft.com> > --- > I discover this stupid mistake when I work on extracting common code > from viridian and the clock source implementation. Does it make a practical difference? > --- > xen/arch/x86/time.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c > index a3c9b927e8..bbcc9b10b8 100644 > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -698,7 +698,8 @@ static int64_t __init init_hyperv_timer(struct platform_timesource *pts) > > static inline uint64_t read_hyperv_timer(void) Also, this function is only ever accessed via pointer. The inline can't be satisfied at all. ~Andrew > { > - uint64_t scale, offset, ret, tsc; > + uint64_t scale, ret, tsc; > + int64_t offset; > uint32_t seq; > const struct ms_hyperv_tsc_page *tsc_page = hyperv_tsc; >
On Fri, 20 Dec 2019 at 19:57, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 20/12/2019 19:47, Wei Liu wrote: > > Fixes: 685d16bd5 (x86: implement Hyper-V clock source) > > Signed-off-by: Wei Liu <liuwe@microsoft.com> > > --- > > I discover this stupid mistake when I work on extracting common code > > from viridian and the clock source implementation. > > Does it make a practical difference? > Probably not. The specs says that field is signed, so our code should reflect that. > > --- > > xen/arch/x86/time.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c > > index a3c9b927e8..bbcc9b10b8 100644 > > --- a/xen/arch/x86/time.c > > +++ b/xen/arch/x86/time.c > > @@ -698,7 +698,8 @@ static int64_t __init init_hyperv_timer(struct platform_timesource *pts) > > > > static inline uint64_t read_hyperv_timer(void) > > Also, this function is only ever accessed via pointer. The inline can't > be satisfied at all. > I can drop the inline here. Wei. > ~Andrew > > > { > > - uint64_t scale, offset, ret, tsc; > > + uint64_t scale, ret, tsc; > > + int64_t offset; > > uint32_t seq; > > const struct ms_hyperv_tsc_page *tsc_page = hyperv_tsc; > > >
On 20/12/2019 20:02, Wei Liu wrote: > On Fri, 20 Dec 2019 at 19:57, Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> On 20/12/2019 19:47, Wei Liu wrote: >>> Fixes: 685d16bd5 (x86: implement Hyper-V clock source) >>> Signed-off-by: Wei Liu <liuwe@microsoft.com> >>> --- >>> I discover this stupid mistake when I work on extracting common code >>> from viridian and the clock source implementation. >> Does it make a practical difference? >> > Probably not. The specs says that field is signed, so our code should > reflect that. Fair enough. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> (with the inline fix as well)
On Fri, Dec 20, 2019 at 08:04:12PM +0000, Andrew Cooper wrote: > On 20/12/2019 20:02, Wei Liu wrote: > > On Fri, 20 Dec 2019 at 19:57, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >> On 20/12/2019 19:47, Wei Liu wrote: > >>> Fixes: 685d16bd5 (x86: implement Hyper-V clock source) > >>> Signed-off-by: Wei Liu <liuwe@microsoft.com> > >>> --- > >>> I discover this stupid mistake when I work on extracting common code > >>> from viridian and the clock source implementation. > >> Does it make a practical difference? > >> > > Probably not. The specs says that field is signed, so our code should > > reflect that. > > Fair enough. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> (with > the inline fix as well) Thanks. I will commit this now. Wei.
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index a3c9b927e8..bbcc9b10b8 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -698,7 +698,8 @@ static int64_t __init init_hyperv_timer(struct platform_timesource *pts) static inline uint64_t read_hyperv_timer(void) { - uint64_t scale, offset, ret, tsc; + uint64_t scale, ret, tsc; + int64_t offset; uint32_t seq; const struct ms_hyperv_tsc_page *tsc_page = hyperv_tsc;
Fixes: 685d16bd5 (x86: implement Hyper-V clock source) Signed-off-by: Wei Liu <liuwe@microsoft.com> --- I discover this stupid mistake when I work on extracting common code from viridian and the clock source implementation. --- xen/arch/x86/time.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)