[PATCH v2 2/2] tools/helpers: set event channel for PVH xenstore-stubdom console

Juergen Gross posted 2 patches 4 years, 2 months ago
There is a newer version of this series
[PATCH v2 2/2] tools/helpers: set event channel for PVH xenstore-stubdom console
Posted by Juergen Gross 4 years, 2 months ago
In contrast to the PFN of the console ring page the event channel of
the console isn't being set automatically by xc_dom_build_image().

Call xc_hvm_param_set() explicitly for that reason.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
 tools/helpers/init-xenstore-domain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 9457d0251b..3eff839827 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -249,6 +249,14 @@ static int build(xc_interface *xch)
             fprintf(stderr, "xc_domain_set_memory_map failed\n");
             goto err;
         }
+
+        rv = xc_hvm_param_set(xch, domid, HVM_PARAM_CONSOLE_EVTCHN,
+                              console_evtchn);
+        if ( rv )
+        {
+            fprintf(stderr, "xc_hvm_param_set failed\n");
+            goto err;
+        }
     }
     rv = xc_dom_build_image(dom);
     if ( rv )
-- 
2.26.2


Re: [PATCH v2 2/2] tools/helpers: set event channel for PVH xenstore-stubdom console
Posted by Andrew Cooper 4 years, 2 months ago
On 06/12/2021 14:29, Juergen Gross wrote:
> In contrast to the PFN of the console ring page the event channel of
> the console isn't being set automatically by xc_dom_build_image().
>
> Call xc_hvm_param_set() explicitly for that reason.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

So, technically, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

However...

That said, there is a distinct lack of joined-up thinking in this interface.

It makes no sense whatsoever to for xc_dom_build_image() to build the
grant, but leave the evtchn to the caller.

And indeed,

xg_dom_x86.c: start_info->console.domU.evtchn = dom->console_evtchn;

we set it up on the PV side of things.  So I think the proper fix is to
wire up the HVM side and prevent the callers needing to do this.


Furthermore, I doubt we skip setting up the xenstore connection.

Really, the users of xc_dom_build_image() want a console Y/n, xenstore
Y/n type interface, and judging by the fields we've already got, that
can reasonably be done on the non-zero-ness of *_evtchn

(It is also weird that the caller is required to bind the evtchn, but
that's so baked into the API that I'd need to rearrange code between
Ocaml daemons to make use of a "library code allocates evtchn+grant
together" option.)

~Andrew