From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1n9bFe-0002zl-Oz for mharc-grub-devel@gnu.org; Mon, 17 Jan 2022 18:17:26 -0500 Received: from eggs.gnu.org ([209.51.188.92]:59432) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9bFd-0002zX-Bq for grub-devel@gnu.org; Mon, 17 Jan 2022 18:17:25 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:57803) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9bFa-0004Tr-BN for grub-devel@gnu.org; Mon, 17 Jan 2022 18:17:24 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642461440; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=dI6PqLWNQ+lOthrSC2HGW8CL3InKpN57J9rGXaqNY0w=; b=Iv2IDALXYJ/wF6Uv+gf2i4TgX1bKMMqITHC98qYXwCAYa5uSFyzfG9gNWAH5WBonLvOrs2 C3356G3id3qTolJOi+3Ie4Qxx0a43aRHPgbd3j6SBcJtvBJFdfda2AqvbqMhFyihm5SPRY Z1LnBxQDxQwbVLYKNvFIAfmmpg974ck= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-126-bpRIiRB6MOa8mUNzY7NK0g-1; Mon, 17 Jan 2022 18:17:15 -0500 X-MC-Unique: bpRIiRB6MOa8mUNzY7NK0g-1 Received: by mail-qk1-f198.google.com with SMTP id y185-20020a3764c2000000b0047a8c8b3febso5659362qkb.1 for ; Mon, 17 Jan 2022 15:17:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=7vFJ9z0x3qCqTu9jMc/FpbrkCRTOAR89eJoZqrinok8=; b=GLXgZawdWXiWQtdD+H7gWlAJa4aVNDw/pi57cWEn+1KSToKDT5gq3rj+qT2Gl8ms9H uGMKPtDdFg1YVmDLeQJgojnEDSltUN/D3nRdP81iYcJyolfgPBBdT0Xyl73CCvo+5ENL nC7zrtT0yB1okXzIVmxmRHzfasbaxFmDpNDtaI/r8sTI7eS+cHeXaESSaw+Ir8Hm9OTx +uuwEtg9vdHbWAIOWiPMWq7oHrxxO8K0pWlDaZo6qUHTxk3TIFut6sY5JzjrX6Sznkch foAZIHoGq8BA/LIimj+tOdeiVk822zUefwKCJ6RMjRK0iNyqAxcGfqhwDEoU/Ou3x+N8 yFVg== X-Gm-Message-State: AOAM530biSzJLWAEeKEXtj3htgY8oEAVqqHiuXm30L5f5yYreoWNcktY t3xKS2FWTTUTi9fs/y85axCgJuR/A4X2qJSHmQaD2OF21STn7uGMjC4fLYP8xqVPd88v3oppRrj 97eMSdFhSkpk= X-Received: by 2002:ac8:74d6:: with SMTP id j22mr14167149qtr.506.1642461432196; Mon, 17 Jan 2022 15:17:12 -0800 (PST) X-Google-Smtp-Source: ABdhPJxbMLW6+9mxjF4RDusyuBbcPWxvKF1eaFleT7Obc5oBpcrD9+zmfY0HDqX34HjKeG2DNSSZCQ== X-Received: by 2002:ac8:74d6:: with SMTP id j22mr14167099qtr.506.1642461431331; Mon, 17 Jan 2022 15:17:11 -0800 (PST) Received: from localhost ([2601:184:4181:74c0:862e:5809:ed9e:e10e]) by smtp.gmail.com with ESMTPSA id q10sm2978773qtw.41.2022.01.17.15.17.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Jan 2022 15:17:10 -0800 (PST) From: Robbie Harwood To: Javier Martinez Canillas , grub-devel@gnu.org Cc: Peter Jones , Javier Martinez Canillas , Daniel Kiper Subject: Re: [PATCH v2] i386: Make pmtimer tsc calibration not take 51 seconds to fail In-Reply-To: <20200529100837.1054806-1-javierm@redhat.com> References: <20200529100837.1054806-1-javierm@redhat.com> Date: Mon, 17 Jan 2022 18:17:07 -0500 Message-ID: MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=rharwood@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=170.10.133.124; envelope-from=rharwood@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.699, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Jan 2022 23:17:25 -0000 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Javier Martinez Canillas writes: > From: Peter Jones > > 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 tr= y > to wait for 5-200us, it decides it's broken in ~0x2626aa0 TSCs, aka ~2.4 > 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. > > If "debug" is includes "pmtimer", you will see one of the following > three outcomes. If pmtimer gives all 0 or all 1 bits, you will see: > > pmtimer: 0xffffff bad_reads: 1 > pmtimer: 0xffffff bad_reads: 2 > pmtimer: 0xffffff bad_reads: 3 > pmtimer: 0xffffff bad_reads: 4 > pmtimer: 0xffffff bad_reads: 5 > pmtimer: 0xffffff bad_reads: 6 > pmtimer: 0xffffff bad_reads: 7 > pmtimer: 0xffffff bad_reads: 8 > pmtimer: 0xffffff bad_reads: 9 > pmtimer: 0xffffff bad_reads: 10 > timer is broken; giving up. > > This outcome was tested using qemu+kvm with UEFI (OVMF) firmware and > these options: -machine pc-q35-2.10 -cpu Broadwell-noTSX > > If pmtimer gives any other bit patterns but is not actually marching > forward fast enough to use for clock calibration, you will see: > > pmtimer delta is 0x0 (1904 iterations) > tsc delta is implausible: 0x2626aa0 > > This outcome was tested using grub compiled with GRUB_PMTIMER_IGNORE_BAD_= READS > defined (so as not to trip the bad read test) using qemu+kvm with UEFI > (OVMF) firmware, and these options: -machine pc-q35-2.10 -cpu Broadwell-n= oTSX > > If pmtimer actually works, you'll see something like: > > pmtimer delta is 0xdff > tsc delta is 0x278756 > > This outcome was tested using qemu+kvm with UEFI (OVMF) firmware, and > these options: -machine pc-i440fx-2.4 -cpu Broadwell-noTSX > > I've also tested this outcome on a real Intel Xeon E3-1275v3 on an Intel > Server Board S1200V3RPS using the SDV.RP.B8 "Release" build here: > https://firmware.intel.com/sites/default/files/UEFIDevKit_S1200RP_vB8.zip > > Signed-off-by: Peter Jones > Signed-off-by: Javier Martinez Canillas > --- > > Hello Daniel, > > I think that addressed all the issues you pointed out to Peter in > https://lists.gnu.org/archive/html/grub-devel/2018-02/msg00078.html > > Please let me know if I missed anything. > > Best regards, > Javier > > Changes in v2: > - Address issues pointed out by Daniel Kiper on previously posted version= . > > grub-core/kern/i386/tsc_pmtimer.c | 112 ++++++++++++++++++++++++------ > 1 file changed, 92 insertions(+), 20 deletions(-) > > diff --git a/grub-core/kern/i386/tsc_pmtimer.c b/grub-core/kern/i386/tsc_= pmtimer.c > index c9c36169978..d9b3765b018 100644 > --- a/grub-core/kern/i386/tsc_pmtimer.c > +++ b/grub-core/kern/i386/tsc_pmtimer.c > @@ -28,40 +28,104 @@ > #include > #include > =20 > +/* > + * Define GRUB_PMTIMER_IGNORE_BAD_READS if you're trying to test a timer= that's > + * present but doesn't keep time well. > + */ > +// #define GRUB_PMTIMER_IGNORE_BAD_READS > + > grub_uint64_t > grub_pmtimer_wait_count_tsc (grub_port_t pmtimer, > =09=09=09 grub_uint16_t num_pm_ticks) > { > grub_uint32_t start; > - grub_uint32_t last; > - grub_uint32_t cur, end; > + grub_uint64_t cur, end; > grub_uint64_t start_tsc; > grub_uint64_t end_tsc; > - int num_iter =3D 0; > + unsigned int num_iter =3D 0; > +#ifndef GRUB_PMTIMER_IGNORE_BAD_READS > + int bad_reads =3D 0; > +#endif > =20 > - start =3D grub_inl (pmtimer) & 0xffffff; > - last =3D start; > + /* > + * Some timers are 24-bit and some are 32-bit, but it doesn't make muc= h > + * difference to us. Caring which one we have isn't really worth it s= ince > + * the low-order digits will give us enough data to calibrate TSC. So= just > + * mask the top-order byte off. > + */ > + cur =3D start =3D grub_inl (pmtimer) & 0x00ffffffUL; > end =3D start + num_pm_ticks; > start_tsc =3D grub_get_tsc (); > while (1) > { > - cur =3D grub_inl (pmtimer) & 0xffffff; > - if (cur < last) > -=09cur |=3D 0x1000000; > - num_iter++; > + cur &=3D 0xffffffffff000000ULL; > + /* > + * Only take the low-order 24-bit for the reason explained above. > + */ > + cur |=3D grub_inl (pmtimer) & 0x00ffffffUL; > + > + end_tsc =3D grub_get_tsc(); > + > +#ifndef GRUB_PMTIMER_IGNORE_BAD_READS > + /* > + * 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 =3D=3D 0xffffffUL || cur =3D=3D 0) > +=09{ > +=09 bad_reads++; > +=09 grub_dprintf ("pmtimer", > +=09=09=09"pmtimer: 0x%"PRIxGRUB_UINT64_T" bad_reads: %d\n", > +=09=09=09cur, bad_reads); > +=09 grub_dprintf ("pmtimer", "timer is broken; giving up.\n"); > + > +=09 if (bad_reads =3D=3D 10) > +=09 return 0; > +=09} > +#endif > + > + if (cur < start) > +=09cur +=3D 0x1000000; > + > if (cur >=3D end) > =09{ > -=09 end_tsc =3D grub_get_tsc (); > +=09 grub_dprintf ("pmtimer", "pmtimer delta is 0x%"PRIxGRUB_UINT64_T"\n= ", > +=09=09=09cur - start); > +=09 grub_dprintf ("pmtimer", "tsc delta is 0x%"PRIxGRUB_UINT64_T"\n", > +=09=09=09end_tsc - start_tsc); > =09 return end_tsc - start_tsc; > =09} > - /* Check for broken PM timer. > -=09 50000000 TSCs is between 5 ms (10GHz) and 200 ms (250 MHz) > -=09 if after this time we still don't have 1 ms on pmtimer, then > -=09 pmtimer is broken. > + > + /* > + * Check for broken PM timer. 1ms at 10GHz should be 1E+7 TSCs; a= t > + * 250MHz it should be 2.5E5. So if after 4E+7 TSCs on a 10GHz ma= chine, > + * we should have seen pmtimer show 4ms of change (i.e. cur =3D~ > + * start+14320); on a 250MHz machine that should be 160ms (start+5= 72800). > + * If after this a time we still don't have 1ms on pmtimer, then p= mtimer > + * is broken. > + * > + * Likewise, if our code is perfectly efficient and introduces no = delays > + * whatsoever, on a 10GHz system we should see a TSC delta of 3580= in > + * ~3580 iterations. On a 250MHz machine that should be ~900 iter= ations. > + * > + * With those factors in mind, there are two limits here. There's= a hard > + * limit here at 8x our desired pm timer delta. This limit was pic= ked as > + * an arbitrarily large value that's still not a lot of time to hu= mans, > + * because if we get that far this is either an implausibly fast m= achine > + * or the pmtimer is not running. And there is another limit on a= 4 ms TSC > + * delta on a 10 GHz clock, without seeing cur converge on our tar= get value. > */ > - if ((num_iter & 0xffffff) =3D=3D 0 && grub_get_tsc () - start_tsc = > 5000000) { > -=09return 0; > - } > + if ((++num_iter > (grub_uint32_t)num_pm_ticks << 3UL) || > +=09 end_tsc - start_tsc > 40000000) > +=09{ > +=09 grub_dprintf ("pmtimer", > +=09=09=09"pmtimer delta is 0x%"PRIxGRUB_UINT64_T" (%u iterations)\n", > +=09=09=09cur - start, num_iter); > +=09 grub_dprintf ("pmtimer", > +=09=09=09"tsc delta is implausible: 0x%"PRIxGRUB_UINT64_T"\n", > +=09=09=09end_tsc - start_tsc); > +=09 return 0; > +=09} > } > } > =20 > @@ -74,12 +138,20 @@ grub_tsc_calibrate_from_pmtimer (void) > =20 > fadt =3D grub_acpi_find_fadt (); > if (!fadt) > - return 0; > + { > + grub_dprintf ("pmtimer", "No FADT found; not using pmtimer.\n"); > + return 0; > + } > pmtimer =3D fadt->pmtimer; > if (!pmtimer) > - return 0; > + { > + grub_dprintf ("pmtimer", "FADT does not specify pmtimer; skipping.= \n"); > + return 0; > + } > =20 > - /* It's 3.579545 MHz clock. Wait 1 ms. */ > + /* > + * It's 3.579545 MHz clock. Wait 1 ms. > + */ > tsc_diff =3D grub_pmtimer_wait_count_tsc (pmtimer, 3580); > if (tsc_diff =3D=3D 0) > return 0; > --=20 > 2.26.2 Reviewed-by: Robbie Harwood Be well, --Robbie --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJIBAEBCgAyFiEEA5qc6hnelQjDaHWqJTL5F2qVpEIFAmHl+PMUHHJoYXJ3b29k QHJlZGhhdC5jb20ACgkQJTL5F2qVpEIYUA//aktHU6ssZRB4zzX1+tjMFqMArRYx YGcX5RoGHbp/0612dtE0MZQmwlj9KgXFNuquh9gc8BU+TQj5yrJAF7P2u9GWPjKP V/Prinwy+OARhslMvYNjitcT1QRidGZA/cZ1lz0hLkGI5irm0Zk3YMoALLw78G+g lq8AG5GZt5kO+eEDXgFv1ROnnOOgs6kFr08wKkDn2WIjdLdT9P8SjBTx9/G5qjtG Jeu1EKY9DjfWDIHrM4om/xUL/MxzWGYqWaDHH8ISfP//IMqEoZ4UFbXRGzQ1CO4d vBaR6iePMoJq4odpFGTTQHQwF/UJTgjZjy+zhowJO+0eu33nQv8vSoYIIrEQkW+V QIFsJFwd/elaQ3fu1ywnKsc2JhOfWCrN542C7xAMQc0iqMYRroJK/5k6R2VltC8x 6V4t9b6IUxzDUWh0SxnZlEPSZOckB7EPZqVGS3mZZSB0dWhu0FgXZ+UgMVRaUVAD eMQTLvkVP65ta16yXQZ24Tm9ugwJInWt1GfhnNO050bd9p5lpgExD8+AlxTZxBsZ BrM2U12+TAPhZp4eEjeR+sqE2HbKhujHcYz4D30PvsPF8DxWwWRUh2Nb/K3RQSC8 0JDlaloyFUeVUnsjV9dbmIp0jpnbwbNSJ4BqFiczTNG0X6X9Q6vZuijrx5Rg0Mlj 6hMxjPA742Xj7Uc= =pRk4 -----END PGP SIGNATURE----- --=-=-=--