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

Re: [PATCH v2 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 09:45:38 +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=rIXjtw8rpt2im2PJqVoJPd6ABNVHoSLEN5afQ+Pr+cQ=; b=kVSv8ff7/zu6Y0riNUm0TEwuBfhXeSbDG0qac3f1PlXjD8J00guK3lzCviNxM7tg5TPrmdgn8xrOyPr0RdyxKw+AjeqFAaKObZnkxrTYIZiUd6T9cTbQCRXegCF+NYiz4kP0jcYx6O2xQ40w/vbaAsbsUt4a2Gsv11FONOc3lgntOL3D6u+JulNNaDl9jPUfqADpRhsUOlFKHzO8vAs8pWT44fZ15GCNPyXKkESdK6w6/8T7la4CTxV0pTYbJVGk+s41uCiMDShgBlmqV1XRn21RI3iA4u7KRvHMYYhJCP1JHKOF2m+jYqXEgIOijWAvC1DZXB6nJOAyU0HxydGh2g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FP5a1peS0NMlG0qOmZ+msQPAWPKTpiiHxNyRYOXGIUfZsNZYPdg1dceXXf4zEoefO7PTGk44ZePUIcP+pflFlPhF+ftfIzU3Jz1dSaLbzEwisY1eVcAjSU92UAT93x3c8RrEY6PFnY96MTKMmhXM4Mr86QLG1s3J1MO0I2RxUtLwYAjEetWikyvL9TThcDguXdfY0bPZEGnmD28d5TcIZnvySh1+tDzOFLtYLQuRdF+fA3rD/f/Q6+eKgTJvQCu0U2UP+qcRa31P0Kf1GcVc7g9W8HFPTNxMirUwyaQrYMmud2eCPZL/PgdskfE1jUBoUOsQSLBtI2scOdl3ebY3GA==
  • Authentication-results: student.uw.edu.pl; dkim=none (message not signed) header.d=none; student.uw.edu.pl; dmarc=none action=none header.from=suse.com;
  • Cc: artur@xxxxxxxxxxxx, Wei Liu <wl@xxxxxxx>, jakub@xxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, marmarek@xxxxxxxxxxxxxxxxxxxxxx, j.nowak26@xxxxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 22 Jun 2020 07:46:34 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 19.06.2020 20:23, Grzegorz Uriasz wrote:
> On 19/06/2020 15:17, Jan Beulich wrote:
>> On 19.06.2020 06:28, Grzegorz Uriasz wrote:
>>> --- a/xen/arch/x86/time.c
>>> +++ b/xen/arch/x86/time.c
>>> @@ -457,16 +457,16 @@ 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 )
>>>          return 0;
>>>  
>>> -    if ( pmtmr_width == 32 )
>>> -    {
>>> -        pts->counter_bits = 32;
>>> -        mask = 0xffffffff;
>>> -    }
>>> +    if ( pmtmr_width != 24 && pmtmr_width != 32 )
>>> +        return 0;
>>> +
>>> +    pts->counter_bits = (int) pmtmr_width;
>>> +    mask = (1ull << pmtmr_width) - 1;
>>>  
>>>      count = inl(pmtmr_ioport) & mask;
>>>      start = rdtsc_ordered();
>>> @@ -486,7 +486,6 @@ static struct platform_timesource __initdata 
>>> plt_pmtimer =
>>>      .name = "ACPI PM Timer",
>>>      .frequency = ACPI_PM_FREQUENCY,
>>>      .read_counter = read_pmtimer_count,
>>> -    .counter_bits = 24,
>>>      .init = init_pmtimer
>>>  };
>> I'm struggling a little to see why this code churn is needed / wanted.
>> The change I had suggested was quite a bit less intrusive. I'm not
>> entirely opposed though, but at the very least please drop the odd
>> (int) cast. If anything we want the struct field changed to unsigned
>> int (but unlikely in this very patch).
>>
>> If you want to stick to this larger change, then also please fold the
>> two if()s at the beginning of the function.
> 
> I wanted to avoid hard coding the masks -> Linux has a nice macro for
> generating the masks but I haven't found a similar macro in xen.
> Additionally this version sets the counter width in a single place
> instead of two.

I guessed this was the goal, but I think such adjustments, if indeed
wanted, would better go into a separate patch. The bug fix here wants
backporting, while this extra cleanup probably doesn't.

Jan



 


Rackspace

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