From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ecJ3V-0008Ei-Hd for mharc-grub-devel@gnu.org; Thu, 18 Jan 2018 17:53:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52612) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ecJ3S-0008Dz-TJ for grub-devel@gnu.org; Thu, 18 Jan 2018 17:53:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ecJ3P-0003ff-RT for grub-devel@gnu.org; Thu, 18 Jan 2018 17:53:06 -0500 Received: from aserp2130.oracle.com ([141.146.126.79]:45398) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ecJ3P-0003b5-JC for grub-devel@gnu.org; Thu, 18 Jan 2018 17:53:03 -0500 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w0IMqttl169029; Thu, 18 Jan 2018 22:53:00 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2017-10-26; bh=1uqkNUPxHGp+nH3qdST0LJcCWqrLrDDUABDAw1WKE2U=; b=kVWqD1HUwPJknlaCT6WL/rB0jExgi3bmGlRuQSLMPbkgtC2gXBDMkfE9mA6T3rlYHGB2 gM5iSm8weYzMFKmT0+KDZJ0Y6VMGPg0fOVAR8ugWwJKNPmjitkj2Ps7qcjwDcZf/RKCa IgB86qWB1izH1zzunrwDW9mh6mjVvw7qIHpbRqM2Myp5i8h//zb12dzTcID3AVCpjiIL iJIU9faiG6KHurT3XZ7xHeWUYo7zJNLIFWv8MafkPf/AdtJgG2ab7ohrnt64QEYuJslZ O1T4y2MJ2eqqPhis+i4gM+TKnOyGF2Xkaibki09p5Z9LEgBTORIySx1GKEDLyGBN+aAB 4w== Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by aserp2130.oracle.com with ESMTP id 2fk4qt80ad-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 18 Jan 2018 22:52:59 +0000 Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w0IMqxdc023794 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 18 Jan 2018 22:52:59 GMT Received: from abhmp0005.oracle.com (abhmp0005.oracle.com [141.146.116.11]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w0IMqww4004441; Thu, 18 Jan 2018 22:52:58 GMT Received: from olila.local.net-space.pl (/10.175.166.72) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 18 Jan 2018 14:52:57 -0800 Date: Thu, 18 Jan 2018 23:52:53 +0100 From: Daniel Kiper To: Peter Jones Cc: grub-devel@gnu.org, "David E . Box" Subject: Re: [PATCH] Make pmtimer tsc calibration not take 51 seconds to fail. Message-ID: <20180118225253.GB21705@olila.local.net-space.pl> References: <20180116181617.2748-1-pjones@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180116181617.2748-1-pjones@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8778 signatures=668654 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1801180296 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 141.146.126.79 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jan 2018 22:53:08 -0000 On Tue, Jan 16, 2018 at 01:16:17PM -0500, Peter Jones wrote: > On my laptop running at 2.4GHz, if I run a VM where tsc calibration > using pmtimer will fail presuming a broken pmtimer, it takes ~51 seconds > to do so (as measured with the stopwatch on my phone), with a tsc delta > of 0x1cd1c85300, or around 125 billion cycles. > > If instead of trying to wait for 5-200ms to show up on the pmtimer, we try > to wait for 5-200us, it decides it's broken in ~0x7998f9e TSCs, aka ~2 > million cycles, or more or less instantly. > > Additionally, this reading the pmtimer was returning 0xffffffff anyway, > and that's obviously an invalid return. I've added a check for that and > 0 so we don't bother waiting for the test if what we're seeing is dead > pins with no response at all. > > Signed-off-by: Peter Jones > --- > grub-core/kern/i386/tsc_pmtimer.c | 43 ++++++++++++++++++++++++++++++--------- > 1 file changed, 33 insertions(+), 10 deletions(-) > > diff --git a/grub-core/kern/i386/tsc_pmtimer.c b/grub-core/kern/i386/tsc_pmtimer.c > index c9c36169978..609402b8376 100644 > --- a/grub-core/kern/i386/tsc_pmtimer.c > +++ b/grub-core/kern/i386/tsc_pmtimer.c > @@ -38,30 +38,53 @@ grub_pmtimer_wait_count_tsc (grub_port_t pmtimer, > grub_uint64_t start_tsc; > grub_uint64_t end_tsc; > int num_iter = 0; > + int bad_reads = 0; > > - start = grub_inl (pmtimer) & 0xffffff; > + start = grub_inl (pmtimer) & 0x3fff; I am not sure why you are changing this to 0x3fff... > last = start; > end = start + num_pm_ticks; > start_tsc = grub_get_tsc (); > while (1) > { > - cur = grub_inl (pmtimer) & 0xffffff; What about 24-bit timers? I would leave this here... > + cur = grub_inl (pmtimer); > + > + /* If we get 10 reads in a row that are obviously dead pins, there's no > + reason to do this thousands of times. > + */ > + if (cur == 0xffffffff || cur == 0) ...and here I would check for 0xffffff and 0. > + { > + bad_reads++; > + grub_dprintf ("pmtimer", "cur: 0x%08x bad_reads: %d\n", cur, bad_reads); > + > + if (bad_reads == 10) > + return 0; > + } > + else if (bad_reads) > + bad_reads = 0; Do we really need to reset this? > + cur &= 0x3fff; > + > if (cur < last) > - cur |= 0x1000000; > + cur |= 0x4000; > num_iter++; > if (cur >= end) > { > end_tsc = grub_get_tsc (); > + grub_dprintf ("pmtimer", "tsc delta is 0x%016lx\n", > + end_tsc - start_tsc); > return end_tsc - start_tsc; > } > - /* Check for broken PM timer. > - 50000000 TSCs is between 5 ms (10GHz) and 200 ms (250 MHz) > - if after this time we still don't have 1 ms on pmtimer, then > - pmtimer is broken. > + /* Check for broken PM timer. 5000 TSCs is between 5us (10GHz) and ^^^ 500ns? > + 200us (250 MHz). If after this time we still don't have 1us on ^^^^^ 20us? > + pmtimer, then pmtimer is broken. > */ > - if ((num_iter & 0xffffff) == 0 && grub_get_tsc () - start_tsc > 5000000) { > - return 0; > - } > + end_tsc = grub_get_tsc(); > + if ((num_iter & 0x3fff) == 0 && end_tsc - start_tsc > 5000) Why 0x3fff here which means, AIUI, 4000 iterations? Daniel