| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200709032050.28121.bzolnier@gmail.com>
Date: Mon, 3 Sep 2007 20:50:27 +0200
From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To: Sergei Shtylyov <sshtylyov@...mvista.com>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] ide: make ide_rate_filter() also respect PIO and SW/MW DMA mode masks (take 2)
Hi,
On Sunday 02 September 2007, Sergei Shtylyov wrote:
> Make ide_rate_filter() also respect PIO/SWDMA/MWDMA mode masks. While at it,
Hm, this seems to be already dealt with by:
http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/patches/ide-mode-limiting-fixes-for-user-requested-speed-changes.patch
after applying the above patch ide_rate_filter() looks like this:
/**
* ide_rate_filter - filter transfer mode
* @drive: IDE device
* @speed: desired speed
*
* Given the available transfer modes this function returns
* the best available speed at or below the speed requested.
*
* TODO: check device PIO capabilities
*/
u8 ide_rate_filter(ide_drive_t *drive, u8 speed)
{
ide_hwif_t *hwif = drive->hwif;
u8 mode = ide_find_dma_mode(drive, speed);
if (mode == 0) {
if (hwif->pio_mask)
mode = fls(hwif->pio_mask) - 1 + XFER_PIO_0;
else
mode = XFER_PIO_4;
}
// printk("%s: mode 0x%02x, speed 0x%02x\n", __FUNCTION__, mode, speed);
return min(speed, mode);
}
What is still missing from IDE tree is "respecting device PIO limits" (sorry
I was quoting from memory it is TODO not FIXME [1]) which probably comes down
to adding ide_get_best_pio_mode() call to ide_rate_filter() (IMO this should
be done before ide-mode-limiting-fixes-for-user-requested-speed-changes.patch
changes).
[1] http://www.mail-archive.com/linux-ide@vger.kernel.org/msg09750.html
Ah, I see the source of the confusion now, I misunderstood your remark about
->mwdma_mask being ignored (for some reason I though that you were referring
to MWDMA mode not being tried if device refuses SET_XFER command w/ UDMA).
ide_find_dma_mode() is a bit tricky because it used also by ide_max_dma_mode()
but it should take care of ->mwdma_mask if ->udma_mask is zero just fine.
> make the udma_filter() method calls take precedence over using the mode masks.
This change is OK - could you make a new patch?
> Signed-off-by: Sergei Shtylyov <sshtylyov@...mvista.com>
>
> ---
> Alas, the previous version of the patch wasn't even worth signing it off --
> the code had to be much trickier. The patch against the current Linus' tree and
take #1 was correct - i.e. if UDMA0 is not available we want to go down
to MWDMA2 and not MWDMA0 (this is what the old code was doing BTW)
> unfortunately I was able to only compile test it since that tree gives MODPOST
> warning and dies early on bootup. I'm hoping to give it more tesring when/if
> I have time... :-)
>
> drivers/ide/ide-dma.c | 7 +++---
> drivers/ide/ide-lib.c | 57 ++++++++++++++++++++++++++++++--------------------
> 2 files changed, 39 insertions(+), 25 deletions(-)
>
> Index: linux-2.6/drivers/ide/ide-dma.c
> ===================================================================
> --- linux-2.6.orig/drivers/ide/ide-dma.c
> +++ linux-2.6/drivers/ide/ide-dma.c
> @@ -664,10 +664,11 @@ static unsigned int ide_get_mode_mask(id
> if ((id->field_valid & 4) == 0)
> break;
>
> - mask = id->dma_ultra & hwif->ultra_mask;
> -
> if (hwif->udma_filter)
> - mask &= hwif->udma_filter(drive);
> + mask = hwif->udma_filter(drive);
> + else
> + mask = hwif->ultra_mask;
> + mask &= id->dma_ultra;
>
> if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
> mask &= 0x07;
> Index: linux-2.6/drivers/ide/ide-lib.c
> ===================================================================
> --- linux-2.6.orig/drivers/ide/ide-lib.c
> +++ linux-2.6/drivers/ide/ide-lib.c
> @@ -75,38 +75,51 @@ EXPORT_SYMBOL(ide_xfer_verbose);
> *
> * Given the available transfer modes this function returns
> * the best available speed at or below the speed requested.
> - *
> - * FIXME: filter also PIO/SWDMA/MWDMA modes
> */
>
> u8 ide_rate_filter(ide_drive_t *drive, u8 speed)
> {
> -#ifdef CONFIG_BLK_DEV_IDEDMA
> ide_hwif_t *hwif = drive->hwif;
> - u8 mask = hwif->ultra_mask, mode = XFER_MW_DMA_2;
> + u8 mask, base = speed & 0xf8, bit = 1 << (speed & 0x07);
>
> - if (hwif->udma_filter)
> - mask = hwif->udma_filter(drive);
> + switch (base) {
> +#ifdef CONFIG_BLK_DEV_IDEDMA
> + case XFER_UDMA_0:
> + mask = hwif->udma_filter ? hwif->udma_filter(drive) :
> + hwif->ultra_mask;
>
> - /*
> - * TODO: speed > XFER_UDMA_2 extra check is needed to avoid false
> - * cable warning from eighty_ninty_three(), moving ide_rate_filter()
> - * calls from ->speedproc to core code will make this hack go away
> - */
> - if (speed > XFER_UDMA_2) {
> - if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
> + /*
> + * TODO: if speed > XFER_UDMA_2, an extra check is needed to
> + * avoid a false cable warning from eighty_ninty_three() --
> + * moving ide_rate_filter() calls from ->speedproc to core
> + * code will make this hack go away...
> + */
> + if (speed > XFER_UDMA_2 && (mask & 0x78) &&
> + eighty_ninty_three(drive) == 0)
> mask &= 0x07;
> - }
> -
> - if (mask)
> - mode = fls(mask) - 1 + XFER_UDMA_0;
>
> -// printk("%s: mode 0x%02x, speed 0x%02x\n", __FUNCTION__, mode, speed);
> -
> - return min(speed, mode);
> -#else /* !CONFIG_BLK_DEV_IDEDMA */
> - return min(speed, (u8)XFER_PIO_4);
> + if (mask & bit)
> + break;
> + base = XFER_MW_DMA_0;
> + /* Fall thru */
> + case XFER_MW_DMA_0:
> + if (hwif->mwdma_mask & bit)
> + break;
> + base = XFER_SW_DMA_0;
> + /* Fall thru */
> + case XFER_SW_DMA_0:
> + if (hwif->swdma_mask & bit)
> + break;
> + base = XFER_PIO_0;
> + /* Fall thru */
> #endif /* CONFIG_BLK_DEV_IDEDMA */
> + case XFER_PIO_0:
> + if (hwif->pio_mask & bit)
> + break;
> + default:
> + BUG();
> + }
> + return base | bit;
> }
>
> EXPORT_SYMBOL(ide_rate_filter);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists