On Tue, 30 Jul 2019 21:46:14 +0800 Huang Yang <yang.hu...@intel.com> wrote:
> It is a virtio based RPMB (Replay Protected Memory Block) device. > > Signed-off-by: Yang Huang <yang.hu...@intel.com> > Reviewed-by: Bing Zhu <bing....@intel.com> > Reviewed-by: Tomas Winkler <tomas.wink...@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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org