Message ID | 20240209212432.750653-2-arkadiusz.kubalewski@intel.com |
---|---|
State | Accepted |
Delegated to: | Anthony Nguyen |
Headers | show |
Series | ice: fix dpll data access on PF reset | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Arkadiusz Kubalewski > Sent: Saturday, February 10, 2024 2:55 AM > To: intel-wired-lan@lists.osuosl.org > Cc: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com> > Subject: [Intel-wired-lan] [PATCH iwl-net v2 1/3] ice: fix dpll and dpll_pin data access on PF reset > > Do not allow to acquire data or alter configuration of dpll and pins > through firmware if PF reset is in progress, this would cause confusing > netlink extack errors as the firmware cannot respond or process the > request properly during the reset time. > > Return (-EBUSY) and extack error for the user who tries access/modify > the config of dpll/pin through firmware during the reset time. > > The PF reset and kernel access to dpll data are both asynchronous. It is > not possible to guard all the possible reset paths with any determinictic > approach. I.e., it is possible that reset starts after reset check is > performed (or if the reset would be checked after mutex is locked), but at > the same time it is not possible to wait for dpll mutex unlock in the > reset flow. > This is best effort solution to at least give a clue to the user > what is happening in most of the cases, knowing that there are possible > race conditions where the user could see a different error received > from firmware due to reset unexpectedly starting. > > Test by looping execution of below steps until netlink error appears: > - perform PF reset > $ echo 1 > /sys/class/net/<ice PF>/device/reset > - i.e. try to alter/read dpll/pin config: > $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \ > --dump pin-get > > Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu") > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > --- > v2: > - remove newline from extack error > - change "pf" -> "PF" in extack error > --- > drivers/net/ethernet/intel/ice/ice_dpll.c | 38 +++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 9c0d739be1e9..58d135764ab0 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -30,6 +30,26 @@ static const char * const pin_type_name[] = { [ICE_DPLL_PIN_TYPE_RCLK_INPUT] = "rclk-input", }; +/** + * ice_dpll_is_reset - check if reset is in progress + * @pf: private board structure + * @extack: error reporting + * + * If reset is in progress, fill extack with error. + * + * Return: + * * false - no reset in progress + * * true - reset in progress + */ +static bool ice_dpll_is_reset(struct ice_pf *pf, struct netlink_ext_ack *extack) +{ + if (ice_is_reset_in_progress(pf->state)) { + NL_SET_ERR_MSG(extack, "PF reset in progress"); + return true; + } + return false; +} + /** * ice_dpll_pin_freq_set - set pin's frequency * @pf: private board structure @@ -109,6 +129,9 @@ ice_dpll_frequency_set(const struct dpll_pin *pin, void *pin_priv, struct ice_pf *pf = d->pf; int ret; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); ret = ice_dpll_pin_freq_set(pf, p, pin_type, frequency, extack); mutex_unlock(&pf->dplls.lock); @@ -584,6 +607,9 @@ ice_dpll_pin_state_set(const struct dpll_pin *pin, void *pin_priv, struct ice_pf *pf = d->pf; int ret; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); if (enable) ret = ice_dpll_pin_enable(&pf->hw, p, d->dpll_idx, pin_type, @@ -687,6 +713,9 @@ ice_dpll_pin_state_get(const struct dpll_pin *pin, void *pin_priv, struct ice_pf *pf = d->pf; int ret; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); ret = ice_dpll_pin_state_update(pf, p, pin_type, extack); if (ret) @@ -811,6 +840,9 @@ ice_dpll_input_prio_set(const struct dpll_pin *pin, void *pin_priv, struct ice_pf *pf = d->pf; int ret; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); ret = ice_dpll_hw_input_prio_set(pf, d, p, prio, extack); mutex_unlock(&pf->dplls.lock); @@ -1090,6 +1122,9 @@ ice_dpll_rclk_state_on_pin_set(const struct dpll_pin *pin, void *pin_priv, int ret = -EINVAL; u32 hw_idx; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); hw_idx = parent->idx - pf->dplls.base_rclk_idx; if (hw_idx >= pf->dplls.num_inputs) @@ -1144,6 +1179,9 @@ ice_dpll_rclk_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv, int ret = -EINVAL; u32 hw_idx; + if (ice_dpll_is_reset(pf, extack)) + return -EBUSY; + mutex_lock(&pf->dplls.lock); hw_idx = parent->idx - pf->dplls.base_rclk_idx; if (hw_idx >= pf->dplls.num_inputs)