[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification
On Wed, Jul 31, 2019 at 02:41:56PM +0000, Huang, Yang wrote:
>
>
> > > ---
> > > conformance.tex | 19 ++++++++++-
> > > content.tex | 3 ++
> > > virtio-rpmb.tex | 100
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644
> > > virtio-rpmb.tex
> > >
> >
> > (...)
>
> Sorry, what do you mean?
>
> >
> > > diff --git a/content.tex b/content.tex index ee0d7c9..7f54f94 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -2717,6 +2717,8 @@ \chapter{Device Types}\label{sec:Device Types}
> > > \hline
> > > 27 & PMEM device \\
> > > \hline
> > > +28 & RPMB device \\
> >
> > Depending on how long it will take to get this into shape, it might be a good idea
> > to reserve the id separately.
>
> Thanks. I will remove this change at current phase.
>
> > > +\hline
> > > \end{tabular}
> > >
> > > Some of the devices above are unspecified by this document, @@
> > > -5677,6 +5679,7 @@ \subsubsection{Legacy Interface: Framing
> > > Requirements}\label{sec:Device \input{virtio-input.tex}
> > > \input{virtio-crypto.tex} \input{virtio-vsock.tex}
> > > +\input{virtio-rpmb.tex}
> > >
> > > \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > >
> > > diff --git a/virtio-rpmb.tex b/virtio-rpmb.tex new file mode 100644
> > > index 0000000..aa113bb
> > > --- /dev/null
> > > +++ b/virtio-rpmb.tex
> > > @@ -0,0 +1,100 @@
> > > +\section{RPMB Device}\label{sec:Device Types / RPMB Device}
> > > +
> > > +virtio-rpmb is a virtio based RPMB (Replay Protected Memory Block)
> > > +device. It is used as a tamper-resistant and anti-replay storage.
> > > +It supports four command requests including read, write, get write
> > > +counter and program key. They are placed in the queue.
> >
> > What about replacing the last two sentences with:
> >
> > "The device is driven via requests including read, write, get write counter, and
> > program key, which are submitted via a request queue." ?
>
> I will update it.
>
> > > +
> > > +\subsection{Device ID}\label{sec:Device Types / RPMB Device / Device
> > > +ID}
> > > +
> > > +28
> > > +
> > > +\subsection{Virtqueues}\label{sec:Device Types / RPMB Device /
> > > +Virtqueues}
> > > +
> > > +\begin{description}
> > > +\item[0] requestq
> > > +\end{description}
> > > +
> > > +\subsection{Feature bits}\label{sec:Device Types / RPMB Device /
> > > +Feature bits}
> > > +
> > > +None.
> > > +
> > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > > +RPMB Device / Device configuration layout}
> > > +
> > > +None.
> > > +
> > > +\devicenormative{\subsection}{Device Initialization}{Device Types /
> > > +RPMB Device / Device Initialization}
> > > +
> > > +\begin{enumerate}
> > > +\item The virtqueue is initialized.
> > > +\item The authentication key of device SHOULD NOT be programmed at the
> > first device initialization.
> > > +\item The device capacity SHOULD be initialized to a multiple of 128 Kbytes
> > and up to 16Mbytes.
> > > +\end{enumerate}
> >
> > I would place a slightly more verbose description of how the device should be
> > initialized into a descriptive section and only leave the normative statements
> > here.
> >
> > Also, why SHOULD/SHOULD NOT instead of MUST/MUST NOT? Especially the
> > device capacity requirements feel like something that should not be too loosely
> > defined.
>
> Agree.
>
> > Would it make sense to expose the capacity through the config space?
>
> Normally, an application could read the address from 16MB to 8MB, 4MB, 2MB ...
> to get the correct capacity from the error code of result.
> But I agree that setting the capacity as an always present config is better.
>
> >
> > > +
> > > +\subsection{Device Operation}\label{sec:Device Types / RPMB Device /
> > > +Device Operation}
> > > +
> > > +The operation of a virtio RPMB device is driven by the requests placed on the
> > virtqueue.
> > > + The type of the request can be program key
> > > +(VIRTIO_RPMB_REQ_PROGRAM_KEY),
> > > + get write counter (VIRTIO_RPMB_REQ_GET_WRITE_COUNTER),
> > > + write (VIRTIO_RPMB_REQ_DATA_WRITE), and read
> > (VIRTIO_RPMB_REQ_DATA_READ).
> > > + A program key or write request can also combine with a
> > > + result read (VIRTIO_RPMB_REQ_RESULT_READ) for a returned result.
> > > +
> > > +\begin{lstlisting}
> > > +#define VIRTIO_RPMB_REQ_PROGRAM_KEY 0x0001
> > > +#define VIRTIO_RPMB_REQ_GET_WRITE_COUNTER 0x0002
> > > +#define VIRTIO_RPMB_REQ_DATA_WRITE 0x0003
> > > +#define VIRTIO_RPMB_REQ_DATA_READ 0x0004
> > > +#define VIRTIO_RPMB_REQ_RESULT_READ 0x0005
> > > +\end{lstlisting}
> >
> > What form do the requests that are placed on the queue take?
>
> This is defined by spec, like eMMC spec:
> https://www.jedec.org/sites/default/files/docs/JESD84-B51.pdf,
> at Chapter 6.6.22: Replay Protected Memory Block.
> It acts as req_resp of RPMB frame:
> /**
> * struct rpmb_frame_jdec - rpmb frame as defined by JDEC specs
> *
> * @stuff : stuff bytes
> * @key_mac : The authentication key or the message authentication
> * code (MAC) depending on the request/response type.
> * The MAC will be delivered in the last (or the only)
> * block of data.
> * @data : Data to be written or read by signed access.
> * @nonce : Random number generated by the host for the requests
> * and copied to the response by the RPMB engine.
> * @write_counter: Counter value for the total amount of the successful
> * authenticated data write requests made by the host.
> * @addr : Address of the data to be programmed to or read
> * from the RPMB. Address is the serial number of
> * the accessed block (half sector 256B).
> * @block_count : Number of blocks (half sectors, 256B) requested to be
> * read/programmed.
> * @result : Includes information about the status of the write counter
> * (valid, expired) and result of the access made to the RPMB.
> * @req_resp : Defines the type of request and response to/from the memory.
> */
> struct rpmb_frame_jdec {
> __u8 stuff[196];
> __u8 key_mac[32];
> __u8 data[256];
> __u8 nonce[16];
> __be32 write_counter;
> __be16 addr;
> __be16 block_count;
> __be16 result;
> __be16 req_resp;
> }
>
> A user should fill the rpmb frame per his request and send it to virtio rpmb driver.
> The driver then format it to virtqueue and send it to device.
>
> Is it necessary to mention this rpmb frame?
> > > +
> > > +\drivernormative{\subsubsection}{Device Operation}{Device Types /
> > > +RPMB Device / Device Operation}
> > > +
> > > +The driver MUST configure and initialize all virtqueues for the requests
> > received.
> >
> > But there is only a single virtqueue, isn't there? This requirement looks a bit odd.
>
> Agree. I will update it.
>
> > > +
> > > +\devicenormative{\subsubsection}{Device Operation}{Device Types /
> > > +RPMB Device / Device Operation}
> > > +
> > > +The device provides a simulated RPMB backed by ordinary file or
> > > + other medium in host. It SHOULD keep consistent behaviors with
> > > + hardware, including but not limited to:
> >
> > If this device is supposed to simulate the behaviour of a real device, the virtio
> > spec needs to point to a specification of that behaviour as a normative
> > reference. From the information contained here, I would have no clue how to
> > actually produce a conforming device or driver implementation.
>
> I think the spec of eMMC RPMB could be a reference:
> https://www.jedec.org/sites/default/files/docs/JESD84-B51.pdf
I wonder whether a 32 bit key is always enough
(e.g. a bigger key might make sense with a
different authentication method).
> > > +\begin{enumerate}
> > > +\item The device maintains an authentication key. Once the first
> > > + successful key programming is performed, the authentication
> > > + key MUST be kept unchanged during device lifecycle.
I wonder what is a lifecycle from point of view of virtualization.
For example, how is a state that can not be rolled back compatible
with snapshoting?
How about migration?
> It cannot
> > > + be overwritten, erased or read. This key MUST be kept regardless
> > > + of device reset or reboot.
> > > +\item The device maintains a monotonic write counter. It MUST be
> > > + initialized to zero and added by one automatically after each
> > > + successful write operation. The value cannot be reset. After
> > > + the counter has reached its maximum value 0xFFFFFFFF, it will
> > > + not be incremented anymore. This counter MUST be kept regardless
> > > + of device reset or reboot.
> > > +\item The RPMB device cannot be successfully accessed until RPMB
> > > + authentication key is programmed. For any operation (read, write,
> > > + program key, get write counter) done to virtio RPMB device after
> > > + authentication key is programmed successfully, the device responds
> > > + with a MAC calculated by HMAC-SHA with authentication key to driver.
> > > +\item For write operation, the device MUST compare the writer counter
> > > + it receives with the one it maintained internally. If the two are
> > > + not equal, a VIRTIO_RPMB_RES_COUNT_FAILURE SHOULD be returned as
> > > + the result. The device MUST calculate the MAC using HMAC-SHA. The
> > > + authentication key acts as an input of the calculation. If the MAC
> > > + are not equal to the one it received, a VIRTIO_RPMB_RES_AUTH_FAILURE
> > > + SHOULD be returned as the result.
> > > +\item
> > > +\end{enumerate}
> >
> > I think the spec would again benefit from putting the verbose description of
> > what the driver needs to do to access the device and how the device needs to
> > react into a descriptive section, and only keep some simple normative
> > statements here.
>
> What the driver need to do is pretty simple. It just need to format the RPMB frames
> sent from user to virtqueue, and send it to device.
> While what the device need to do is pretty complex. Besides providing RPMB spec,
> is it necessary to illustrate the behaviors of four requests that device should react?
>
> > > +
> > > +One of the below error codes MUST be returned to the driver
> > > + based on the operation result.
> > > +
> > > +\begin{lstlisting}
> > > +#define VIRTIO_RPMB_RES_OK 0x0000
> > > +#define VIRTIO_RPMB_RES_GENERAL_FAILURE 0x0001
> > > +#define VIRTIO_RPMB_RES_AUTH_FAILURE 0x0002
> > > +#define VIRTIO_RPMB_RES_COUNT_FAILURE 0x0003
> > > +#define VIRTIO_RPMB_RES_ADDR_FAILURE 0x0004
> > > +#define VIRTIO_RPMB_RES_WRITE_FAILURE 0x0005
> > > +#define VIRTIO_RPMB_RES_READ_FAILURE 0x0006
> > > +#define VIRTIO_RPMB_RES_NO_AUTH_KEY 0x0007
> > > +#define VIRTIO_RPMB_RES_WRITE_COUNTER_EXPIRED 0x0080
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]