RE: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: "arve@android.com" <arve@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	"Hunter, Adrian" <adrian.hunter@intel.com>,
	"James Bottomley" <James.Bottomley@hansenpartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Vinayak Holikatti <vinholikatti@gmail.com>,
	Andy Lutomirski <luto@kernel.org>,
	Michael Ryleev <gmar@google.com>,
	"Joao Pinto" <Joao.Pinto@synopsys.com>,
	Christoph Hellwig <hch@lst.de>,
	Yaniv Gardi <ygardi@codeaurora.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: RE: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem
Date: Thu, 2 Jun 2016 13:17:42 +0000	[thread overview]
Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B5427E98E@hasmsx108.ger.corp.intel.com> (raw)
In-Reply-To: <CAMP5Xgfo-4h3DYki0VWhQFjBtAE9=f9dqCZQV8VDvfJ3Rc_2UQ@mail.gmail.com>

> 
> On Wed, Jun 1, 2016 at 2:41 PM, Tomas Winkler <tomas.winkler@intel.com>
> wrote:
> > Few storage technology such is EMMC, UFS, and NVMe support RPMB
> >hardware partition with common protocol and frame layout.
> > The RPMB partition cannot be accessed via standard block layer, but
> >by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
> >PROGRAM_KEY.
> >...
> 
> If the same protocol is used by all these standards, why not export it directly
> (including the RESULT_READ command or not even knowing the command
> types)?
The protocol is the  same, but the wrapping of the packets is storage type specific so 
I believe that the best abstraction is on those  4 operations level. I'm not sure if the code would
be simpler if it would be exposed on a lower level.  
RESULT_READ  is  a command to be issued for getting preceeding write operation status, it's a kind of work around about the fact that you have to issue a read operation
in order to retrieve data in this case a  write operation result.  It can be successfully hidden and final result of the operation is delivered
to the user.

> While I would prefer an rpmb specific interface over the existing raw
> mmc command interface, all I need is an rpmb operation that lets me send
> and receive buffers without interruption.

I currently don't see an obstacle on doing the same with proposed interface, what is the issue are seeing.

> You can find our exiting user-space code here at
> https://android.googlesource.com/platform/system/core/+/master/trusty/s
> torage/proxy/rpmb.c.
> If you use an interface more similar to this, I think your emmc and ufs
> specific code would be simpler. Also, if you don't need the in-kernel
> interface, the kernel would not need to know the details of the rpmb
> protocol at all.

My major interest is the in-kernel protocol the user space API was more intended for debug, but I found it would be even more useful. 
The store type  access is very RPMB specific  for both MMC and UFS and needs to do special operations for RPMB so I don't see how this awareness can be removed or I'm not reading your proposal correctly.
Accessing RPMB is a multiple stepsoperation, the steps can be driven from the user space as done in EMMC ioctl but hidning would reduce the number of system calls and amount of data passed, 
in some sense the same as in the  new mmc  MMC_IOC_MULTI_CMD is trying ot achive.

> I have not tested your code, but it looks like we would have to modify the
> storage proxy to interpret the data it currently passes through and remove all
> RESULT_READ packets.
Your proxy driver just sends ioctls so this wouldn't be much difference and the rpmb code on the trusty w need rewrite just rpmb_send () function,
actually removing one set of buffer size. I will try to make that modification if it helps?

Thanks for interest and your review. 
Tomas 