[virtio-dev] [PATCH v2] Add virtio rpmb device specification

[virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Huang Yang]

It is a virtio based RPMB (Replay Protected Memory Block) device.

Signed-off-by: Yang Huang <yang.huang@intel.com>
Reviewed-by: Bing Zhu <bing.zhu@intel.com>
Reviewed-by: Tomas Winkler <tomas.winkler@intel.com>

v1 -> v2:
1. update conformance.
2. wordings change:
   first initialization -> first device initialization
   device size -> device capacity
3. update Device Operation:
   add more decriptions on write counter, key and write operations.
---
 conformance.tex |  19 ++++++++++-
 content.tex     |   3 ++
 virtio-rpmb.tex | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 virtio-rpmb.tex

diff --git a/conformance.tex b/conformance.tex
index 0ac58aa..e0c55e0 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -22,7 +22,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
   \begin{itemize}
     \item Clause \ref{sec:Conformance / Device Conformance}.
     \item One of clauses \ref{sec:Conformance / Device Conformance / PCI Device Conformance}, \ref{sec:Conformance / Device Conformance / MMIO Device Conformance} or \ref{sec:Conformance / Device Conformance / Channel I/O Device Conformance}.
-    \item One of clauses \ref{sec:Conformance / Device Conformance / Network Device Conformance}, \ref{sec:Conformance / Device Conformance / Block Device Conformance}, \ref{sec:Conformance / Device Conformance / Console Device Conformance}, \ref{sec:Conformance / Device Conformance / Entropy Device Conformance}, \ref{sec:Conformance / Device Conformance / Traditional Memory Balloon Device Conformance}, \ref{sec:Conformance / Device Conformance / SCSI Host Device Conformance}, \ref{sec:Conformance / Device Conformance / Input Device Conformance}, \ref{sec:Conformance / Device Conformance / Crypto Device Conformance} or \ref{sec:Conformance / Device Conformance / Socket Device Conformance}.
+    \item One of clauses \ref{sec:Conformance / Device Conformance / Network Device Conformance}, \ref{sec:Conformance / Device Conformance / Block Device Conformance}, \ref{sec:Conformance / Device Conformance / Console Device Conformance}, \ref{sec:Conformance / Device Conformance / Entropy Device Conformance}, \ref{sec:Conformance / Device Conformance / Traditional Memory Balloon Device Conformance}, \ref{sec:Conformance / Device Conformance / SCSI Host Device Conformance}, \ref{sec:Conformance / Device Conformance / Input Device Conformance}, \ref{sec:Conformance / Device Conformance / Crypto Device Conformance}, \ref{sec:Conformance / Device Conformance / Socket Device Conformance} or \ref{sec:Conformance / Device Conformance / RPMB Device Conformance}.
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
 \end{description}
@@ -183,6 +183,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Device Events}
 \end{itemize}
 
+\conformance{\subsection}{RPMB Driver Conformance}\label{sec:Conformance / Driver Conformance / RPMB Driver Conformance}
+
+A RPMB driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / RPMB Device / Device Operation}
+\end{itemize}
+
 \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -338,6 +346,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Receive and Transmit}
 \end{itemize}
 
+\conformance{\subsection}{RPMB Device Conformance}\label{sec:Conformance / Device Conformance / RPMB Device Conformance}
+
+An RPMB device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / RPMB Device / Device Initialization}
+\item \ref{devicenormative:Device Types / RPMB Device / Device Operation}
+\end{itemize}
+
 \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
 A conformant implementation MUST be either transitional or
 non-transitional, see \ref{intro:Legacy
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 \\
+\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.
+
+\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}
+
+\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}
+
+\drivernormative{\subsubsection}{Device Operation}{Device Types / RPMB Device / Device Operation}
+
+The driver MUST configure and initialize all virtqueues for the requests received.
+
+\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:
+\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. 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}
+
+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
+\end{lstlisting}
-- 
2.7.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Cornelia Huck]

On Tue, 30 Jul 2019 21:46:14 +0800
Huang Yang <yang.huang@intel.com> wrote:

> It is a virtio based RPMB (Replay Protected Memory Block) device.
> 
> Signed-off-by: Yang Huang <yang.huang@intel.com>
> Reviewed-by: Bing Zhu <bing.zhu@intel.com>
> Reviewed-by: Tomas Winkler <tomas.winkler@intel.com>
> 
> v1 -> v2:
> 1. update conformance.
> 2. wordings change:
>    first initialization -> first device initialization
>    device size -> device capacity
> 3. update Device Operation:
>    add more decriptions on write counter, key and write operations.
> ---
>  conformance.tex |  19 ++++++++++-
>  content.tex     |   3 ++
>  virtio-rpmb.tex | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 121 insertions(+), 1 deletion(-)
>  create mode 100644 virtio-rpmb.tex
> 

(...)

> 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.

> +\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." ?

> +
> +\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.

Would it make sense to expose the capacity through the config space?


> +
> +\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?

> +
> +\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.

> +
> +\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.

> +\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. 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.

> +
> +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

These return codes probably need some more description. Also, how are
they returned?

> +\end{lstlisting}


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Huang, Yang]

> > ---
> >  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 

> > +\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. 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


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On Tue, Jul 30, 2019 at 09:46:14PM +0800, Huang Yang wrote:
> It is a virtio based RPMB (Replay Protected Memory Block) device.

Please include the request structs.  There is not enough information in
this spec to implement the device.

> +\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

Or it could be a real hardware?  The specification shouldn't discuss
these implementation details except to say that virtio-rpmb could be
backed in a number of ways.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Cornelia Huck]

On Wed, 31 Jul 2019 14:41:56 +0000
"Huang, Yang" <yang.huang@intel.com> 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?

I'm just indicating that I cut some text :)

(...)

> > > +\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;
> }

Ok, this seems to properly define the structure including endianness.
You should explicitly refer to it by name, then it should be fine (when
the document you refer to is added as a normative reference).

> 
> 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 there any formatting on top of the frame?

> 
> Is it necessary to mention this rpmb frame? 

Yes; otherwise, how is an implementer to know what is supposed to go on
the queue?

(...)

> > > +\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 haven't looked at it; but from the excerpt you cited above this looks
like a good plan.

> 
> > > +\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. 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.

Yes.

> 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?

The spec needs to be concrete enough so that it can be used as a
reference (together with the pdf you mentioned above) for implementing
a conforming device. I have not looked at the linked spec to find out
how concrete you have to get when describing the behaviours of the
requests :) Implementation details, on the other hand, do not really
belong here, only maybe as a non-normative example to make things
easier to understand.

> 
> > > +
> > > +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  
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Huang, Yang]

> -----Original Message-----
> From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> On Behalf Of Stefan Hajnoczi
> Sent: Wednesday, July 31, 2019 22:57
> To: Huang, Yang <yang.huang@intel.com>
> Cc: virtio-dev@lists.oasis-open.org; mst@redhat.com; Zhu, Bing
> <bing.zhu@intel.com>; Winkler, Tomas <tomas.winkler@intel.com>
> Subject: Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification
> 
> On Tue, Jul 30, 2019 at 09:46:14PM +0800, Huang Yang wrote:
> > It is a virtio based RPMB (Replay Protected Memory Block) device.
> 
> Please include the request structs.  There is not enough information in this spec
> to implement the device.

OK.

> > +\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
> 
> Or it could be a real hardware?  The specification shouldn't discuss these
> implementation details except to say that virtio-rpmb could be backed in a
> number of ways.

This is a good catch. 

But how to implement a physical RPMB backed solution is out of scope.

If there are multiple guests who want to use RPMB. And their devices are backed by a same physical RPMB.
1. Who owns key, who owns RPMB. They should share a same key. If Guest1 programmed a Key1, the Guest2 MUST use the same Key1.
2. Device SHOULD  isolate the RPMB space to protect from overwritten between guests.
    But address remapping cannot work. Because address remapping by virtio device will cause to a wrong MAC, it will result to write failure.
3. RPMB capacity is limited up to 16MB. It cannot support the sharing among a large number of guests.

If users want to implement it backed by hardware RPMB, they should have a whole picture design, which is out of scope of the spec.
Of course, physical backed RPMB still adapts to the rules of the spec.
I will change it to "could be backed in a number of ways."

Both simulated and physical RPMB backed solution are implemented on Project ACRN.
If you are interested, "Secure Storage Virtualization" in this slide shows a high level design of physical RPMB backed solution: 
https://events.linuxfoundation.org/wp-content/uploads/2017/12/Implement-Android-Tamper-Resistant-Secure-Storage-Bing-Zhu_and-Secure-it-in-Virtualization-Bing-Zhu-Intel-Corporation.pdf




---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Huang, Yang]

> >  */
> > 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;
> > }
> 
> Ok, this seems to properly define the structure including endianness.
> You should explicitly refer to it by name, then it should be fine (when the
> document you refer to is added as a normative reference).
> 
> >
> > 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 there any formatting on top of the frame?

No such requirement for driver. Just sending to device via virtqueue. 

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2968 bytes --]

On Thu, Aug 01, 2019 at 01:14:11AM +0000, Huang, Yang wrote:
> > -----Original Message-----
> > From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> > On Behalf Of Stefan Hajnoczi
> > Sent: Wednesday, July 31, 2019 22:57
> > To: Huang, Yang <yang.huang@intel.com>
> > Cc: virtio-dev@lists.oasis-open.org; mst@redhat.com; Zhu, Bing
> > <bing.zhu@intel.com>; Winkler, Tomas <tomas.winkler@intel.com>
> > Subject: Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification
> > 
> > On Tue, Jul 30, 2019 at 09:46:14PM +0800, Huang Yang wrote:
> > > It is a virtio based RPMB (Replay Protected Memory Block) device.
> > 
> > Please include the request structs.  There is not enough information in this spec
> > to implement the device.
> 
> OK.
> 
> > > +\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
> > 
> > Or it could be a real hardware?  The specification shouldn't discuss these
> > implementation details except to say that virtio-rpmb could be backed in a
> > number of ways.
> 
> This is a good catch. 
> 
> But how to implement a physical RPMB backed solution is out of scope.
> 
> If there are multiple guests who want to use RPMB. And their devices are backed by a same physical RPMB.
> 1. Who owns key, who owns RPMB. They should share a same key. If Guest1 programmed a Key1, the Guest2 MUST use the same Key1.
> 2. Device SHOULD  isolate the RPMB space to protect from overwritten between guests.
>     But address remapping cannot work. Because address remapping by virtio device will cause to a wrong MAC, it will result to write failure.
> 3. RPMB capacity is limited up to 16MB. It cannot support the sharing among a large number of guests.
> 
> If users want to implement it backed by hardware RPMB, they should have a whole picture design, which is out of scope of the spec.
> Of course, physical backed RPMB still adapts to the rules of the spec.
> I will change it to "could be backed in a number of ways."

Yes, all of this is out of scope.  I think the VIRTIO spec should avoid
saying "guest" and "host" whenever possible and just stick to "driver"
and "device".

VIRTIO is used in other contexts too, like hardware PCI devices or for
cross-CPU communication without virtualization, so it's nice to be as
general as we can.

> Both simulated and physical RPMB backed solution are implemented on Project ACRN.
> If you are interested, "Secure Storage Virtualization" in this slide shows a high level design of physical RPMB backed solution: 
> https://events.linuxfoundation.org/wp-content/uploads/2017/12/Implement-Android-Tamper-Resistant-Secure-Storage-Bing-Zhu_and-Secure-it-in-Virtualization-Bing-Zhu-Intel-Corporation.pdf

Thanks for the link!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Michael S. Tsirkin]

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

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] [PATCH v2] Add virtio rpmb device specification

[Winkler, Tomas]

> >
> >
> > > > ---
> > > >  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).

In JDEC specs no, but in NVMe spec, it is extensible 

> 
> 
> 
> 
> > > > +\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?

This can really vary, in virtualized environment that can be per guest boot,  but in that case the master key/seed has to be shared OOB. 
> 
> 
> 
> > 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