| 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
| ||
|
Message-ID: <CALs4sv2cg9ojQNn+V0sVDCW0b-ukL3xZ4jgm5qcVbx232vzJNQ@mail.gmail.com>
Date: Fri, 10 Mar 2023 10:34:45 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: 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>,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
netdev@...r.kernel.org
Subject: Re: [PATCH net v3] bnxt_en: reset PHC frequency in free-running mode
On Fri, Mar 10, 2023 at 3:24 AM Vadim Fedorenko <vadfed@...a.com> wrote:
>
> When using a PHC in shared between multiple hosts, the previous
> frequency value may not be reset and could lead to host being unable to
> compensate the offset with timecounter adjustments. To avoid such state
> reset the hardware frequency of PHC to zero on init. Some refactoring is
> needed to make code readable.
>
> Fixes: 85036aee1938 ("bnxt_en: Add a non-real time mode to access NIC clock")
> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 +
> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 60 ++++++++++---------
> 3 files changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 808236dc898b..e2e2c986c82b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6990,11 +6990,9 @@ static int bnxt_hwrm_func_qcfg(struct bnxt *bp)
> if (flags & FUNC_QCFG_RESP_FLAGS_FW_DCBX_AGENT_ENABLED)
> bp->fw_cap |= BNXT_FW_CAP_DCBX_AGENT;
> }
> - if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST)) {
> + if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST))
> bp->flags |= BNXT_FLAG_MULTI_HOST;
> - if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> - bp->fw_cap &= ~BNXT_FW_CAP_PTP_RTC;
> - }
> +
> if (flags & FUNC_QCFG_RESP_FLAGS_RING_MONITOR_ENABLED)
> bp->fw_cap |= BNXT_FW_CAP_RING_MONITOR;
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index dcb09fbe4007..c0628ac1b798 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2000,6 +2000,8 @@ struct bnxt {
> u32 fw_dbg_cap;
>
> #define BNXT_NEW_RM(bp) ((bp)->fw_cap & BNXT_FW_CAP_NEW_RM)
> +#define BNXT_PTP_USE_RTC(bp) (!BNXT_MH(bp) && \
> + ((bp)->fw_cap & BNXT_FW_CAP_PTP_RTC))
> u32 hwrm_spec_code;
> u16 hwrm_cmd_seq;
> u16 hwrm_cmd_kong_seq;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index 4ec8bba18cdd..a96adde97e60 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -63,7 +63,7 @@ static int bnxt_ptp_settime(struct ptp_clock_info *ptp_info,
> ptp_info);
> u64 ns = timespec64_to_ns(ts);
>
> - if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> + if (BNXT_PTP_USE_RTC(ptp->bp))
> return bnxt_ptp_cfg_settime(ptp->bp, ns);
>
> spin_lock_bh(&ptp->ptp_lock);
> @@ -196,7 +196,7 @@ static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
> struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
> ptp_info);
>
> - if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> + if (BNXT_PTP_USE_RTC(ptp->bp))
> return bnxt_ptp_adjphc(ptp, delta);
>
> spin_lock_bh(&ptp->ptp_lock);
> @@ -205,34 +205,39 @@ static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
> return 0;
> }
>
> +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);
> + return rc;
> +}
> +
> static int bnxt_ptp_adjfine(struct ptp_clock_info *ptp_info, long scaled_ppm)
> {
> struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
> ptp_info);
> - struct hwrm_port_mac_cfg_input *req;
> struct bnxt *bp = ptp->bp;
> - int rc = 0;
>
> - if (!(ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)) {
> - spin_lock_bh(&ptp->ptp_lock);
> - timecounter_read(&ptp->tc);
> - ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
> - spin_unlock_bh(&ptp->ptp_lock);
> - } else {
> - s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
> -
> - rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
> - if (rc)
> - return rc;
> + if (BNXT_PTP_USE_RTC(bp))
> + return bnxt_ptp_adjfine_rtc(bp, scaled_ppm);
>
> - 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(ptp->bp, req);
> - if (rc)
> - netdev_err(ptp->bp->dev,
> - "ptp adjfine failed. rc = %d\n", rc);
> - }
> - return rc;
> + spin_lock_bh(&ptp->ptp_lock);
> + timecounter_read(&ptp->tc);
> + ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
> + spin_unlock_bh(&ptp->ptp_lock);
> + return 0;
> }
>
> void bnxt_ptp_pps_event(struct bnxt *bp, u32 data1, u32 data2)
> @@ -879,7 +884,7 @@ int bnxt_ptp_init_rtc(struct bnxt *bp, bool phc_cfg)
> u64 ns;
> int rc;
>
> - if (!bp->ptp_cfg || !(bp->fw_cap & BNXT_FW_CAP_PTP_RTC))
> + if (!bp->ptp_cfg || !BNXT_PTP_USE_RTC(bp))
> return -ENODEV;
>
> if (!phc_cfg) {
> @@ -932,13 +937,14 @@ 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_MH(bp)) {
Using BNXT_MH(bp) inside bnxt_ptp_init() will not work because
bnxt_ptp_init() is being called much before we determine it is an MH
system.
I may have alluded to this change in v2 comments but I realize that we
don't have the MH determined yet. I am sure your test would not pass
with this patch.
We plan to make that change in the upstream driver to move the
bnxt_ptp_init() to a later point, after we know the type of host.
For now, I think you can simply call bnxt_ptp_adjfine_rtc(bp, 0); when
you see it is a non-RTC host. Much like in v2, except the condition.
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);
bnxt_ptp_adjfine_rtc(bp, 0);
}
Rest everything looks OK.
> + bnxt_ptp_timecounter_init(bp, true);
> + bnxt_ptp_adjfine_rtc(bp, 0);
> + } else {
> 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);
> }
>
> ptp->ptp_info = bnxt_ptp_caps;
> --
> 2.34.1
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)
Powered by blists - more mailing lists