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@xxxxxxxxxxxx> > > > > > > > > > > > > > > > > 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.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
