Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 02:06:11PM +0200, Johannes Thumshirn wrote: > On Wed, Aug 01, 2018 at 08:00:44PM +0800, Ming Lei wrote: > > On Wed, Aug 01, 2018 at 07:52:19PM +0800, Ming Lei wrote: > > > On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > > > > On 1 August 2018 at 11:59, Mark Brown wrote: > > > > > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > > > > > > > >> You may have to provide some clue, such as dmesg log, boot disk, ... > > > > > > > > > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > > > > >> mode at default, even though without d5038a13eca72fb. > > > > > > > > > > Boot logs and so on can be found here: > > > > > > > > > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > > > > > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > > > > > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > > > > > > > > > (these are today's but the symptoms are the same.) The ramdisk is > > > > > unfortunately not linked through the UI, though we don't get that far. > > > > > > > > And a full LAVA boot log from my lab > > > > http://lava.streamtester.net/scheduler/job/138067 > > > > > > > > QEMU command line here: > > > > http://lava.streamtester.net/scheduler/job/138067#L75 > > > > > > > > And a LAVA job definition, which includes the url of the ramdisk and > > > > kernel: > > > > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > > > > > > > > > > Thanks for the sharing! > > > > > > I can reproduce this issue with above script/initrd/kernel config, and > > > looks > > > the issue disappeared after 'scsi_mod.use_blk_mq=0' is passed. > > > > > > Not see such issue with zero-day ktest config. > > > > > > Looks a bit weird, given SCSI_MQ is nothing related with ramdisk. > > > > Seems related with sr: > > > > 1) with scsi-mq > > [2.808204] ata2.01: NODEV after polling detection > > [2.809807] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 > > [2.827377] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM > > 2.5+ PQ: 0 ANSI: 5 > > > > 2) without scsi_mq > > [5.549135] ata2.01: NODEV after polling detection > > [5.554404] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 > > [5.596143] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM > > 2.5+ PQ: 0 ANSI: 5 > > [5.637126] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray > > [5.637870] cdrom: Uniform CD-ROM driver Revision: 3.20 > > [5.648940] sr 1:0:0:0: Attached scsi CD-ROM sr0 > > [5.661605] sr 1:0:0:0: Attached scsi generic sg0 type 5 > > > > > > We may need to take a look at recent SCSI change. > > [2.052168] ata2.01: NODEV after polling detection > [2.053690] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 > [2.072269] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ > P5 > [2.107220] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray > [2.107675] cdrom: Uniform CD-ROM driver Revision: 3.20 > [2.111851] sr 1:0:0:0: Attached scsi CD-ROM sr0 > [2.123560] sr 1:0:0:0: Attached scsi generic sg0 type 5 > > # cat /proc/cmdline > console=ttyS0,115200 root=/dev/ram0 debug verbose > > $ grep SCSI_MQ .config > CONFIG_SCSI_MQ_DEFAULT=y > > on Martin's latest 4.19/scsi-queue. I just checked my daily test log, looks this issue is reported 1st time on next-20180731. Thanks, Ming
Re: [PATCH 2/2] Avoid that SCSI device removal through sysfs triggers a deadlock
On Wed, 2018-08-01 at 21:16 +, Bart Van Assche wrote: > During my kernel tests of today I noticed that this patch makes booting > significantly slower: boot time for a VM increases from 6s to 157s. Martin, > please drop this patch series. Please ignore my previous message - these two patches are fine. The trouble was caused by a patch in another tree that I had merged in during my tests. I will repost this series. Bart.
Re: [PATCH 2/2] Avoid that SCSI device removal through sysfs triggers a deadlock
On Mon, 2018-07-30 at 11:40 -0700, Bart Van Assche wrote: > A long time ago the unfortunate decision was taken to add a self- > deletion attribute to the sysfs SCSI device directory. That decision > was unfortunate because self-deletion is really tricky. We can't drop > that attribute because widely used user space software depends on it, > namely the rescan-scsi-bus.sh script. Hence this patch that avoids > that writing into that attribute triggers a deadlock. See also commit > 7973cbd9fbd9 ("[PATCH] add sysfs attributes to scan and delete > scsi_devices"). [ ... ] During my kernel tests of today I noticed that this patch makes booting significantly slower: boot time for a VM increases from 6s to 157s. Martin, please drop this patch series. Thanks, Bart.
Re: [RFC 0/6] scsi: scsi transport for ufs devices
On Mon, 2018-07-30 at 14:52 -0400, Douglas Gilbert wrote: > I designed 'struct sg_io_v4' found in include/uapi/linux/bsg.h to handle > storage > related protocols, not just the SCSI command set. After the guard, the next > two fields in that structure are: > __u32 protocol; /* [i] 0 -> SCSI , */ > __u32 subprotocol; /* [i] 0 -> SCSI command, 1 -> SCSI task > management function, */ > > So there is lots of room for other protocols. Hi Doug, That's great, but it seems like most bsg drivers use other data structures than struct sg_io_v4. See e.g. struct fc_bsg_request and struct fc_bsg_reply in include/uapi/scsi/scsi_bsg_fc.h. See also struct iscsi_bsg_request and struct iscsi_bsg_reply in include/scsi/scsi_bsg_iscsi.h. The output of git grep -nH ' = job->request;' did not reveal any bsg driver that uses struct sg_io_v4. Did I perhaps misunderstand something? Thanks, Bart.
Re: [PATCH 11/15] target: export initiator port values for all sessions
On Wed, 2018-08-01 at 11:44 -0500, Mike Christie wrote: > I agree knowing the acl info is useful, so how about the following: > > 1. Create a file named "acl" in the session's dir. > 2. For dynamic_node_acl=0 the acl file will return a empty string or > "generate_node_acls=1" or "demo mode enabled". > 3. For dynamic_node_acl=1 the acl file will return > se_node_acl->initiatorname which is the name of the acl in > /tpgt_$N/acls/. Hello Mike, That sounds fine to me. My personal preference is that the "acl" file is empty if demo mode is enabled. Thanks, Bart.
Re: [PATCH 11/15] target: export initiator port values for all sessions
On 08/01/2018 11:44 AM, Mike Christie wrote: > 1. Create a file named "acl" in the session's dir. > 2. For dynamic_node_acl=0 the acl file will return a empty string or Miswrote this. Above should be dynamic_node_acl=1 > "generate_node_acls=1" or "demo mode enabled". > 3. For dynamic_node_acl=1 the acl file will return This should be dynamic_node_acl=0 > se_node_acl->initiatorname which is the name of the acl in > /tpgt_$N/acls/. >
Re: [PATCH 11/15] target: export initiator port values for all sessions
On 07/19/2018 12:07 PM, Bart Van Assche wrote: > On Thu, 2018-07-19 at 11:38 -0500, Mike Christie wrote: >> On 07/19/2018 10:37 AM, Bart Van Assche wrote: >>> The general recommendation for configfs is that each attribute contains a >>> single value, just like for sysfs. Patch 11/15 exports two values through >>> a single attribute. Have you considered to split the above into two >> >> What about just making it the initiator port transport id so it aligns >> with the get_initiator_port_transport_id() comment for the other patch. >> For iscsi it would be 1 value with the format from SPC/SAM >> "target_name,i,0x,isid"? > > That sounds fine to me. > >>> attributes, namely the initiator name and the ISID? Can the initiator name >>> be changed into a soft link to the se_node_acl configfs directory to make >>> it easy for shell scripts to retrieve additional initiator configuration >>> information? >> >> Because the kernel is creating the session instead of it being driven >> from a mkdir, there are no existing functions for this. I do not know >> configfs code well, but I think making a function to do this is possible >> though. > > Initially configfs did not support creation of a directory from the kernel > side. Last time I brought this up with Christoph he replied that this > functionality has been added to configfs (if I understood Christoph > correctly). > >> What about the dynamic_acl case? Just make those a normal file? > > I'm not that familiar with dynamic ACLs. Is there a difference between > "dynamic ACL" generation and "demo mode"? How does this interact with the > generate_node_acls mode? Ah sorry, I think I made up that term. I was referring to when se_node_acl->dynamic_node_acl=1, so generate_node_acls=1 and demo mode and dynamic_node_acl=1 is the same state. > >> Just to make sure we are on the same page too. The initiator name is >> always the name of the acl right? It looked like that from >> target_fabric_make_nodeacl but I was wondering if you are asking for the >> symlink because there are some fabric module quirks where that is not >> the case. If it's the same names, then you know the acl already from the >> initiator name file. > > It depends on what is called the "initiator name". E.g. the SRP target > driver supports multiple formats to refer to a single initiator port. The > first version of the ib_srpt driver supported only 128-bit GIDs as initiator > name. Since the 64-bit prefix of a GID may change due to a reboot, later > on support for 64-bit GUIDs was added. During login three formats for > the initiator name are verified: GID, GUID without "0x" prefix and GUID > with "0x" prefix. In any case, target_alloc_session() will store a > pointer to the first struct se_node_acl that matches in sess->se_node_acl. > I think using the name stored in struct se_node_acl is fine in all cases. > Hey Bart, I did patches to add symlinks. There is one problem that this will break userspace though. The userspace tools assume that they can tear down a tpgt while sessions are running because currently a rmdir on the acl will force running sessions to be shutdown. For example, a FC or iscsi initiator can be logged into the target and userspace currently does something like this simplified sequence: for each object under a tpgt ulink luns rmdir luns rmdir acls rmdir tpt The problem is that because the session has a symlink to the acl and configfs has done a config_item_get on the acl config_item to make sure it does not get freed while in use the "rmdir acl" will now fail. I agree knowing the acl info is useful, so how about the following: 1. Create a file named "acl" in the session's dir. 2. For dynamic_node_acl=0 the acl file will return a empty string or "generate_node_acls=1" or "demo mode enabled". 3. For dynamic_node_acl=1 the acl file will return se_node_acl->initiatorname which is the name of the acl in /tpgt_$N/acls/.
Re: [PATCH 2/2] Avoid that SCSI device removal through sysfs triggers a deadlock
On Mon, Jul 30, 2018 at 11:40:52AM -0700, Bart Van Assche wrote: > A long time ago the unfortunate decision was taken to add a self- > deletion attribute to the sysfs SCSI device directory. That decision > was unfortunate because self-deletion is really tricky. We can't drop > that attribute because widely used user space software depends on it, > namely the rescan-scsi-bus.sh script. Hence this patch that avoids > that writing into that attribute triggers a deadlock. See also commit > 7973cbd9fbd9 ("[PATCH] add sysfs attributes to scan and delete > scsi_devices"). Acked-by: Tejun Heo Thanks. -- tejun
Re: [PATCH 1/2] sysfs: Introduce sysfs_{un,}break_active_protection()
On Mon, Jul 30, 2018 at 11:40:51AM -0700, Bart Van Assche wrote: > Introduce these two functions and export them such that the next patch > can add calls to these functions from the SCSI core. > > Signed-off-by: Bart Van Assche > Cc: Greg Kroah-Hartman > Cc: Tejun Heo > Cc: Acked-by: Tejun Heo Thanks. -- tejun
Re: [PATCH 6/6] scsi: ufs-bsg: Add support for uic commands in ufs_bsg_request()
On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > + struct uic_command uc = {0}; Please use "{ }" or "{}" for structure initialization as is done elsewhere in the kernel instead of "{0}". > +#define UIC_CMD_SIZE (sizeof(u32) * 4) Please add a comment above this definition that explains whether this constant comes from the spec or whether it has another origin. Thanks, Bart.
Re: [PATCH 5/6] scsi: ufs-bsg: Add support for raw upiu in ufs_bsg_request()
On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > + if (qr->opcode != UPIU_QUERY_OPCODE_WRITE_DESC || > + desc_size <= 0) > + return -EINVAL; Please use the full line length and don't split lines if that is not necessary. > + ret = ufshcd_map_desc_id_to_length(bsg_host->hba, desc_id, buff_len); > + > + if (ret || !buff_len) > + return -EINVAL; Why is buff_len only tested after it has been passed as an argument to ufshcd_map_desc_id_to_length()? That seems weird to me. > +static int ufs_bsg_verify_query_size(unsigned int request_len, > + unsigned int reply_len, > + int rw, int buff_len) > +{ > + int min_req_len = sizeof(struct ufs_bsg_request); > + int min_rsp_len = sizeof(struct ufs_bsg_reply); > + > + if (rw == UFS_BSG_NOP) > + goto null_buff; > + > + if (rw == WRITE) > + min_req_len += buff_len; Can the "goto" statement be avoided by using a switch/case on 'rw'? Thanks, Bart.
Re: [PATCH 4/6] scsi: ufs: Add API to execute raw upiu commands
On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > +/** > + * ufshcd_exec_raw_upiu_cmd - API function for sending raw upiu commands > + * @hba: per-adapter instance > + * @req_upiu:upiu request - 8 dwards > + * @rsp_upiu:upiu reply - 8 dwards > + * @msgcode: message code, one of UPIU Transaction Codes Initiator to Target > + * @desc_buff: pointer to descriptor buffer, NULL if NA > + * @buff_len:descriptor size, 0 if NA > + * @rw: either READ or WRITE > + * > + * Supports UTP Transfer requests (nop and query), and UTP Task > + * Management requests. > + * It is up to the caller to fill the upiu conent properly, as it will > + * be copied without any further input validations. > + */ > +int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba, > + __be32 *req_upiu, __be32 *rsp_upiu, > + int msgcode, > + u8 *desc_buff, int *buff_len, int rw) > +{ Again, what are "dwards"? Documenting the size of a data structure in a function header is very weird. Please change the data type of req_upiu and rsp_upio into a pointer to a structure of the proper size. Thanks, Bart.
Re: [PATCH 4/6] scsi: ufs: Add API to execute raw upiu commands
On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, _slot)); The above is the weirdest API I have seen so far for tag allocation. Why does ufshcd_get_tm_free_slot() does a linear search through a bitmap instead of using the sbitmap data structure? > + ufshcd_writel(hba, 1 << free_slot, REG_UTP_TASK_REQ_DOOR_BELL); > + /* Make sure that doorbell is committed immediately */ > + wmb(); This is the first time that I see someone claiming that issuing a write memory barrier causes the write to be executed faster than without memory barrier. Are you sure that comment is correct and that that wmb() is necessary? > + spin_unlock_irqrestore(host->host_lock, flags); > + > + /* wait until the task management command is completed */ > + err = wait_event_timeout(hba->tm_wq, > + test_bit(free_slot, >tm_condition), > + msecs_to_jiffies(TM_CMD_TIMEOUT)); Did you perhaps start implementing the ufshcd_issue_tm_upiu_cmd() function by copy/pasting ufshcd_issue_tm_cmd()? Please don't do that and instead avoid code duplication by moving shared code in a new function. > + /* ignore the returning value here - ufshcd_check_query_response is > + * bound to fail since dev_cmd.query and dev_cmd.type were left empty. > + * read the response directly ignoring all errors. > + */ Have you verified this patch with checkpatch? This is not the proper kernel comment style. Thanks, Bart.
Re: [PATCH 2/6] scsi: ufs: Add ufs-bsg module
On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > +++ b/drivers/scsi/ufs/ufs_bsg.c > @@ -0,0 +1,127 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SSCSI transport companion for UFS HBA What is "SSCSI"? > diff --git a/drivers/scsi/ufs/ufs_bsg.h b/drivers/scsi/ufs/ufs_bsg.h > new file mode 100644 [ ... ] > +struct ufs_bsg_request { > + uint32_t msgcode; > + struct utp_upiu_header header; > + union { > + struct utp_upiu_query qr; > + struct utp_upiu_task_reqtr; > + /* use utp_upiu_query to host the 4 dwards of uic command */ What are "dwards"? > + struct utp_upiu_query uc; > + } tsf; > + uint8_t data[0]; The offset of the data member will change if a member will be added to the union with a larger size than the existing members. That seems like an API design bug to me. > +} __packed; Would the data member offsets be the same without "__packed"? If so, __packed should be left out because it results in generation of suboptimal code on architectures that do not support unaligned multi-byte reads (e.g. IA-64). > +struct ufs_bsg_reply { > + /* > + * The completion result. Result exists in two forms: > + * if negative, it is an -Exxx system errno value. There will > + * be no further reply information supplied. > + * else, it's the 4-byte scsi error result, with driver, host, > + * msg and status fields. The per-msgcode reply structure > + * will contain valid data. > + */ > + uint32_t result; > + > + /* If there was reply_payload, how much was recevied ? */ Did you perhaps mean "received"? > + uint32_t reply_payload_rcv_len; > + > + struct utp_upiu_header header; > + union { > + struct utp_upiu_query qr; > + struct utp_upiu_task_rsptr; > + struct utp_upiu_query uc; > + } tsf; > + uint8_t data[0]; > +}; > + > +struct ufs_bsg_raw_upiu { > + struct ufs_bsg_upiu request; > + struct ufs_bsg_upiu reply; > +}; Are any of the above data structures needed by user space software? Should these data structure definitions perhaps be moved to a header file under include/uapi? Thanks, Bart.
RE: [PATCH 1/6] scsi: Add ufs transport class
Johannes, Thanks a lot for your comments. Cheers, Avri > -Original Message- > From: linux-scsi-ow...@vger.kernel.org > On Behalf Of Johannes Thumshirn > Sent: Wednesday, August 01, 2018 2:17 PM > To: Avri Altman > Cc: Christoph Hellwig ; Hannes Reinecke ; > Bart Van Assche ; James E.J. Bottomley > ; Martin K. Petersen > ; linux-scsi@vger.kernel.org; Stanislav > Nijnikov ; Avi Shchislowski > ; Alex Lemberg ; > Subhash Jadavani ; Vinayak Holikatti > > Subject: Re: [PATCH 1/6] scsi: Add ufs transport class > > Hi Avri, > > On Wed, Aug 01, 2018 at 11:04:27AM +0300, Avri Altman wrote: > [... > > > +#include > > Why do you include bsg.h here and bsg-lib.h in the scsi_transport_ufs.h? > > [...] Right, will move both to the same place. > > > +#define to_ufs_internal(tmpl) container_of(tmpl, struct > ufs_internal, t) > > I'd personally prefer this to be a inline function instead of a define > for type safety reasons. Ok. > > > + > > +struct ufs_host_attrs { > > + atomic_t next_port_id; > > +}; > > +#define to_ufs_host_attrs(x) ((struct ufs_host_attrs *)(x)->shost_data) > > Ditto. Ok. > > [...] > > > + > > + port->id = atomic_inc_return(_host->next_port_id); > > Any reason you can't use an IDA for the port->id? Ok. Will change to use it. > > [...] > > + > > + error = device_add(dev); > > + > > + if (error) > > + return error; > > No blank line please. Done. > > [...] > > > +#define dev_to_ufs_port(d) \ > > + container_of((d), struct ufs_port, dev) > > Inline function as well, please. Done. > > -- > Johannes Thumshirn Storage > jthumsh...@suse.de+49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 02:50:40PM +0100, Guillaume Tucker wrote: > > Sure, although it didn't make any apparent difference, still on > next-20180731: > > https://lava.collabora.co.uk/scheduler/job/1215417 > > The .config file I used is available here, with just > CONFIG_BLK_DEV_RAM=y on top of defconfig: > > > https://people.collabora.com/~gtucker/lava/boot/debug/bzImage-85eac382fa06-blk-dev.config OK, this is a deviation from what I see here (on mkp's 4.19/scsi-queue not next though). -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On 01/08/18 13:40, Johannes Thumshirn wrote: On Wed, Aug 01, 2018 at 01:37:17PM +0100, Guillaume Tucker wrote: The kernel images I used have the git revision in their file name to make it clear where they came from, they were built with x86_64_defconfig. The links to the kernel and ramdisks can be found in the job definition: https://lava.collabora.co.uk/scheduler/job/1215154/definition Please let me know if you need any more details. I'm happy to run other boot tests on that same setup if that helps verifying things (such as enabling some VIRTIO configs...). Yes, can you please enable CONFIG_BLK_DEV_RAM? See my other mails in this thread for details. Sure, although it didn't make any apparent difference, still on next-20180731: https://lava.collabora.co.uk/scheduler/job/1215417 The .config file I used is available here, with just CONFIG_BLK_DEV_RAM=y on top of defconfig: https://people.collabora.com/~gtucker/lava/boot/debug/bzImage-85eac382fa06-blk-dev.config Best wishes, Guillaume
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On 01/08/18 12:25, Johannes Thumshirn wrote: On Wed, Aug 01, 2018 at 11:59:19AM +0100, Mark Brown wrote: On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: You may have to provide some clue, such as dmesg log, boot disk, ... I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq mode at default, even though without d5038a13eca72fb. Boot logs and so on can be found here: https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ (these are today's but the symptoms are the same.) The ramdisk is unfortunately not linked through the UI, though we don't get that far. Can you give us a bit more information about you test setups? Like Qemu command line, etc..? From you kernel config I see you're not using virtio (as Ming already suggested). What medium are you booting off? Sure, sorry I should have put that in my first email. Here's a couple of LAVA boot tests, one passing with the commit reverted and one failing with plain next-20180731: https://lava.collabora.co.uk/scheduler/job/1215154 https://lava.collabora.co.uk/scheduler/job/1215173 The qemu command line can be found in the log, copying it here: /usr/bin/qemu-system-x86_64 -cpu host -enable-kvm -nographic -net nic,model=virtio,macaddr=DE:AD:BE:EF:AE:1B -net user -m 1024 -monitor none -kernel /var/lib/lava/dispatcher/tmp/1215173/deployimages-3p80s2zk/kernel/bzImage-85eac382fa06 -append "console=ttyS0,115200 root=/dev/ram0 debug verbose" -initrd /var/lib/lava/dispatcher/tmp/1215173/deployimages-3p80s2zk/ramdisk/rootfs.cpio.gz The kernel images I used have the git revision in their file name to make it clear where they came from, they were built with x86_64_defconfig. The links to the kernel and ramdisks can be found in the job definition: https://lava.collabora.co.uk/scheduler/job/1215154/definition Please let me know if you need any more details. I'm happy to run other boot tests on that same setup if that helps verifying things (such as enabling some VIRTIO configs...). Best wishes, Guillaume
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 08:00:44PM +0800, Ming Lei wrote: > On Wed, Aug 01, 2018 at 07:52:19PM +0800, Ming Lei wrote: > > On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > > > On 1 August 2018 at 11:59, Mark Brown wrote: > > > > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > > > > > >> You may have to provide some clue, such as dmesg log, boot disk, ... > > > > > > > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > > > >> mode at default, even though without d5038a13eca72fb. > > > > > > > > Boot logs and so on can be found here: > > > > > > > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > > > > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > > > > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > > > > > > > (these are today's but the symptoms are the same.) The ramdisk is > > > > unfortunately not linked through the UI, though we don't get that far. > > > > > > And a full LAVA boot log from my lab > > > http://lava.streamtester.net/scheduler/job/138067 > > > > > > QEMU command line here: > > > http://lava.streamtester.net/scheduler/job/138067#L75 > > > > > > And a LAVA job definition, which includes the url of the ramdisk and > > > kernel: > > > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > > > > > > > Thanks for the sharing! > > > > I can reproduce this issue with above script/initrd/kernel config, and looks > > the issue disappeared after 'scsi_mod.use_blk_mq=0' is passed. > > > > Not see such issue with zero-day ktest config. > > > > Looks a bit weird, given SCSI_MQ is nothing related with ramdisk. > > Seems related with sr: > > 1) with scsi-mq > [2.808204] ata2.01: NODEV after polling detection > [2.809807] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 > [2.827377] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ > PQ: 0 ANSI: 5 > > 2) without scsi_mq > [5.549135] ata2.01: NODEV after polling detection > [5.554404] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 > [5.596143] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ > PQ: 0 ANSI: 5 > [5.637126] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray > [5.637870] cdrom: Uniform CD-ROM driver Revision: 3.20 > [5.648940] sr 1:0:0:0: Attached scsi CD-ROM sr0 > [5.661605] sr 1:0:0:0: Attached scsi generic sg0 type 5 > > > We may need to take a look at recent SCSI change. [2.052168] ata2.01: NODEV after polling detection [2.053690] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 [2.072269] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ P5 [2.107220] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray [2.107675] cdrom: Uniform CD-ROM driver Revision: 3.20 [2.111851] sr 1:0:0:0: Attached scsi CD-ROM sr0 [2.123560] sr 1:0:0:0: Attached scsi generic sg0 type 5 # cat /proc/cmdline console=ttyS0,115200 root=/dev/ram0 debug verbose $ grep SCSI_MQ .config CONFIG_SCSI_MQ_DEFAULT=y on Martin's latest 4.19/scsi-queue. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 07:52:19PM +0800, Ming Lei wrote: > On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > > On 1 August 2018 at 11:59, Mark Brown wrote: > > > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > > > >> You may have to provide some clue, such as dmesg log, boot disk, ... > > > > > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > > >> mode at default, even though without d5038a13eca72fb. > > > > > > Boot logs and so on can be found here: > > > > > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > > > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > > > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > > > > > (these are today's but the symptoms are the same.) The ramdisk is > > > unfortunately not linked through the UI, though we don't get that far. > > > > And a full LAVA boot log from my lab > > http://lava.streamtester.net/scheduler/job/138067 > > > > QEMU command line here: > > http://lava.streamtester.net/scheduler/job/138067#L75 > > > > And a LAVA job definition, which includes the url of the ramdisk and kernel: > > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > > > > Thanks for the sharing! > > I can reproduce this issue with above script/initrd/kernel config, and looks > the issue disappeared after 'scsi_mod.use_blk_mq=0' is passed. > > Not see such issue with zero-day ktest config. > > Looks a bit weird, given SCSI_MQ is nothing related with ramdisk. Seems related with sr: 1) with scsi-mq [2.808204] ata2.01: NODEV after polling detection [2.809807] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 [2.827377] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ PQ: 0 ANSI: 5 2) without scsi_mq [5.549135] ata2.01: NODEV after polling detection [5.554404] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 [5.596143] scsi 1:0:0:0: CD-ROMQEMU QEMU DVD-ROM 2.5+ PQ: 0 ANSI: 5 [5.637126] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray [5.637870] cdrom: Uniform CD-ROM driver Revision: 3.20 [5.648940] sr 1:0:0:0: Attached scsi CD-ROM sr0 [5.661605] sr 1:0:0:0: Attached scsi generic sg0 type 5 We may need to take a look at recent SCSI change. Thanks, Ming
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 07:52:21PM +0800, Ming Lei wrote: > On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > > On 1 August 2018 at 11:59, Mark Brown wrote: > > > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > > > >> You may have to provide some clue, such as dmesg log, boot disk, ... > > > > > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > > >> mode at default, even though without d5038a13eca72fb. > > > > > > Boot logs and so on can be found here: > > > > > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > > > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > > > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > > > > > (these are today's but the symptoms are the same.) The ramdisk is > > > unfortunately not linked through the UI, though we don't get that far. > > > > And a full LAVA boot log from my lab > > http://lava.streamtester.net/scheduler/job/138067 > > > > QEMU command line here: > > http://lava.streamtester.net/scheduler/job/138067#L75 > > > > And a LAVA job definition, which includes the url of the ramdisk and kernel: > > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > > > > Thanks for the sharing! > > I can reproduce this issue with above script/initrd/kernel config, and looks > the issue disappeared after 'scsi_mod.use_blk_mq=0' is passed. > > Not see such issue with zero-day ktest config. > > Looks a bit weird, given SCSI_MQ is nothing related with ramdisk. Ahm and: qemu [...] -append "console=ttyS0,115200 root=/dev/ram0 debug verbose" $ grep CONFIG_BLK_DEV_RAM .config # CONFIG_BLK_DEV_RAM is not set Something is fishy here. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > On 1 August 2018 at 11:59, Mark Brown wrote: > > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > >> You may have to provide some clue, such as dmesg log, boot disk, ... > > > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > >> mode at default, even though without d5038a13eca72fb. > > > > Boot logs and so on can be found here: > > > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > > > (these are today's but the symptoms are the same.) The ramdisk is > > unfortunately not linked through the UI, though we don't get that far. > > And a full LAVA boot log from my lab > http://lava.streamtester.net/scheduler/job/138067 > > QEMU command line here: > http://lava.streamtester.net/scheduler/job/138067#L75 > > And a LAVA job definition, which includes the url of the ramdisk and kernel: > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > Thanks for the sharing! I can reproduce this issue with above script/initrd/kernel config, and looks the issue disappeared after 'scsi_mod.use_blk_mq=0' is passed. Not see such issue with zero-day ktest config. Looks a bit weird, given SCSI_MQ is nothing related with ramdisk. Thanks, Ming
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 12:24:00PM +0100, Matt Hart wrote: > And a full LAVA boot log from my lab > http://lava.streamtester.net/scheduler/job/138067 > > QEMU command line here: > http://lava.streamtester.net/scheduler/job/138067#L75 > > And a LAVA job definition, which includes the url of the ramdisk and kernel: > http://lava.streamtester.net/scheduler/job/138067/definition#defline76 I grabbed your qemu cmdline and kernel config and try to reproduce it locally. Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
On Wed, Aug 01, 2018 at 08:33:23AM +0200, Hannes Reinecke wrote: > On 07/31/2018 05:07 PM, Varun Prakash wrote: > > If number of interrupt vectors are more than num_online_cpus() > > then pci_alloc_irq_vectors_affinity() assigns cpumask based > > on num_possible_cpus() to the remaining vectors because of > > this interrupt does not generate for these vectors. > > > > This patch fixes this issue by using pci_alloc_irq_vectors() > > instead of pci_alloc_irq_vectors_affinity(). > > > > Signed-off-by: Varun Prakash > > --- > > drivers/scsi/csiostor/csio_isr.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt, > > - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, ); > > + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX); > > if (cnt < 0) > > return cnt; > > > > > Hmm. > That patch (and the reasoning leading up to it) really sound dodgy. > I'd rather fix the interrupt affinity code to handle this case correctly. Yes, it is better to fix the affinity assignment code, I posted this patch to start a discussion on this issue. I can confirm that if number of interrupt vectors are more than num_online cpus() then pci_alloc_irq_vectors_affinity() assigns cpumask based on cpu_possible_mask to the remaining vectors. Following is the logic for creating affinity masks struct cpumask * irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd) { ... /* Spread on present CPUs starting from affd->pre_vectors */ usedvecs = irq_build_affinity_masks(affd, curvec, affvecs, node_to_cpumask, cpu_present_mask, nmsk, masks); /* * Spread on non present CPUs starting from the next vector to be * handled. If the spreading of present CPUs already exhausted the * vector space, assign the non present CPUs to the already spread * out vectors. */ if (usedvecs >= affvecs) curvec = affd->pre_vectors; else curvec = affd->pre_vectors + usedvecs; cpumask_andnot(npresmsk, cpu_possible_mask, cpu_present_mask); usedvecs += irq_build_affinity_masks(affd, curvec, affvecs, node_to_cpumask, npresmsk, nmsk, masks); ... } > Thomas, can you help here? >
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 11:59:19AM +0100, Mark Brown wrote: > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > > > You may have to provide some clue, such as dmesg log, boot disk, ... > > > I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > > mode at default, even though without d5038a13eca72fb. > > Boot logs and so on can be found here: > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > (these are today's but the symptoms are the same.) The ramdisk is > unfortunately not linked through the UI, though we don't get that far. Can you give us a bit more information about you test setups? Like Qemu command line, etc..? From you kernel config I see you're not using virtio (as Ming already suggested). What medium are you booting off? Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On 1 August 2018 at 11:59, Mark Brown wrote: > On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > >> You may have to provide some clue, such as dmesg log, boot disk, ... > >> I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq >> mode at default, even though without d5038a13eca72fb. > > Boot logs and so on can be found here: > > https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ > https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ > https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ > > (these are today's but the symptoms are the same.) The ramdisk is > unfortunately not linked through the UI, though we don't get that far. And a full LAVA boot log from my lab http://lava.streamtester.net/scheduler/job/138067 QEMU command line here: http://lava.streamtester.net/scheduler/job/138067#L75 And a LAVA job definition, which includes the url of the ramdisk and kernel: http://lava.streamtester.net/scheduler/job/138067/definition#defline76 > > ___ > Kernel-build-reports mailing list > kernel-build-repo...@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/kernel-build-reports >
Re: [PATCH 1/6] scsi: Add ufs transport class
Hi Avri, On Wed, Aug 01, 2018 at 11:04:27AM +0300, Avri Altman wrote: [... > +#include Why do you include bsg.h here and bsg-lib.h in the scsi_transport_ufs.h? [...] > +#define to_ufs_internal(tmpl)container_of(tmpl, struct ufs_internal, > t) I'd personally prefer this to be a inline function instead of a define for type safety reasons. > + > +struct ufs_host_attrs { > + atomic_t next_port_id; > +}; > +#define to_ufs_host_attrs(x) ((struct ufs_host_attrs *)(x)->shost_data) Ditto. [...] > + > + port->id = atomic_inc_return(_host->next_port_id); Any reason you can't use an IDA for the port->id? [...] > + > + error = device_add(dev); > + > + if (error) > + return error; No blank line please. [...] > +#define dev_to_ufs_port(d) \ > + container_of((d), struct ufs_port, dev) Inline function as well, please. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 06:51:09PM +0800, Ming Lei wrote: > You may have to provide some clue, such as dmesg log, boot disk, ... > I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq > mode at default, even though without d5038a13eca72fb. Boot logs and so on can be found here: https://kernelci.org/boot/id/5b618c9f59b514931f96ba97/ https://kernelci.org/boot/id/5b618ca359b514904d96bac5/ https://kernelci.org/boot/id/5b618cbc59b51492e896baad/ (these are today's but the symptoms are the same.) The ramdisk is unfortunately not linked through the UI, though we don't get that far. signature.asc Description: PGP signature
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 11:33:03AM +0100, Mark Brown wrote: > On Wed, Aug 01, 2018 at 11:05:36AM +0100, Guillaume Tucker wrote: > > On 31/07/18 16:14, kernelci.org bot wrote: > > > > Boot Regressions Detected: > > [...] > > > x86: > > > > > > x86_64_defconfig: > > > qemu: > > > lab-baylibre: failing since 1 day (last pass: next-20180727 > > > - first fail: next-20180730) > > > lab-mhart: failing since 1 day (last pass: next-20180727 - > > > first fail: next-20180730) > > > lab-linaro-lkft: failing since 1 day (last pass: > > > next-20180727 - first fail: next-20180730) > > > I've run a few automated bisection on kernelci.org, it initially > > landed on this merge commit: > > > ff719be3476a Merge remote-tracking branch 'scsi/for-next' > > > The 2 parent commits boot fine, but not the resulting merge. So I > > did another bisection based on the first branch while merging the > > incoming one in each iteration, and that landed on this commit: > > > commit d5038a13eca72fb216c07eb717169092e92284f1 > > Author: Johannes Thumshirn > > Date: Wed Jul 4 10:53:56 2018 +0200 > > > > scsi: core: switch to scsi-mq by default > > > > > > I then tried to revert it on top of next-20180731 and it did make it > > boot again. Now, I haven't looked much further in the code, it's > > entirely possible that the problem is on another incoming branch, in > > the code that this config enables. At least it seems to be narrowing > > down the scope for where to look for a problem. > > Copying in everyone else who signed off/acked/reviewed that commit. You may have to provide some clue, such as dmesg log, boot disk, ... I guess you don't use virtio-scsi/virtio-blk since both run at blk-mq mode at default, even though without d5038a13eca72fb. Thanks, Ming
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On Wed, Aug 01, 2018 at 11:05:36AM +0100, Guillaume Tucker wrote: > On 31/07/18 16:14, kernelci.org bot wrote: > > Boot Regressions Detected: > [...] > > x86: > > > > x86_64_defconfig: > > qemu: > > lab-baylibre: failing since 1 day (last pass: next-20180727 - > > first fail: next-20180730) > > lab-mhart: failing since 1 day (last pass: next-20180727 - > > first fail: next-20180730) > > lab-linaro-lkft: failing since 1 day (last pass: next-20180727 > > - first fail: next-20180730) > I've run a few automated bisection on kernelci.org, it initially > landed on this merge commit: > ff719be3476a Merge remote-tracking branch 'scsi/for-next' > The 2 parent commits boot fine, but not the resulting merge. So I > did another bisection based on the first branch while merging the > incoming one in each iteration, and that landed on this commit: > commit d5038a13eca72fb216c07eb717169092e92284f1 > Author: Johannes Thumshirn > Date: Wed Jul 4 10:53:56 2018 +0200 > > scsi: core: switch to scsi-mq by default > > > I then tried to revert it on top of next-20180731 and it did make it > boot again. Now, I haven't looked much further in the code, it's > entirely possible that the problem is on another incoming branch, in > the code that this config enables. At least it seems to be narrowing > down the scope for where to look for a problem. Copying in everyone else who signed off/acked/reviewed that commit. signature.asc Description: PGP signature
Re: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731)
On 31/07/18 16:14, kernelci.org bot wrote: next/master boot: 179 boots: 11 failed, 167 passed with 1 offline (next-20180731) Full Boot Summary: https://kernelci.org/boot/all/job/next/branch/master/kernel/next-20180731/ Full Build Summary: https://kernelci.org/build/next/branch/master/kernel/next-20180731/ Tree: next Branch: master Git Describe: next-20180731 Git Commit: 85eac382fa06ac72adf891d04bf4d08fb09d80fa Git URL: http://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git Tested: 66 unique boards, 26 SoC families, 21 builds out of 200 Boot Regressions Detected: [...] x86: x86_64_defconfig: qemu: lab-baylibre: failing since 1 day (last pass: next-20180727 - first fail: next-20180730) lab-mhart: failing since 1 day (last pass: next-20180727 - first fail: next-20180730) lab-linaro-lkft: failing since 1 day (last pass: next-20180727 - first fail: next-20180730) I've run a few automated bisection on kernelci.org, it initially landed on this merge commit: ff719be3476a Merge remote-tracking branch 'scsi/for-next' The 2 parent commits boot fine, but not the resulting merge. So I did another bisection based on the first branch while merging the incoming one in each iteration, and that landed on this commit: commit d5038a13eca72fb216c07eb717169092e92284f1 Author: Johannes Thumshirn Date: Wed Jul 4 10:53:56 2018 +0200 scsi: core: switch to scsi-mq by default I then tried to revert it on top of next-20180731 and it did make it boot again. Now, I haven't looked much further in the code, it's entirely possible that the problem is on another incoming branch, in the code that this config enables. At least it seems to be narrowing down the scope for where to look for a problem. Hope this helps! Best wishes, Guillaume [...] --- For more info write to ___ Kernel-build-reports mailing list kernel-build-repo...@lists.linaro.org https://lists.linaro.org/mailman/listinfo/kernel-build-reports
[PATCH V7 0/2] Add UFS provisioning support in driver
This patch adds Configfs support to provision UFS device at runtime. This feature can be primarily useful in factory or assembly line as some devices may be required to be configured multiple times during initial system development phase. Configuration Descriptors can be written multiple times until bConfigDescrLock attribute is zero. Configuration descriptor buffer consists of Device and Unit descriptor configurable parameters which are parsed from vendor specific provisioning file and then passed via configfs node at runtime to provision ufs device. Changes since V6: 1)scsi: ufs: set the device reference clock setting Re-introduced this patch to provisioning patch set(as per comments from Evan). Used of_clk_get_by_name() and clk_get_rate() to set ref_clk frequency instead of passing freq via DT. 2)scsi: ufs: Add configfs support for UFS provisioning Updated error handling in case if kstrtoint fails while parsing input configuration buffer. Changes since V5: 1)scsi: ufs: set the device reference clock setting Removed this patch from provisioning patch set(as its not required to be set as dependent changes). This will be uploaded as a separate patch later. 2)scsi: ufs: Add configfs support for UFS provisioning Removed few extra debug prints. Updated permission of ufs_provision attribute from 0666 to 0644. Pass UFS device name as part of ufshcd_configfs_init() to support multiple UFS controller for embedded and removable UFS card. Changes since V4: 1)scsi: ufs: set the device reference clock setting Used "assigned-clock-rates" DT property to pass required ref clk frequency. 2)scsi: ufs: Add configfs support for ufs provisioning Combined previous patch(2) and patch(3) into single patch which adds configfs provisioning support in driver. Removed extra sw provisioning related fields (like lun_to_grow, commit) and its related code. Updated Documentation to match configuration descriptor buffer parameters to be passed as per specs. Removed global ufs_hba ptr added in ufs-configfs file and instead passed *hba in ufs configfs init()/store()/show() api's. This is to support embedded as well as removable ufs card provisioning via configfs. Changes since V3: 1)scsi: ufs: set the device reference clock setting Updated logic to retain default ref_clk frequency setting programmed in device in case if invalid value is passed via devicetree setting. Replaced of_property_read_u32() with device_property_read_u32(). Removed invalid checks. 2)scsi: ufs: Add ufs provisioning support Added pm_runtime_get/put_sync and scsi_block/unblock_request in runtime provisioning for stable operation. 3)scsi: ufs: Add configfs support for ufs provisioning Updated Documentation with missing buffer entries required for runtime provisioning. Used config option to support conditional compilation for configfs api's. Changes since V2: Added configfs support for ufs provisioning and removed sysfs support. Changes since V1: Added device tree entry to parse reference clock frequency instead of hardcoding 19.2 MHz, as it can vary for different vendors. Also removed setting ref_clk again during runtime provisioning as it will be already set during probe. Used get_unaligned_be*/put_unaligned_be* where applicable. Changes since RFC: Added check to avoid ufs runtime provisioning if Configuration decriptor lock attribute is set to one. Instead of parsing ref_clk frequency via device tree, used correct enum ref_clk_freq value(19.2 Mhz for proviosioning). Added config_descriptor sysfs entry to provision ufs and also updated documentation for its correct usage. Added more protection against bad data handling in sysfs code. Sayali Lokhande (1): scsi: ufs: Add configfs support for UFS provisioning Subhash Jadavani (1): scsi: ufs: set the device reference clock setting Documentation/ABI/testing/configfs-driver-ufs | 18 +++ drivers/scsi/ufs/Kconfig | 10 ++ drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-configfs.c | 162 ++ drivers/scsi/ufs/ufs.h| 8 ++ drivers/scsi/ufs/ufshcd-pltfrm.c | 2 + drivers/scsi/ufs/ufshcd.c | 91 ++- drivers/scsi/ufs/ufshcd.h | 12 ++ 8 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/configfs-driver-ufs create mode 100644 drivers/scsi/ufs/ufs-configfs.c -- The Qualcomm Innovation Center,
[PATCH 6/6] scsi: ufs-bsg: Add support for uic commands in ufs_bsg_request()
Add support to those uic commands, that are currently supported by ufshcd api: the variants of dme_{peer}_{set_get}. At this point better not to add any new api, as careless uic command may turn the device into a brick. Signed-off-by: Avri Altman --- drivers/scsi/ufs/ufs_bsg.c | 56 +- drivers/scsi/ufs/ufs_bsg.h | 1 + 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c index d077e42..441d096 100644 --- a/drivers/scsi/ufs/ufs_bsg.c +++ b/drivers/scsi/ufs/ufs_bsg.c @@ -63,6 +63,55 @@ static int ufs_bsg_verify_query_size(unsigned int request_len, return 0; } +static int ufs_bsg_exec_uic_cmd(struct uic_command *uc) +{ + u32 attr_sel = uc->argument1; + u8 attr_set = (uc->argument2 >> 16) & 0xff; + u32 mib_val = uc->argument3; + int cmd = uc->command; + int ret = 0; + + switch (cmd) { + case UIC_CMD_DME_GET: + ret = ufshcd_dme_get_attr(bsg_host->hba, attr_sel, + _val, DME_LOCAL); + break; + case UIC_CMD_DME_SET: + ret = ufshcd_dme_set_attr(bsg_host->hba, attr_sel, attr_set, + mib_val, DME_LOCAL); + break; + case UIC_CMD_DME_PEER_GET: + ret = ufshcd_dme_get_attr(bsg_host->hba, attr_sel, + _val, DME_PEER); + break; + case UIC_CMD_DME_PEER_SET: + ret = ufshcd_dme_set_attr(bsg_host->hba, attr_sel, attr_set, + mib_val, DME_PEER); + break; + case UIC_CMD_DME_POWERON: + case UIC_CMD_DME_POWEROFF: + case UIC_CMD_DME_ENABLE: + case UIC_CMD_DME_RESET: + case UIC_CMD_DME_END_PT_RST: + case UIC_CMD_DME_LINK_STARTUP: + case UIC_CMD_DME_HIBER_ENTER: + case UIC_CMD_DME_HIBER_EXIT: + case UIC_CMD_DME_TEST_MODE: + ret = -ENOTSUPP; + pr_err("%s unsupported command 0x%x\n", __func__, cmd); + break; + default: + ret = -EINVAL; + } + + if (ret) + pr_err("%s error in command 0x%x\n", __func__, cmd); + + uc->argument3 = mib_val; + + return ret; +} + static int ufs_bsg_request(struct bsg_job *job) { struct ufs_bsg_request *bsg_request = job->request; @@ -70,6 +119,7 @@ static int ufs_bsg_request(struct bsg_job *job) unsigned int request_len = job->request_len; unsigned int reply_len = job->reply_len; struct utp_upiu_query *qr; + struct uic_command uc = {0}; __be32 *req_upiu = NULL; __be32 *rsp_upiu = NULL; int msgcode; @@ -124,7 +174,11 @@ static int ufs_bsg_request(struct bsg_job *job) break; case UPIU_TRANSACTION_UIC_CMD: - /* later */ + memcpy(, _request->tsf.uc, UIC_CMD_SIZE); + ret = ufs_bsg_exec_uic_cmd(); + memcpy(_reply->tsf.uc, , UIC_CMD_SIZE); + + break; case UPIU_TRANSACTION_COMMAND: case UPIU_TRANSACTION_DATA_OUT: not_supported: diff --git a/drivers/scsi/ufs/ufs_bsg.h b/drivers/scsi/ufs/ufs_bsg.h index 0fd2859..651ffb3 100644 --- a/drivers/scsi/ufs/ufs_bsg.h +++ b/drivers/scsi/ufs/ufs_bsg.h @@ -15,6 +15,7 @@ #define UFS_BSG_NOP (-1) #define UPIU_TRANSACTION_UIC_CMD 0x1F +#define UIC_CMD_SIZE (sizeof(u32) * 4) enum { REQ_UPIU_SIZE_DWORDS= 8, -- 1.9.1
[PATCH 5/6] scsi: ufs-bsg: Add support for raw upiu in ufs_bsg_request()
Do that for the currently supported UPIUs: query, nop out, and task management. We do not support UPIU of type scsi command yet, while we are using the job's request and reply pointers to hold the payload. We will look into it in later patches. We might need to elaborate the raw upiu api for that. We also still not supporting uic commands: For first phase, we plan to use the existing api, and send only uic commands that are already supported. Anyway, all that will come in the next patch. Signed-off-by: Avri Altman --- drivers/scsi/ufs/ufs_bsg.c | 119 +++-- drivers/scsi/ufs/ufs_bsg.h | 1 + 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c index 71826ba..d077e42 100644 --- a/drivers/scsi/ufs/ufs_bsg.c +++ b/drivers/scsi/ufs/ufs_bsg.c @@ -19,18 +19,129 @@ struct ufs_bsg { .bsg_request = ufs_bsg_request, }; +static int ufs_bsg_get_query_desc_size(struct utp_upiu_query *qr, + int *buff_len) +{ + int desc_size = be16_to_cpu(qr->length); + int desc_id = qr->idn; + int ret = 0; + + if (qr->opcode != UPIU_QUERY_OPCODE_WRITE_DESC || + desc_size <= 0) + return -EINVAL; + + ret = ufshcd_map_desc_id_to_length(bsg_host->hba, desc_id, buff_len); + + if (ret || !buff_len) + return -EINVAL; + + *buff_len = min_t(int, *buff_len, desc_size); + + return ret; +} + +static int ufs_bsg_verify_query_size(unsigned int request_len, +unsigned int reply_len, +int rw, int buff_len) +{ + int min_req_len = sizeof(struct ufs_bsg_request); + int min_rsp_len = sizeof(struct ufs_bsg_reply); + + if (rw == UFS_BSG_NOP) + goto null_buff; + + if (rw == WRITE) + min_req_len += buff_len; + +null_buff: + if (min_req_len > request_len) + return -EINVAL; + + if (min_rsp_len > reply_len) + return -EINVAL; + + return 0; +} + static int ufs_bsg_request(struct bsg_job *job) { struct ufs_bsg_request *bsg_request = job->request; struct ufs_bsg_reply *bsg_reply = job->reply; - int ret = -ENOTSUPP; + unsigned int request_len = job->request_len; + unsigned int reply_len = job->reply_len; + struct utp_upiu_query *qr; + __be32 *req_upiu = NULL; + __be32 *rsp_upiu = NULL; + int msgcode; + uint8_t *desc_buff = NULL; + int buff_len = 0; + int rw = UFS_BSG_NOP; + int ret; + ret = ufs_bsg_verify_query_size(request_len, reply_len, rw, buff_len); + if (ret) { + dev_err(job->dev, "not enough space assigned\n"); + goto out; + } bsg_reply->reply_payload_rcv_len = 0; - /* Do Nothing for now */ - dev_err(job->dev, "unsupported message_code 0x%x\n", - bsg_request->msgcode); + msgcode = bsg_request->msgcode; + switch (msgcode) { + case UPIU_TRANSACTION_QUERY_REQ: + qr = _request->tsf.qr; + if (qr->opcode == UPIU_QUERY_OPCODE_READ_DESC) + goto not_supported; + + if (ufs_bsg_get_query_desc_size(qr, _len)) + goto null_desc_buff; + + if (qr->opcode == UPIU_QUERY_OPCODE_WRITE_DESC) { + rw = WRITE; + desc_buff = ((uint8_t *)bsg_request) + + sizeof(struct ufs_bsg_request); + } + +null_desc_buff: + /* fall through */ + case UPIU_TRANSACTION_NOP_OUT: + case UPIU_TRANSACTION_TASK_REQ: + /* Now that we know if its a read or write, verify again */ + if (rw != UFS_BSG_NOP || buff_len) { + ret = ufs_bsg_verify_query_size(request_len, reply_len, + rw, buff_len); + if (ret) { + dev_err(job->dev, + "not enough space assigned\n"); + goto out; + } + } + + req_upiu = (__be32 *)_request->header; + rsp_upiu = (__be32 *)_reply->header; + ret = ufshcd_exec_raw_upiu_cmd(bsg_host->hba, + req_upiu, rsp_upiu, msgcode, + desc_buff, _len, rw); + + break; + case UPIU_TRANSACTION_UIC_CMD: + /* later */ + case UPIU_TRANSACTION_COMMAND: + case UPIU_TRANSACTION_DATA_OUT: +not_supported: + /* for the time being, we do not support data transfer upiu */ + ret = -ENOTSUPP; + dev_err(job->dev, "unsupported msgcode 0x%x\n",
[PATCH 4/6] scsi: ufs: Add API to execute raw upiu commands
The UFS host software uses a combination of a host register set, and Transfer Request Descriptors in system memory to communicate with host controller hardware. In its mmio space, a separate places are assigned to UTP Transfer Request Descriptor ("utrd") list, and to UTP Task Management Request Descriptor ("utmrd") list. The provided API supports utrd-typed requests: nop out and device management commands. It also supports utmrd-type requests: task management requests. Other UPIU types are not supported for now. We utilize the already existing code for tag and task work queues. That is, all utrd-typed UPIUs are "disguised" as device management commands. Similarly, the utmrd-typed UPUIs uses the task management infrastructure. It is up to the caller to fill the upiu request properly, as it will be copied without any further input validations. Signed-off-by: Avri Altman --- drivers/scsi/ufs/ufshcd.c | 225 ++ drivers/scsi/ufs/ufshcd.h | 4 + 2 files changed, 229 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c2ae406..5abf697 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -257,6 +257,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, static irqreturn_t ufshcd_intr(int irq, void *__hba); static int ufshcd_change_power_mode(struct ufs_hba *hba, struct ufs_pa_layer_attr *pwr_mode); +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp); + static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag) { return tag >= 0 && tag < hba->nutrs; @@ -3106,6 +3108,229 @@ int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, EXPORT_SYMBOL(ufshcd_map_desc_id_to_length); /** + * ufshcd_issue_tm_upiu_cmd - API for sending "utmrd" requests + * @hba - UFS hba + * @req_upiu - bsg request + * @rsp_upiu - bsg reply + * + * Those requests uses UTP Task Management Request Descriptor - utmrd. + * Therefore, it "rides" the task management infrastructure: uses its tag and + * tasks work queues. + */ +static int ufshcd_issue_tm_upiu_cmd(struct ufs_hba *hba, + __be32 *req_upiu, + __be32 *rsp_upiu) +{ + struct utp_task_req_desc *task_req_descp; + struct Scsi_Host *host = hba->host; + unsigned long flags; + int free_slot; + u8 tm_response = 0xF; + int err; + int task_tag; + + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, _slot)); + ufshcd_hold(hba, false); + + spin_lock_irqsave(host->host_lock, flags); + + task_req_descp = hba->utmrdl_base_addr; + task_req_descp += free_slot; + + task_req_descp->header.dword_0 = cpu_to_le32(UTP_REQ_DESC_INT_CMD); + task_req_descp->header.dword_2 = + cpu_to_le32(OCS_INVALID_COMMAND_STATUS); + + task_tag = hba->nutrs + free_slot; + + req_upiu[0] |= cpu_to_be32(task_tag); + + /* just copy the upiu request as it is */ + memcpy(task_req_descp->task_req_upiu, req_upiu, + GENERAL_UPIU_REQUEST_SIZE); + + /* send command to the controller */ + __set_bit(free_slot, >outstanding_tasks); + + /* Make sure descriptors are ready before ringing the task doorbell */ + wmb(); + + ufshcd_writel(hba, 1 << free_slot, REG_UTP_TASK_REQ_DOOR_BELL); + /* Make sure that doorbell is committed immediately */ + wmb(); + + spin_unlock_irqrestore(host->host_lock, flags); + + /* wait until the task management command is completed */ + err = wait_event_timeout(hba->tm_wq, + test_bit(free_slot, >tm_condition), + msecs_to_jiffies(TM_CMD_TIMEOUT)); + if (!err) { + dev_err(hba->dev, "%s: bsg task management cmd timed-out\n", + __func__); + if (ufshcd_clear_tm_cmd(hba, free_slot)) + dev_WARN(hba->dev, +"%s: unable clear tm cmd (slot %d) timeout\n", + __func__, free_slot); + err = -ETIMEDOUT; + } else { + err = ufshcd_task_req_compl(hba, free_slot, _response); + } + + /* just copy the upiu response as it is */ + memcpy(rsp_upiu, task_req_descp->task_rsp_upiu, + GENERAL_UPIU_REQUEST_SIZE); + + clear_bit(free_slot, >tm_condition); + ufshcd_put_tm_slot(hba, free_slot); + wake_up(>tm_tag_wq); + + ufshcd_release(hba); + return err; +} + +/** + * ufshcd_issue_devman_upiu_cmd - API for sending "utrd" type requests + * @hba: per-adapter instance + * @req_upiu: upiu request - 8 dwards + * @rsp_upiu: upiu reply - 8 dwards + * @msgcode: message code, one of UPIU Transaction Codes Initiator to Target + * @desc_buff: pointer to descriptor buffer, NULL if NA + * @buff_len: descriptor
[PATCH 3/6] scsi: ufs: Instantiate a ufs transport if its available
Call the Attach/Probe/Remove APIs. Signed-off-by: Avri Altman --- drivers/scsi/ufs/ufshcd.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 3560185..c2ae406 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -46,6 +46,7 @@ #include "ufs_quirks.h" #include "unipro.h" #include "ufs-sysfs.h" +#include "ufs_bsg.h" #define CREATE_TRACE_POINTS #include @@ -6651,6 +6652,9 @@ static int ufshcd_probe_hba(struct ufs_hba *hba) hba->clk_scaling.is_allowed = true; } + if (hba->host->transportt) + ufs_bsg_probe(hba); + scsi_scan_host(hba->host); pm_runtime_put_sync(hba->dev); } @@ -7874,6 +7878,7 @@ int ufshcd_shutdown(struct ufs_hba *hba) */ void ufshcd_remove(struct ufs_hba *hba) { + ufs_bsg_remove(hba->host); ufs_sysfs_remove_nodes(hba->dev); scsi_remove_host(hba->host); /* disable interrupts */ @@ -8067,6 +8072,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) hba->is_irq_enabled = true; } + ufs_bsg_attach_transport(hba); + err = scsi_add_host(host, hba->dev); if (err) { dev_err(hba->dev, "scsi_add_host failed\n"); -- 1.9.1
[PATCH 2/6] scsi: ufs: Add ufs-bsg module
A LLD companion for the ufs scsi transport. For now, just provide an API to instantiate the ufs transport, allocating and removing ufs-ports. As we are attaching the transport template to the scsi host that was already alocated for the ufs hba, we'll need to pay extra attention on where to call the provided API from ufshcd. For the time being, implements an empty bsg_request() - will add some more functionality in coming patches. Nonetheless, we reveal here the protocol we are planning to use: UFS Transport Protocol Transactions. UFS transactions consist of packets called UFS Protocol Information Units (UPIU). There are UPIU’s defined for UFS SCSI commands, responses, data in and data out, task management, utility functions, vendor functions, transaction synchronization and control, and more. By using UPIUs, we get access to the most fine-grained internals of this protocol, and able to communicate with the device in ways, that are sometimes beyond the capacity of the ufs driver. Signed-off-by: Avri Altman --- drivers/scsi/ufs/Makefile| 1 + drivers/scsi/ufs/ufs_bsg.c | 127 +++ drivers/scsi/ufs/ufs_bsg.h | 74 +++ include/uapi/scsi/scsi_bsg_ufs.h | 56 + 4 files changed, 258 insertions(+) create mode 100644 drivers/scsi/ufs/ufs_bsg.c create mode 100644 drivers/scsi/ufs/ufs_bsg.h create mode 100644 include/uapi/scsi/scsi_bsg_ufs.h diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile index 2c50f03..5227bfb 100644 --- a/drivers/scsi/ufs/Makefile +++ b/drivers/scsi/ufs/Makefile @@ -8,3 +8,4 @@ ufshcd-core-objs := ufshcd.o ufs-sysfs.o obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o +obj-$(CONFIG_SCSI_UFS_ATTRS) += ufs_bsg.o diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c new file mode 100644 index 000..71826ba --- /dev/null +++ b/drivers/scsi/ufs/ufs_bsg.c @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SSCSI transport companion for UFS HBA + * + * Copyright (C) 2018 Western Digital Corporation + */ +#include "ufs_bsg.h" + +struct ufs_bsg { + struct ufs_hba *hba; + struct ufs_port *port; +}; +static struct ufs_bsg *bsg_host; + +static int ufs_bsg_request(struct bsg_job *job); + +static struct scsi_transport_template *ufs_transport_template; +static struct ufs_function_template ufs_transport_functions = { + .bsg_request = ufs_bsg_request, +}; + +static int ufs_bsg_request(struct bsg_job *job) +{ + struct ufs_bsg_request *bsg_request = job->request; + struct ufs_bsg_reply *bsg_reply = job->reply; + int ret = -ENOTSUPP; + + bsg_reply->reply_payload_rcv_len = 0; + + /* Do Nothing for now */ + dev_err(job->dev, "unsupported message_code 0x%x\n", + bsg_request->msgcode); + + bsg_reply->result = ret; + if (ret) + job->reply_len = sizeof(uint32_t); + else + job->reply_len = sizeof(struct ufs_bsg_reply) + +bsg_reply->reply_payload_rcv_len; + + bsg_job_done(job, bsg_reply->result, bsg_reply->reply_payload_rcv_len); + + return ret; +} + +/** + * ufs_bsg_attach_transport - instantiate UFS transport template + * @hba: hba to attach the transport template to + * + * Call this before scsi_add_host() publishes it to the scsi midlayer + */ +void ufs_bsg_attach_transport(struct ufs_hba *hba) +{ + ufs_transport_template = + ufs_attach_transport(_transport_functions); + if (!ufs_transport_template) + return; + + hba->host->transportt = ufs_transport_template; +} + +/** + * ufs_bsg_remove - detach and remove the added ufs-ports + * @shost: scsi host that is parenting the ufs-ports + * + * Should be called when unloading the driver. + */ +void ufs_bsg_remove(struct Scsi_Host *shost) +{ + if (!bsg_host) + return; + + ufs_remove_host(shost); + + if (ufs_transport_template) { + ufs_release_transport(ufs_transport_template); + ufs_transport_template = NULL; + } + + kfree(bsg_host); +} + +/** + * ufs_bsg_probe - Add and report ufs device to ufs transport + * @hba: per adapter object + * + * Called during initial loading of the driver, and before scsi_scan_host. + * Should be called only if the attach phase was successful + */ + +int ufs_bsg_probe(struct ufs_hba *hba) +{ + struct ufs_port *port; + int ret; + + if (WARN_ON(!ufs_transport_template)) + return -ENODEV; + + bsg_host = kzalloc(sizeof(*bsg_host), GFP_KERNEL); + if (!bsg_host) { + ret = -ENOMEM; + goto out; + } + + port = ufs_port_alloc(hba->host); + if (!port) { + ret = -ENOMEM; + goto out_free_bsg_host; + } + + ret =
[PATCH 1/6] scsi: Add ufs transport class
Scsi transport is a framework that allow to send scsi commands to a non-scsi devices. Still, it is flexible enough to allow sending non-scsi commands as well. We will use this framework to manage ufs devices by sending UPIU transactions. In addition to the basic SCSI core objects this transport class introduces two additional (currently empty) class objects: “ufs-host” and “ufs-port”. There is only one “ufs-host” in the system, but can be more-than-one “ufs-ports”. Those classes are left empty for now, as ufs-sysfs already contains an abundant amount of attributes. A “ufs-port” is purely a software object. Evidently, the function template takes no port as an argument, as the driver has no concept of "port". We only need it as a hanging point in the bsg device tree, so maybe a more appropriate term would be: "ufs-bsg-dev-node". The ufs-port has a pretty lean structure. This is because we are using upiu transactions that contains the outmost detailed info, so we don't really need complex constructs to support it. The transport will keep track of its ufs-ports via its scsi host. Signed-off-by: Avri Altman --- drivers/scsi/Kconfig | 13 ++ drivers/scsi/Makefile | 1 + drivers/scsi/scsi_transport_ufs.c | 337 ++ include/scsi/scsi_transport_ufs.h | 40 + 4 files changed, 391 insertions(+) create mode 100644 drivers/scsi/scsi_transport_ufs.c create mode 100644 include/scsi/scsi_transport_ufs.h diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 8fc851a..9e99275 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -290,6 +290,19 @@ config SCSI_SAS_ATTRS If you wish to export transport-specific information about each attached SAS device to sysfs, say Y. +config SCSI_UFS_ATTRS + tristate "UFS Transport Attributes" + depends on SCSI + select BLK_DEV_BSGLIB + help + Scsi transport is a framework that allow to send scsi commands to + a non-scsi devices. Still, it is flexible enough to allow sending + non-scsi commands as well. We will use this framework to manage + ufs devices by sending UPIU transactions. + + If you wish to export transport-specific information about + each attached UFS device to sysfs, say Y. + source "drivers/scsi/libsas/Kconfig" config SCSI_SRP_ATTRS diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index d1bad43..e0efc06 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_SCSI_ISCSI_ATTRS)+= scsi_transport_iscsi.o obj-$(CONFIG_SCSI_SAS_ATTRS) += scsi_transport_sas.o obj-$(CONFIG_SCSI_SAS_LIBSAS) += libsas/ obj-$(CONFIG_SCSI_SRP_ATTRS) += scsi_transport_srp.o +obj-$(CONFIG_SCSI_UFS_ATTRS) += scsi_transport_ufs.o obj-$(CONFIG_SCSI_DH) += device_handler/ obj-$(CONFIG_LIBFC)+= libfc/ diff --git a/drivers/scsi/scsi_transport_ufs.c b/drivers/scsi/scsi_transport_ufs.c new file mode 100644 index 000..62aec49 --- /dev/null +++ b/drivers/scsi/scsi_transport_ufs.c @@ -0,0 +1,337 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SCSI UFS transport class + * + * Copyright (C) 2018 Western Digital Corporation + */ + + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + + +#define UFS_HOST_ATTRS 0 +#define UFS_PORT_ATTRS 0 + +struct ufs_internal { + struct scsi_transport_template t; + struct ufs_function_template *f; + + struct device_attribute *host_attrs[UFS_HOST_ATTRS + 1]; + struct device_attribute *port_attrs[UFS_PORT_ATTRS + 1]; + struct transport_container port_attr_cont; +}; +#define to_ufs_internal(tmpl) container_of(tmpl, struct ufs_internal, t) + +struct ufs_host_attrs { + atomic_t next_port_id; +}; +#define to_ufs_host_attrs(x) ((struct ufs_host_attrs *)(x)->shost_data) + +static int ufs_bsg_dispatch(struct bsg_job *job) +{ + struct Scsi_Host *shost = dev_to_shost(job->dev); + struct ufs_internal *i = to_ufs_internal(shost->transportt); + + /* check for payload_len when we'll support data transfer upiu */ + + return i->f->bsg_request(job); +} + +static int ufs_bsg_initialize(struct Scsi_Host *shost, struct ufs_port *port) +{ + struct request_queue *q; + struct ufs_internal *i = to_ufs_internal(shost->transportt); + + if (!i->f->bsg_request) { + pr_info("%s can't handle ufs requests\n", shost->hostt->name); + return 0; + } + + q = bsg_setup_queue(>dev, dev_name(>dev), + ufs_bsg_dispatch, 0, NULL); + if (IS_ERR(q)) + return PTR_ERR(q); + port->q = q; + + return 0; +} + +static int ufs_host_setup(struct transport_container *tc, struct device *dev, + struct device *cdev) +{ + struct Scsi_Host *shost = dev_to_shost(dev);
[PATCH 0/6] scsi: scsi transport for ufs devices
Here is a proposal to use the scsi transport subsystem to manage ufs devices. scsi transport is a framework that allow to send scsi commands to a non-scsi devices. Still, it is flexible enough to allow sending non-scsi commands as well. We will use this framework to manage ufs devices by sending UPIU transactions. We added a new scsi transport module, a ufs-bsg LLD companion, and some new API to the ufs driver. For the time being, we will not use it for data transfer, but just for device management per-se. We are planning to add this capability in the future. We tested it on a Hikey-960 platform with a V4.14 kernel, "modernized" by recent bsg and ufs patches. We also used a htc11 with a V4.4 kernel, but needed much more transport/bsg/ufs patches to make it relevant. Avri Altman (6): scsi: Add ufs transport class scsi: ufs: Add ufs-bsg module scsi: ufs: Instantiate a ufs transport if its available scsi: ufs: Add API to execute raw upiu commands scsi: ufs-bsg: Add support for raw upiu in ufs_bsg_request() scsi: ufs-bsg: Add support for uic commands in ufs_bsg_request() drivers/scsi/Kconfig | 13 ++ drivers/scsi/Makefile | 1 + drivers/scsi/scsi_transport_ufs.c | 337 ++ drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs_bsg.c| 292 + drivers/scsi/ufs/ufs_bsg.h| 76 + drivers/scsi/ufs/ufshcd.c | 232 ++ drivers/scsi/ufs/ufshcd.h | 4 + include/scsi/scsi_transport_ufs.h | 40 + include/uapi/scsi/scsi_bsg_ufs.h | 56 +++ 10 files changed, 1052 insertions(+) create mode 100644 drivers/scsi/scsi_transport_ufs.c create mode 100644 drivers/scsi/ufs/ufs_bsg.c create mode 100644 drivers/scsi/ufs/ufs_bsg.h create mode 100644 include/scsi/scsi_transport_ufs.h create mode 100644 include/uapi/scsi/scsi_bsg_ufs.h -- 1.9.1
RE: [RFC 0/6] scsi: scsi transport for ufs devices
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org > On Behalf Of Douglas Gilbert > Sent: Monday, July 30, 2018 9:53 PM > To: Hannes Reinecke ; Bart Van Assche > ; h...@lst.de; j...@linux.vnet.ibm.com; linux- > s...@vger.kernel.org; jthumsh...@suse.de; martin.peter...@oracle.com; > Avri Altman > Cc: Vinayak Holikatti ; Avi Shchislowski > ; Alex Lemberg ; > Stanislav Nijnikov ; subha...@codeaurora.org > Subject: Re: [RFC 0/6] scsi: scsi transport for ufs devices > > On 2018-07-30 02:13 PM, Hannes Reinecke wrote: > > On 07/30/2018 08:01 PM, Bart Van Assche wrote: > >> On Sun, 2018-07-29 at 16:33 +0300, Avri Altman wrote: > >>> Here is a proposal to use the scsi transport subsystem to manage > >>> ufs devices. > >>> > >>> scsi transport is a framework that allow to send scsi commands to > >>> a non-scsi devices. Still, it is flexible enough to allow > >>> sending non-scsi commands as well. We will use this framework to > >>> manage ufs devices by sending UPIU transactions. > >>> > >>> We added a new scsi transport module, a ufs-bsg LLD companion, > >>> and some new API to the ufs driver. > >> > >> My understanding is that all upstream code uses the bsg interface for > *SCSI* > >> commands. Sending UPIU commands over a bsg interface seems like > abuse of that > >> interface to me. Aren't you opening a can of worms with such an > approach? > >> > > I beg to disagree. > > > > bsg was precisely designed to handle non-SCSI commands, as this was the > main > > limitation of the original 'sg' interface. > > The original intention was to allow sending of transport frames for the > various > > SCSI transports (like FC or SAS), but there is no direct requirement for > > bsg to > > be limited to SCSI. > > Quite the contrary. > > I designed 'struct sg_io_v4' found in include/uapi/linux/bsg.h to handle > storage > related protocols, not just the SCSI command set. After the guard, the next > two fields in that structure are: > __u32 protocol; /* [i] 0 -> SCSI , */ > __u32 subprotocol; /* [i] 0 -> SCSI command, 1 -> SCSI task > management function, */ > > So there is lots of room for other protocols. Also the naming of fields is in > terms of request, response, data-in and data-out rather than SCSI command > specific terms (e.g. SCSI sense data maps to the response, while the SCSI > status is returned via a layered error mechanism). It also handles bidi data > transfers (which the sg_io_v3 interface does not). > > > NVMe didn't exist when struct sg_io_v4 was designed. If it was then I would > have made provisions for "metadata" transfers. One use for metadata > transfer might be to send protection information separately (i.e. as > metadata) rather than interleave with the user data as SCSI does. Is > NVMe metadata much used? And the extreme case: > bidi_data+bidi_metadata, > any examples of that? > > Doug Gilbert OK then. Let's give it a try. Will re-send it as patchset. Thanks, Avri
Re: [PATCH] scsi: csiostor: remove automatic irq affinity assignment
On 07/31/2018 05:07 PM, Varun Prakash wrote: > If number of interrupt vectors are more than num_online_cpus() > then pci_alloc_irq_vectors_affinity() assigns cpumask based > on num_possible_cpus() to the remaining vectors because of > this interrupt does not generate for these vectors. > > This patch fixes this issue by using pci_alloc_irq_vectors() > instead of pci_alloc_irq_vectors_affinity(). > > Signed-off-by: Varun Prakash > --- > drivers/scsi/csiostor/csio_isr.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/scsi/csiostor/csio_isr.c > b/drivers/scsi/csiostor/csio_isr.c > index 7c88147..8b92c59 100644 > --- a/drivers/scsi/csiostor/csio_isr.c > +++ b/drivers/scsi/csiostor/csio_isr.c > @@ -480,7 +480,6 @@ csio_enable_msix(struct csio_hw *hw) > int i, j, k, n, min, cnt; > int extra = CSIO_EXTRA_VECS; > struct csio_scsi_cpu_info *info; > - struct irq_affinity desc = { .pre_vectors = 2 }; > > min = hw->num_pports + extra; > cnt = hw->num_sqsets + extra; > @@ -491,8 +490,7 @@ csio_enable_msix(struct csio_hw *hw) > > csio_dbg(hw, "FW supp #niq:%d, trying %d msix's\n", hw->cfg_niq, cnt); > > - cnt = pci_alloc_irq_vectors_affinity(hw->pdev, min, cnt, > - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, ); > + cnt = pci_alloc_irq_vectors(hw->pdev, min, cnt, PCI_IRQ_MSIX); > if (cnt < 0) > return cnt; > > Hmm. That patch (and the reasoning leading up to it) really sound dodgy. I'd rather fix the interrupt affinity code to handle this case correctly. Thomas, can you help here? Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
[PATCH v2 12/16] qla2xxx: Fix race between switch cmd completion and timeout
From: Quinn Tran Fix race condition between switch cmd completion and timeout timer. Timer has popped triggers command free. On IOCB completion, stale sp point was reused. Instead, an abort will be sent to FW to nudge the command out of FW, where the normal completion will take place. RIP: 0010:qla2x00_chk_ms_status+0xf3/0x1b0 [qla2xxx] Call Trace: qla24xx_els_ct_entry.isra.15+0x1d4/0x2b0 [qla2xxx] qla24xx_msix_rsp_q+0x39/0xf0 [qla2xxx] qla24xx_process_response_queue+0xbc/0x2b0 [qla2xxx] qla24xx_msix_rsp_q+0x8a/0xf0 [qla2xxx] __handle_irq_event_percpu+0xa0/0x1f0 Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 1 + drivers/scsi/qla2xxx/qla_gbl.h | 2 +- drivers/scsi/qla2xxx/qla_init.c | 75 ++--- 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 40bcf938cf4f..0fc563572fad 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -313,6 +313,7 @@ struct srb_cmd { #define SRB_CRC_CTX_DMA_VALID BIT_2 /* DIF: context DMA valid */ #define SRB_CRC_PROT_DMA_VALID BIT_4 /* DIF: prot DMA valid */ #define SRB_CRC_CTX_DSD_VALID BIT_5 /* DIF: dsd_list valid */ +#define SRB_WAKEUP_ON_COMP BIT_6 /* To identify if a srb is of T10-CRC type. @sp => srb_t pointer */ #define IS_PROT_IO(sp) (sp->flags & SRB_CRC_CTX_DSD_VALID) diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 00fbd49a9a7a..6f2a37220a55 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -213,7 +213,7 @@ extern int qla24xx_post_upd_fcport_work(struct scsi_qla_host *, fc_port_t *); void qla2x00_handle_login_done_event(struct scsi_qla_host *, fc_port_t *, uint16_t *); int qla24xx_post_gnl_work(struct scsi_qla_host *, fc_port_t *); -int qla24xx_async_abort_cmd(srb_t *); +int qla24xx_async_abort_cmd(srb_t *, bool); int qla24xx_post_relogin_work(struct scsi_qla_host *vha); /* diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index e92f60a73bc3..b5c1a1dea087 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -50,16 +50,15 @@ qla2x00_sp_timeout(struct timer_list *t) { srb_t *sp = from_timer(sp, t, u.iocb_cmd.timer); struct srb_iocb *iocb; - scsi_qla_host_t *vha = sp->vha; struct req_que *req; unsigned long flags; - spin_lock_irqsave(>hw->hardware_lock, flags); - req = vha->hw->req_q_map[0]; + spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags); + req = sp->qpair->req; req->outstanding_cmds[sp->handle] = NULL; iocb = >u.iocb_cmd; + spin_unlock_irqrestore(sp->qpair->qp_lock_ptr, flags); iocb->timeout(sp); - spin_unlock_irqrestore(>hw->hardware_lock, flags); } void @@ -100,6 +99,8 @@ qla2x00_async_iocb_timeout(void *data) srb_t *sp = data; fc_port_t *fcport = sp->fcport; struct srb_iocb *lio = >u.iocb_cmd; + int rc, h; + unsigned long flags; if (fcport) { ql_dbg(ql_dbg_disc, fcport->vha, 0x2071, @@ -114,11 +115,26 @@ qla2x00_async_iocb_timeout(void *data) switch (sp->type) { case SRB_LOGIN_CMD: - /* Retry as needed. */ - lio->u.logio.data[0] = MBS_COMMAND_ERROR; - lio->u.logio.data[1] = lio->u.logio.flags & SRB_LOGIN_RETRIED ? - QLA_LOGIO_LOGIN_RETRIED : 0; - sp->done(sp, QLA_FUNCTION_TIMEOUT); + rc = qla24xx_async_abort_cmd(sp, false); + if (rc) { + /* Retry as needed. */ + lio->u.logio.data[0] = MBS_COMMAND_ERROR; + lio->u.logio.data[1] = + lio->u.logio.flags & SRB_LOGIN_RETRIED ? + QLA_LOGIO_LOGIN_RETRIED : 0; + spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags); + for (h = 1; h < sp->qpair->req->num_outstanding_cmds; + h++) { + if (sp->qpair->req->outstanding_cmds[h] == + sp) { + sp->qpair->req->outstanding_cmds[h] = + NULL; + break; + } + } + spin_unlock_irqrestore(sp->qpair->qp_lock_ptr, flags); + sp->done(sp, QLA_FUNCTION_TIMEOUT); + } break; case SRB_LOGOUT_CMD: case SRB_CT_PTHRU_CMD: @@ -127,7 +143,21 @@ qla2x00_async_iocb_timeout(void *data) case SRB_NACK_PRLI: case SRB_NACK_LOGO: case SRB_CTRL_VP: - sp->done(sp, QLA_FUNCTION_TIMEOUT); +
[PATCH v2 08/16] qla2xxx: Fix session state stuck in Get Port DB
From: Quinn Tran This patch sets discovery state back to GNL (Get Name List) when session is stuck at GPDB (Get Port DataBase). This will allow state machine to retry login and move session state ahead in discovery. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 4a89aa93c315..f94335301c70 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -1152,9 +1152,12 @@ void qla24xx_handle_gpdb_event(scsi_qla_host_t *vha, struct event_arg *ea) case PDS_PLOGI_COMPLETE: case PDS_PRLI_PENDING: case PDS_PRLI2_PENDING: - ql_dbg(ql_dbg_disc, vha, 0x20d5, "%s %d %8phC relogin needed\n", - __func__, __LINE__, fcport->port_name); - set_bit(RELOGIN_NEEDED, >dpc_flags); + /* Set discovery state back to GNL to Relogin attempt */ + if (qla_dual_mode_enabled(vha) || + qla_ini_mode_enabled(vha)) { + fcport->disc_state = DSC_GNL; + set_bit(RELOGIN_NEEDED, >dpc_flags); + } return; case PDS_LOGO_PENDING: case PDS_PORT_UNAVAILABLE: -- 2.12.0
[PATCH v2 11/16] qla2xxx: Fix Management Server NPort handle reservation logic
From: Quinn Tran After selecting the NPort handle/loop_id, set a bit in the loop_id_map to prevent others from selecting the same NPort handle. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_gbl.h | 1 + drivers/scsi/qla2xxx/qla_init.c | 28 drivers/scsi/qla2xxx/qla_mid.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 3 ++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index f68eb6096559..00fbd49a9a7a 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -118,6 +118,7 @@ extern int qla2x00_post_async_prlo_done_work(struct scsi_qla_host *, fc_port_t *, uint16_t *); int qla_post_iidma_work(struct scsi_qla_host *vha, fc_port_t *fcport); void qla_do_iidma_work(struct scsi_qla_host *vha, fc_port_t *fcport); +int qla2x00_reserve_mgmt_server_loop_id(scsi_qla_host_t *); /* * Global Data in qla_os.c source file. */ diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 7a984f5e6f11..e92f60a73bc3 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -5615,6 +5615,34 @@ qla2x00_find_new_loop_id(scsi_qla_host_t *vha, fc_port_t *dev) } +/* FW does not set aside Loop id for MGMT Server/FAh */ +int +qla2x00_reserve_mgmt_server_loop_id(scsi_qla_host_t *vha) +{ + int loop_id = FC_NO_LOOP_ID; + int lid = NPH_MGMT_SERVER - vha->vp_idx; + unsigned long flags; + struct qla_hw_data *ha = vha->hw; + + if (vha->vp_idx == 0) { + set_bit(NPH_MGMT_SERVER, ha->loop_id_map); + return NPH_MGMT_SERVER; + } + + /* pick id from high and work down to low */ + spin_lock_irqsave(>vport_slock, flags); + for (; lid > 0; lid--) { + if (!test_bit(lid, vha->hw->loop_id_map)) { + set_bit(lid, vha->hw->loop_id_map); + loop_id = lid; + break; + } + } + spin_unlock_irqrestore(>vport_slock, flags); + + return loop_id; +} + /* * qla2x00_fabric_login * Issue fabric login command. diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c index f6f0a759a7c2..14bc88bc4a5a 100644 --- a/drivers/scsi/qla2xxx/qla_mid.c +++ b/drivers/scsi/qla2xxx/qla_mid.c @@ -485,7 +485,7 @@ qla24xx_create_vhost(struct fc_vport *fc_vport) "Couldn't allocate vp_id.\n"); goto create_vhost_failed; } - vha->mgmt_svr_loop_id = NPH_MGMT_SERVER; + vha->mgmt_svr_loop_id = qla2x00_reserve_mgmt_server_loop_id(vha); vha->dpc_flags = 0L; diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 04e0c7f51e68..48d1003c8178 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -3048,7 +3048,8 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id) host = base_vha->host; base_vha->req = req; if (IS_QLA2XXX_MIDTYPE(ha)) - base_vha->mgmt_svr_loop_id = NPH_MGMT_SERVER; + base_vha->mgmt_svr_loop_id = + qla2x00_reserve_mgmt_server_loop_id(base_vha); else base_vha->mgmt_svr_loop_id = MANAGEMENT_SERVER + base_vha->vp_idx; -- 2.12.0
[PATCH v2 03/16] qla2xxx: Fix login retry count
From: Quinn Tran Login retry count was not properly decrementing, which lead to endless login retry. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_gs.c | 16 +++ drivers/scsi/qla2xxx/qla_init.c | 23 +++--- drivers/scsi/qla2xxx/qla_mbx.c| 1 + drivers/scsi/qla2xxx/qla_os.c | 90 --- drivers/scsi/qla2xxx/qla_target.c | 3 +- 5 files changed, 70 insertions(+), 63 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 4bc2b66b299f..5139a3577bb3 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3475,6 +3475,14 @@ void qla24xx_handle_gpnid_event(scsi_qla_host_t *vha, struct event_arg *ea) fcport->rscn_gen++; fcport->scan_state = QLA_FCPORT_FOUND; fcport->flags |= FCF_FABRIC_DEVICE; + if (fcport->login_retry == 0) { + fcport->login_retry = + vha->hw->login_retry_count; + ql_dbg(ql_dbg_disc, vha, 0x, + "Port login retry %8phN, lid 0x%04x cnt=%d.\n", + fcport->port_name, fcport->loop_id, + fcport->login_retry); + } switch (fcport->disc_state) { case DSC_LOGIN_COMPLETE: /* recheck session is still intact. */ @@ -3967,6 +3975,14 @@ void qla24xx_async_gnnft_done(scsi_qla_host_t *vha, srb_t *sp) } else { if (fcport->rscn_rcvd || fcport->disc_state != DSC_LOGIN_COMPLETE) { + if (fcport->login_retry == 0) { + fcport->login_retry = + vha->hw->login_retry_count; + ql_dbg(ql_dbg_disc, vha, 0x20a3, + "Port login retry %8phN, lid 0x%04x retry cnt=%d.\n", + fcport->port_name, fcport->loop_id, + fcport->login_retry); + } fcport->rscn_rcvd = 0; qla24xx_fcport_handle_login(vha, fcport); } diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 17c679102fcc..848ffd602e0b 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -213,8 +213,6 @@ qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport, if (fcport->fc4f_nvme) lio->u.logio.flags |= SRB_LOGIN_SKIP_PRLI; - if (data[1] & QLA_LOGIO_LOGIN_RETRIED) - lio->u.logio.flags |= SRB_LOGIN_RETRIED; } rval = qla2x00_start_sp(sp); @@ -485,7 +483,6 @@ static void qla24xx_handle_gnl_done_event(scsi_qla_host_t *vha, if (ea->rc) { /* rval */ if (fcport->login_retry == 0) { - fcport->login_retry = vha->hw->login_retry_count; ql_dbg(ql_dbg_disc, vha, 0x20de, "GNL failed Port login retry %8phN, retry cnt=%d.\n", fcport->port_name, fcport->login_retry); @@ -1255,11 +1252,10 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport) return 0; } - if (fcport->login_retry > 0) - fcport->login_retry--; switch (fcport->disc_state) { case DSC_DELETED: + fcport->login_retry--; wwn = wwn_to_u64(fcport->node_name); if (wwn == 0) { ql_dbg(ql_dbg_disc, vha, 0x, @@ -1272,6 +1268,7 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport) __func__, __LINE__, fcport->port_name); qla24xx_post_gnl_work(vha, fcport); } else { + fcport->login_retry--; qla_chk_n2n_b4_login(vha, fcport); } break; @@ -1288,6 +1285,7 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport) break; case DSC_LOGIN_FAILED: + fcport->login_retry--; ql_dbg(ql_dbg_disc, vha, 0x20d0, "%s %d %8phC post gidpn\n", __func__, __LINE__, fcport->port_name); @@ -1302,6 +1300,7 @@ int qla24xx_fcport_handle_login(struct scsi_qla_host *vha, fc_port_t *fcport) ql_dbg(ql_dbg_disc, vha, 0x20d1, "%s %d %8phC post adisc\n", __func__,
[PATCH v2 10/16] qla2xxx: Flush mailbox commands on chip reset
From: Quinn Tran Flush pending mailbox commands on chip reset. Wake up command that's waiting for an interrupt and wait for mailbox counters to go to zero. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 5 + drivers/scsi/qla2xxx/qla_init.c | 21 - drivers/scsi/qla2xxx/qla_mbx.c | 50 + drivers/scsi/qla2xxx/qla_os.c | 3 +++ 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index ee4d1f4fdf95..40bcf938cf4f 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -3598,6 +3598,7 @@ struct qla_hw_data { uint32_tdetected_lr_sfp:1; uint32_tusing_lr_setting:1; uint32_trida_fmt2:1; + uint32_tpurge_mbox:1; } flags; uint16_t max_exchg; @@ -3843,6 +3844,9 @@ struct qla_hw_data { int port_down_retry_count; uint8_t mbx_count; uint8_t aen_mbx_count; + atomic_tnum_pend_mbx_stage1; + atomic_tnum_pend_mbx_stage2; + atomic_tnum_pend_mbx_stage3; uint32_tlogin_retry_count; /* SNS command interfaces. */ @@ -4156,6 +4160,7 @@ struct qla_hw_data { struct work_struct board_disable; struct mr_data_fx00 mr; + uint32_t chip_reset; struct qlt_hw_data tgt; int allow_cna_fw_dump; diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index f94335301c70..7a984f5e6f11 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -6282,6 +6282,7 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha) ql_log(ql_log_info, vha, 0x00af, "Performing ISP error recovery - ha=%p.\n", ha); + ha->flags.purge_mbox = 1; /* For ISP82XX, reset_chip is just disabling interrupts. * Driver waits for the completion of the commands. * the interrupts need to be enabled. @@ -6296,13 +6297,31 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha) ha->current_topology = 0; ha->flags.fw_started = 0; ha->flags.fw_init_done = 0; - ha->base_qpair->chip_reset++; + ha->chip_reset++; + ha->base_qpair->chip_reset = ha->chip_reset; for (i = 0; i < ha->max_qpairs; i++) { if (ha->queue_pair_map[i]) ha->queue_pair_map[i]->chip_reset = ha->base_qpair->chip_reset; } + /* purge MBox commands */ + if (atomic_read(>num_pend_mbx_stage3)) { + clear_bit(MBX_INTR_WAIT, >mbx_cmd_flags); + complete(>mbx_intr_comp); + } + + i = 0; + while (atomic_read(>num_pend_mbx_stage3) || + atomic_read(>num_pend_mbx_stage2) || + atomic_read(>num_pend_mbx_stage1)) { + msleep(20); + i++; + if (i > 50) + break; + } + ha->flags.purge_mbox = 0; + atomic_set(>loop_down_timer, LOOP_DOWN_TIME); if (atomic_read(>loop_state) != LOOP_DOWN) { atomic_set(>loop_state, LOOP_DOWN); diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index 17537f0b3b54..10847cdca093 100644 --- a/drivers/scsi/qla2xxx/qla_mbx.c +++ b/drivers/scsi/qla2xxx/qla_mbx.c @@ -110,6 +110,7 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) unsigned long wait_time; struct qla_hw_data *ha = vha->hw; scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev); + u32 chip_reset; ql_dbg(ql_dbg_mbx, vha, 0x1000, "Entered %s.\n", __func__); @@ -140,7 +141,7 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) rval = QLA_SUCCESS; abort_active = test_bit(ABORT_ISP_ACTIVE, _vha->dpc_flags); - + chip_reset = ha->chip_reset; if (ha->flags.pci_channel_io_perm_failure) { ql_log(ql_log_warn, vha, 0x1003, @@ -167,6 +168,7 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) return QLA_FUNCTION_TIMEOUT; } + atomic_inc(>num_pend_mbx_stage1); /* * Wait for active mailbox commands to finish by waiting at most tov * seconds. This is to serialize actual issuing of mailbox cmds during @@ -177,8 +179,14 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) ql_log(ql_log_warn, vha, 0xd035, "Cmd access timeout, cmd=0x%x, Exiting.\n", mcp->mb[0]); + atomic_dec(>num_pend_mbx_stage1); return QLA_FUNCTION_TIMEOUT; } + atomic_dec(>num_pend_mbx_stage1); + if (ha->flags.purge_mbox || chip_reset != ha->chip_reset) { + rval = QLA_ABORTED;
[PATCH v2 02/16] qla2xxx: Fix N2N link re-connect
From: Quinn Tran In case of N2N connect, when sg_regset for bus/device/host was causing driver and firmware state to go out of sync. This patch fixes this link instablity when reconnect is attempted after link flap. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h| 3 +- drivers/scsi/qla2xxx/qla_init.c | 226 +- drivers/scsi/qla2xxx/qla_iocb.c | 15 ++- drivers/scsi/qla2xxx/qla_isr.c| 3 +- drivers/scsi/qla2xxx/qla_mbx.c| 27 + drivers/scsi/qla2xxx/qla_os.c | 5 + drivers/scsi/qla2xxx/qla_target.c | 9 ++ 7 files changed, 180 insertions(+), 108 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 9442e18aef6f..ee4d1f4fdf95 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -377,6 +377,7 @@ struct srb_iocb { #define SRB_LOGIN_COND_PLOGI BIT_1 #define SRB_LOGIN_SKIP_PRLIBIT_2 #define SRB_LOGIN_NVME_PRLIBIT_3 +#define SRB_LOGIN_PRLI_ONLYBIT_4 uint16_t data[2]; u32 iop[2]; } logio; @@ -4236,7 +4237,7 @@ typedef struct scsi_qla_host { #define FCOE_CTX_RESET_NEEDED 18 /* Initiate FCoE context reset */ #define MPI_RESET_NEEDED 19 /* Initiate MPI FW reset */ #define ISP_QUIESCE_NEEDED 20 /* Driver need some quiescence */ -#define FREE_BIT 21 +#define N2N_LINK_RESET 21 #define PORT_UPDATE_NEEDED 22 #define FX00_RESET_RECOVERY23 #define FX00_TARGET_SCAN 24 diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 1de78697dc0d..17c679102fcc 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -160,6 +160,22 @@ qla2x00_async_login_sp_done(void *ptr, int res) sp->free(sp); } +static inline bool +fcport_is_smaller(fc_port_t *fcport) +{ + if (wwn_to_u64(fcport->port_name) < + wwn_to_u64(fcport->vha->port_name)) + return true; + else + return false; +} + +static inline bool +fcport_is_bigger(fc_port_t *fcport) +{ + return !fcport_is_smaller(fcport); +} + int qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport, uint16_t *data) @@ -189,13 +205,18 @@ qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport, qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->done = qla2x00_async_login_sp_done; - lio->u.logio.flags |= SRB_LOGIN_COND_PLOGI; + if (N2N_TOPO(fcport->vha->hw) && fcport_is_bigger(fcport)) { + lio->u.logio.flags |= SRB_LOGIN_PRLI_ONLY; + } else { + lio->u.logio.flags |= SRB_LOGIN_COND_PLOGI; - if (fcport->fc4f_nvme) - lio->u.logio.flags |= SRB_LOGIN_SKIP_PRLI; + if (fcport->fc4f_nvme) + lio->u.logio.flags |= SRB_LOGIN_SKIP_PRLI; + + if (data[1] & QLA_LOGIO_LOGIN_RETRIED) + lio->u.logio.flags |= SRB_LOGIN_RETRIED; + } - if (data[1] & QLA_LOGIO_LOGIN_RETRIED) - lio->u.logio.flags |= SRB_LOGIN_RETRIED; rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) { fcport->flags |= FCF_LOGIN_NEEDED; @@ -497,15 +518,18 @@ static void qla24xx_handle_gnl_done_event(scsi_qla_host_t *vha, for (i = 0; i < n; i++) { e = >gnl.l[i]; wwn = wwn_to_u64(e->port_name); + id.b.domain = e->port_id[2]; + id.b.area = e->port_id[1]; + id.b.al_pa = e->port_id[0]; + id.b.rsvd_1 = 0; if (memcmp((u8 *), fcport->port_name, WWN_SIZE)) continue; + if (IS_SW_RESV_ADDR(id)) + continue; + found = 1; - id.b.domain = e->port_id[2]; - id.b.area = e->port_id[1]; - id.b.al_pa = e->port_id[0]; - id.b.rsvd_1 = 0; loop_id = le16_to_cpu(e->nport_handle); loop_id = (loop_id & 0x7fff); @@ -518,14 +542,18 @@ static void qla24xx_handle_gnl_done_event(scsi_qla_host_t *vha, fcport->d_id.b.domain, fcport->d_id.b.area, fcport->d_id.b.al_pa, loop_id, fcport->loop_id); - if ((id.b24 != fcport->d_id.b24) || - ((fcport->loop_id != FC_NO_LOOP_ID) && - (fcport->loop_id != loop_id))) { - ql_dbg(ql_dbg_disc, vha, 0x20e3, - "%s %d %8phC post del sess\n", - __func__, __LINE__, fcport->port_name); - qlt_schedule_sess_for_deletion(fcport); - return; + switch (fcport->disc_state) { + case DSC_DELETE_PEND: + case DSC_DELETED: +
[PATCH v2 07/16] qla2xxx: Fix redundant fc_rport registration
From: Quinn Tran Prevent multiple registration with transport layer for the same remote port. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 79738294b069..4a89aa93c315 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -5001,6 +5001,9 @@ qla2x00_reg_remote_port(scsi_qla_host_t *vha, fc_port_t *fcport) struct fc_rport *rport; unsigned long flags; + if (atomic_read(>state) == FCS_ONLINE) + return; + rport_ids.node_name = wwn_to_u64(fcport->node_name); rport_ids.port_name = wwn_to_u64(fcport->port_name); rport_ids.port_id = fcport->d_id.b.domain << 16 | @@ -5056,25 +5059,18 @@ qla2x00_update_fcport(scsi_qla_host_t *vha, fc_port_t *fcport) if (IS_SW_RESV_ADDR(fcport->d_id)) return; - ql_dbg(ql_dbg_disc, vha, 0x20ef, "%s %8phC\n", - __func__, fcport->port_name); - - if (IS_QLAFX00(vha->hw)) { - qla2x00_set_fcport_state(fcport, FCS_ONLINE); - } else { - fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT); - fcport->disc_state = DSC_LOGIN_COMPLETE; - fcport->deleted = 0; - fcport->logout_on_delete = 1; - fcport->login_retry = vha->hw->login_retry_count; - qla2x00_set_fcport_state(fcport, FCS_ONLINE); - } + fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT); + fcport->disc_state = DSC_LOGIN_COMPLETE; + fcport->deleted = 0; + fcport->logout_on_delete = 1; + fcport->login_retry = vha->hw->login_retry_count; - qla2x00_set_fcport_state(fcport, FCS_ONLINE); qla2x00_iidma_fcport(vha, fcport); if (fcport->fc4f_nvme) { qla_nvme_register_remote(vha, fcport); + fcport->disc_state = DSC_LOGIN_COMPLETE; + qla2x00_set_fcport_state(fcport, FCS_ONLINE); return; } @@ -5115,6 +5111,7 @@ qla2x00_update_fcport(scsi_qla_host_t *vha, fc_port_t *fcport) qla24xx_post_gpsc_work(vha, fcport); } } + qla2x00_set_fcport_state(fcport, FCS_ONLINE); } /* -- 2.12.0
[PATCH v2 00/16] qla2xxx: Updates for the driver
Hi Martin, This patch series addresses issue with N2N connection for FCP and FC-NVMe by moving login to state machine and handle various state change. Please apply this series to 4.19/scsi-queue at your earliest. Changes from v1 -> v2 o Rebased on 4.19/scsi-queue Thanks, Himanshu Himanshu Madhani (3): qla2xxx: Cleanup for N2N code qla2xxx: Fix stalled relogin qla2xxx: Update driver version to 10.00.00.08-k Quinn Tran (13): qla2xxx: Fix N2N link re-connect qla2xxx: Fix login retry count qla2xxx: Add longer window for Chip reset qla2xxx: Prevent SysFS access when chip is down qla2xxx: Silent erroneous message qla2xxx: Fix redundant fc_rport registration qla2xxx: Fix session state stuck in Get Port DB qla2xxx: Fix unintended Logout qla2xxx: Flush mailbox commands on chip reset qla2xxx: Fix Management Server NPort handle reservation logic qla2xxx: Fix race between switch cmd completion and timeout qla2xxx: Save frame payload size from ICB qla2xxx: Migrate NVME N2N handling into state machine drivers/scsi/qla2xxx/qla_attr.c| 33 +- drivers/scsi/qla2xxx/qla_dbg.c | 3 + drivers/scsi/qla2xxx/qla_def.h | 22 +- drivers/scsi/qla2xxx/qla_fw.h | 5 + drivers/scsi/qla2xxx/qla_gbl.h | 6 +- drivers/scsi/qla2xxx/qla_gs.c | 67 +++- drivers/scsi/qla2xxx/qla_init.c| 728 +++-- drivers/scsi/qla2xxx/qla_inline.h | 8 +- drivers/scsi/qla2xxx/qla_iocb.c| 153 +--- drivers/scsi/qla2xxx/qla_isr.c | 3 +- drivers/scsi/qla2xxx/qla_mbx.c | 155 +--- drivers/scsi/qla2xxx/qla_mid.c | 2 +- drivers/scsi/qla2xxx/qla_nvme.c| 15 +- drivers/scsi/qla2xxx/qla_nvme.h| 2 +- drivers/scsi/qla2xxx/qla_os.c | 133 --- drivers/scsi/qla2xxx/qla_target.c | 15 +- drivers/scsi/qla2xxx/qla_tmpl.c| 13 +- drivers/scsi/qla2xxx/qla_version.h | 2 +- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 3 - 19 files changed, 888 insertions(+), 480 deletions(-) -- 2.12.0
[PATCH v2 14/16] qla2xxx: Save frame payload size from ICB
From: Quinn Tran Save frame payload size from init control block. This field/data is used to register with switch data base. This allow the init control block temp buf to be reuse Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 1 + drivers/scsi/qla2xxx/qla_gs.c | 4 +--- drivers/scsi/qla2xxx/qla_init.c | 6 +++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 0fc563572fad..d1ff1b8f1ae5 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -3848,6 +3848,7 @@ struct qla_hw_data { atomic_tnum_pend_mbx_stage1; atomic_tnum_pend_mbx_stage2; atomic_tnum_pend_mbx_stage3; + uint16_tframe_payload_size; uint32_tlogin_retry_count; /* SNS command interfaces. */ diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 5139a3577bb3..af9a75df6700 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -2134,9 +2134,7 @@ qla2x00_fdmiv2_rhba(scsi_qla_host_t *vha) /* MAX CT Payload Length */ eiter = entries + size; eiter->type = cpu_to_be16(FDMI_HBA_MAXIMUM_CT_PAYLOAD_LENGTH); - eiter->a.max_ct_len = IS_FWI2_CAPABLE(ha) ? - le16_to_cpu(icb24->frame_payload_size) : - le16_to_cpu(ha->init_cb->frame_payload_size); + eiter->a.max_ct_len = cpu_to_be32(ha->frame_payload_size); eiter->a.max_ct_len = cpu_to_be32(eiter->a.max_ct_len); eiter->len = cpu_to_be16(4 + 4); size += 4 + 4; diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index a0c730bed312..924fe4265590 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -4417,7 +4417,7 @@ qla2x00_nvram_config(scsi_qla_host_t *vha) cnt = (uint8_t *)icb->reserved_3 - (uint8_t *)icb->add_firmware_options; while (cnt--) *dptr1++ = *dptr2++; - + ha->frame_payload_size = le16_to_cpu(icb->frame_payload_size); /* Use alternate WWN? */ if (nv->host_p[1] & BIT_7) { memcpy(icb->node_name, nv->alternate_node_name, WWN_SIZE); @@ -6892,7 +6892,7 @@ qla24xx_nvram_config(scsi_qla_host_t *vha) (uint8_t *)>interrupt_delay_timer; while (cnt--) *dptr1++ = *dptr2++; - + ha->frame_payload_size = le16_to_cpu(icb->frame_payload_size); /* * Setup driver NVRAM options. */ @@ -7948,7 +7948,7 @@ qla81xx_nvram_config(scsi_qla_host_t *vha) /* Use extended-initialization control block. */ memcpy(ha->ex_init_cb, >ex_version, sizeof(*ha->ex_init_cb)); - + ha->frame_payload_size = le16_to_cpu(icb->frame_payload_size); /* * Setup driver NVRAM options. */ -- 2.12.0
[PATCH v2 09/16] qla2xxx: Fix unintended Logout
From: Quinn Tran During normal IO, FW can return IO with 'port unavailble' status. Driver would send a LOGO to remote port for session resync. On an off chance, a PLOGI could arrive before sending the LOGO. This patch will skip sendiing LOGO, if a PLOGI just came in. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_target.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 4f9c315813df..f9036004f179 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -986,8 +986,9 @@ void qlt_free_session_done(struct work_struct *work) logo.id = sess->d_id; logo.cmd_count = 0; + if (!own) + qlt_send_first_logo(vha, ); sess->send_els_logo = 0; - qlt_send_first_logo(vha, ); } if (sess->logout_on_delete && sess->loop_id != FC_NO_LOOP_ID) { -- 2.12.0
[PATCH v2 15/16] qla2xxx: Migrate NVME N2N handling into state machine
From: Quinn Tran This patch fixes regression introduced for the N2N support for FC-NVMe. For FC-NVMe with N2N connection, instead of FW initiating the Login, Driver starts Login process. This patch migrate that new process from a standalone path into existing session management state machine. With this state change now driver will not wait for pull NPort ID from FW. Fixes: edd05de197592 ("scsi: qla2xxx: Changes to support N2N logins") Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 12 +- drivers/scsi/qla2xxx/qla_fw.h | 5 + drivers/scsi/qla2xxx/qla_gbl.h | 3 +- drivers/scsi/qla2xxx/qla_gs.c | 47 +--- drivers/scsi/qla2xxx/qla_init.c| 229 ++--- drivers/scsi/qla2xxx/qla_inline.h | 2 - drivers/scsi/qla2xxx/qla_iocb.c| 112 +- drivers/scsi/qla2xxx/qla_mbx.c | 77 ++--- drivers/scsi/qla2xxx/qla_nvme.c| 15 ++- drivers/scsi/qla2xxx/qla_nvme.h| 2 +- drivers/scsi/qla2xxx/qla_os.c | 32 -- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 3 - 12 files changed, 413 insertions(+), 126 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index d1ff1b8f1ae5..a03e12797f30 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -398,6 +398,8 @@ struct srb_iocb { struct completion comp; struct els_plogi_payload *els_plogi_pyld; struct els_plogi_payload *els_resp_pyld; + u32 tx_size; + u32 rx_size; dma_addr_t els_plogi_pyld_dma; dma_addr_t els_resp_pyld_dma; uint32_tfw_status[3]; @@ -2312,6 +2314,7 @@ enum fcport_mgt_event { FCME_ADISC_DONE, FCME_GNNID_DONE, FCME_GFPNID_DONE, + FCME_ELS_PLOGI_DONE, }; enum rscn_addr_format { @@ -2408,6 +2411,7 @@ typedef struct fc_port { struct ct_sns_desc ct_desc; enum discovery_state disc_state; enum login_state fw_login_state; + unsigned long dm_login_expire; unsigned long plogi_nack_done_deadline; u32 login_gen, last_login_gen; @@ -2418,7 +2422,8 @@ typedef struct fc_port { u8 iocb[IOCB_SIZE]; u8 current_login_state; u8 last_login_state; - struct completion n2n_done; + u16 n2n_link_reset_cnt; + u16 n2n_chip_reset; } fc_port_t; #define QLA_FCPORT_SCAN1 @@ -3228,6 +3233,7 @@ enum qla_work_type { QLA_EVT_GFPNID, QLA_EVT_SP_RETRY, QLA_EVT_IIDMA, + QLA_EVT_ELS_PLOGI, }; @@ -3600,6 +3606,7 @@ struct qla_hw_data { uint32_tusing_lr_setting:1; uint32_trida_fmt2:1; uint32_tpurge_mbox:1; + uint32_tn2n_bigger:1; } flags; uint16_t max_exchg; @@ -3908,6 +3915,9 @@ struct qla_hw_data { int exchoffld_size; int exchoffld_count; + /* n2n */ + struct els_plogi_payload plogi_els_payld; + void*swl; /* These are used by mailbox operations. */ diff --git a/drivers/scsi/qla2xxx/qla_fw.h b/drivers/scsi/qla2xxx/qla_fw.h index 5d8688e5bc7c..50c1e6c62e31 100644 --- a/drivers/scsi/qla2xxx/qla_fw.h +++ b/drivers/scsi/qla2xxx/qla_fw.h @@ -1366,6 +1366,11 @@ struct vp_rpt_id_entry_24xx { /* format 1 fabric */ uint8_t vpstat1_subcode; /* vp_status=1 subcode */ uint8_t flags; +#define TOPO_MASK 0xE +#define TOPO_FL0x2 +#define TOPO_N2N 0x4 +#define TOPO_F 0x6 + uint16_t fip_flags; uint8_t rsv2[12]; diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 6f2a37220a55..035ab18bd534 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -45,8 +45,7 @@ extern int qla2x00_fabric_login(scsi_qla_host_t *, fc_port_t *, uint16_t *); extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *); extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t); -extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, - port_id_t); +extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool); extern void qla2x00_update_fcports(scsi_qla_host_t *); diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index af9a75df6700..3a93a9389a49 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3384,19 +3384,40 @@ int qla24xx_post_gpnid_work(struct scsi_qla_host *vha, port_id_t *id) void qla24xx_sp_unmap(scsi_qla_host_t *vha, srb_t *sp) { - if (sp->u.iocb_cmd.u.ctarg.req) { -
[PATCH v2 04/16] qla2xxx: Add longer window for Chip reset
From: Quinn Tran The qla2x00_reset_active only cover the window of turning the chip off, add check to cover Chip on. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_dbg.c | 3 +++ drivers/scsi/qla2xxx/qla_tmpl.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index 5fd44c50bbac..c7533fa7f46e 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -1130,6 +1130,7 @@ qla24xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked) ha->fw_dump); goto qla24xx_fw_dump_failed; } + QLA_FW_STOPPED(ha); fw = >fw_dump->isp.isp24; qla2xxx_prep_dump(ha, ha->fw_dump); @@ -1384,6 +1385,7 @@ qla25xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked) ha->fw_dump); goto qla25xx_fw_dump_failed; } + QLA_FW_STOPPED(ha); fw = >fw_dump->isp.isp25; qla2xxx_prep_dump(ha, ha->fw_dump); ha->fw_dump->version = htonl(2); @@ -2036,6 +2038,7 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked) "request...\n", ha->fw_dump); goto qla83xx_fw_dump_failed; } + QLA_FW_STOPPED(ha); fw = >fw_dump->isp.isp83; qla2xxx_prep_dump(ha, ha->fw_dump); diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c index 731ca0d8520a..b170eb54aab5 100644 --- a/drivers/scsi/qla2xxx/qla_tmpl.c +++ b/drivers/scsi/qla2xxx/qla_tmpl.c @@ -1028,8 +1028,10 @@ qla27xx_fwdump(scsi_qla_host_t *vha, int hardware_locked) ql_log(ql_log_warn, vha, 0xd300, "Firmware has been previously dumped (%p)," " -- ignoring request\n", vha->hw->fw_dump); - else + else { + QLA_FW_STOPPED(vha->hw); qla27xx_execute_fwdt_template(vha); + } #ifndef __CHECKER__ if (!hardware_locked) -- 2.12.0
[PATCH v2 13/16] qla2xxx: Fix stalled relogin
This patch sets and clears FCF_ASYNC_{SENT|ACTIVE} flags to prevent stalling of relogin attempt. Once flag are correctly set/cleared, relogin timer can retry relogin attempt for driver to continue login. Fixes: fa83e65885b9 ("scsi: qla2xxx: ensure async flags are reset correctly") Cc: sta...@vger.kernel.org #4.17 Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index b5c1a1dea087..a0c730bed312 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -431,7 +431,7 @@ qla2x00_async_adisc_sp_done(void *ptr, int res) "Async done-%s res %x %8phC\n", sp->name, res, sp->fcport->port_name); - sp->fcport->flags &= ~FCF_ASYNC_SENT; + sp->fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE); memset(, 0, sizeof(ea)); ea.event = FCME_ADISC_DONE; diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index e1ff2e27e59f..ba14db8d0b8d 100644 --- a/drivers/scsi/qla2xxx/qla_iocb.c +++ b/drivers/scsi/qla2xxx/qla_iocb.c @@ -2634,6 +2634,7 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode, ql_dbg(ql_dbg_io, vha, 0x3073, "Enter: PLOGI portid=%06x\n", fcport->d_id.b24); + fcport->flags |= FCF_ASYNC_SENT; sp->type = SRB_ELS_DCMD; sp->name = "ELS_DCMD"; sp->fcport = fcport; -- 2.12.0
[PATCH v2 16/16] qla2xxx: Update driver version to 10.00.00.08-k
Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_version.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h index 1ad7582220c3..3850b28518e5 100644 --- a/drivers/scsi/qla2xxx/qla_version.h +++ b/drivers/scsi/qla2xxx/qla_version.h @@ -7,7 +7,7 @@ /* * Driver version */ -#define QLA2XXX_VERSION "10.00.00.07-k" +#define QLA2XXX_VERSION "10.00.00.08-k" #define QLA_DRIVER_MAJOR_VER 10 #define QLA_DRIVER_MINOR_VER 0 -- 2.12.0
[PATCH v2 01/16] qla2xxx: Cleanup for N2N code
Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 112 drivers/scsi/qla2xxx/qla_iocb.c | 25 - 2 files changed, 137 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 7b675243bd16..1de78697dc0d 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -4687,110 +4687,6 @@ qla2x00_configure_loop(scsi_qla_host_t *vha) } /* - * N2N Login - * Updates Fibre Channel Device Database with local loop devices. - * - * Input: - * ha = adapter block pointer. - * - * Returns: - */ -static int qla24xx_n2n_handle_login(struct scsi_qla_host *vha, - fc_port_t *fcport) -{ - struct qla_hw_data *ha = vha->hw; - int res = QLA_SUCCESS, rval; - int greater_wwpn = 0; - int logged_in = 0; - - if (ha->current_topology != ISP_CFG_N) - return res; - - if (wwn_to_u64(vha->port_name) > - wwn_to_u64(vha->n2n_port_name)) { - ql_dbg(ql_dbg_disc, vha, 0x2002, - "HBA WWPN is greater %llx > target %llx\n", - wwn_to_u64(vha->port_name), - wwn_to_u64(vha->n2n_port_name)); - greater_wwpn = 1; - fcport->d_id.b24 = vha->n2n_id; - } - - fcport->loop_id = vha->loop_id; - fcport->fc4f_nvme = 0; - fcport->query = 1; - - ql_dbg(ql_dbg_disc, vha, 0x4001, - "Initiate N2N login handler: HBA port_id=%06x loopid=%d\n", - fcport->d_id.b24, vha->loop_id); - - /* Fill in member data. */ - if (!greater_wwpn) { - rval = qla2x00_get_port_database(vha, fcport, 0); - ql_dbg(ql_dbg_disc, vha, 0x1051, - "Remote login-state (%x/%x) port_id=%06x loop_id=%x, rval=%d\n", - fcport->current_login_state, fcport->last_login_state, - fcport->d_id.b24, fcport->loop_id, rval); - - if (((fcport->current_login_state & 0xf) == 0x4) || - ((fcport->current_login_state & 0xf) == 0x6)) - logged_in = 1; - } - - if (logged_in || greater_wwpn) { - if (!vha->nvme_local_port && vha->flags.nvme_enabled) - qla_nvme_register_hba(vha); - - /* Set connected N_Port d_id */ - if (vha->flags.nvme_enabled) - fcport->fc4f_nvme = 1; - - fcport->scan_state = QLA_FCPORT_FOUND; - fcport->fw_login_state = DSC_LS_PORT_UNAVAIL; - fcport->disc_state = DSC_GNL; - fcport->n2n_flag = 1; - fcport->flags = 3; - vha->hw->flags.gpsc_supported = 0; - - if (greater_wwpn) { - ql_dbg(ql_dbg_disc, vha, 0x20e5, - "%s %d PLOGI ELS %8phC\n", - __func__, __LINE__, fcport->port_name); - - res = qla24xx_els_dcmd2_iocb(vha, ELS_DCMD_PLOGI, - fcport, fcport->d_id); - } - - if (res != QLA_SUCCESS) { - ql_log(ql_log_info, vha, 0xd04d, - "PLOGI Failed: portid=%06x - retrying\n", - fcport->d_id.b24); - res = QLA_SUCCESS; - } else { - /* State 0x6 means FCP PRLI complete */ - if ((fcport->current_login_state & 0xf) == 0x6) { - ql_dbg(ql_dbg_disc, vha, 0x2118, - "%s %d %8phC post GPDB work\n", - __func__, __LINE__, fcport->port_name); - fcport->chip_reset = - vha->hw->base_qpair->chip_reset; - qla24xx_post_gpdb_work(vha, fcport, 0); - } else { - ql_dbg(ql_dbg_disc, vha, 0x2118, - "%s %d %8phC post NVMe PRLI\n", - __func__, __LINE__, fcport->port_name); - qla24xx_post_prli_work(vha, fcport); - } - } - } else { - /* Wait for next database change */ - set_bit(N2N_LOGIN_NEEDED, >dpc_flags); - } - - return res; -} - -/* * qla2x00_configure_local_loop * Updates Fibre Channel Device Database with local loop devices. * @@ -4847,14 +4743,6 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha) } new_fcport->flags &= ~FCF_FABRIC_DEVICE; - /* Inititae N2N login. */ - if (test_and_clear_bit(N2N_LOGIN_NEEDED, >dpc_flags)) { - rval = qla24xx_n2n_handle_login(vha, new_fcport); - if (rval != QLA_SUCCESS) -