Re: [PATCH 4/4] xen/version: Introduce non-truncating XENVER_* subops

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.01.2023 19:34, Andrew Cooper wrote:
> On 04/01/2023 5:04 pm, Jan Beulich wrote:
>> On 03.01.2023 21:09, Andrew Cooper wrote:
>>> API/ABI wise, XENVER_build_id could be merged into xenver_varstring_op(), 
>>> but
>>> the internal infrastructure is awkward.
>> I guess build-id also doesn't fit the rest because of not returning strings,
>> but indeed an array of bytes. You also couldn't use strlen() on the array.
> 
> The format is unspecified, but it is a base64 encoded ASCII string (not
> NUL terminated).

Where are you taking this base64 encoding from? I see the build ID being pure
binary data, which could easily have an embedded nul.

>>> +    if ( sz > INT32_MAX )
>>> +        return -E2BIG; /* Compat guests.  2G ought to be plenty. */
>> While the comment here and in the public header mention compat guests,
>> the check is uniform. What's the deal?
> 
> Well its either this, or a (comat ? INT32_MAX : INT64_MAX) check, along
> with the ifdefary and predicates required to make that compile.
> 
> But there's not a CPU today which can actually move 2G of data (which is
> 4G of L1d bandwidth) without suffering the watchdog (especially as we've
> just read it once for strlen(), so that's 6G of bandwidth), nor do I
> expect this to change in the forseeable future.
> 
> There's some boundary (probably far lower) beyond which we can't use the
> algorithm here.
> 
> There wants to be some limit, and I don't feel it is necessary to make
> it variable on the guest type.

Sure. My question was merely because of the special mentioning of 32-bit /
compat guests. I'm fine with the universal limit, and I'd also be fine
with a lower (universal) bound. All I'm after is that the (to me at least)
confusing comments be adjusted.

> But overall, I'm not seeing a major objection to this change?  In which
> case I'll go ahead and do the tools/ cleanup too for v2.

Well, I'm not entirely convinced of the need for the new subops (we could
as well introduce build time checks to make sure no truncation will occur
for the existing ones), but I also won't object to their introduction. At
least for the command line I can see that we will want (need) to support
more than 1k worth of a string, considering how many options we have
accumulated.

Jan