Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e72812a9-1e5b-a826-d490-62ed23d94116@acm.org>
Date:   Wed, 6 Apr 2022 10:32:44 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
Content-Language: en-US
To:     Bean Huo <huobean@gmail.com>,
        =?UTF-8?Q?Alex_Benn=c3=a9e?= <alex.bennee@linaro.org>
Cc:     linux-kernel@vger.kernel.org, maxim.uvarov@linaro.org,
        joakim.bech@linaro.org, ulf.hansson@linaro.org,
        ilias.apalodimas@linaro.org, arnd@linaro.org,
        ruchika.gupta@linaro.org, tomas.winkler@intel.com,
        yang.huang@intel.com, bing.zhu@intel.com,
        Matti.Moell@opensynergy.com, hmo@opensynergy.com,
        linux-mmc@vger.kernel.org, linux-scsi@vger.kernel.org
References: <20220405093759.1126835-1-alex.bennee@linaro.org>
 <8b3ce88f65fd11523a4d2daab3c617f7089eb1ce.camel@gmail.com>
 <87r16bk013.fsf@linaro.org>
 <aeb64d1c4bbddfd8463c07a40ab1fc78be0d158d.camel@gmail.com>
 <87ee2ajuky.fsf@linaro.org>
 <c18a2b359046a14250cafa5a619dbe53f2872b5a.camel@gmail.com>
From:   Bart Van Assche <bvanassche@acm.org>
In-Reply-To: <c18a2b359046a14250cafa5a619dbe53f2872b5a.camel@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 4/6/22 10:19, Bean Huo wrote:
> On Wed, 2022-04-06 at 12:22 +0100, Alex Bennée wrote:
>>
>> Bean Huo <huobean@gmail.com> writes:
>>
>>> On Tue, 2022-04-05 at 16:43 +0100, Alex Bennée wrote:
>>>>
>>>> Bean Huo <huobean@gmail.com> writes:
>>>>
>>>>> Hi Alex,
>>>>>
>>>>> Thanks for this unified RPMB interface, I wanted to verify this
>>>>> on
>>>>> our
>>>>> UFS, it seems you didn't add the UFS access interface in this
>>>>> version
>>>>> from your userspace tools, right?
>>>>
>>>> No I didn't but it should be easy enough to add some function
>>>> pointer
>>>> redirection everywhere one of the op_* functions calls a vrpmb_*
>>>> function. Do you already have a UFS RPMB device driver?
>>>>
>>>
>>> Hi Alex,
>>> Thanks for your feedback.
>>>
>>> We now access UFS RPMB through the RPMB LUN BSG device, RPMB is a
>>> well-
>>> known LU and we have a userspace tool to access it.
>>>
>>> I see that if we're going to use your interface, "static struct
>>> rpmb_ops" should be registered from a lower-level driver, for
>>> example
>>> in a UFS driver, yes there should be no problem with this
>>> registration,
>>> but I don't know with the current way Compared, what are the
>>> advantages
>>> to add a driver. maybe the main advantage is that we will have an
>>> unified user space tool for RPMB. right?
>>
>> Pretty much. The main issue for virtio-rpmb is it doesn't really fit
>> neatly into the block stack because all it does is the RPMB part so a
>> non-block orientate API makes sense.
>>
>> Can you point be to where the UFS driver does it's current RPMB
>> stuff?
>>
> 
> It's the SCSI BSG driver, in fact, we don't have a dedicated UFS RPMB
> driver in the kernel. RPMB is a well known LU, we are using userspace
> tools to issue SCSI commands directly to the UFS RPMB LU via ioctl()
> from the BSG device node in the /dev/sg/ folder.
> 
> Here is the BSG part of the code in the userspace tools:
> 
>          io_hdr_v4.guard = 'Q';
>          io_hdr_v4.protocol = BSG_PROTOCOL_SCSI;
>          io_hdr_v4.subprotocol = BSG_SUB_PROTOCOL_SCSI_CMD;
>          io_hdr_v4.response = (__u64)sense_buffer;
>          io_hdr_v4.max_response_len = SENSE_BUFF_LEN;
>          io_hdr_v4.request_len = cmd_len;
>          io_hdr_v4.request = (__u64)cdb;
>                                                                                                                            
>                                                                                   
>          ioctl(fd, SG_IO, &io_hdr_v4))

Hi Bean,

I'm not sure where the above comes from? The Android RPMB client uses 
slightly different code. Additionally, the retry loop around the 
submission of SG/IO commands is very important. See also the 
check_sg_io_hdr() call in send_ufs_rpmb_req(). See also 
https://cs.android.com/android/platform/superproject/+/master:system/core/trusty/storage/proxy/rpmb.c

Thanks,

Bart.