[U-Boot] [PATCH 1/1] avb2.0: add get_size_of_partition()

[Igor Opaniuk]

Hi Andrew,
Sorry I missed your message.

On 9 July 2018 at 18:21, Andrew F. Davis <afd at ti.com> wrote:
> On 07/09/2018 09:52 AM, Sam Protsenko wrote:
>> On Mon, Jul 9, 2018 at 3:15 PM, Igor Opaniuk <igor.opaniuk at linaro.org> wrote:
>>> Implement get_size_of_partition() operation,
>>> which is required by the latest upstream libavb [1].
>>>
>>> [1] https://android.googlesource.com/platform/external/avb/+/master/README.md
>>>
>
>
> I may have missed it, where in here do we need this information? I looks
> to be passed in on the command line for most ops. Has a new function
> been added?

right, it was introduced in 417e8133af46 ("libavb: Load entire
partition if |allow_verification_error| is true.").
When I included the latest libavb for the avb v2 patch series (in v1
the was out-dated version, AFAIK ~1y old),
I noticed this new Avb operation, although it didn't have any impact
on avb verify functionality.
(I was receiving just a warning that no get_size_of_partition is set
in AvbOps structure)

So I decided to introduce the implementation of this function in a
separate patch.

>
>>> Signed-off-by: Igor Opaniuk <igor.opaniuk at linaro.org>
>>> ---
>>
>> Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>
>>
>>>  common/avb_verify.c | 33 ++++++++++++++++++++++++++++++++-
>>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/avb_verify.c b/common/avb_verify.c
>>> index f9a00f8..5eabab0 100644
>>> --- a/common/avb_verify.c
>>> +++ b/common/avb_verify.c
>>> @@ -699,6 +699,37 @@ static AvbIOResult get_unique_guid_for_partition(AvbOps *ops,
>>>  }
>>>
>>>  /**
>>> + * get_size_of_partition() - gets the size of a partition identified
>>> + * by a string name
>>> + *
>>> + * @ops: contains AVB ops handlers
>>> + * @partition: partition name (NUL-terminated UTF-8 string)
>>> + * @out_size_num_bytes: returns the value of a partition size
>>> + *
>>> + * @return:
>>> + *      AVB_IO_RESULT_OK, on success (GUID found)
>>> + *      AVB_IO_RESULT_ERROR_IO, out_size_num_bytes is NULL
>
>
> This does not seems to be the right error code for this, this implies a
> hardware error, maybe AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE is a better
> choice? 'out_size_num_bytes' is a buffer in a way (to 8 bytes).

Frankly, I chose the most "abstract" (if I can say that) error code,
as there is no any in AVB that resembles POSIX EINVAL.
Regarding AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE, which
"is returned if a buffer is too small for the requested operation",
I'm not sure that it's proper error either.

There is a question spinning in my mind "should we do any param verification?",
as only libavb is using these functions,
and no theoretical case is possible when an invalid value is provided.

>
> Andrew
>
>
>>> + *      AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION, if partition was not found
>>> + */
>>> +static AvbIOResult get_size_of_partition(AvbOps *ops,
>>> +                                        const char *partition,
>>> +                                        u64 *out_size_num_bytes)
>>> +{
>>> +       struct mmc_part *part;
>>> +
>>> +       if (!out_size_num_bytes)
>>> +               return AVB_IO_RESULT_ERROR_IO;
>>> +
>>> +       part = get_partition(ops, partition);
>>> +       if (!part)
>>> +               return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION;
>>> +
>>> +       *out_size_num_bytes = part->info.blksz * part->info.size;
>>> +
>>> +       return AVB_IO_RESULT_OK;
>>> +}
>>> +
>>> +/**
>>>   * ============================================================================
>>>   * AVB2.0 AvbOps alloc/initialisation/free
>>>   * ============================================================================
>>> @@ -721,7 +752,7 @@ AvbOps *avb_ops_alloc(int boot_device)
>>>         ops_data->ops.read_is_device_unlocked = read_is_device_unlocked;
>>>         ops_data->ops.get_unique_guid_for_partition =
>>>                 get_unique_guid_for_partition;
>>> -
>>> +       ops_data->ops.get_size_of_partition = get_size_of_partition;
>>>         ops_data->mmc_dev = boot_device;
>>>
>>>         return &ops_data->ops;
>>> --
>>> 2.7.4
>>>



-- 
Regards,
Igor Opaniuk