From: Christoph Hellwig <hch@infradead.org> To: Scott Bauer <scott.bauer@intel.com> Cc: Christoph Hellwig <hch@infradead.org>, Keith Busch <keith.busch@intel.com>, linux-nvme@lists.infradead.org, Rafael.Antognolli@intel.com, axboe@fb.com, jonathan.derrick@intel.com, viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, sagi@grimberg.me Subject: Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code. Date: Wed, 21 Dec 2016 01:37:46 -0800 [thread overview] Message-ID: <20161221093746.GA32084@infradead.org> (raw) In-Reply-To: <20161220175239.GA2426@sbauer-Z170X-UD5> On Tue, Dec 20, 2016 at 10:52:41AM -0700, Scott Bauer wrote: > 2) Do we want to continue passing around a sed_context to the core? > Instead of a block_device struct like we did in previous versions. My personal preference would be the block_device, but in this case my preference collides with the (sad) reality, and that reality is that TCG did a major, major fuckup in specifyiong the NVMe interaction in the SIIG and does not treat TPer as per-Namespaces, as it does in SCSI for LUNs. So in NVMe all the interactions are on a per-controller level, not per namespaces. And the block_device maps to the namespacespace in NVMe. So I fear we'll have to keep the sed_context. > 2a) If we do wish to do wish to continue passng sed_contexts to the core I > have to add a new variable to the block_device structure for our sed_context. > Will this be acceptable? We have a lot less block_device structures, and they are limited to block devices, so I think it should be acceptable. > It wasn't acceptable for the file struct. The reason > I need a new variable in the struct is: > On the ioctl path, if we intercept the SED call in the block layer ioctl and > have the call chain be: > uland -> blk_ioctl -> sed_ioctl() -> sedcore -> sec_send/recv -> nvme But I really don't think we strictly need it for this reason. The above callchain should be: userland -> blk_ioctl -> nvme_ioctl -> sed_opal_iocl This allows passing the sec context including the submission method to sed_opal_ioctl from the driver and should not require anything in the block device. > then I need to be able to pass a sed_ctx struct in blk_ioctl to sed-ioctl and > the only way is to have it sitting in our block_device structure. > > The other way which was sorta nack'd last time is the following call chain: > uland -> blk_ioctl -> nvme_ioctl -> sed_ioctl -> sedcore -> send/rcv -> nvme Why was this nacked? This is still my preference, except that it could still be simplified a bit per the other comments, e.g. I don't think we really need a sec_core. > In this call chain in nvme_ioctl we have access to our block device struct > and from there we can do blkdev->bd_disk->private_data to get our ns and then > eventually our sed_ctx to pass to sed_ioctl. I could add the ns to the sec_data > pointer in sed_context. This would give us access to ns without having to pass > around a block device or store it anywhere. > 3) For NVMe we need access to our ns ID. It's in the block_device behind a few > pointers. What I can do is if we want to continue with the first ioctl path > described above is something like: The think is the ns does not matter for the OPAL device. I suspect the right answer is to pass 0xffffffff as the nsid. I need to verify this with some devices I should have access to, and you should verity it with Intel. I've also kicked a mail to some people involved with TCG to see if we can get some agreement on this behavior. If we can't get agreement on that we'll just need to have a variable in the nvme_ctrl that stores some valid nsid just for security send/receive. > While this works for NVMe I don't know if this is acceptible for *all* users. The other users are SCSI, ATA and eMMC. For SCSI the scsi_device is per-lun, and the SIIS specifies that all TPers are per-lun, so we'll simply have a context per scsi_device, otherwise it should work the same as NVMe. ATA doesn't support LUNs (except for ATAPI, which is SCSI over ATA), so it's even simpler. Additionally Linux hides ATA behind the SCSI layer, so the only thing we'll need to implement is the translation between the two. I don't really know enough about eMMC to comment on it.
WARNING: multiple messages have this Message-ID (diff)
From: hch@infradead.org (Christoph Hellwig) Subject: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code. Date: Wed, 21 Dec 2016 01:37:46 -0800 [thread overview] Message-ID: <20161221093746.GA32084@infradead.org> (raw) In-Reply-To: <20161220175239.GA2426@sbauer-Z170X-UD5> On Tue, Dec 20, 2016@10:52:41AM -0700, Scott Bauer wrote: > 2) Do we want to continue passing around a sed_context to the core? > Instead of a block_device struct like we did in previous versions. My personal preference would be the block_device, but in this case my preference collides with the (sad) reality, and that reality is that TCG did a major, major fuckup in specifyiong the NVMe interaction in the SIIG and does not treat TPer as per-Namespaces, as it does in SCSI for LUNs. So in NVMe all the interactions are on a per-controller level, not per namespaces. And the block_device maps to the namespacespace in NVMe. So I fear we'll have to keep the sed_context. > 2a) If we do wish to do wish to continue passng sed_contexts to the core I > have to add a new variable to the block_device structure for our sed_context. > Will this be acceptable? We have a lot less block_device structures, and they are limited to block devices, so I think it should be acceptable. > It wasn't acceptable for the file struct. The reason > I need a new variable in the struct is: > On the ioctl path, if we intercept the SED call in the block layer ioctl and > have the call chain be: > uland -> blk_ioctl -> sed_ioctl() -> sedcore -> sec_send/recv -> nvme But I really don't think we strictly need it for this reason. The above callchain should be: userland -> blk_ioctl -> nvme_ioctl -> sed_opal_iocl This allows passing the sec context including the submission method to sed_opal_ioctl from the driver and should not require anything in the block device. > then I need to be able to pass a sed_ctx struct in blk_ioctl to sed-ioctl and > the only way is to have it sitting in our block_device structure. > > The other way which was sorta nack'd last time is the following call chain: > uland -> blk_ioctl -> nvme_ioctl -> sed_ioctl -> sedcore -> send/rcv -> nvme Why was this nacked? This is still my preference, except that it could still be simplified a bit per the other comments, e.g. I don't think we really need a sec_core. > In this call chain in nvme_ioctl we have access to our block device struct > and from there we can do blkdev->bd_disk->private_data to get our ns and then > eventually our sed_ctx to pass to sed_ioctl. I could add the ns to the sec_data > pointer in sed_context. This would give us access to ns without having to pass > around a block device or store it anywhere. > 3) For NVMe we need access to our ns ID. It's in the block_device behind a few > pointers. What I can do is if we want to continue with the first ioctl path > described above is something like: The think is the ns does not matter for the OPAL device. I suspect the right answer is to pass 0xffffffff as the nsid. I need to verify this with some devices I should have access to, and you should verity it with Intel. I've also kicked a mail to some people involved with TCG to see if we can get some agreement on this behavior. If we can't get agreement on that we'll just need to have a variable in the nvme_ctrl that stores some valid nsid just for security send/receive. > While this works for NVMe I don't know if this is acceptible for *all* users. The other users are SCSI, ATA and eMMC. For SCSI the scsi_device is per-lun, and the SIIS specifies that all TPers are per-lun, so we'll simply have a context per scsi_device, otherwise it should work the same as NVMe. ATA doesn't support LUNs (except for ATAPI, which is SCSI over ATA), so it's even simpler. Additionally Linux hides ATA behind the SCSI layer, so the only thing we'll need to implement is the translation between the two. I don't really know enough about eMMC to comment on it.
next prev parent reply other threads:[~2016-12-21 9:37 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer 2016-12-19 19:35 ` Scott Bauer 2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer 2016-12-19 19:35 ` Scott Bauer 2016-12-20 6:46 ` Christoph Hellwig 2016-12-20 6:46 ` Christoph Hellwig 2016-12-25 14:15 ` Jethro Beekman 2016-12-25 14:15 ` Jethro Beekman 2016-12-27 22:14 ` Scott Bauer 2016-12-27 22:14 ` Scott Bauer 2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer 2016-12-19 19:35 ` Scott Bauer 2016-12-19 21:34 ` Keith Busch 2016-12-19 21:34 ` Keith Busch 2016-12-20 6:07 ` Christoph Hellwig 2016-12-20 6:07 ` Christoph Hellwig 2016-12-20 3:21 ` kbuild test robot 2016-12-20 3:21 ` kbuild test robot 2016-12-20 3:48 ` kbuild test robot 2016-12-20 3:48 ` kbuild test robot 2016-12-20 6:50 ` Al Viro 2016-12-20 6:50 ` Al Viro 2016-12-20 7:28 ` Christoph Hellwig 2016-12-20 7:28 ` Christoph Hellwig 2016-12-20 21:55 ` Scott Bauer 2016-12-20 21:55 ` Scott Bauer 2016-12-21 9:42 ` Christoph Hellwig 2016-12-21 9:42 ` Christoph Hellwig 2016-12-20 22:07 ` Jon Derrick 2016-12-20 22:07 ` Jon Derrick 2016-12-21 9:47 ` Christoph Hellwig 2016-12-21 9:47 ` Christoph Hellwig 2016-12-19 19:35 ` [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl Scott Bauer 2016-12-19 19:35 ` Scott Bauer 2016-12-20 6:21 ` Christoph Hellwig 2016-12-20 6:21 ` Christoph Hellwig 2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer 2016-12-19 19:35 ` Scott Bauer 2016-12-19 21:59 ` Keith Busch 2016-12-19 21:59 ` Keith Busch 2016-12-19 22:23 ` Scott Bauer 2016-12-19 22:23 ` Scott Bauer 2016-12-20 6:17 ` Christoph Hellwig 2016-12-20 6:17 ` Christoph Hellwig 2016-12-20 15:49 ` Keith Busch 2016-12-20 15:49 ` Keith Busch 2016-12-20 15:46 ` Christoph Hellwig 2016-12-20 15:46 ` Christoph Hellwig 2016-12-20 16:05 ` Scott Bauer 2016-12-20 16:05 ` Scott Bauer 2016-12-21 9:01 ` Christoph Hellwig 2016-12-21 9:01 ` Christoph Hellwig 2016-12-20 17:52 ` Scott Bauer 2016-12-20 17:52 ` Scott Bauer 2016-12-21 9:37 ` Christoph Hellwig [this message] 2016-12-21 9:37 ` Christoph Hellwig 2016-12-20 4:11 ` kbuild test robot 2016-12-20 4:11 ` kbuild test robot 2016-12-20 6:21 ` Christoph Hellwig 2016-12-20 6:21 ` Christoph Hellwig 2016-12-20 6:49 ` Christoph Hellwig 2016-12-20 6:49 ` Christoph Hellwig 2016-12-25 14:15 ` Jethro Beekman 2016-12-25 14:15 ` Jethro Beekman 2016-12-27 22:12 ` Scott Bauer 2016-12-27 22:12 ` Scott Bauer 2016-12-28 8:39 ` Christoph Hellwig 2016-12-28 8:39 ` Christoph Hellwig 2016-12-19 19:35 ` [PATCH v3 5/5] Maintainers: Add Information for SED Opal library Scott Bauer 2016-12-19 19:35 ` 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=20161221093746.GA32084@infradead.org \ --to=hch@infradead.org \ --cc=Rafael.Antognolli@intel.com \ --cc=axboe@fb.com \ --cc=jonathan.derrick@intel.com \ --cc=keith.busch@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=sagi@grimberg.me \ --cc=scott.bauer@intel.com \ --cc=viro@zeniv.linux.org.uk \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.