| Message ID | 20200618024736.97207-1-satyat@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Inline Encryption support for UFS | expand |
Avri, > This patch series adds support for inline encryption to UFS using > the inline encryption support in the block layer. It follows the JEDEC > UFSHCI v2.1 specification, which defines inline encryption for UFS. I'd appreciate it if you could review this series. Thanks!
> Avri, > > > This patch series adds support for inline encryption to UFS using > > the inline encryption support in the block layer. It follows the JEDEC > > UFSHCI v2.1 specification, which defines inline encryption for UFS. > > I'd appreciate it if you could review this series. I need help doing it. I will ask around. Thanks, Avri
+Alim & Asutosh Hi Satya, > > Avri, > > > This patch series adds support for inline encryption to UFS using > > the inline encryption support in the block layer. It follows the JEDEC > > UFSHCI v2.1 specification, which defines inline encryption for UFS. > > I'd appreciate it if you could review this series. > > Thanks! > > -- > Martin K. Petersen Oracle Linux Engineering A quick question and a comment: Does the IE infrastructure that you've added to the block layer invented for ufs? Do you see other devices using it in the future? Today, chipset vendors are using a different scheme for their IE. Need their ack before reviewing your patches. Thanks, Avri
> +Alim & Asutosh > > Hi Satya, > > > > > Avri, > > > > > This patch series adds support for inline encryption to UFS using > > > the inline encryption support in the block layer. It follows the JEDEC > > > UFSHCI v2.1 specification, which defines inline encryption for UFS. > > > > I'd appreciate it if you could review this series. > > > > Thanks! > > > > -- > > Martin K. Petersen Oracle Linux Engineering > A quick question and a comment: > > Does the IE infrastructure that you've added to the block layer invented for > ufs? And how this infrastructure relates to Eric's RFC: Inline crypto support on DragonBoard 845c https://www.spinics.net/lists/linux-scsi/msg141472.html > Do you see other devices using it in the future? > > Today, chipset vendors are using a different scheme for their IE. > Need their ack before reviewing your patches. > > Thanks, > Avri
On Sun, Jun 21, 2020 at 12:47:11PM +0000, Avri Altman wrote: > > > +Alim & Asutosh > > > > Hi Satya, > > > > > > > > Avri, > > > > > > > This patch series adds support for inline encryption to UFS using > > > > the inline encryption support in the block layer. It follows the JEDEC > > > > UFSHCI v2.1 specification, which defines inline encryption for UFS. > > > > > > I'd appreciate it if you could review this series. > > > > > > Thanks! > > > > > > -- > > > Martin K. Petersen Oracle Linux Engineering > > A quick question and a comment: > > > > Does the IE infrastructure that you've added to the block layer invented for > > ufs? > And how this infrastructure relates to Eric's RFC: Inline crypto support on DragonBoard 845c > https://www.spinics.net/lists/linux-scsi/msg141472.html > My patches are based on top of Satya's patches. My patches just introduce the quirks needed for Qualcomm ICE to work, since it doesn't follow the UFS standard exactly. - Eric
Thanks Avri for CCing me. > -----Original Message----- > From: Avri Altman <Avri.Altman@wdc.com> > Sent: 21 June 2020 18:05 > To: Martin K. Petersen <martin.petersen@oracle.com>; Satya Tangirala > <satyat@google.com>; alim.akhtar@samsung.com; asutoshd@codeaurora.org > Cc: linux-scsi@vger.kernel.org; Barani Muthukumaran > <bmuthuku@qti.qualcomm.com>; Kuohong Wang > <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com> > Subject: RE: [PATCH v2 0/3] Inline Encryption support for UFS > > +Alim & Asutosh > > Hi Satya, > > > > > Avri, > > > > > This patch series adds support for inline encryption to UFS using > > > the inline encryption support in the block layer. It follows the > > > JEDEC UFSHCI v2.1 specification, which defines inline encryption for UFS. > > > > I'd appreciate it if you could review this series. > > > > Thanks! > > > > -- > > Martin K. Petersen Oracle Linux Engineering > A quick question and a comment: > > Does the IE infrastructure that you've added to the block layer invented for ufs? > Do you see other devices using it in the future? > > Today, chipset vendors are using a different scheme for their IE. > Need their ack before reviewing your patches. > Yes, as of today at least in Samsung HCI, we use additional HW blocks to handle all the crypto part. (Though I need to check the status on the recent SoCs). However given the fact that UFSHCI 2.1 spec does includes Crypto support, and going by threads that you shared, looks like other Vendors does uses IE. I am inclined toward getting this reviewed. > Thanks, > Avri
On Sun, Jun 21, 2020 at 12:35:13PM +0000, Avri Altman wrote: > +Alim & Asutosh > > Hi Satya, > > > > > Avri, > > > > > This patch series adds support for inline encryption to UFS using > > > the inline encryption support in the block layer. It follows the JEDEC > > > UFSHCI v2.1 specification, which defines inline encryption for UFS. > > > > I'd appreciate it if you could review this series. > > > > Thanks! > > > > -- > > Martin K. Petersen Oracle Linux Engineering > A quick question and a comment: > > Does the IE infrastructure that you've added to the block layer invented for ufs? It wasn't invented "just" for UFS, but it was guided by how UFS inline crypto works. It is meant to be generic, and should be how we support any arbitrary IE hardware in the kernel. > Do you see other devices using it in the future? > Certainly - at the very least, I think it's likely we'll add support for eMMC inline crypto (although inline encryption support for eMMC was only added in an unreleased/yet to be released spec). It's certainly my hope that we add IE support for other classes of devices too. > Today, chipset vendors are using a different scheme for their IE. > Need their ack before reviewing your patches. > Sure :). The Inline Encryption patches (the block layer, fscrypt, f2fs, ext4 and these UFS patches) have been part of the Android common kernel https://android.googlesource.com/kernel/common/ for quite a while now, and chipset vendors have been working with us on ensuring that their UFS cards are supported by these patches (by adding code similar to Eric's RFC: Inline crypto support on DragonBoard 845c at https://www.spinics.net/lists/linux-scsi/msg141472.html or Mediatek's "scsi: ufs-mediatek: add inline encryption support" at https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/ for their individual chipsets). > Thanks, > Avri
On Sun, Jun 21, 2020 at 06:24:36PM +0000, Satya Tangirala wrote: > > > Today, chipset vendors are using a different scheme for their IE. > > Need their ack before reviewing your patches. > > > Sure :). The Inline Encryption patches (the block layer, fscrypt, f2fs, > ext4 and these UFS patches) have been part of the Android common kernel > https://android.googlesource.com/kernel/common/ > for quite a while now, and chipset vendors have been working with us > on ensuring that their UFS cards are supported by these patches (by adding > code similar to Eric's RFC: Inline crypto support on DragonBoard 845c at > https://www.spinics.net/lists/linux-scsi/msg141472.html > or Mediatek's "scsi: ufs-mediatek: add inline encryption support" at > https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/ > for their individual chipsets). Just as a practical matter, I don't think this patchset should be blocked waiting for a formal Acked-by from every vendor, as some of them don't to be participating in LKML at all despite being Cc'ed on these patches for a year and working with us outside LKML to support their hardware using the new framework. So I think a formal Acked-by on LKML is not always guaranteed. (It would certainly be preferable if everyone would participate though!) - Eric
On Sun, Jun 21, 2020 at 11:24:30PM +0530, Alim Akhtar wrote: > Thanks Avri for CCing me. > > > -----Original Message----- > > From: Avri Altman <Avri.Altman@wdc.com> > > Sent: 21 June 2020 18:05 > > To: Martin K. Petersen <martin.petersen@oracle.com>; Satya Tangirala > > <satyat@google.com>; alim.akhtar@samsung.com; asutoshd@codeaurora.org > > Cc: linux-scsi@vger.kernel.org; Barani Muthukumaran > > <bmuthuku@qti.qualcomm.com>; Kuohong Wang > > <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com> > > Subject: RE: [PATCH v2 0/3] Inline Encryption support for UFS > > > > +Alim & Asutosh > > > > Hi Satya, > > > > > > > > Avri, > > > > > > > This patch series adds support for inline encryption to UFS using > > > > the inline encryption support in the block layer. It follows the > > > > JEDEC UFSHCI v2.1 specification, which defines inline encryption for > UFS. > > > > > > I'd appreciate it if you could review this series. > > > > > > Thanks! > > > > > > -- > > > Martin K. Petersen Oracle Linux Engineering > > A quick question and a comment: > > > > Does the IE infrastructure that you've added to the block layer invented > for ufs? > > Do you see other devices using it in the future? > > > > Today, chipset vendors are using a different scheme for their IE. > > Need their ack before reviewing your patches. > > > Yes, as of today at least in Samsung HCI, we use additional HW blocks to > handle all the crypto part. > (Though I need to check the status on the recent SoCs). > However given the fact that UFSHCI 2.1 spec does includes Crypto support, > and going by threads that you shared, looks like other > Vendors does uses IE. I am inclined toward getting this reviewed. Note that Boojin Kim, who has been Cc'ed on all these patches, has already been working on replacing Samsung's legacy inline encryption implementation with one using the new framework. Unfortunately, Samsung's UFS inline encryption hardware doesn't follow the UFS specification, so it needs custom driver code and doesn't take much advantage of ufshcd-crypto (this patchset). However, it can still use the blk-crypto framework. So only the driver needs to differ, not the rest of the storage stack. This differs from the "old world" where every vendor had to customize the entire storage stack to support their inline encryption hardware. Anyway, ufshcd-crypto (this patchset) is still needed for all vendors who did mostly/fully follow the UFS specification, e.g. Mediatek (https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com) and Qualcomm (https://lkml.kernel.org/linux-scsi/20200621173713.132879-1-ebiggers@kernel.org). More reviews are always appreciated, though note that this patchset has already been out for review for over a year. (This is really v15; Satya started the numbering over after blk-crypto was merged in v5.8-rc1.) So I'm not sure we should count on many more formal reviews. - Eric
> > > A quick question and a comment: > > > > > > Does the IE infrastructure that you've added to the block layer invented > > for ufs? > > > Do you see other devices using it in the future? > > > > > > Today, chipset vendors are using a different scheme for their IE. > > > Need their ack before reviewing your patches. > > > > > Yes, as of today at least in Samsung HCI, we use additional HW blocks to > > handle all the crypto part. > > (Though I need to check the status on the recent SoCs). > > However given the fact that UFSHCI 2.1 spec does includes Crypto support, > > and going by threads that you shared, looks like other > > Vendors does uses IE. I am inclined toward getting this reviewed. > > Note that Boojin Kim, who has been Cc'ed on all these patches, has already > been > working on replacing Samsung's legacy inline encryption implementation with > one > using the new framework. > > Unfortunately, Samsung's UFS inline encryption hardware doesn't follow the > UFS > specification, so it needs custom driver code and doesn't take much > advantage of > ufshcd-crypto (this patchset). However, it can still use the blk-crypto > framework. So only the driver needs to differ, not the rest of the storage > stack. This differs from the "old world" where every vendor had to customize > the entire storage stack to support their inline encryption hardware. > > Anyway, ufshcd-crypto (this patchset) is still needed for all vendors who did > mostly/fully follow the UFS specification, e.g. Mediatek > (https://lkml.kernel.org/linux-scsi/20200304022101.14165-1- > stanley.chu@mediatek.com) > and Qualcomm > (https://lkml.kernel.org/linux-scsi/20200621173713.132879-1- > ebiggers@kernel.org). > > More reviews are always appreciated, though note that this patchset has > already > been out for review for over a year. (This is really v15; Satya started the > numbering over after blk-crypto was merged in v5.8-rc1.) So I'm not sure we > should count on many more formal reviews. Ok then. This works as a practical ack by most of the large chipset vendors. Thanks, Avri
>ufshcd_prepare_req_desc_hdr maybe pack all your additions to ufshcd_prepare_req_desc_hdr into a ufshcd_prepare_req_desc_hdr_crypto helper, so the ifdefs can be avoided? >ufshcd_compose_dev_cmd same goes to ufshcd_compose_dev_cmd and ufshcd_issue_devman_upiu_cmd - maybe just call ufshcd_prepare_lrbp_crypto (with some minor changes) instead of those ifdefs?