Re: [RFC PATCH 12/25] xen: Replace sysctl/readconsole with autogenerated version

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2024 15:36, Alejandro Vallejo wrote:
> On Tue Nov 26, 2024 at 1:20 PM GMT, Jan Beulich wrote:
>> On 26.11.2024 13:27, Alejandro Vallejo wrote:
>>> On Tue Nov 26, 2024 at 9:40 AM GMT, Jan Beulich wrote:
>>>> On 25.11.2024 19:51, Alejandro Vallejo wrote:
>>>>> On Mon Nov 25, 2024 at 12:05 PM GMT, Jan Beulich wrote:
>>>>>> On 15.11.2024 12:51, Alejandro Vallejo wrote:
>>>>>>> Describe sysctl/readconsole as a TOML specification, remove old
>>>>>>> hand-coded version and replace it with autogenerated file.
>>>>>>>
>>>>>>> While at it, transform the console driver to use uint8_t rather than
>>>>>>> char in order to mandate the type to be unsigned and ensure the ABI is
>>>>>>> not defined with regards to C-specific types.
>>>>>>
>>>>>> Yet the derived C representation imo then should still be using char, not
>>>>>> uint8_t.
>>>>>
>>>>> There's 2 issued addressed by this patch.
>>>>>
>>>>>   1. The removal of char from the external headers (and the Xen driver).
>>>>>   2. The replacement of the existing struct by the autogenerated one.
>>>>>
>>>>> (1) wants doing irrespective of (2). char has neither a fixed width nor a 
>>>>> fixed
>>>>> sign. Which is irrelevant for ABI purposes in this case because what we 
>>>>> really
>>>>> meant is "give me a pointer" in this hypercall, but it may be important in
>>>>> other cases.
>>>>>
>>>>> IOW, char should've never made it to the definition of the public ABI, 
>>>>> and I'm
>>>>> merely taking the chance to take it out. Happy to extract this patch and 
>>>>> send
>>>>> it separately.
>>>>
>>>> Well, work towards fully getting char out of the public headers may indeed 
>>>> be
>>>> worthwhile. Otoh with char being the basic addressing granularity, I think
>>>> the ABI is pretty much tied to sizeof(char) == 1, imo limiting the
>>>> worthwhile-ness quite a bit.
>>>
>>> Let me put it another way. If I were to create a separate patch stripping 
>>> char
>>> and using uint8_t instead, what are my chances of getting an Acked-by? Or 
>>> not a
>>> NAK, at least. (there's other maintainers that I need that from, but one 
>>> step
>>> at a time).
>>
>> That would to some degree depend on what other maintainers think. Not a 
>> straight
>> NAK in any event.
>>
>>>> Signed-ness of plain char doesn't really matter as long as it's used only 
>>>> for
>>>> what really are characters (or strings thereof). And that looks the be 
>>>> pretty
>>>> much the case throughout the public headers.
>>>
>>> Maybe. Still, as a general principle caller and callee ought to agree on 
>>> size,
>>> alignment and sign for every type. I'd rather not make exceptions for that
>>> invariant unless truly well motivated. And in this case it's a case of
>>> requiring trivial non-functional changes.
>>
>> In how far they're non-functional will need to be seen. You also need to keep
>> consumers in mind: They may face sudden type disagreement that compilers may
>> complain about. Yet "stable" for the public headers means not just the ABI
>> itself being stable, but updated headers also being usable as drop-in
>> replacements for older versions.
> 
> Would it be fair to say that users of the Xen low-level API strictly go via
> xenctrl et al?

No - recall we're meanwhile talking about all public headers, not just ones
limited to toolstack use. Kernels (and alike, e.g. xtf) obviously won't use
xenctrl.

Jan