[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 08.10.21 11:13, Jan Beulich wrote:

Hi Jan

On 07.10.2021 22:23, Stefano Stabellini wrote:
On Thu, 7 Oct 2021, Jan Beulich wrote:
On 07.10.2021 15:12, Oleksandr wrote:
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.
Well, that's still a mail you sent, not Andrew's. And while I have yours
in my mailbox, I don't have Andrew's for whatever reason.

Nevertheless there's enough context to be halfway certain that this
wasn't meant as an extension to the create domctl, but rather a separate
new one (merely replacing what you had originally as a sysctl to become
per-domain, to allow returning varying [between domains] values down the
road). I continue to think that if such a field was added to "create",
it would be an input (only).
During the Xen Community Call on Tuesday, we briefly spoke about this.
Andrew confirmed that what he meant with his original email is to use a
domctl. We didn't discuss which domctl specifically.

This patch now follows the same pattern of clock_frequency and
gic_version (see xen/include/public/arch-arm.h:struct xen_arch_domainconfig).
Note that gic_version is an IN/OUT parameter, showing that if in the
future we want the ability to set gpaddr_bits (in addition to get
gpaddr_bits), it will be possible.
Well, as said before - I'm not convinced gic_version being IN/OUT is
appropriate. At the very least a 2nd way to merely retrieve the value
would seem to be necessary, so that it's not only the party creating
the guest which would be able to know.

Since here's we're solely after retrieving the value, I don't see
the point in altering "create". As you say, domctl can be changed,
and hence at the point this needs to become an input to "create", it
could easily be added there.


Just a quick question. What do you think can XEN_DOMCTL_getdomaininfo be reused to retrieve gpaddr_bits? I don't see why not at the moment, but maybe there are some implications/concerns which are invisible to me.

I see that arch_get_domain_info() is present, so the field will be common, and each arch will write a value it considers appropriate. This could be a good compromise to not add an extra domctl and to not alter domain_create.


Oleksandr Tyshchenko



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