Hi Eric, On Tue, 2020-02-25 at 17:12 -0800, Eric Biggers wrote: > On Tue, Feb 25, 2020 at 03:21:25PM +0800, Stanley Chu wrote: > > Hi Christoph, > > > > On Mon, 2020-02-24 at 15:37 -0800, Christoph Hellwig wrote: > > > On Sun, Feb 23, 2020 at 09:47:36PM +0800, Stanley Chu wrote: > > > > Yes, MediaTek is keeping work closely with inline encryption patch sets. > > > > Currently the v6 version can work well (without > > > > UFSHCD_QUIRK_BROKEN_CRYPTO quirk) at least in our MT6779 SoC platform > > > > which basic SoC support and some other peripheral drivers are under > > > > upstreaming as below link, > > > > > > > > https://patchwork.kernel.org/project/linux-mediatek/list/?state=% > > > > 2A&q=6779&series=&submitter=&delegate=&archive=both > > > > > > > > The integration with inline encryption patch set needs to patch > > > > ufs-mediatek and patches are ready in downstream. We plan to upstream > > > > them soon after inline encryption patch sets get merged. > > > > > > What amount of support do you need in ufs-mediatek? It seems like > > > pretty much every ufs low-level driver needs some kind of specific > > > support now, right? I wonder if we should instead opt into the support > > > instead of all the quirking here. > > > > The patch in ufs-mediatek is aimed to issue vendor-specific SMC calls > > for host initialization and configuration. This is because MediaTek UFS > > host has some "secure-protected" registers/features which need to be > > accessed/switched in secure world. > > > > Such protection is not mentioned by UFSHCI specifications thus inline > > encryption patch set assumes that every registers in UFSHCI can be > > accessed normally in kernel. This makes sense and surely the patchset > > can work fine in any "standard" UFS host. However if host has special > > design then it can work normally only if some vendor-specific treatment > > is applied. > > > > I think one of the reason to apply UFSHCD_QUIRK_BROKEN_CRYPTO quirk in > > ufs-qcom host is similar to above case. > > So, I had originally assumed that most kernel developers would prefer to make > the UFS crypto support opt-out rather than opt-in, since that matches the normal > Linux way of doing things. I.e. normally the kernel's default assumption is > that devices implement the relevant standard, and only when a device is known to > deviate from the standard does the driver apply quirks. > > But indeed, as we've investigated more vendors' UFS hardware, it seems that > everyone has some quirk that needs to be handled in the platform driver: > > - ufs-qcom (tested on DragonBoard 845c with upstream kernel) needs > vendor-specific crypto initialization logic and SMC calls to set keys > > - ufs-mediatek needs the quirks that Stanley mentioned above > > - ufs-hisi (tested on Hikey960 with upstream kernel) needs to write a > vendor-specific register to use high keyslots, but even then I still > couldn't get the crypto support working correctly. > > I'm not sure about the UFS controllers from Synopsys, Cadence, or Samsung, all > of which apparently have implemented some form of the crypto support too. But I > wouldn't get my hopes up that everyone followed the UFS standard precisely. > > So if there are no objections, IMO we should make the crypto support opt-in. > > That makes it even more important to upstream the crypto support for specific > hardware like ufs-qcom and ufs-mediatek, since otherwise the ufshcd-crypto code > would be unusable even theoretically. I'm volunteering to handle ufs-qcom with > https://lkml.kernel.org/linux-block/20200110061634.46742-1-ebiggers@xxxxxxxxxx/. > Stanley, could you send out ufs-mediatek support as an RFC so people can see > better what it involves? Sure, I will send out our RFC patches. Please allow me some time for submission. Thanks, Stanley Chu > - Eric
