linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: hch@infradead.org (Christoph Hellwig)
Subject: [PATCH v1 6/7] nvme: Implement SED Unlock from suspend
Date: Thu, 17 Nov 2016 05:16:51 -0800	[thread overview]
Message-ID: <20161117131651.GB15852@infradead.org> (raw)
In-Reply-To: <1479338252-8777-7-git-send-email-scott.bauer@intel.com>

> +{
> +	struct opal_suspend_unlk ulk = { 0 };
> +	struct nvme_ns *ns;
> +	char diskname[DISK_NAME_LEN];
> +	mutex_lock(&ctrl->namespaces_mutex);
> +	if (list_empty(&ctrl->namespaces))
> +		goto out_no_namespace;
> +	ulk.data = ns =list_first_entry(&ctrl->namespaces, struct nvme_ns, list);

Simply grabbing a namespace without locking is broken.  That being
said..

> +	mutex_unlock(&ctrl->namespaces_mutex);
> +	snprintf(diskname, sizeof(diskname), "%sn%d",
> +		 dev_name(ctrl->device), ns->instance);
> +	ulk.name = diskname;
> +
> +	ulk.ops.send = nvme_sec_send;
> +	ulk.ops.recv = nvme_sec_recv;
> +	opal_unlock_from_suspend(&ulk);

passing a device _name_ to a lower level interface is even more
broken.  The Security Send/Receive commands operate on the NVMe
admin queue, and for SCSI and ATA that'd operate on the device.
So what we need to do here is to pass an object that identifies
the device - either the request queue if the opal code wants
to use it directly, or an opaqueue object that allows us to find
the nvme_ctrl.

Looking a bit at the actual low-level OPAL code it seems
like the driver should allocate the opal_dev structure when
setting up the device, and we should always pass it in.  But
maybe I need to understand that code a bit better first.

  reply	other threads:[~2016-11-17 13:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16 23:17 [PATCH v1 0/7] SED OPAL Library Scott Bauer
2016-11-16 23:17 ` [PATCH v1 1/7] Include: Add definitions for sed Scott Bauer
2016-11-17 15:22   ` Christoph Hellwig
2016-11-17 16:10     ` Scott Bauer
2016-11-16 23:17 ` [PATCH v1 2/7] lib: Add Sed-opal library Scott Bauer
2016-11-17  0:35   ` Keith Busch
2016-11-17 15:38   ` Christoph Hellwig
2016-11-16 23:17 ` [PATCH v1 3/7] lib: Add Sed to Kconfig and Makefile Scott Bauer
2016-11-16 23:17 ` [PATCH v1 4/7] include: Add sec_ops to block device operations Scott Bauer
2016-11-16 23:17 ` [PATCH v1 5/7] nvme: Implement SED Security Operations Scott Bauer
2016-11-17  0:09   ` Keith Busch
2016-11-16 23:17 ` [PATCH v1 6/7] nvme: Implement SED Unlock from suspend Scott Bauer
2016-11-17 13:16   ` Christoph Hellwig [this message]
2016-11-16 23:17 ` [PATCH v1 7/7] block: ioctl: Wire up Sed to block ioctls Scott Bauer
2016-11-17 13:12 ` [PATCH v1 0/7] SED OPAL Library Christoph Hellwig
2016-11-17 17:36   ` Scott Bauer
2016-11-17 18:21     ` Rafael Antognolli
2016-11-17 19:28     ` Christoph Hellwig
2016-11-17 19:33       ` Scott Bauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161117131651.GB15852@infradead.org \
    --to=hch@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).