Re: [Xen-devel] [PATCH v3 04/11] hvm/hpet: Correctly limit period to a maximum.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.04.14 at 19:42, <dslutz@xxxxxxxxxxx> wrote:
> In the code section after the comment:
> 
>     /*
>      * Clamp period to reasonable min/max values:
>      *  - minimum is 100us, same as timers controlled by vpt.c
>      *  - maximum is to prevent overflow in time_after() calculations
>      */
> 
> The current maximum limit actually allows "bad" values like 0 and 1.
> This is because it uses a mask not a maximum.
> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

>  xen/arch/x86/hvm/hpet.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
> index 910d87d..c64c547 100644
> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -404,6 +404,8 @@ static int hpet_write(
>          if ( timer_is_periodic(h, tn) &&
>               !(h->hpet.timers[tn].config & HPET_TN_SETVAL) )
>          {
> +            uint64_t max_period = (timer_is_32bit(h, tn) ? ~0u : ~0ull) >> 1;
> +
>              /*
>               * Clamp period to reasonable min/max values:
>               *  - minimum is 100us, same as timers controlled by vpt.c
> @@ -411,7 +413,8 @@ static int hpet_write(
>               */
>              if ( hpet_tick_to_ns(h, new_val) < MICROSECS(100) )
>                  new_val = (MICROSECS(100) << 10) / h->hpet_to_ns_scale;
> -            new_val &= (timer_is_32bit(h, tn) ? ~0u : ~0ull) >> 1;
> +            if ( new_val > max_period )
> +                new_val = max_period;
>              h->hpet.period[tn] = new_val;
>          }
>          else
> -- 
> 1.8.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel