* [PATCH v2 0/2] virtio-scsi: Implement FC_HOST feature @ 2017-01-26 3:41 Fam Zheng 2017-01-26 3:41 ` [PATCH v2 1/2] virtio_scsi: Add fc_host definitions Fam Zheng 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng 0 siblings, 2 replies; 8+ messages in thread From: Fam Zheng @ 2017-01-26 3:41 UTC (permalink / raw) To: linux-kernel Cc: Paolo Bonzini, famz, linux-scsi, James E.J. Bottomley, Michael S. Tsirkin, Jason Wang, Martin K. Petersen, stefanha, virtualization v2: Fix endianness of WWNN/WWPN. [Paolo] This series implements the proposed fc_host feature of virtio-scsi. The first patch updates the data structure changes according to the spec proposal; the second patch actually implements the operations. Fam Zheng (2): virtio_scsi: Add fc_host definitions virtio_scsi: Implement fc_host drivers/scsi/virtio_scsi.c | 60 +++++++++++++++++++++++++++++++++++++++- include/uapi/linux/virtio_scsi.h | 6 ++++ 2 files changed, 65 insertions(+), 1 deletion(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] virtio_scsi: Add fc_host definitions 2017-01-26 3:41 [PATCH v2 0/2] virtio-scsi: Implement FC_HOST feature Fam Zheng @ 2017-01-26 3:41 ` Fam Zheng 2017-01-26 21:49 ` Michael S. Tsirkin 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng 1 sibling, 1 reply; 8+ messages in thread From: Fam Zheng @ 2017-01-26 3:41 UTC (permalink / raw) To: linux-kernel Cc: Paolo Bonzini, famz, linux-scsi, James E.J. Bottomley, Michael S. Tsirkin, Jason Wang, Martin K. Petersen, stefanha, virtualization Signed-off-by: Fam Zheng <famz@redhat.com> --- include/uapi/linux/virtio_scsi.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/uapi/linux/virtio_scsi.h b/include/uapi/linux/virtio_scsi.h index cc18ef8..a26fb31 100644 --- a/include/uapi/linux/virtio_scsi.h +++ b/include/uapi/linux/virtio_scsi.h @@ -113,6 +113,11 @@ struct virtio_scsi_config { __u16 max_channel; __u16 max_target; __u32 max_lun; + __u8 primary_wwpn[8]; + __u8 primary_wwnn[8]; + __u8 secondary_wwpn[8]; + __u8 secondary_wwnn[8]; + __u8 primary_active; } __attribute__((packed)); /* Feature Bits */ @@ -120,6 +125,7 @@ struct virtio_scsi_config { #define VIRTIO_SCSI_F_HOTPLUG 1 #define VIRTIO_SCSI_F_CHANGE 2 #define VIRTIO_SCSI_F_T10_PI 3 +#define VIRTIO_SCSI_F_FC_HOST 4 /* Response codes */ #define VIRTIO_SCSI_S_OK 0 -- 2.9.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] virtio_scsi: Add fc_host definitions 2017-01-26 3:41 ` [PATCH v2 1/2] virtio_scsi: Add fc_host definitions Fam Zheng @ 2017-01-26 21:49 ` Michael S. Tsirkin 0 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2017-01-26 21:49 UTC (permalink / raw) To: Fam Zheng Cc: linux-kernel, Paolo Bonzini, linux-scsi, James E.J. Bottomley, Jason Wang, Martin K. Petersen, stefanha, virtualization On Thu, Jan 26, 2017 at 11:41:08AM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > --- I pefer combining this with implementation, hard to reason about interface alone. > include/uapi/linux/virtio_scsi.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/uapi/linux/virtio_scsi.h b/include/uapi/linux/virtio_scsi.h > index cc18ef8..a26fb31 100644 > --- a/include/uapi/linux/virtio_scsi.h > +++ b/include/uapi/linux/virtio_scsi.h > @@ -113,6 +113,11 @@ struct virtio_scsi_config { > __u16 max_channel; > __u16 max_target; > __u32 max_lun; > + __u8 primary_wwpn[8]; > + __u8 primary_wwnn[8]; > + __u8 secondary_wwpn[8]; > + __u8 secondary_wwnn[8]; > + __u8 primary_active; Is this in fact a binary value? Also pls add padding to align on 8 byte boundary. > } __attribute__((packed)); > > /* Feature Bits */ > @@ -120,6 +125,7 @@ struct virtio_scsi_config { > #define VIRTIO_SCSI_F_HOTPLUG 1 > #define VIRTIO_SCSI_F_CHANGE 2 > #define VIRTIO_SCSI_F_T10_PI 3 > +#define VIRTIO_SCSI_F_FC_HOST 4 > > /* Response codes */ > #define VIRTIO_SCSI_S_OK 0 > -- > 2.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] virtio_scsi: Implement fc_host 2017-01-26 3:41 [PATCH v2 0/2] virtio-scsi: Implement FC_HOST feature Fam Zheng 2017-01-26 3:41 ` [PATCH v2 1/2] virtio_scsi: Add fc_host definitions Fam Zheng @ 2017-01-26 3:41 ` Fam Zheng 2017-01-26 8:34 ` Christoph Hellwig ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: Fam Zheng @ 2017-01-26 3:41 UTC (permalink / raw) To: linux-kernel Cc: Paolo Bonzini, famz, linux-scsi, James E.J. Bottomley, Michael S. Tsirkin, Jason Wang, Martin K. Petersen, stefanha, virtualization This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config fields and presenting them as sysfs fc_host attributes. The config change handler is added here because primary_active will toggle during migration. Signed-off-by: Fam Zheng <famz@redhat.com> --- drivers/scsi/virtio_scsi.c | 60 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index ec91bd0..1bb330c 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -28,6 +28,7 @@ #include <scsi/scsi_device.h> #include <scsi/scsi_cmnd.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_transport_fc.h> #include <linux/seqlock.h> #define VIRTIO_SCSI_MEMPOOL_SZ 64 @@ -795,6 +796,15 @@ static struct scsi_host_template virtscsi_host_template_multi = { .track_queue_depth = 1, }; +static struct fc_function_template virtscsi_fc_template = { + .show_host_node_name = 1, + .show_host_port_name = 1, + .show_host_port_type = 1, + .show_host_port_state = 1, +}; + +static struct scsi_transport_template *virtscsi_fc_transport_template; + #define virtscsi_config_get(vdev, fld) \ ({ \ typeof(((struct virtio_scsi_config *)0)->fld) __val; \ @@ -956,15 +966,42 @@ static int virtscsi_init(struct virtio_device *vdev, return err; } +static void virtscsi_update_fc_host_attrs(struct virtio_device *vdev) +{ + struct Scsi_Host *shost = vdev->priv; + u8 node_name[8], port_name[8]; + + if (virtscsi_config_get(vdev, primary_active)) { + virtio_cread_bytes(vdev, + offsetof(struct virtio_scsi_config, primary_wwnn), + &node_name, 8); + virtio_cread_bytes(vdev, + offsetof(struct virtio_scsi_config, primary_wwpn), + &port_name, 8); + } else { + virtio_cread_bytes(vdev, + offsetof(struct virtio_scsi_config, secondary_wwnn), + &node_name, 8); + virtio_cread_bytes(vdev, + offsetof(struct virtio_scsi_config, secondary_wwpn), + &port_name, 8); + } + fc_host_node_name(shost) = wwn_to_u64(node_name); + fc_host_port_name(shost) = wwn_to_u64(port_name); + fc_host_port_type(shost) = FC_PORTTYPE_NPORT; + fc_host_port_state(shost) = FC_PORTSTATE_ONLINE; +} + static int virtscsi_probe(struct virtio_device *vdev) { - struct Scsi_Host *shost; + struct Scsi_Host *shost = NULL; struct virtio_scsi *vscsi; int err; u32 sg_elems, num_targets; u32 cmd_per_lun; u32 num_queues; struct scsi_host_template *hostt; + bool fc_host_enabled; if (!vdev->config->get) { dev_err(&vdev->dev, "%s failure: config access disabled\n", @@ -987,6 +1024,9 @@ static int virtscsi_probe(struct virtio_device *vdev) if (!shost) return -ENOMEM; + fc_host_enabled = virtio_has_feature(vdev, VIRTIO_SCSI_F_FC_HOST); + if (fc_host_enabled) + shost->transportt = virtscsi_fc_transport_template; sg_elems = virtscsi_config_get(vdev, seg_max) ?: 1; shost->sg_tablesize = sg_elems; vscsi = shost_priv(shost); @@ -1032,6 +1072,9 @@ static int virtscsi_probe(struct virtio_device *vdev) if (err) goto scsi_add_host_failed; + if (fc_host_enabled) + virtscsi_update_fc_host_attrs(vdev); + virtio_device_ready(vdev); if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) @@ -1098,6 +1141,11 @@ static int virtscsi_restore(struct virtio_device *vdev) } #endif +static void virtscsi_config_changed(struct virtio_device *vdev) +{ + virtscsi_update_fc_host_attrs(vdev); +} + static struct virtio_device_id id_table[] = { { VIRTIO_ID_SCSI, VIRTIO_DEV_ANY_ID }, { 0 }, @@ -1109,6 +1157,7 @@ static unsigned int features[] = { #ifdef CONFIG_BLK_DEV_INTEGRITY VIRTIO_SCSI_F_T10_PI, #endif + VIRTIO_SCSI_F_FC_HOST, }; static struct virtio_driver virtio_scsi_driver = { @@ -1123,12 +1172,20 @@ static struct virtio_driver virtio_scsi_driver = { .restore = virtscsi_restore, #endif .remove = virtscsi_remove, + .config_changed = virtscsi_config_changed, }; static int __init init(void) { int ret = -ENOMEM; + virtscsi_fc_transport_template = + fc_attach_transport(&virtscsi_fc_template); + if (!virtscsi_fc_transport_template) { + pr_err("fc_attach_transport() failed\n"); + goto error; + } + virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0); if (!virtscsi_cmd_cache) { pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n"); @@ -1176,6 +1233,7 @@ static int __init init(void) static void __exit fini(void) { + fc_release_transport(virtscsi_fc_transport_template); unregister_virtio_driver(&virtio_scsi_driver); cpuhp_remove_multi_state(virtioscsi_online); cpuhp_remove_multi_state(CPUHP_VIRT_SCSI_DEAD); -- 2.9.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng @ 2017-01-26 8:34 ` Christoph Hellwig 2017-01-26 8:55 ` kbuild test robot 2017-01-26 22:06 ` Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2017-01-26 8:34 UTC (permalink / raw) To: Fam Zheng Cc: linux-kernel, Paolo Bonzini, linux-scsi, James E.J. Bottomley, Michael S. Tsirkin, Jason Wang, Martin K. Petersen, stefanha, virtualization, Cathy Avery On Thu, Jan 26, 2017 at 11:41:09AM +0800, Fam Zheng wrote: > This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config > fields and presenting them as sysfs fc_host attributes. The config > change handler is added here because primary_active will toggle during > migration. This needs a way, way better description on why the heck we would even bother with something odd like this. Also please coordinate with Cathy's "lightweight" FC attrs for storsvcs as the code seems to be clearly following the "lead" set by Hyper-V. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng 2017-01-26 8:34 ` Christoph Hellwig @ 2017-01-26 8:55 ` kbuild test robot 2017-01-26 22:06 ` Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: kbuild test robot @ 2017-01-26 8:55 UTC (permalink / raw) To: Fam Zheng Cc: kbuild-all, linux-kernel, Paolo Bonzini, famz, linux-scsi, James E.J. Bottomley, Michael S. Tsirkin, Jason Wang, Martin K. Petersen, stefanha, virtualization [-- Attachment #1: Type: text/plain, Size: 858 bytes --] Hi Fam, [auto build test ERROR on linus/master] [also build test ERROR on v4.10-rc5 next-20170125] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Fam-Zheng/virtio-scsi-Implement-FC_HOST-feature/20170126-114655 config: i386-randconfig-h0-01261251 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> ERROR: "fc_release_transport" [drivers/scsi/virtio_scsi.ko] undefined! >> ERROR: "fc_attach_transport" [drivers/scsi/virtio_scsi.ko] undefined! --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28153 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng 2017-01-26 8:34 ` Christoph Hellwig 2017-01-26 8:55 ` kbuild test robot @ 2017-01-26 22:06 ` Michael S. Tsirkin 2017-01-27 8:43 ` Paolo Bonzini 2 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2017-01-26 22:06 UTC (permalink / raw) To: Fam Zheng Cc: linux-kernel, Paolo Bonzini, linux-scsi, James E.J. Bottomley, Jason Wang, Martin K. Petersen, stefanha, virtualization On Thu, Jan 26, 2017 at 11:41:09AM +0800, Fam Zheng wrote: > This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config > fields and presenting them as sysfs fc_host attributes. The config > change handler is added here because primary_active will toggle during > migration. Looks like there's active discussion on virtio tc mailing list. It's ok to post patches meanwhile but best as RFC, and repost after controversy is resolved. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > drivers/scsi/virtio_scsi.c | 60 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 59 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c > index ec91bd0..1bb330c 100644 > --- a/drivers/scsi/virtio_scsi.c > +++ b/drivers/scsi/virtio_scsi.c > @@ -28,6 +28,7 @@ > #include <scsi/scsi_device.h> > #include <scsi/scsi_cmnd.h> > #include <scsi/scsi_tcq.h> > +#include <scsi/scsi_transport_fc.h> > #include <linux/seqlock.h> > > #define VIRTIO_SCSI_MEMPOOL_SZ 64 > @@ -795,6 +796,15 @@ static struct scsi_host_template virtscsi_host_template_multi = { > .track_queue_depth = 1, > }; > > +static struct fc_function_template virtscsi_fc_template = { > + .show_host_node_name = 1, > + .show_host_port_name = 1, > + .show_host_port_type = 1, > + .show_host_port_state = 1, > +}; > + > +static struct scsi_transport_template *virtscsi_fc_transport_template; > + > #define virtscsi_config_get(vdev, fld) \ > ({ \ > typeof(((struct virtio_scsi_config *)0)->fld) __val; \ > @@ -956,15 +966,42 @@ static int virtscsi_init(struct virtio_device *vdev, > return err; > } > > +static void virtscsi_update_fc_host_attrs(struct virtio_device *vdev) > +{ > + struct Scsi_Host *shost = vdev->priv; > + u8 node_name[8], port_name[8]; > + > + if (virtscsi_config_get(vdev, primary_active)) { > + virtio_cread_bytes(vdev, > + offsetof(struct virtio_scsi_config, primary_wwnn), > + &node_name, 8); > + virtio_cread_bytes(vdev, > + offsetof(struct virtio_scsi_config, primary_wwpn), > + &port_name, 8); > + } else { > + virtio_cread_bytes(vdev, > + offsetof(struct virtio_scsi_config, secondary_wwnn), > + &node_name, 8); > + virtio_cread_bytes(vdev, > + offsetof(struct virtio_scsi_config, secondary_wwpn), > + &port_name, 8); > + } This is racy, isn't it? You need to wrap this in a generation check otherwise read can race with primary_active changing. And you might want a wrapper to virtio_cread_bytes that does not include generation check. > + fc_host_node_name(shost) = wwn_to_u64(node_name); > + fc_host_port_name(shost) = wwn_to_u64(port_name); > + fc_host_port_type(shost) = FC_PORTTYPE_NPORT; > + fc_host_port_state(shost) = FC_PORTSTATE_ONLINE; > +} > + > static int virtscsi_probe(struct virtio_device *vdev) > { > - struct Scsi_Host *shost; > + struct Scsi_Host *shost = NULL; > struct virtio_scsi *vscsi; > int err; > u32 sg_elems, num_targets; > u32 cmd_per_lun; > u32 num_queues; > struct scsi_host_template *hostt; > + bool fc_host_enabled; > > if (!vdev->config->get) { > dev_err(&vdev->dev, "%s failure: config access disabled\n", > @@ -987,6 +1024,9 @@ static int virtscsi_probe(struct virtio_device *vdev) > if (!shost) > return -ENOMEM; > > + fc_host_enabled = virtio_has_feature(vdev, VIRTIO_SCSI_F_FC_HOST); > + if (fc_host_enabled) > + shost->transportt = virtscsi_fc_transport_template; > sg_elems = virtscsi_config_get(vdev, seg_max) ?: 1; > shost->sg_tablesize = sg_elems; > vscsi = shost_priv(shost); > @@ -1032,6 +1072,9 @@ static int virtscsi_probe(struct virtio_device *vdev) > if (err) > goto scsi_add_host_failed; > > + if (fc_host_enabled) > + virtscsi_update_fc_host_attrs(vdev); > + > virtio_device_ready(vdev); > > if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) > @@ -1098,6 +1141,11 @@ static int virtscsi_restore(struct virtio_device *vdev) > } > #endif > > +static void virtscsi_config_changed(struct virtio_device *vdev) > +{ This is called unconditionally here, will access invalid config fields if feature is off. In fact this is wasting an MSIX vector when feature is not negotiated. No easy way not to, but best document this in a code comment. > + virtscsi_update_fc_host_attrs(vdev); > +} > + > static struct virtio_device_id id_table[] = { > { VIRTIO_ID_SCSI, VIRTIO_DEV_ANY_ID }, > { 0 }, > @@ -1109,6 +1157,7 @@ static unsigned int features[] = { > #ifdef CONFIG_BLK_DEV_INTEGRITY > VIRTIO_SCSI_F_T10_PI, > #endif > + VIRTIO_SCSI_F_FC_HOST, > }; > > static struct virtio_driver virtio_scsi_driver = { > @@ -1123,12 +1172,20 @@ static struct virtio_driver virtio_scsi_driver = { > .restore = virtscsi_restore, > #endif > .remove = virtscsi_remove, > + .config_changed = virtscsi_config_changed, > }; > > static int __init init(void) > { > int ret = -ENOMEM; > > + virtscsi_fc_transport_template = > + fc_attach_transport(&virtscsi_fc_template); > + if (!virtscsi_fc_transport_template) { > + pr_err("fc_attach_transport() failed\n"); > + goto error; > + } > + > virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0); > if (!virtscsi_cmd_cache) { > pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n"); > @@ -1176,6 +1233,7 @@ static int __init init(void) > > static void __exit fini(void) > { > + fc_release_transport(virtscsi_fc_transport_template); > unregister_virtio_driver(&virtio_scsi_driver); > cpuhp_remove_multi_state(virtioscsi_online); > cpuhp_remove_multi_state(CPUHP_VIRT_SCSI_DEAD); > -- > 2.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host 2017-01-26 22:06 ` Michael S. Tsirkin @ 2017-01-27 8:43 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2017-01-27 8:43 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Fam Zheng, linux-kernel, linux-scsi, James E.J. Bottomley, Jason Wang, Martin K. Petersen, stefanha, virtualization ----- Original Message ----- > From: "Michael S. Tsirkin" <mst@redhat.com> > To: "Fam Zheng" <famz@redhat.com> > Cc: linux-kernel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, linux-scsi@vger.kernel.org, "James E.J. > Bottomley" <jejb@linux.vnet.ibm.com>, "Jason Wang" <jasowang@redhat.com>, "Martin K. Petersen" > <martin.petersen@oracle.com>, stefanha@redhat.com, virtualization@lists.linux-foundation.org > Sent: Thursday, January 26, 2017 11:06:27 PM > Subject: Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host > > On Thu, Jan 26, 2017 at 11:41:09AM +0800, Fam Zheng wrote: > > This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config > > fields and presenting them as sysfs fc_host attributes. The config > > change handler is added here because primary_active will toggle during > > migration. > > Looks like there's active discussion on virtio tc mailing list. > It's ok to post patches meanwhile but best as RFC, > and repost after controversy is resolved. Discussion on the TC mailing list was not about the merit of the feature, only about the timing of the vote. Paolo > > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > drivers/scsi/virtio_scsi.c | 60 > > +++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 59 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c > > index ec91bd0..1bb330c 100644 > > --- a/drivers/scsi/virtio_scsi.c > > +++ b/drivers/scsi/virtio_scsi.c > > @@ -28,6 +28,7 @@ > > #include <scsi/scsi_device.h> > > #include <scsi/scsi_cmnd.h> > > #include <scsi/scsi_tcq.h> > > +#include <scsi/scsi_transport_fc.h> > > #include <linux/seqlock.h> > > > > #define VIRTIO_SCSI_MEMPOOL_SZ 64 > > @@ -795,6 +796,15 @@ static struct scsi_host_template > > virtscsi_host_template_multi = { > > .track_queue_depth = 1, > > }; > > > > +static struct fc_function_template virtscsi_fc_template = { > > + .show_host_node_name = 1, > > + .show_host_port_name = 1, > > + .show_host_port_type = 1, > > + .show_host_port_state = 1, > > +}; > > + > > +static struct scsi_transport_template *virtscsi_fc_transport_template; > > + > > #define virtscsi_config_get(vdev, fld) \ > > ({ \ > > typeof(((struct virtio_scsi_config *)0)->fld) __val; \ > > @@ -956,15 +966,42 @@ static int virtscsi_init(struct virtio_device *vdev, > > return err; > > } > > > > +static void virtscsi_update_fc_host_attrs(struct virtio_device *vdev) > > +{ > > + struct Scsi_Host *shost = vdev->priv; > > + u8 node_name[8], port_name[8]; > > + > > + if (virtscsi_config_get(vdev, primary_active)) { > > + virtio_cread_bytes(vdev, > > + offsetof(struct virtio_scsi_config, primary_wwnn), > > + &node_name, 8); > > + virtio_cread_bytes(vdev, > > + offsetof(struct virtio_scsi_config, primary_wwpn), > > + &port_name, 8); > > + } else { > > + virtio_cread_bytes(vdev, > > + offsetof(struct virtio_scsi_config, secondary_wwnn), > > + &node_name, 8); > > + virtio_cread_bytes(vdev, > > + offsetof(struct virtio_scsi_config, secondary_wwpn), > > + &port_name, 8); > > + } > > This is racy, isn't it? You need to wrap this in a generation check > otherwise read can race with primary_active changing. > And you might want a wrapper to virtio_cread_bytes that does not > include generation check. > > > + fc_host_node_name(shost) = wwn_to_u64(node_name); > > + fc_host_port_name(shost) = wwn_to_u64(port_name); > > + fc_host_port_type(shost) = FC_PORTTYPE_NPORT; > > + fc_host_port_state(shost) = FC_PORTSTATE_ONLINE; > > +} > > + > > static int virtscsi_probe(struct virtio_device *vdev) > > { > > - struct Scsi_Host *shost; > > + struct Scsi_Host *shost = NULL; > > struct virtio_scsi *vscsi; > > int err; > > u32 sg_elems, num_targets; > > u32 cmd_per_lun; > > u32 num_queues; > > struct scsi_host_template *hostt; > > + bool fc_host_enabled; > > > > if (!vdev->config->get) { > > dev_err(&vdev->dev, "%s failure: config access disabled\n", > > @@ -987,6 +1024,9 @@ static int virtscsi_probe(struct virtio_device *vdev) > > if (!shost) > > return -ENOMEM; > > > > + fc_host_enabled = virtio_has_feature(vdev, VIRTIO_SCSI_F_FC_HOST); > > + if (fc_host_enabled) > > + shost->transportt = virtscsi_fc_transport_template; > > sg_elems = virtscsi_config_get(vdev, seg_max) ?: 1; > > shost->sg_tablesize = sg_elems; > > vscsi = shost_priv(shost); > > @@ -1032,6 +1072,9 @@ static int virtscsi_probe(struct virtio_device *vdev) > > if (err) > > goto scsi_add_host_failed; > > > > + if (fc_host_enabled) > > + virtscsi_update_fc_host_attrs(vdev); > > + > > virtio_device_ready(vdev); > > > > if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) > > @@ -1098,6 +1141,11 @@ static int virtscsi_restore(struct virtio_device > > *vdev) > > } > > #endif > > > > +static void virtscsi_config_changed(struct virtio_device *vdev) > > +{ > > This is called unconditionally here, will access > invalid config fields if feature is off. > In fact this is wasting an MSIX vector when feature is not > negotiated. No easy way not to, but best document this > in a code comment. > > > + virtscsi_update_fc_host_attrs(vdev); > > +} > > + > > static struct virtio_device_id id_table[] = { > > { VIRTIO_ID_SCSI, VIRTIO_DEV_ANY_ID }, > > { 0 }, > > @@ -1109,6 +1157,7 @@ static unsigned int features[] = { > > #ifdef CONFIG_BLK_DEV_INTEGRITY > > VIRTIO_SCSI_F_T10_PI, > > #endif > > + VIRTIO_SCSI_F_FC_HOST, > > }; > > > > static struct virtio_driver virtio_scsi_driver = { > > @@ -1123,12 +1172,20 @@ static struct virtio_driver virtio_scsi_driver = { > > .restore = virtscsi_restore, > > #endif > > .remove = virtscsi_remove, > > + .config_changed = virtscsi_config_changed, > > }; > > > > static int __init init(void) > > { > > int ret = -ENOMEM; > > > > + virtscsi_fc_transport_template = > > + fc_attach_transport(&virtscsi_fc_template); > > + if (!virtscsi_fc_transport_template) { > > + pr_err("fc_attach_transport() failed\n"); > > + goto error; > > + } > > + > > virtscsi_cmd_cache = KMEM_CACHE(virtio_scsi_cmd, 0); > > if (!virtscsi_cmd_cache) { > > pr_err("kmem_cache_create() for virtscsi_cmd_cache failed\n"); > > @@ -1176,6 +1233,7 @@ static int __init init(void) > > > > static void __exit fini(void) > > { > > + fc_release_transport(virtscsi_fc_transport_template); > > unregister_virtio_driver(&virtio_scsi_driver); > > cpuhp_remove_multi_state(virtioscsi_online); > > cpuhp_remove_multi_state(CPUHP_VIRT_SCSI_DEAD); > > -- > > 2.9.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-27 8:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-26 3:41 [PATCH v2 0/2] virtio-scsi: Implement FC_HOST feature Fam Zheng 2017-01-26 3:41 ` [PATCH v2 1/2] virtio_scsi: Add fc_host definitions Fam Zheng 2017-01-26 21:49 ` Michael S. Tsirkin 2017-01-26 3:41 ` [PATCH v2 2/2] virtio_scsi: Implement fc_host Fam Zheng 2017-01-26 8:34 ` Christoph Hellwig 2017-01-26 8:55 ` kbuild test robot 2017-01-26 22:06 ` Michael S. Tsirkin 2017-01-27 8:43 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).