linux-kernel - Re: [PATCH] SCSI driver for VMware's virtual HBA

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:	Tue, 08 Sep 2009 18:07:45 -0700
From:	Alok Kataria <akataria@...are.com>
To:	Brian King <brking@...ux.vnet.ibm.com>
Cc:	James Bottomley <James.Bottomley@...e.de>,
	Rolf Eike Beer <eike-kernel@...tec.de>,
	Matthew Wilcox <matthew@....cx>,
	Roland Dreier <rdreier@...co.com>,
	Bart Van Assche <bvanassche@....org>,
	Robert Love <robert.w.love@...el.com>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	Mike Christie <michaelc@...wisc.edu>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"pv-drivers@...are.com" <pv-drivers@...are.com>,
	"Chetan.Loke@...lex.Com" <Chetan.Loke@...lex.Com>
Subject: Re: [PATCH] SCSI driver for VMware's virtual HBA - V3.

Hi all,

Thanks for taking another look. I have addressed all of Eike and Bryan's
comments except the host_lock one, see below for details. 
Also I have rolled out a v4 patch with all the changes, which I will
send out separately. 

On Thu, 2009-09-03 at 14:52 -0700, Brian King wrote:
> Alok Kataria wrote:
> > +
> > +struct pvscsi_adapter {
> > +	char			*mmioBase;
> > +	unsigned int		irq;
> > +	u8			rev;
> > +	bool			use_msi;
> > +	bool			use_msix;
> > +	bool			use_msg;
> > +
> > +	spinlock_t		hw_lock;
> 
> Why not just use host_lock in the scsi_host structure?
> 

I agree with Chetan here, lets keep these locks separate. 

> > +
> > +/*
> > + * Map all data buffers for a command into PCI space and
> > + * setup the scatter/gather list if needed.
> > + */
> > +static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
> > +			       struct pvscsi_ctx *ctx,
> > +			       struct scsi_cmnd *cmd, PVSCSIRingReqDesc *e)
> > +{
> > +	unsigned count;
> > +	unsigned bufflen = scsi_bufflen(cmd);
> > +	struct scatterlist *sg;
> > +
> > +	e->dataLen = bufflen;
> > +	e->dataAddr = 0;
> > +	if (bufflen == 0)
> > +		return;
> > +
> > +	sg = scsi_sglist(cmd);
> > +	count = scsi_sg_count(cmd);
> > +	if (count != 0) {
> > +		int segs = pci_map_sg(adapter->dev, sg, count,
> > +				      cmd->sc_data_direction);
> 
> Should use scsi_dma_map here instead.

Done. 
> > +		if (segs > 1) {
> > +			pvscsi_create_sg(ctx, sg, segs);
> > +
> > +			e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST;
> > +			ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl,
> > +						    SGL_SIZE, PCI_DMA_TODEVICE);
> > +			e->dataAddr = ctx->sglPA;
> > +		} else
> > +			e->dataAddr = sg_dma_address(sg);
> > +	} else {
> > +		/*
> > +		 * In case there is no S/G list, scsi_sglist points
> > +		 * directly to the buffer.
> > +		 */
> > +		ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen,
> > +					     cmd->sc_data_direction);
> > +		e->dataAddr = ctx->dataPA;
> > +	}
> > +}
> > +
> > +static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter,
> > +				 struct pvscsi_ctx *ctx)
> > +{
> > +	struct scsi_cmnd *cmd;
> > +	unsigned bufflen;
> > +
> > +	cmd = ctx->cmd;
> > +	bufflen = scsi_bufflen(cmd);
> > +
> > +	if (bufflen != 0) {
> > +		unsigned count = scsi_sg_count(cmd);
> > +
> > +		if (count != 0) {
> > +			pci_unmap_sg(adapter->dev, scsi_sglist(cmd), count,
> > +				     cmd->sc_data_direction);
> 
> Use scsi_dma_unmap here instead.
Done.
> 
> > +			if (ctx->sglPA) {
> > +				pci_unmap_single(adapter->dev, ctx->sglPA,
> > +						 SGL_SIZE, PCI_DMA_TODEVICE);
> > +				ctx->sglPA = 0;
> > +			}
> > +		} else
> > +			pci_unmap_single(adapter->dev, ctx->dataPA, bufflen,
> > +					 cmd->sc_data_direction);
> > +	}
> > +	if (cmd->sense_buffer)
> > +		pci_unmap_single(adapter->dev, ctx->sensePA,
> > +				 SCSI_SENSE_BUFFERSIZE, PCI_DMA_FROMDEVICE);
> > +}
> > +
> 
> 

Thanks,
Alok

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