[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/1] x86/acpi: Use FADT flags to determine the PMTMR width


  • To: Grzegorz Uriasz <gorbak25@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 22 Jun 2020 10:49:04 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cZH7/xZQX+TdFs0CqjEu3za0269PNeZnFsQkTcfLmfw=; b=HE8XhBBv02af3Pp5ZtQtqgUEDO50/Yga1qOzpSyu+D7iQsy8iOmhtMHm8UCru3WsSShduJ0Ra3Z1eXDOQSMy5ABvtSxxSBWmrPuBG1W1MB6TxU4PYONk7+t8ehTu3+TJYzmBCO0dMfrKF2tmnLL2BaUxXKjYPjD+mo/aqWJbF7Mgq+0RxNbctIAZe9VawVp+bRcclgF0JP/RDZoZ9kXwqLk2gFd8rrN4WiW7+Y6+D92i6cSO/Xet5nIs+8omkvWt5T3NHovC9JQ47PCOxx7sbbFqn2gigpcB71RpoptesxIVmksz37f9Q6smxzOjv4ihVRfNci7hDJLxEOsFRnpMLw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BtrdZACy1TtRB/PDBLiEMnxyYN1zrYzs+KVydMoFOXZ738pRif1Okef6FExYrrwgug30tNW1bgOq52xB4Zre82icWIK2pkli1P7jfKQ5JHW4adZoZc/gKyhzn7zbeP9Vm9wR/B8fdk6GEEDn0oQB1OdgtjpfHvd8++2KndjPvFM2tHLaeQhjbnIuWZhfECcl5i2ClUmR7Lbd8kyoP4S1D0q+1ITK5mcH2r3LVHQO316/oQnjOlFBNxV+rU+KxhRZ4mXTvDxZuhxVYqgMUlbFoqFHXMUxf7flvj98jyDTjfpOwEyeOo1PPtbSIpEwMtYY8Tt06zqCnzrMtOqU5MM6NA==
  • Authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, jakub@xxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, marmarek@xxxxxxxxxxxxxxxxxxxxxx, j.nowak26@xxxxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, contact@xxxxxxxxxxxx, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 22 Jun 2020 08:49:31 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 20.06.2020 00:00, Grzegorz Uriasz wrote:
> @@ -490,8 +497,12 @@ static int __init acpi_parse_fadt(struct 
> acpi_table_header *table)
>        */
>       if (!pmtmr_ioport) {
>               pmtmr_ioport = fadt->pm_timer_block;
> -             pmtmr_width = fadt->pm_timer_length == 4 ? 24 : 0;
> +             pmtmr_width = fadt->pm_timer_length == 4 ? 32 : 0;
>       }
> +     if (pmtmr_width < 32 && fadt->flags & ACPI_FADT_32BIT_TIMER)
> +        printk(KERN_WARNING PREFIX "PM-Timer is too short\n");

Indentation is screwed up here.

> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -457,16 +457,13 @@ static u64 read_pmtimer_count(void)
>  static s64 __init init_pmtimer(struct platform_timesource *pts)
>  {
>      u64 start;
> -    u32 count, target, mask = 0xffffff;
> +    u32 count, target, mask;
>  
> -    if ( !pmtmr_ioport || !pmtmr_width )
> +    if ( !pmtmr_ioport || (pmtmr_width != 24 && pmtmr_width != 32) )
>          return 0;
>  
> -    if ( pmtmr_width == 32 )
> -    {
> -        pts->counter_bits = 32;
> -        mask = 0xffffffff;
> -    }
> +    pts->counter_bits = pmtmr_width;
> +    mask = (1ull << pmtmr_width) - 1;

"mask" being of "u32" type, the use of 1ull is certainly a little odd
here, and one of the reasons why I think this extra "cleanup" would
better go in a separate patch.

Seeing that besides cosmetic aspects the patch looks okay now, I'd be
willing to leave the bigger adjustment mostly as is, albeit I'd
prefer the 1ull to go away, by instead switching to e.g.

    mask = 0xffffffff >> (32 - pmtmr_width);

With the adjustments (which I'd be happy to make while committing,
provided you agree)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Also Cc-ing Paul for a possible release ack (considering this is a
backporting candidate).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.