netdev - Re: [PATCH net v2] bnxt_en: reset PHC frequency in free-running mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALs4sv2cOFEVFwJ_UgV5T0iJOAzM7X=jvDqsdAtjiuQjTs5U8g@mail.gmail.com>
Date:   Thu, 9 Mar 2023 15:41:19 +0530
From:   Pavan Chebbi <pavan.chebbi@...adcom.com>
To:     Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc:     Vadim Fedorenko <vadfed@...a.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Andy Gospodarek <andrew.gospodarek@...adcom.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Richard Cochran <richardcochran@...il.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net v2] bnxt_en: reset PHC frequency in free-running mode

On Thu, Mar 9, 2023 at 3:02 PM Vadim Fedorenko
<vadim.fedorenko@...ux.dev> wrote:
>
> On 09.03.2023 04:40, Pavan Chebbi wrote:
> > On Wed, Mar 8, 2023 at 8:12 PM Vadim Fedorenko <vadfed@...a.com> wrote:

> >> @@ -932,13 +937,15 @@ int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg)
> >>          atomic_set(&ptp->tx_avail, BNXT_MAX_TX_TS);
> >>          spin_lock_init(&ptp->ptp_lock);
> >>
> >> -       if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) {
> >> +       if (BNXT_PTP_USE_RTC(ptp->bp)) {
> >>                  bnxt_ptp_timecounter_init(bp, false);
> >>                  rc = bnxt_ptp_init_rtc(bp, phc_cfg);
> >>                  if (rc)
> >>                          goto out;
> >>          } else {
> >>                  bnxt_ptp_timecounter_init(bp, true);
> >> +               if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> >
> > I understand from your response on v1 as to why it will not affect you
> > if a new firmware does not report RTC on MH.
> > However, once you update the fw, any subsequent kernels upgrades will
> > prevent resetting the freq stored in the PHC.
> > Would changing the check to if (BNXT_MH(bp)) instead be a better option?
>
> How will it affect hardware without RTC support? The one which doesn't have
> BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will

For single hosts, it should not matter if we reset the PHC frequency.
bnxt_ptp_init() is [re]initializing the host-timecounter, and this
function being called on a single host means everything is going to
[re]start from scratch.

> not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to
> !BNXT_MH() and there will be no need for additional check in this else clause.

You are right, hence my original suggestion of resetting the PHC freq
unconditionally is better.
One more thing, the function bnxt_ptp_adjfine() should select
non-realtime adjustments only for MH systems.
You may need to flip the check, something like

if (!BNXT_MH(ptp->bp))
    return bnxt_ptp_adjfine_rtc(bp, scaled_ppm);

This is because there can be a very old firmware which does not have
RTC on single hosts but we still want to make HW adjustments.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)