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