[virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[Alex Bennée]

Even if 0 meant no limit we are still limited by the field size of the
request. That said for a maximum sized partition (* 80 128 1024) you
could only actually request 40960 blocks before running out of device.
Perhaps it would be better to mark 0 as invalid?

Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: Harald Mommer <hmo@opensynergy.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 device-types/rpmb/description.tex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/device-types/rpmb/description.tex b/device-types/rpmb/description.tex
index 1dae3fd..2ce8a5b 100644
--- a/device-types/rpmb/description.tex
+++ b/device-types/rpmb/description.tex
@@ -37,7 +37,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / RPMB Device /
    The values MUST range between 0x00 and 0x80 inclusive.
 \item[\field{max_wr_cnt and max_rd_cnt}] are the maximum numbers of RPMB
    block count (256B) that can be performed to device in one request. 0 implies
-   no limitation.
+   no limitation other than the maximum value you can store in \field{block_count} (65535).
 \end{description}
 
 \devicenormative{\subsection}{Device Initialization}{Device Types / RPMB Device / Device Initialization}
-- 
2.39.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[Cornelia Huck]

On Mon, Jun 19 2023, Alex Bennée <alex.bennee@linaro.org> wrote:

> Even if 0 meant no limit we are still limited by the field size of the
> request. That said for a maximum sized partition (* 80 128 1024) you
> could only actually request 40960 blocks before running out of device.
> Perhaps it would be better to mark 0 as invalid?

I don't think we can mark 0 as invalid, as it has been in a published
spec and implementations may be already using it.

That said, what is the actual limitation? Knowing nothing about RPMB, is
it 64k (what can fit in block_count), or the 40960 you mentioned above
as a value defined by other reasons (maximum size of a partition -- I
assume that cannot change?)

>
> Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Cc: Harald Mommer <hmo@opensynergy.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  device-types/rpmb/description.tex | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/device-types/rpmb/description.tex b/device-types/rpmb/description.tex
> index 1dae3fd..2ce8a5b 100644
> --- a/device-types/rpmb/description.tex
> +++ b/device-types/rpmb/description.tex
> @@ -37,7 +37,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / RPMB Device /
>     The values MUST range between 0x00 and 0x80 inclusive.
>  \item[\field{max_wr_cnt and max_rd_cnt}] are the maximum numbers of RPMB
>     block count (256B) that can be performed to device in one request. 0 implies
> -   no limitation.
> +   no limitation other than the maximum value you can store in \field{block_count} (65535).

So, I'm now wondering what these fields actually refer to. They are u8,
so obviously less than what fits into be16... are reads/writes limited
by anything else?

>  \end{description}
>  
>  \devicenormative{\subsection}{Device Initialization}{Device Types / RPMB Device / Device Initialization}


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[Alex Bennée]

Cornelia Huck <cohuck@redhat.com> writes:

> On Mon, Jun 19 2023, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> Even if 0 meant no limit we are still limited by the field size of the
>> request. That said for a maximum sized partition (* 80 128 1024) you
>> could only actually request 40960 blocks before running out of device.
>> Perhaps it would be better to mark 0 as invalid?
>
> I don't think we can mark 0 as invalid, as it has been in a published
> spec and implementations may be already using it.

Certainly no open source implementations as this was pointed out in a
code review for a kernel driver implementation. But yes there may be
other virtio-rpmb in the field we don't know about.

Current feedback though is there are some difficulties integrating
virtio-rpmb into driver stacks which are designed around accessing RPMB
partitions on exiting storage devices.

I'm not sure how much virtio-rpmb should be adapted to more easily fit
into driver stacks though - there was lukewarm interest in integrating a
common RPMB kernel char based user-space API which doesn't need to
involve the block devices. However more likely consumers of virtio-rpmb
are going to be secure firmwares and early boot code.

> That said, what is the actual limitation? Knowing nothing about RPMB, is
> it 64k (what can fit in block_count), or the 40960 you mentioned above
> as a value defined by other reasons (maximum size of a partition -- I
> assume that cannot change?)

Yes, just above we state:

  \item[\field{capacity}] is the capacity of the device (expressed in 128KB units).
     The values MUST range between 0x00 and 0x80 inclusive.

that said the capacity field has space for a bit more although it itself
is only 8 bit as well.

>
>>
>> Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> Cc: Harald Mommer <hmo@opensynergy.com>
>> Cc: Will Deacon <will@kernel.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  device-types/rpmb/description.tex | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/device-types/rpmb/description.tex b/device-types/rpmb/description.tex
>> index 1dae3fd..2ce8a5b 100644
>> --- a/device-types/rpmb/description.tex
>> +++ b/device-types/rpmb/description.tex
>> @@ -37,7 +37,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / RPMB Device /
>>     The values MUST range between 0x00 and 0x80 inclusive.
>>  \item[\field{max_wr_cnt and max_rd_cnt}] are the maximum numbers of RPMB
>>     block count (256B) that can be performed to device in one request. 0 implies
>> -   no limitation.
>> +   no limitation other than the maximum value you can store in \field{block_count} (65535).
>
> So, I'm now wondering what these fields actually refer to. They are u8,
> so obviously less than what fits into be16... are reads/writes limited
> by anything else?
>
>>  \end{description}
>>  
>>  \devicenormative{\subsection}{Device Initialization}{Device Types / RPMB Device / Device Initialization}


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[Harald Mommer]

Hello,

this damned "unlimited" makes me crazy. Nothing in a computer is unlimited.

How easy could life have been if the virtio spec had used 16 bit values 
for max_wr_cnt and max_rd_cnt in the config space but it is as it is and 
this cannot be changed any more.

On 19.06.23 14:01, Alex Bennée wrote:
> Even if 0 meant no limit we are still limited by the field size of the
> request. That said for a maximum sized partition (* 80 128 1024) you
> could only actually request 40960 blocks before running out of device.
> Perhaps it would be better to mark 0 as invalid?

Where comes this 80 from? I saw a 0x80 in the virtio specification, 
which would be 128 and not 80 decimal.

> Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Cc: Harald Mommer <hmo@opensynergy.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   device-types/rpmb/description.tex | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/device-types/rpmb/description.tex b/device-types/rpmb/description.tex
> index 1dae3fd..2ce8a5b 100644
> --- a/device-types/rpmb/description.tex
> +++ b/device-types/rpmb/description.tex
> @@ -37,7 +37,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / RPMB Device /
>      The values MUST range between 0x00 and 0x80 inclusive.
>   \item[\field{max_wr_cnt and max_rd_cnt}] are the maximum numbers of RPMB
>      block count (256B) that can be performed to device in one request. 0 implies
> -   no limitation.
> +   no limitation other than the maximum value you can store in \field{block_count} (65535).
>   \end{description}

Yes, 65535 is what fits in the 2 byte block count of various underlying 
storage technology specs. So 65535 is an upper limit. Smallest upper 
limit which makes sense? I guess no as there is underlying storage 
technology.

JESD85-B51 for eMMC:

Looking at 6.6.22.4.3 Authenticated Data Write I see that at least for 
write 2 or 3 sizes may be supported:

1 block, 2 blocks and optionally 32 blocks but nothing in between of 2 
and 32.

This is strange. If I understand this correctly (Maybe but I'm not sure) 
the limits of the underlying storage technology cannot be always 
represented cleanly by the virtio config space definitions due to the 
missing 3..31 value range.

JESD220C-2.2 for UFS is more friendly:

14.1.4.4 Geometry descriptor

bRPMB_ReadWriteSize is an 8 bit value, 0 seems not to be foreseen. 255 
is the theoretical maximum and we don't have to cope with 0 means 
"unlimited".

And then I found something in an NVMe spec (NVM Express® Base 
Specification, revision 2.0b)

Figure 275 says that in bytes 315:312 some bits 31:24 (8 bits)

"Access Size: If the Number of RPMB Units field is non-zero, then this 
field indicates the maximum number of 512B units of data that may be 
read or written per RPMB access by Security Send or Security Receive 
commands for the controller. This is a 0’s based value. A value of 0h 
indicates support for one unit of 512B of data."

So if I understand this correctly NVMe has the biggest limit with 255 + 1.

Considering current storage technology it could be that in practice for 
now the best implementation choice when interpreting "unlimited" may be 
to interpret this as "256".

Replacing the word "unlimited" by "65535" may not be what should be done 
here.

>   
>   \devicenormative{\subsection}{Device Initialization}{Device Types / RPMB Device / Device Initialization}


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [RFC PATCH] virtio-rpmb: fix spec land mine in max_[wr|rd]_cnt

[Alex Bennée]

Harald Mommer <harald.mommer@opensynergy.com> writes:

> Hello,
>
> this damned "unlimited" makes me crazy. Nothing in a computer is unlimited.
>
> How easy could life have been if the virtio spec had used 16 bit
> values for max_wr_cnt and max_rd_cnt in the config space but it is as
> it is and this cannot be changed any more.
>
> On 19.06.23 14:01, Alex Bennée wrote:
>> Even if 0 meant no limit we are still limited by the field size of the
>> request. That said for a maximum sized partition (* 80 128 1024) you
>> could only actually request 40960 blocks before running out of device.
>> Perhaps it would be better to mark 0 as invalid?
>
> Where comes this 80 from? I saw a 0x80 in the virtio specification,
> which would be 128 and not 80 decimal.

Ahh yes of course so a potential maximum of 16777216, but as you say...

>> Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> Cc: Harald Mommer <hmo@opensynergy.com>
>> Cc: Will Deacon <will@kernel.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   device-types/rpmb/description.tex | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/device-types/rpmb/description.tex b/device-types/rpmb/description.tex
>> index 1dae3fd..2ce8a5b 100644
>> --- a/device-types/rpmb/description.tex
>> +++ b/device-types/rpmb/description.tex
>> @@ -37,7 +37,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / RPMB Device /
>>      The values MUST range between 0x00 and 0x80 inclusive.
>>   \item[\field{max_wr_cnt and max_rd_cnt}] are the maximum numbers of RPMB
>>      block count (256B) that can be performed to device in one request. 0 implies
>> -   no limitation.
>> +   no limitation other than the maximum value you can store in \field{block_count} (65535).
>>   \end{description}
>
> Yes, 65535 is what fits in the 2 byte block count of various
> underlying storage technology specs. So 65535 is an upper limit.
> Smallest upper limit which makes sense? I guess no as there is
> underlying storage technology.
>
> JESD85-B51 for eMMC:
>
> Looking at 6.6.22.4.3 Authenticated Data Write I see that at least for
> write 2 or 3 sizes may be supported:
>
> 1 block, 2 blocks and optionally 32 blocks but nothing in between of 2
> and 32.
>
> This is strange. If I understand this correctly (Maybe but I'm not
> sure) the limits of the underlying storage technology cannot be always
> represented cleanly by the virtio config space definitions due to the
> missing 3..31 value range.

I guess this depends on the backing store. Currently all the backends
I'm aware of emulate the RPMB storage but surely sometimes the
virtio-rpmb interface will be backed by a "real" RPMB partition.


> JESD220C-2.2 for UFS is more friendly:
>
> 14.1.4.4 Geometry descriptor
>
> bRPMB_ReadWriteSize is an 8 bit value, 0 seems not to be foreseen. 255
> is the theoretical maximum and we don't have to cope with 0 means
> "unlimited".
>
> And then I found something in an NVMe spec (NVM Express® Base
> Specification, revision 2.0b)
>
> Figure 275 says that in bytes 315:312 some bits 31:24 (8 bits)
>
> "Access Size: If the Number of RPMB Units field is non-zero, then this
> field indicates the maximum number of 512B units of data that may be
> read or written per RPMB access by Security Send or Security Receive
> commands for the controller. This is a 0’s based value. A value of 0h
> indicates support for one unit of 512B of data."
>
> So if I understand this correctly NVMe has the biggest limit with 255 + 1.
>
> Considering current storage technology it could be that in practice
> for now the best implementation choice when interpreting "unlimited"
> may be to interpret this as "256".
>
> Replacing the word "unlimited" by "65535" may not be what should be
> done here.

Seems reasonable.

>
>>     \devicenormative{\subsection}{Device Initialization}{Device
>> Types / RPMB Device / Device Initialization}


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/