Re: [PATCH v3 3/9] vfio: selftests: igb: Use advanced TX and RX descriptors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Programming `SRRCTL` with `E1000_SRRCTL_DESCTYPE_ADV_ONEBUF` overwrites the register, clearing the `BSIZEPACKET` (Receive Buffer Size) field to 0, which violates the 82576 datasheet.
- [Medium] The commit message incorrectly claims to add new header macros and remove legacy CMD macros, but the patch contains no such header modifications.
--

--- Patch [3]: [PATCH v3 3/9] vfio: selftests: igb: Use advanced TX and RX descriptors ---
commit 44f58d3e0349aa95894e8741ae9ff09ee319dcae
Author: Alex Williamson <alex.williamson@nvidia.com>

vfio: selftests: igb: Use advanced TX and RX descriptors

The submitted driver builds a partial legacy TX descriptor (just
DTALEN | CMD_EOP) and never programs SRRCTL.DESCTYPE.  QEMU's emulated
igb tolerates this by treating descriptors as advanced regardless of
DESCTYPE, but real 82576 hardware does not.

[ ... ]

>     Remove the legacy CMD macros (IGB_TXD_CMD_EOP, IGB_TXD_CMD_IFCS,
>     IGB_TXD_CMD_RS, IGB_TXD_CMD_SHIFT, IGB_TXD_CMD_LEGACY_FORMAT) that
>     become unused, and add the SRRCTL register offset, DESCTYPE encoding,
>     and advanced TX descriptor field macros.

[Severity: Medium]
Does the patch actually include these macro changes?

The commit message claims to remove legacy CMD macros and add new ones, but
there are no header modifications in the diff.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> index 58c6aceea63a9..f4f6d2ee262da 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> @@ -235,6 +235,17 @@ static void igb_init(struct vfio_pci_device *device)
>  	igb_write32(igb, E1000_RDLEN(0), RING_SIZE * sizeof(struct igb_rx_desc));
>  	igb_write32(igb, E1000_RDH(0), 0);
>  	igb_write32(igb, E1000_RDT(0), 0);
> +
> +	/*
> +	 * Select the advanced one-buffer descriptor format.  Per 82576
> +	 * datasheet section 7.1.5.2: "SRRCTL[n].DESCTYPE must be set to a
> +	 * value other than 000b for the 82576 to write back the special
> +	 * descriptors."  struct igb_rx_desc matches the advanced one-buffer
> +	 * writeback layout (section 7.1.5.2), so polling rx.wb.status_error
> +	 * requires this format.  Section 8.10.2 specifies DESCTYPE[27:25].
> +	 */
> +	igb_write32(igb, E1000_SRRCTL(0), E1000_SRRCTL_DESCTYPE_ADV_ONEBUF);

[Severity: Medium]
Will this overwrite the SRRCTL register and clear the BSIZEPACKET field to 0?

By clearing BSIZEPACKET (bits 6:0 of SRRCTL), real 82576 hardware may see a
0-byte receive buffer and truncate or drop received packets, causing the test
to fail on hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-igb_v3_b4-v3-0-1272650ca614@google.com?part=3