Skip to contentSkip to conversation

ata: libata-core: Ignore forced transfer modes the host does not support

PATCH Applied Rand Deeb 3 participants 4 messages Oct 15 – Oct 22 libata

lore
Share & tools
lore

Rand Deeb

author
#0 lore raw

PATCH v2 1/1 ata: libata-core: Ignore forced transfer modes the host does not support

The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4, udma0-7) for a port or device. ata_force_xfermask() applies it in ata_do_set_mode() right after ata_dev_xfermask() has limited the device masks to what the host supports. It replaces those masks with the forced mode without checking the host masks, so a mode the controller does not support is passed to the driver's ->set_piomode() or ->set_dmamode().

Several PATA drivers rely on the host masks to keep the mode number in range when indexing their timing tables. ata_piix, pata_efar, pata_it8213, pata_oldpiix, pata_radisys and pata_rdc declare ATA_PIO4 and ATA_MWDMA12_ONLY, index a five-entry PIO timing table with pio_mode - XFER_PIO_0, and index a three-entry needed_pio[] table with dma_mode - XFER_MW_DMA_0. Booting with libata.force=pio5, pio6, mwdma3 or mwdma4 makes them read past those tables, use the values read as further indices into the timing table, and write the result to the controller's timing registers.

Make ata_force_xfermask() skip, with a warning, a forced transfer mode that is not in the port's PIO, MWDMA or UDMA mask, and keep looking for an earlier matching libata.force entry, as it already does for entries that do not apply to the device. With libata.force=pio4,1.0:pio5 on a PIO4 host, device 1.0 now gets pio4 instead of pio5. Forcing a mode the host supports still works as before, including a mode that ata_dev_xfermask() removed for the device or cable.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes 332673257056 ("libata: implement libata.force module parameter")

---

Notes:
v2:
- Skip the unsupported entry and keep searching instead of returning, so an earlier matching entry still applies, e.g. libata.force=pio4,1.0:pio5 (Sashiko AI review).
- Rewrite the check and the warning as suggested by Damien.
- Update the commit message to match.
v1: https://lore.kernel.org/r/[email protected]

 drivers/ata/libata-core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/ata/libata-core.c +12 -0
index 92233fb25..578071eea 100644
@@ -434,6 +434,7 @@ static void ata_force_link_limits(struct ata_link *link)
  */
 static void ata_force_xfermask(struct ata_device *dev)
 {
+	struct ata_port *ap = dev->link->ap;
 	int devno = dev->link->pmp + dev->devno;
 	int alt_devno = devno;
 	int i;
@@ -458,6 +459,17 @@ static void ata_force_xfermask(struct ata_device *dev)
 
 		ata_unpack_xfermask(fe->param.xfer_mask,
 				    &pio_mask, &mwdma_mask, &udma_mask);
+
+		/* Make sure that the port supports the specified xfer modes. */
+		if (!(pio_mask & ap->pio_mask) &&
+		    !(mwdma_mask & ap->mwdma_mask) &&
+		    !(udma_mask & ap->udma_mask)) {
+			ata_dev_warn(dev,
+				     "FORCE: ignoring unsupported xfer_mask %s\n",
+				     fe->param.name);
+			continue;
+		}
+
 		if (udma_mask)
 			dev->udma_mask = udma_mask;
 		else if (mwdma_mask) {

base-commit: 045958acaa75205b31578cb22c4345e28dd2e4bf

Signature · 1 line
2.39.5
maintainer Applied
#1 lore raw
Quoted 8 lines · Rand Deeb

The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4, udma0-7) for a port or device. ata_force_xfermask() applies it in ata_do_set_mode() right after ata_dev_xfermask() has limited the device masks to what the host supports. It replaces those masks with the forced mode without checking the host masks, so a mode the controller does not support is passed to the driver's ->set_piomode() or ->set_dmamode().

[...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: libata-core: Ignore forced transfer modes the host does not support

      https://git.kernel.org/libata/linux/c/abbb03d0

Kind regards,
Niklas

maintainer Reviewed-by on v2 1/1
#2 lore raw
In reply to Rand Deeb
Quoted 25 lines · Rand Deeb

The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4, udma0-7) for a port or device. ata_force_xfermask() applies it in ata_do_set_mode() right after ata_dev_xfermask() has limited the device masks to what the host supports. It replaces those masks with the forced mode without checking the host masks, so a mode the controller does not support is passed to the driver's ->set_piomode() or ->set_dmamode().

Several PATA drivers rely on the host masks to keep the mode number in range when indexing their timing tables. ata_piix, pata_efar, pata_it8213, pata_oldpiix, pata_radisys and pata_rdc declare ATA_PIO4 and ATA_MWDMA12_ONLY, index a five-entry PIO timing table with pio_mode - XFER_PIO_0, and index a three-entry needed_pio[] table with dma_mode - XFER_MW_DMA_0. Booting with libata.force=pio5, pio6, mwdma3 or mwdma4 makes them read past those tables, use the values read as further indices into the timing table, and write the result to the controller's timing registers.

Make ata_force_xfermask() skip, with a warning, a forced transfer mode that is not in the port's PIO, MWDMA or UDMA mask, and keep looking for an earlier matching libata.force entry, as it already does for entries that do not apply to the device. With libata.force=pio4,1.0:pio5 on a PIO4 host, device 1.0 now gets pio4 instead of pio5. Forcing a mode the host supports still works as before, including a mode that ata_dev_xfermask() removed for the device or cable.

Quoted 4 lines

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes 332673257056 ("libata: implement libata.force module parameter")

Looks good.

Signature · 2 lines
Damien Le Moal
Western Digital Research
maintainer
#3 lore raw
Quoted 26 lines · Damien Le Moal
Quoted 25 lines · Rand Deeb

The libata.force parameter can force a transfer mode (pio0-6, mwdma0-4, udma0-7) for a port or device. ata_force_xfermask() applies it in ata_do_set_mode() right after ata_dev_xfermask() has limited the device masks to what the host supports. It replaces those masks with the forced mode without checking the host masks, so a mode the controller does not support is passed to the driver's ->set_piomode() or ->set_dmamode().

Several PATA drivers rely on the host masks to keep the mode number in range when indexing their timing tables. ata_piix, pata_efar, pata_it8213, pata_oldpiix, pata_radisys and pata_rdc declare ATA_PIO4 and ATA_MWDMA12_ONLY, index a five-entry PIO timing table with pio_mode - XFER_PIO_0, and index a three-entry needed_pio[] table with dma_mode - XFER_MW_DMA_0. Booting with libata.force=pio5, pio6, mwdma3 or mwdma4 makes them read past those tables, use the values read as further indices into the timing table, and write the result to the controller's timing registers.

Make ata_force_xfermask() skip, with a warning, a forced transfer mode that is not in the port's PIO, MWDMA or UDMA mask, and keep looking for an earlier matching libata.force entry, as it already does for entries that do not apply to the device. With libata.force=pio4,1.0:pio5 on a PIO4 host, device 1.0 now gets pio4 instead of pio5. Forcing a mode the host supports still works as before, including a mode that ata_dev_xfermask() removed for the device or cable.

Quoted 8 lines
Quoted 4 lines

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes 332673257056 ("libata: implement libata.force module parameter")

Looks good.

Repushed commit to pick up Damien's tag.

Code unchanged.

Kind regards,
Niklas