x86/hpet: Fix return value of hpet_setup()

[PATCH] x86/hpet: Fix return value of hpet_setup()

Andrew Cooper posted 1 patch 3 years, 3 months ago

Download mbox

Test gitlab-ci passed

Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20201230160208.18877-1-andrew.cooper3@citrix.com

xen/arch/x86/hpet.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

Expand all Fold all

[PATCH] x86/hpet: Fix return value of hpet_setup()

Posted by Andrew Cooper 3 years, 3 months ago

hpet_setup() is idempotent if the rate has already been calculated, and
returns the cached value.  However, this only works correctly when the return
statements are identical.

Use a sensibly named local variable, rather than a dead one with a bad name.

Fixes: a60bb68219 ("x86/time: reduce rounding errors in calculations")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/hpet.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index a55e68e6f7..e6fab8acd8 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -759,7 +759,7 @@ u64 __init hpet_setup(void)
 {
     static u64 __initdata hpet_rate;
     u32 hpet_id, hpet_period;
-    unsigned int last;
+    unsigned int last, rem;
 
     if ( hpet_rate )
         return hpet_rate;
@@ -789,9 +789,11 @@ u64 __init hpet_setup(void)
     hpet_resume(hpet_boot_cfg);
 
     hpet_rate = 1000000000000000ULL; /* 10^15 */
-    last = do_div(hpet_rate, hpet_period);
+    rem = do_div(hpet_rate, hpet_period);
+    if ( (rem * 2) > hpet_period )
+        hpet_rate++;
 
-    return hpet_rate + (last * 2 > hpet_period);
+    return hpet_rate;
 }
 
 void hpet_resume(u32 *boot_cfg)
-- 
2.11.0

Re: [PATCH] x86/hpet: Fix return value of hpet_setup()

Posted by Roger Pau Monné 3 years, 3 months ago

On Wed, Dec 30, 2020 at 04:02:08PM +0000, Andrew Cooper wrote:
> hpet_setup() is idempotent if the rate has already been calculated, and
> returns the cached value.  However, this only works correctly when the return
> statements are identical.
> 
> Use a sensibly named local variable, rather than a dead one with a bad name.
> 
> Fixes: a60bb68219 ("x86/time: reduce rounding errors in calculations")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.