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

[Alex BennÃe <alex.bennee@linaro.org>]

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