netdev - RE: [PATCH net-next 4/8] qlcnic: Add support for PEX DMA method to read memory section of adapter dump

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]

Message-ID: <AE90C24D6B3A694183C094C60CF0A2F6026B72A0@saturn3.aculab.com>
Date:	Mon, 24 Jun 2013 09:56:06 +0100
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Jitendra Kalsaria" <jitendra.kalsaria@...gic.com>,
	<davem@...emloft.net>
Cc:	<netdev@...r.kernel.org>, <sony.chacko@...gic.com>,
	<shahed.shaikh@...gic.com>, <Dept_NX_Linux_NIC_Driver@...gic.com>
Subject: RE: [PATCH net-next 4/8] qlcnic: Add support for PEX DMA method to read memory section of adapter dump

> From: Shahed Shaikh <shahed.shaikh@...gic.com>
> 
> This patch adds support to read memory section of adapter
> dump using PEX DMA method. This method significantly improves
> total adapter dump collection time.

Which DMA engine is this using?
My first thought was that it was the one in the CSB bridge
(which ought to have a shared interface), but the structures
don't seem to match the code I have for that.

...
> +struct qlcnic_pex_dma_descriptor {
> +	u32	read_data_size;
> +	u32	dma_desc_cmd;
> +	u32	src_addr_low;
> +	u32	src_addr_high;
> +	u32	dma_bus_addr_low;
> +	u32	dma_bus_addr_high;
> +	u32	rsvd[6];
> +} __packed;

Why __packed??
Why are the structures following also marked __packed.

...
> +	/* Wait for DMA to complete */
> +	temp_addr = dma_base_addr + QLC_DMA_CMD_STATUS_CTRL;
> +	for (i = 0; i < 400; i++) {
> +		dma_sts = qlcnic_ind_rd(adapter, temp_addr);
> +
> +		if (dma_sts & BIT_1)
> +			usleep_range(250, 500);
> +		else
> +			break;
> +	}

Do these DMA actually take long enough to need to sleep
for completion?
I'd have thought that they'd normally finish almost immediately.
Sleeping might be reasonable as a 'last hope' check.

I'd also consider putting the 'wait for complete' code into
a separate function - allowing overlapped operations.

...
> +
> +	if ((tmpl_hdr->version & 0xffffff) >= 0x20001)
> +		ahw->fw_dump.use_pex_dma = true;
> +	else
> +		ahw->fw_dump.use_pex_dma = false;

You know what's wrong with the above....

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html