[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V5 1/3] xen/arm: Introduce gpaddr_bits field to struct xen_arch_domainconfig

On 07.10.21 15:43, Jan Beulich wrote:

Hi Jan.

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 think, this is the email [1] you are referring to. The subject was changed
to reflect changes in the particular version. This is the third proposition of this patch
(the first two were with arch and common field in sysctl).

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.



[1] https://lore.kernel.org/xen-devel/6a2a183d-c9d8-df2a-41aa-b25283fab197@xxxxxxxxx/


Oleksandr Tyshchenko



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.