Re: [PATCH 12/12] hw/xen: add support for Xen primary console in emulated mode

[Paul Durrant]

On 24/10/2023 17:34, David Woodhouse wrote:
> On Tue, 2023-10-24 at 17:25 +0100, Paul Durrant wrote:
> > On 24/10/2023 16:49, David Woodhouse wrote:
> > > On Tue, 2023-10-24 at 16:39 +0100, Paul Durrant wrote:
> > > > On 24/10/2023 16:37, David Woodhouse wrote:
> > > > > On Tue, 2023-10-24 at 15:20 +0100, Paul Durrant wrote:
> > > > > > On 16/10/2023 16:19, David Woodhouse wrote:
> > > > > > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > > > > >
> > > > > > > The primary console is special because the toolstack maps a page at a
> > > > > > > fixed GFN and also allocates the guest-side event channel. Add support
> > > > > > > for that in emulated mode, so that we can have a primary console.
> > > > > > >
> > > > > > > Add a *very* rudimentary stub of foriegnmem ops for emulated mode, which
> > > > > > > supports literally nothing except a single-page mapping of the console
> > > > > > > page. This might as well have been a hack in the xen_console driver, but
> > > > > > > this way at least the special-casing is kept within the Xen emulation
> > > > > > > code, and it gives us a hook for a more complete implementation if/when
> > > > > > > we ever do need one.
> > > > > > Why can't you map the console page via the grant table like the xenstore
> > > > > > page?
> > > > >
> > > > > I suppose we could, but I didn't really want the generic xen-console
> > > > > device code having any more of a special case for 'Xen emulation' than
> > > > > it does already by having to call xen_primary_console_create().
> > > >
> > > > But doesn't is save you the whole foreignmem thing? You can use the
> > > > grant table for primary and secondary consoles.
> > >
> > > Yes. And I could leave the existing foreignmem thing just for the case
> > > of primary console under true Xen. It's probably not that awful a
> > > special case, in the end.
> > >
> > > Then again, I was surprised I didn't *already* have a foreignmem ops
> > > for the emulated case, and we're probably going to want to continue
> > > fleshing it out later, so I don't really mind adding it.
> >
> > True. We'll need it for some of the other more fun protocols like vkbd
> > or fb. Still, I think it'd be nicer to align the xenstore and primary
> > console code to look similar and punt the work until then :-)
>
> I don't think it ends up looking like xenstore either way, does it?
> Xenstore is special because it gets to use the original pointer to its
> own page.

Not sure what you mean there? A guest can query the PFN for either 
xenstore or console using HVM params, or it can find them in its own 
grant table entries 0 or 1.

> I don't think I want to hack the xen_console code to explicitly call a
> xen_console_give_me_your_page() function. If not foreignmem, I think
> you were suggesting that we actually call the grant mapping code to get
> a pointer to the underlying page, right?

I'm suggesting that the page be mapped in the same way that the xenstore 
backend does:

1462    /* 

1463     * We don't actually access the guest's page through the grant, 
because
1464     * this isn't real Xen, and we can just use the page we gave it 
in the
1465     * first place. Map the grant anyway, mostly for cosmetic 
purposes so
1466     * it *looks* like it's in use in the guest-visible grant table. 

1467     */
1468    s->gt = qemu_xen_gnttab_open();
1469    uint32_t xs_gntref = GNTTAB_RESERVED_XENSTORE;
1470    s->granted_xs = qemu_xen_gnttab_map_refs(s->gt, 1, xen_domid, 
&xs_gntref,
1471                                             PROT_READ | PROT_WRITE);


> I could kind of live with that... except that Xen has this ugly
> convention that the "ring-ref" frontend node for the primary console
> actually has the *MFN* not a grant ref. Which I don't understand since
> the toolstack *does* populate the grant table for it (just as it does
> for the xenstore page). But we'd have to add a special case exception
> to that special case, so that in the emu case it's an actual grant ref
> again. I think I prefer just having a stub of foreignmem, TBH.

You're worried about the guest changing the page it uses for the primary 
console and putting a new one in xenstore? I'd be amazed if that even 
works on Xen unless the guest is careful to write it into 
GNTTAB_RESERVED_CONSOLE.

> (I didn't yet manage to get Xen to actually create a primary console of
> type iomem, FWIW)

No, that doesn't entirely surprise me.

  Paul