On 07.10.2021 14:30, Oleksandr wrote:
> On 07.10.21 10:42, Jan Beulich wrote:
>> On 06.10.2021 13:22, Oleksandr Tyshchenko wrote:
>>> Changes V4 -> V5:
>>> - update patch subject and description
>>> - drop Michal's R-b
>>> - pass gpaddr_bits via createdomain domctl
>>> (struct xen_arch_domainconfig)
>> I'm afraid I can't bring this in line with ...
>>
>>> --- a/xen/include/public/arch-arm.h
>>> +++ b/xen/include/public/arch-arm.h
>>> @@ -333,6 +333,11 @@ struct xen_arch_domainconfig {
>>> *
>>> */
>>> uint32_t clock_frequency;
>>> + /*
>>> + * OUT
>>> + * Guest physical address space size
>>> + */
>>> + uint8_t gpaddr_bits;
>> ... this being an OUT field. Is this really what Andrew had asked for?
>> I would have expected the entire struct to be IN (and the comment at
>> the top of the containing struct in public/domctl.h also suggests so,
>> i.e. your new field renders that comment stale). gic_version being
>> IN/OUT is already somewhat in conflict ...
> I am sorry but I'm totally confused now, we want the Xen to provide
> gpaddr_bits to the toolstack, but not the other way around.
> As I understand the main ask was to switch to domctl for which I wanted
> to get some clarification on how it would look like... Well, this patch
> switches to use
> domctl, and I think exactly as it was suggested at [1] in case if a
> common one is a difficult to achieve. I have to admit, I felt it was
> indeed difficult to achieve.
Sadly the mail you reference isn't the one I was referring to. It's not
even from Andrew. Unfortunately I also can't seem to be able to locate
this, i.e. I'm now wondering whether this was under a different subject.
Julien, in any event, confirmed in a recent reply on this thread that
there was such a mail (otherwise I would have started wondering whether
the request was made on irc). In any case it is _that_ mail that would
need going through again.
> I thought that a comment for struct xen_domctl_createdomain in
> public/domctl.h was rather related to the struct fields described in the
> public header than to the arch sub-struct internals described elsewhere.
> But if my assumption is incorrect, then yes the comment got stale and
> probably not by changes in current patch, but after adding
> clock_frequency field (OUT). If so we can add a comment on top of arch
> field clarifying that internal fields *might* be OUT.
>
>
>> One of the problems with
>> _any_ of the fields being OUT is that then it is unclear how the output
>> is intended to be propagated to consumers other than the entity
>> creating the domain.
> If I *properly* understood your concern, we could hide that field in
> struct libxl__domain_build_state and not expose it to struct
> libxl_domain_build_info. Shall I?
I'm afraid I'm lost: I didn't talk about the tool stack at all. While
"consumer" generally means the tool stack, the remark was of more
abstract nature.
Jan
> [1]
> https://lore.kernel.org/xen-devel/[email protected]/
>
>
