linux-kernel - Re: [PATCH] sata_sil24: Use memory barriers before issuing commands

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Jun 2010 19:32:22 -0700
From:	Grant Grundler <grundler@...gle.com>
To:	Catalin Marinas <catalin.marinas@....com>
Cc:	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>,
	Colin Tuckley <colin.tuckley@....com>
Subject: Re: [PATCH] sata_sil24: Use memory barriers before issuing commands

On Thu, Jun 10, 2010 at 7:57 AM, Catalin Marinas
<catalin.marinas@....com> wrote:
> The data in the cmd_block buffers may reach the main memory after the
> writel() to the device ports.

"ia-64 Linux Kernel" (mosberger and eranian) uses exactly this sequence
as an example for wmb() on page 303.

I'm curious about the system that exposed this problem. I believe wmb() fixes
 an issue not exposed on most machines. Can any general comments be
made about cache coherency, memory ordering (weak?), instruction ordering
 (super scalar?), etc. ?

The explanation above is a bit short (most people won't understand it).


> This patch introduces two calls to wmb() to ensure the relative ordering.

And as Tejun asked, the comment where wmb() gets used should clearly
explain which host memory writes are targetted by the wmb().

thanks,
grant

>
> Signed-off-by: Catalin Marinas <catalin.marinas@....com>
> Tested-by: Colin Tuckley <colin.tuckley@....com>
> Cc: Tejun Heo <tj@...nel.org>
> ---
>  drivers/ata/sata_sil24.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index e925051..6392fdb 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -622,6 +622,7 @@ static int sil24_exec_polled_cmd(struct ata_port *ap, int pmp,
>        irq_enabled = readl(port + PORT_IRQ_ENABLE_SET);
>        writel(PORT_IRQ_COMPLETE | PORT_IRQ_ERROR, port + PORT_IRQ_ENABLE_CLR);
>
> +       wmb();
>        writel((u32)paddr, port + PORT_CMD_ACTIVATE);
>        writel((u64)paddr >> 32, port + PORT_CMD_ACTIVATE + 4);
>
> @@ -895,6 +896,7 @@ static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc)
>        paddr = pp->cmd_block_dma + tag * sizeof(*pp->cmd_block);
>        activate = port + PORT_CMD_ACTIVATE + tag * 8;
>
> +       wmb();
>        writel((u32)paddr, activate);
>        writel((u64)paddr >> 32, activate + 4);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
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/