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]

Date:   Thu, 9 Mar 2023 09:32:09 +0000
From:   Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To:     Pavan Chebbi <pavan.chebbi@...adcom.com>,
        Vadim Fedorenko <vadfed@...a.com>
Cc:     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 09.03.2023 04:40, Pavan Chebbi wrote:
> On Wed, Mar 8, 2023 at 8:12 PM Vadim Fedorenko <vadfed@...a.com> wrote:
>>
>> +static int bnxt_ptp_adjfine_rtc(struct bnxt *bp, long scaled_ppm)
>> +{
>> +       s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
>> +       struct hwrm_port_mac_cfg_input *req;
>> +       int rc;
>> +
>> +       rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
>> +       if (rc)
>> +               return rc;
>> +
>> +       req->ptp_freq_adj_ppb = cpu_to_le32(ppb);
>> +       req->enables = cpu_to_le32(PORT_MAC_CFG_REQ_ENABLES_PTP_FREQ_ADJ_PPB);
>> +       rc = hwrm_req_send(bp, req);
>> +       if (rc)
>> +               netdev_err(bp->dev,
>> +                          "ptp adjfine failed. rc = %d\n", rc);
> 
> nit: can be a single line.
> 
>> +       return rc;
>> +}
>> +
> 
>> @@ -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 
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.


>> +                       bnxt_ptp_adjfine_rtc(bp, 0);
>>          }
>>
>>          ptp->ptp_info = bnxt_ptp_caps;
>> --
>> 2.30.2
>>