diff mbox series

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

Message ID 20231016151909.22133-13-dwmw2@infradead.org (mailing list archive)
State New, archived
Headers show
Series Get Xen PV shim running in qemu | expand

Commit Message

David Woodhouse Oct. 16, 2023, 3:19 p.m. UTC
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.

Now at last we can boot the Xen PV shim and run PV kernels in QEMU.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/char/xen_console.c             |  12 ++-
 hw/i386/kvm/meson.build           |   1 +
 hw/i386/kvm/trace-events          |   2 +
 hw/i386/kvm/xen-stubs.c           |   5 +
 hw/i386/kvm/xen_gnttab.c          |  32 +++++-
 hw/i386/kvm/xen_primary_console.c | 167 ++++++++++++++++++++++++++++++
 hw/i386/kvm/xen_primary_console.h |  22 ++++
 hw/i386/kvm/xen_xenstore.c        |   9 ++
 target/i386/kvm/xen-emu.c         |  23 +++-
 9 files changed, 266 insertions(+), 7 deletions(-)
 create mode 100644 hw/i386/kvm/xen_primary_console.c
 create mode 100644 hw/i386/kvm/xen_primary_console.h

Comments

Paul Durrant Oct. 24, 2023, 2:20 p.m. UTC | #1
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?

   Paul
David Woodhouse Oct. 24, 2023, 3:37 p.m. UTC | #2
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().
Paul Durrant Oct. 24, 2023, 3:39 p.m. UTC | #3
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.

   Paul
David Woodhouse Oct. 24, 2023, 3:49 p.m. UTC | #4
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.
Paul Durrant Oct. 24, 2023, 4:25 p.m. UTC | #5
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 :-)

   Paul
David Woodhouse Oct. 24, 2023, 4:34 p.m. UTC | #6
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.

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 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.

(I didn't yet manage to get Xen to actually create a primary console of
type iomem, FWIW)
Paul Durrant Oct. 25, 2023, 8:31 a.m. UTC | #7
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
David Woodhouse Oct. 25, 2023, 9 a.m. UTC | #8
On Wed, 2023-10-25 at 09:31 +0100, Paul Durrant wrote:
> 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.

The code in our xen_xenstore.c uses its *own* pointer (s->xs) to the
MemoryRegion that it created (s->xenstore_page). It is its own backend,
as well as doing the "magic" to create the guest-side mapping and event
channel.

The difference for the console code is that we actually have a
*separation* between the standard backend code in xen_console.c, and
the magic frontend parts for the emulated mode.


> 
> > 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);

It already *is*. But as with xen_xenstore.c, nothing ever *uses* the
s->granted_xs pointer. It's just cosmetic to make the grant table look
right.

But that doesn't help the *backend* code. The backend doesn't even know
the grant ref#, because the convention we inherited from Xen is that
the `ring-ref` in XenStore for the primary console is actually the MFN,
to be mapped as foreignmem.

Of course, we *do* know the grant-ref for the primary console, as it's
always GNTTAB_RESERVED_CONSOLE. So I suppose we could put a hack into
the xen_console backend to map *that* in the case of primary console
under emu? In fact that would probably do the right thing even under
Xen if we could persuade Xen to make an ioemu primary console?





> > 
> > 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.

Not worried about the guest changing it. I was mostly just concerned
about the xen-console having to have another special case and magically
"know" it. But I suppose I can live with it being hard-coded to
GNTTAB_RESERVED_CONSOLE. I'll knock that up and see how it makes me
feel.

I'm reworking some of that connect/disconnect code anyway, to have the
backend tell the primary_console code directly what the backend port#
is, so I can remove the soft-reset hacks in xen_evtchn.c entirely.
Paul Durrant Oct. 25, 2023, 10:44 a.m. UTC | #9
On 25/10/2023 10:00, David Woodhouse wrote:
> On Wed, 2023-10-25 at 09:31 +0100, Paul Durrant wrote:
>> 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.
> 
> The code in our xen_xenstore.c uses its *own* pointer (s->xs) to the
> MemoryRegion that it created (s->xenstore_page). It is its own backend,
> as well as doing the "magic" to create the guest-side mapping and event
> channel.
> 
> The difference for the console code is that we actually have a
> *separation* between the standard backend code in xen_console.c, and
> the magic frontend parts for the emulated mode.
> 
> 
>>
>>> 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);
> 
> It already *is*. But as with xen_xenstore.c, nothing ever *uses* the
> s->granted_xs pointer. It's just cosmetic to make the grant table look
> right.
> 
> But that doesn't help the *backend* code. The backend doesn't even know
> the grant ref#, because the convention we inherited from Xen is that
> the `ring-ref` in XenStore for the primary console is actually the MFN,
> to be mapped as foreignmem.
> 
> Of course, we *do* know the grant-ref for the primary console, as it's
> always GNTTAB_RESERVED_CONSOLE. So I suppose we could put a hack into
> the xen_console backend to map *that* in the case of primary console
> under emu? In fact that would probably do the right thing even under
> Xen if we could persuade Xen to make an ioemu primary console?
> 

That's exactly what I am getting at :-) I don't think we need care about 
the ring-ref in xenstore for the primary console.

   Paul

> 
> 
> 
> 
>>>
>>> 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.
> 
> Not worried about the guest changing it. I was mostly just concerned
> about the xen-console having to have another special case and magically
> "know" it. But I suppose I can live with it being hard-coded to
> GNTTAB_RESERVED_CONSOLE. I'll knock that up and see how it makes me
> feel.
> 
> I'm reworking some of that connect/disconnect code anyway, to have the
> backend tell the primary_console code directly what the backend port#
> is, so I can remove the soft-reset hacks in xen_evtchn.c entirely.
diff mbox series

Patch

diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 1a0f5ed3e1..dfc4be0aa1 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -32,6 +32,7 @@ 
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "hw/xen/interface/io/console.h"
+#include "hw/i386/kvm/xen_primary_console.h"
 #include "trace.h"
 
 struct buffer {
@@ -334,8 +335,8 @@  static char *xen_console_get_name(XenDevice *xendev, Error **errp)
     XenConsole *con = XEN_CONSOLE_DEVICE(xendev);
 
     if (con->dev == -1) {
+        int idx = (xen_mode == XEN_EMULATE) ? 0 : 1;
         char name[11];
-        int idx = 1;
 
         /* Theoretically we could go up to INT_MAX here but that's overkill */
         while (idx < 100) {
@@ -386,10 +387,13 @@  static void xen_console_realize(XenDevice *xendev, Error **errp)
      * be mapped directly as foreignmem (not a grant ref), and the guest port
      * was allocated *for* the guest by the toolstack. The guest gets these
      * through HVMOP_get_param and can use the console long before it's got
-     * XenStore up and running. We cannot create those for a Xen guest.
+     * XenStore up and running. We cannot create those for a true Xen guest,
+     * but we can for Xen emulation.
      */
     if (!con->dev) {
-        if (xen_device_frontend_scanf(xendev, "ring-ref", "%u", &u) != 1 ||
+        if (xen_mode == XEN_EMULATE) {
+            xen_primary_console_create();
+        } else if (xen_device_frontend_scanf(xendev, "ring-ref", "%u", &u) != 1 ||
             xen_device_frontend_scanf(xendev, "port", "%u", &u) != 1) {
             error_setg(errp, "cannot create primary Xen console");
             return;
@@ -404,7 +408,7 @@  static void xen_console_realize(XenDevice *xendev, Error **errp)
     }
 
     /* No normal PV driver initialization for the primary console */
-    if (!con->dev) {
+    if (!con->dev && xen_mode != XEN_EMULATE) {
         xen_console_connect(xendev, errp);
     }
 }
diff --git a/hw/i386/kvm/meson.build b/hw/i386/kvm/meson.build
index ab143d6474..a4a2e23c06 100644
--- a/hw/i386/kvm/meson.build
+++ b/hw/i386/kvm/meson.build
@@ -9,6 +9,7 @@  i386_kvm_ss.add(when: 'CONFIG_XEN_EMU', if_true: files(
   'xen_evtchn.c',
   'xen_gnttab.c',
   'xen_xenstore.c',
+  'xen_primary_console.c',
   'xenstore_impl.c',
   ))
 
diff --git a/hw/i386/kvm/trace-events b/hw/i386/kvm/trace-events
index e4c82de6f3..67bf7f174e 100644
--- a/hw/i386/kvm/trace-events
+++ b/hw/i386/kvm/trace-events
@@ -18,3 +18,5 @@  xenstore_watch(const char *path, const char *token) "path %s token %s"
 xenstore_unwatch(const char *path, const char *token) "path %s token %s"
 xenstore_reset_watches(void) ""
 xenstore_watch_event(const char *path, const char *token) "path %s token %s"
+xen_primary_console_create(void) ""
+xen_primary_console_reset(int port) "port %u"
diff --git a/hw/i386/kvm/xen-stubs.c b/hw/i386/kvm/xen-stubs.c
index ae406e0b02..10068970fe 100644
--- a/hw/i386/kvm/xen-stubs.c
+++ b/hw/i386/kvm/xen-stubs.c
@@ -15,6 +15,7 @@ 
 #include "qapi/qapi-commands-misc-target.h"
 
 #include "xen_evtchn.h"
+#include "xen_primary_console.h"
 
 void xen_evtchn_snoop_msi(PCIDevice *dev, bool is_msix, unsigned int vector,
                           uint64_t addr, uint32_t data, bool is_masked)
@@ -30,6 +31,10 @@  bool xen_evtchn_deliver_pirq_msi(uint64_t address, uint32_t data)
     return false;
 }
 
+void xen_primary_console_create(void)
+{
+}
+
 #ifdef TARGET_I386
 EvtchnInfoList *qmp_xen_event_list(Error **errp)
 {
diff --git a/hw/i386/kvm/xen_gnttab.c b/hw/i386/kvm/xen_gnttab.c
index 21c30e3659..ea201cd582 100644
--- a/hw/i386/kvm/xen_gnttab.c
+++ b/hw/i386/kvm/xen_gnttab.c
@@ -25,6 +25,7 @@ 
 #include "hw/xen/xen_backend_ops.h"
 #include "xen_overlay.h"
 #include "xen_gnttab.h"
+#include "xen_primary_console.h"
 
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_xen.h"
@@ -38,6 +39,7 @@  OBJECT_DECLARE_SIMPLE_TYPE(XenGnttabState, XEN_GNTTAB)
 #define ENTRIES_PER_FRAME_V1 (XEN_PAGE_SIZE / sizeof(grant_entry_v1_t))
 
 static struct gnttab_backend_ops emu_gnttab_backend_ops;
+static struct foreignmem_backend_ops emu_foreignmem_backend_ops;
 
 struct XenGnttabState {
     /*< private >*/
@@ -100,6 +102,7 @@  static void xen_gnttab_realize(DeviceState *dev, Error **errp)
     s->map_track = g_new0(uint8_t, s->max_frames * ENTRIES_PER_FRAME_V1);
 
     xen_gnttab_ops = &emu_gnttab_backend_ops;
+    xen_foreignmem_ops = &emu_foreignmem_backend_ops;
 }
 
 static int xen_gnttab_post_load(void *opaque, int version_id)
@@ -524,6 +527,29 @@  static struct gnttab_backend_ops emu_gnttab_backend_ops = {
     .unmap = xen_be_gnttab_unmap,
 };
 
+/* Dummy implementation of foriegnmem; just enough for console */
+static void *xen_be_foreignmem_map(uint32_t dom, void *addr, int prot,
+                                   size_t pages, xen_pfn_t *pfns,
+                                   int *errs)
+{
+    if (dom == xen_domid && !addr && pages == 1 &&
+        pfns[0] == xen_primary_console_get_pfn()) {
+        return xen_primary_console_get_map();
+    }
+
+    return NULL;
+}
+
+static int xen_be_foreignmem_unmap(void *addr, size_t pages)
+{
+    return 0;
+}
+
+static struct foreignmem_backend_ops emu_foreignmem_backend_ops = {
+    .map = xen_be_foreignmem_map,
+    .unmap = xen_be_foreignmem_unmap,
+};
+
 int xen_gnttab_reset(void)
 {
     XenGnttabState *s = xen_gnttab_singleton;
@@ -537,10 +563,14 @@  int xen_gnttab_reset(void)
     s->nr_frames = 0;
 
     memset(s->entries.v1, 0, XEN_PAGE_SIZE * s->max_frames);
-
     s->entries.v1[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
     s->entries.v1[GNTTAB_RESERVED_XENSTORE].frame = XEN_SPECIAL_PFN(XENSTORE);
 
+    if (xen_primary_console_get_pfn()) {
+        s->entries.v1[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
+        s->entries.v1[GNTTAB_RESERVED_CONSOLE].frame = XEN_SPECIAL_PFN(CONSOLE);
+    }
+
     memset(s->map_track, 0, s->max_frames * ENTRIES_PER_FRAME_V1);
 
     return 0;
diff --git a/hw/i386/kvm/xen_primary_console.c b/hw/i386/kvm/xen_primary_console.c
new file mode 100644
index 0000000000..0aa1c16ad6
--- /dev/null
+++ b/hw/i386/kvm/xen_primary_console.c
@@ -0,0 +1,167 @@ 
+/*
+ * QEMU Xen emulation: Primary console support
+ *
+ * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Authors: David Woodhouse <dwmw2@infradead.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+
+#include "hw/sysbus.h"
+#include "hw/xen/xen.h"
+#include "hw/xen/xen_backend_ops.h"
+#include "xen_evtchn.h"
+#include "xen_overlay.h"
+#include "xen_primary_console.h"
+
+#include "sysemu/kvm.h"
+#include "sysemu/kvm_xen.h"
+
+#include "trace.h"
+
+#include "hw/xen/interface/event_channel.h"
+#include "hw/xen/interface/grant_table.h"
+
+#define TYPE_XEN_PRIMARY_CONSOLE "xen-primary-console"
+OBJECT_DECLARE_SIMPLE_TYPE(XenPrimaryConsoleState, XEN_PRIMARY_CONSOLE)
+
+struct XenPrimaryConsoleState {
+    /*< private >*/
+    SysBusDevice busdev;
+    /*< public >*/
+
+    MemoryRegion console_page;
+    void *cp;
+
+    evtchn_port_t guest_port;
+    evtchn_port_t be_port;
+
+    struct xengntdev_handle *gt;
+    void *granted_xs;
+};
+
+struct XenPrimaryConsoleState *xen_primary_console_singleton;
+
+static void xen_primary_console_realize(DeviceState *dev, Error **errp)
+{
+    XenPrimaryConsoleState *s = XEN_PRIMARY_CONSOLE(dev);
+
+    if (xen_mode != XEN_EMULATE) {
+        error_setg(errp, "Xen primary console support is for Xen emulation");
+        return;
+    }
+
+    memory_region_init_ram(&s->console_page, OBJECT(dev), "xen:console_page",
+                           XEN_PAGE_SIZE, &error_abort);
+    memory_region_set_enabled(&s->console_page, true);
+    s->cp = memory_region_get_ram_ptr(&s->console_page);
+    memset(s->cp, 0, XEN_PAGE_SIZE);
+
+    /* We can't map it this early as KVM isn't ready */
+    xen_primary_console_singleton = s;
+}
+
+static void xen_primary_console_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = xen_primary_console_realize;
+}
+
+static const TypeInfo xen_primary_console_info = {
+    .name          = TYPE_XEN_PRIMARY_CONSOLE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XenPrimaryConsoleState),
+    .class_init    = xen_primary_console_class_init,
+};
+
+
+void xen_primary_console_create(void)
+{
+    DeviceState *dev = sysbus_create_simple(TYPE_XEN_PRIMARY_CONSOLE, -1, NULL);
+
+    trace_xen_primary_console_create();
+
+    xen_primary_console_singleton = XEN_PRIMARY_CONSOLE(dev);
+
+    /*
+     * Defer the init (xen_primary_console_reset()) until KVM is set up and the
+     * overlay page can be mapped.
+     */
+}
+
+static void xen_primary_console_register_types(void)
+{
+    type_register_static(&xen_primary_console_info);
+}
+
+type_init(xen_primary_console_register_types)
+
+uint16_t xen_primary_console_get_port(void)
+{
+    XenPrimaryConsoleState *s = xen_primary_console_singleton;
+    if (!s) {
+        return 0;
+    }
+    return s->guest_port;
+}
+
+uint64_t xen_primary_console_get_pfn(void)
+{
+    XenPrimaryConsoleState *s = xen_primary_console_singleton;
+    if (!s) {
+        return 0;
+    }
+    return XEN_SPECIAL_PFN(CONSOLE);
+}
+
+void *xen_primary_console_get_map(void)
+{
+    XenPrimaryConsoleState *s = xen_primary_console_singleton;
+    if (!s) {
+        return 0;
+    }
+    return s->cp;
+}
+
+static void alloc_guest_port(XenPrimaryConsoleState *s)
+{
+    struct evtchn_alloc_unbound alloc = {
+        .dom = DOMID_SELF,
+        .remote_dom = DOMID_QEMU,
+    };
+
+    if (!xen_evtchn_alloc_unbound_op(&alloc)) {
+        s->guest_port = alloc.port;
+    }
+}
+
+int xen_primary_console_reset(void)
+{
+    XenPrimaryConsoleState *s = xen_primary_console_singleton;
+    if (!s) {
+        return 0;
+    }
+
+    if (!memory_region_is_mapped(&s->console_page)) {
+        uint64_t gpa = XEN_SPECIAL_PFN(CONSOLE) << TARGET_PAGE_BITS;
+        xen_overlay_do_map_page(&s->console_page, gpa);
+    }
+
+    alloc_guest_port(s);
+
+    trace_xen_primary_console_reset(s->guest_port);
+
+    s->gt = qemu_xen_gnttab_open();
+    uint32_t xs_gntref = GNTTAB_RESERVED_CONSOLE;
+    s->granted_xs = qemu_xen_gnttab_map_refs(s->gt, 1, xen_domid, &xs_gntref,
+                                             PROT_READ | PROT_WRITE);
+
+    return 0;
+}
diff --git a/hw/i386/kvm/xen_primary_console.h b/hw/i386/kvm/xen_primary_console.h
new file mode 100644
index 0000000000..dd4922f3f4
--- /dev/null
+++ b/hw/i386/kvm/xen_primary_console.h
@@ -0,0 +1,22 @@ 
+/*
+ * QEMU Xen emulation: Primary console support
+ *
+ * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Authors: David Woodhouse <dwmw2@infradead.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_XEN_PRIMARY_CONSOLE_H
+#define QEMU_XEN_PRIMARY_CONSOLE_H
+
+void xen_primary_console_create(void);
+int xen_primary_console_reset(void);
+
+uint16_t xen_primary_console_get_port(void);
+uint64_t xen_primary_console_get_pfn(void);
+void *xen_primary_console_get_map(void);
+
+#endif /* QEMU_XEN_PRIMARY_CONSOLE_H */
diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 3300e0614a..9f8946e0ce 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -25,6 +25,7 @@ 
 #include "hw/xen/xen_backend_ops.h"
 #include "xen_overlay.h"
 #include "xen_evtchn.h"
+#include "xen_primary_console.h"
 #include "xen_xenstore.h"
 
 #include "sysemu/kvm.h"
@@ -1432,6 +1433,7 @@  static void alloc_guest_port(XenXenstoreState *s)
 int xen_xenstore_reset(void)
 {
     XenXenstoreState *s = xen_xenstore_singleton;
+    int console_port;
     GList *perms;
     int err;
 
@@ -1467,6 +1469,13 @@  int xen_xenstore_reset(void)
     relpath_printf(s, perms, "store/ring-ref", "%lu", XEN_SPECIAL_PFN(XENSTORE));
     relpath_printf(s, perms, "store/port", "%u", s->be_port);
 
+    console_port = xen_primary_console_get_port();
+    if (console_port) {
+        relpath_printf(s, perms, "console/ring-ref", "%lu", XEN_SPECIAL_PFN(CONSOLE));
+        relpath_printf(s, perms, "console/port", "%u", console_port);
+        relpath_printf(s, perms, "console/state", "%u", XenbusStateInitialised);
+    }
+
     g_list_free_full(perms, g_free);
 
     /*
diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c
index 477e93cd92..9f57786e95 100644
--- a/target/i386/kvm/xen-emu.c
+++ b/target/i386/kvm/xen-emu.c
@@ -28,6 +28,7 @@ 
 #include "hw/i386/kvm/xen_overlay.h"
 #include "hw/i386/kvm/xen_evtchn.h"
 #include "hw/i386/kvm/xen_gnttab.h"
+#include "hw/i386/kvm/xen_primary_console.h"
 #include "hw/i386/kvm/xen_xenstore.h"
 
 #include "hw/xen/interface/version.h"
@@ -182,7 +183,8 @@  int kvm_xen_init(KVMState *s, uint32_t hypercall_msr)
         return ret;
     }
 
-    /* The page couldn't be overlaid until KVM was initialized */
+    /* The pages couldn't be overlaid until KVM was initialized */
+    xen_primary_console_reset();
     xen_xenstore_reset();
 
     return 0;
@@ -811,11 +813,23 @@  static bool handle_get_param(struct kvm_xen_exit *exit, X86CPU *cpu,
     case HVM_PARAM_STORE_EVTCHN:
         hp.value = xen_xenstore_get_port();
         break;
+    case HVM_PARAM_CONSOLE_PFN:
+        hp.value = xen_primary_console_get_pfn();
+        if (!hp.value) {
+            err = -EINVAL;
+        }
+        break;
+    case HVM_PARAM_CONSOLE_EVTCHN:
+        hp.value = xen_primary_console_get_port();
+        if (!hp.value) {
+            err = -EINVAL;
+        }
+        break;
     default:
         return false;
     }
 
-    if (kvm_copy_to_gva(cs, arg, &hp, sizeof(hp))) {
+    if (!err && kvm_copy_to_gva(cs, arg, &hp, sizeof(hp))) {
         err = -EFAULT;
     }
 out:
@@ -1426,6 +1440,11 @@  int kvm_xen_soft_reset(void)
         return err;
     }
 
+    err = xen_primary_console_reset();
+    if (err) {
+        return err;
+    }
+
     err = xen_xenstore_reset();
     if (err) {
         return err;