linux-kernel - Re: [PATCHv3] block: Add support for Sony SxS cards

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] [day] [month] [year] [list]

Date:	Sat, 27 Sep 2014 20:31:03 -0600
From:	Jens Axboe <axboe@...nel.dk>
To:	Kieran Kunhya <kieran@...hya.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Joe Perches <joe@...ches.com>,
	Mauro Carvalho Chehab <m.chehab@...sung.com>,
	Antti Palosaari <crope@....fi>,
	Jingoo Han <jg1.han@...sung.com>,
	Ramprasad Chinthekindi <rchinthekindi@...c-inc.com>,
	Pekka Enberg <penberg@...nel.org>,
	Minchan Kim <minchan@...nel.org>,
	Fabian Frederick <fabf@...net.be>,
	Akhil Bhansali <abhansali@...c-inc.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Alan <gnomes@...rguk.ukuu.org.uk>
CC:	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv3] block: Add support for Sony SxS cards

On 2014-09-27 17:43, Kieran Kunhya wrote:
> +static void sxs_memcpy_read(struct sxs_device *dev, unsigned long sector,
> +			    unsigned long nsect, char *buffer)
> +{
> +	struct pci_dev *pdev = dev->pci_dev;
> +	u32 status;
> +	u32 data[4];
> +	u16 *tmp;
> +	u32 *tmp2;
> +
> +	void *dma2;
> +	dma_addr_t dma2_handle;
> +	void *dma3;
> +	dma_addr_t dma3_handle;
> +
> +	sector >>= dev->sector_shift;
> +	nsect >>= dev->sector_shift;
> +
> +	/* Read */
> +	dma2 = pci_alloc_consistent(pdev, 8192, &dma2_handle);
> +	dma3 = pci_alloc_consistent(pdev, 8192, &dma3_handle);
> +
> +	tmp = dma2;
> +	tmp2 = dma3;
> +	tmp2[0] = dma2_handle;
> +	tmp2[2] = dma3_handle;
> +
> +	dev_dbg_ratelimited(&pdev->dev, "CALL %lu %lu\n",
> +			    sector & 0xffffffff, nsect & 0xffffffff);
> +
> +	reinit_completion(&dev->irq_response);
> +	status = readl(dev->mmio+SXS_STATUS_REG);
> +	data[0] = cpu_to_le32(0x00010028);
> +	data[1] = cpu_to_le32(sector & 0xffffffff);
> +	data[2] = 0x0;
> +	data[3] = cpu_to_le32(nsect & 0xffffffff);
> +	memcpy_toio(dev->mmio, data, sizeof(data));
> +	writel(0xa0, dev->mmio+SXS_ENABLE_REG);
> +	writel(0x80, dev->mmio+SXS_CONTROL_REG);
> +
> +	if (!wait_for_completion_timeout(&dev->irq_response,
> +					 msecs_to_jiffies(5000))) {
> +		dev_dbg(&pdev->dev, "No IRQ\n");
> +	}
> +
> +	reinit_completion(&dev->irq_response);
> +	writel(dma3_handle, dev->mmio+SXS_MASTER_LINK_REG_L);
> +	writel(0x0, dev->mmio+SXS_MASTER_LINK_REG_H);
> +	writel(0x20, dev->mmio+SXS_CONTROL_REG);
> +
> +	if (!wait_for_completion_timeout(&dev->irq_response,
> +					 msecs_to_jiffies(5000))) {
> +		dev_dbg(&pdev->dev, "No IRQ\n");
> +	}
> +
> +	/* FIXME: Use DMA properly */
> +	memcpy(buffer, dma2, dev->sector_size * nsect);
> +
> +	dev_dbg_ratelimited(&pdev->dev, "boot-signature %x\n",
> +			    tmp[255]);
> +
> +	writel(0, dev->mmio+SXS_ENABLE_REG);
> +
> +	pci_free_consistent(pdev, 8192, dma3, dma3_handle);
> +	pci_free_consistent(pdev, 8192, dma2, dma2_handle);
> +}
> +
> +static void sxs_request(struct request_queue *q, struct bio *bio)
> +{
> +	struct bvec_iter iter;
> +	struct bio_vec bvec;
> +	char *buffer;
> +	unsigned long flags;
> +	struct sxs_device *dev = q->queuedata;
> +	struct pci_dev *pdev = dev->pci_dev;
> +	sector_t sector = bio->bi_iter.bi_sector;
> +
> +	bio_for_each_segment(bvec, bio, iter) {
> +		dev_dbg_ratelimited(&pdev->dev, "REQUEST %i %i %i\n",
> +				    bio_cur_bytes(bio), bio->bi_vcnt,
> +				    bvec.bv_len);
> +		buffer = bvec_kmap_irq(&bvec, &flags);
> +		sxs_memcpy_read(dev, sector, bio_cur_bytes(bio) >> 9,
> +				buffer);
> +		sector += bio_cur_bytes(bio) >> 9;
> +		bvec_kunmap_irq(buffer, &flags);
> +	}
> +
> +	bio_endio(bio, 0);

So a few questions here... This device only does reads? And it seems to 
be assuming that only reads end up in the request handler? How so?

Second question. IO never fails? There's no status checking and IO is 
always ended successfully. This too seems odd.

Third, this wong work as-is, at least not of HIGHMEM is used. After the 
bvec_kmap_irq(), irqs will be disabled. But your sxs_memcpy_read() 
invokes schedule through the completion waits, that will instantly go bad.

-- 
Jens Axboe

--
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/