2020-01-14 15:57:02

by Moshe Shemesh

[permalink] [raw]
Subject: [PATCH net-next RFC 3/3] net/mlx5: Add FW upgrade reset support

Add support for FW upgrade reset.
On devlink reload the driver checks if there is a FW stored pending
upgrade reset. In such case the driver will set the device to FW upgrade
reset on next PCI link toggle and do link toggle after unload.

To do PCI link toggle, the driver ensures that no other device ID under
the same bridge by checking that all the PF functions under the same PCI
bridge have same device ID. If no other device it uses PCI bridge link
control to turn link down and up.

Signed-off-by: Moshe Shemesh <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 81 ++++++++++++++++++++++-
1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index ac108f1..2aa9e99 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -85,12 +85,91 @@ static u16 mlx5_fw_ver_subminor(u32 version)
return 0;
}

+static int mlx5_pci_link_toggle(struct mlx5_core_dev *dev)
+{
+ struct pci_bus *bridge_bus = dev->pdev->bus;
+ struct pci_dev *bridge = bridge_bus->self;
+ struct pci_dev *sdev;
+ u16 dev_id, sdev_id;
+ u16 reg16;
+ int cap;
+ int err;
+
+ /* Check that all functions under the pci bridge are VFs and PFs of
+ * this device otherwise fail this function.
+ */
+ pci_read_config_word(dev->pdev, PCI_DEVICE_ID, &dev_id);
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ pci_read_config_word(sdev, PCI_DEVICE_ID, &sdev_id);
+ if (sdev_id != 0xFFFF && sdev_id != dev_id)
+ return -EPERM;
+ }
+
+ cap = pci_find_capability(bridge, PCI_CAP_ID_EXP);
+ if (!cap)
+ return -EOPNOTSUPP;
+
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ pci_save_state(sdev);
+ pci_cfg_access_lock(sdev);
+ }
+ err = pci_read_config_word(bridge, cap + PCI_EXP_LNKCTL, &reg16);
+ if (err)
+ return err;
+ reg16 |= PCI_EXP_LNKCTL_LD;
+ err = pci_write_config_word(bridge, cap + PCI_EXP_LNKCTL, reg16);
+ if (err)
+ return err;
+ msleep(100);
+ reg16 &= ~PCI_EXP_LNKCTL_LD;
+ err = pci_write_config_word(bridge, cap + PCI_EXP_LNKCTL, reg16);
+ if (err)
+ return err;
+ msleep(100);
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ pci_cfg_access_unlock(sdev);
+ pci_restore_state(sdev);
+ }
+
+ return 0;
+}
+
static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
struct netlink_ext_ack *extack)
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
+ bool pci_link_toggle_required = false;
+ u32 running_fw, stored_fw;
+ u8 reset_level;
+ int err;
+
+ err = mlx5_fw_version_query(dev, &running_fw, &stored_fw);
+ if (err)
+ return err;

- return mlx5_unload_one(dev, false);
+ if (stored_fw) {
+ err = mlx5_fw_query_reset_level(dev, &reset_level);
+ if (err)
+ return err;
+ if (reset_level & MLX5_MFRL_REG_RESET_LEVEL3) {
+ err = mlx5_fw_set_reset_level(dev,
+ MLX5_MFRL_REG_RESET_LEVEL3);
+ if (err)
+ return err;
+ pci_link_toggle_required = true;
+ } else {
+ mlx5_core_warn(dev, "FW upgrade requires reboot\n");
+ }
+ }
+
+ err = mlx5_unload_one(dev, false);
+ if (err)
+ return err;
+
+ if (pci_link_toggle_required)
+ return mlx5_pci_link_toggle(dev);
+
+ return 0;
}

static int mlx5_devlink_reload_up(struct devlink *devlink,
--
1.8.3.1


2020-01-15 15:03:10

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next RFC 3/3] net/mlx5: Add FW upgrade reset support

On Tue, 14 Jan 2020 17:55:28 +0200, Moshe Shemesh wrote:
> Add support for FW upgrade reset.
> On devlink reload the driver checks if there is a FW stored pending
> upgrade reset. In such case the driver will set the device to FW upgrade
> reset on next PCI link toggle and do link toggle after unload.
>
> To do PCI link toggle, the driver ensures that no other device ID under
> the same bridge by checking that all the PF functions under the same PCI
> bridge have same device ID. If no other device it uses PCI bridge link
> control to turn link down and up.
>
> Signed-off-by: Moshe Shemesh <[email protected]>

I'd have a slight preference for the reset to be an explicit command
rather than something the driver does automatically on the reload if
there are pending changes. Won't there ever be scenarios where users
just want to hard reset the device for their own reason?

If multiple devices under one bridge are a real concern (or otherwise
interdependencies) would it make sense to mark the devices as "reload
pending" and perform the reloads once all devices in the group has this
mark set?

2020-01-16 14:54:06

by Moshe Shemesh

[permalink] [raw]
Subject: Re: [PATCH net-next RFC 3/3] net/mlx5: Add FW upgrade reset support


On 1/15/2020 5:01 PM, Jakub Kicinski wrote:
> On Tue, 14 Jan 2020 17:55:28 +0200, Moshe Shemesh wrote:
>> Add support for FW upgrade reset.
>> On devlink reload the driver checks if there is a FW stored pending
>> upgrade reset. In such case the driver will set the device to FW upgrade
>> reset on next PCI link toggle and do link toggle after unload.
>>
>> To do PCI link toggle, the driver ensures that no other device ID under
>> the same bridge by checking that all the PF functions under the same PCI
>> bridge have same device ID. If no other device it uses PCI bridge link
>> control to turn link down and up.
>>
>> Signed-off-by: Moshe Shemesh <[email protected]>
> I'd have a slight preference for the reset to be an explicit command
> rather than something the driver does automatically on the reload if
> there are pending changes. Won't there ever be scenarios where users
> just want to hard reset the device for their own reason?


Good point, I will take it with Jiri, may add a new devlink dev command
for that.

>
> If multiple devices under one bridge are a real concern (or otherwise
> interdependencies) would it make sense to mark the devices as "reload
> pending" and perform the reloads once all devices in the group has this
> mark set?

All mlx5 current devices support PCI - Express only.

PCI-Express device should have its own PCI-Express bridge, it is 1x1
connection.

So the check here is just to verify, all functions found under the
bridge are expected to be the same device functions (PFs and VFs).

2020-01-17 03:55:24

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next RFC 3/3] net/mlx5: Add FW upgrade reset support

On Thu, 16 Jan 2020 14:52:35 +0000, Moshe Shemesh wrote:
> > If multiple devices under one bridge are a real concern (or otherwise
> > interdependencies) would it make sense to mark the devices as "reload
> > pending" and perform the reloads once all devices in the group has this
> > mark set?
>
> All mlx5 current devices support PCI - Express only.
>
> PCI-Express device should have its own PCI-Express bridge, it is 1x1
> connection.
>
> So the check here is just to verify, all functions found under the
> bridge are expected to be the same device functions (PFs and VFs).

Ah, good, I couldn't confirm that PCIe fact with google fast enough :)
The check sounds good then, with perhaps a small suggestion to add
a helper in PCIe core if it's already done by two drivers? Can be as
a follow up..

2020-01-18 20:00:58

by Moshe Shemesh

[permalink] [raw]
Subject: Re: [PATCH net-next RFC 3/3] net/mlx5: Add FW upgrade reset support


On 1/17/2020 3:26 AM, Jakub Kicinski wrote:
> On Thu, 16 Jan 2020 14:52:35 +0000, Moshe Shemesh wrote:
>>> If multiple devices under one bridge are a real concern (or otherwise
>>> interdependencies) would it make sense to mark the devices as "reload
>>> pending" and perform the reloads once all devices in the group has this
>>> mark set?
>> All mlx5 current devices support PCI - Express only.
>>
>> PCI-Express device should have its own PCI-Express bridge, it is 1x1
>> connection.
>>
>> So the check here is just to verify, all functions found under the
>> bridge are expected to be the same device functions (PFs and VFs).
> Ah, good, I couldn't confirm that PCIe fact with google fast enough :)
> The check sounds good then, with perhaps a small suggestion to add
> a helper in PCIe core if it's already done by two drivers? Can be as
> a follow up..

OK, we will follow up.

We are preparing larger series for FW upgrade reset that relies on this RFC.

Such shared function should go to pcie subsystem, we will follow up with
it for both drivers once the full mlx5 solution is upstream.