On 10/15/24 3:43 AM, Simon Horman wrote:
On Mon, Oct 14, 2024 at 12:18:36PM +0100, Vadim Fedorenko wrote:
On 12/10/2024 03:34, Lee Trager wrote:
fbnic supports updating firmware using a PLDM image signed and distributed
by Meta. PLDM images are written into stored flashed. Flashing does not
interrupt operation.
On host reboot the newly flashed UEFI driver will be used. To run new
control or cmrt firmware the NIC must be power cycled.
Signed-off-by: Lee Trager <lee@xxxxxxxxx>
...
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_devlink.c b/drivers/net/ethernet/meta/fbnic/fbnic_devlink.c
...
+/**
+ * fbnic_send_component_table - Send PLDM component table to the firmware
+ * @context: PLDM FW update structure
+ * @component: The component to send
+ * @transfer_flag: Flag indication location in component tables
+ *
+ * Read relevant data from component table and forward it to the firmware.
+ * Check response to verify if the firmware indicates that it wishes to
+ * proceed with the update.
+ *
+ * Return: zero on success
+ * negative error code on failure
+ */
+static int fbnic_send_component_table(struct pldmfw *context,
+ struct pldmfw_component *component,
+ u8 transfer_flag)
+{
+ struct device *dev = context->dev;
+ u16 id = component->identifier;
+ u8 test_string[80];
+
+ switch (id) {
+ case QSPI_SECTION_CMRT:
+ case QSPI_SECTION_CONTROL_FW:
+ case QSPI_SECTION_OPTION_ROM:
+ break;
+ default:
+ dev_err(dev, "Unknown component ID %u\n", id);
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "Sending PLDM component table to firmware\n");
+
+ /* Temp placeholder */
+ memcpy(test_string, component->version_string,
+ min_t(u8, component->version_len, 79));
+ test_string[min_t(u8, component->version_len, 79)] = 0;
Looks like this construction can be replaced with strscpy().
There were several patchsets in the tree to use strscpy(), let's follow
the pattern.
While looking at these lines, I'm unsure why min_t() is being used
instead of min() here. As version_len is unsigned and 79 is a positive
constant, I believe min() should be fine here.
clang complains if I'm not explicit with the type by using min_t()
/home/ltrager/fbnic/src/fbnic_devlink.c:194:3: warning: comparison of
distinct pointer types ('typeof (component->version_len) *' (aka
'unsigned char *') and 'typeof (79) *' (aka 'int *'))
[-Wcompare-distinct-pointer-types]
194 | min(component->version_len, 79));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/minmax.h:45:19: note: expanded from macro 'min'
45 | #define min(x, y) __careful_cmp(x, y, <)
| ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
36 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~~~~~~~
./include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
26 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~~~~~~~
./include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
1 warning generated.
+ dev_info(dev, "PLDMFW: Component ID: %u version %s\n",
+ id, test_string);
+
+ return 0;
+}
