On 2/20/19 1:09 PM, Paolo Bonzini wrote:
On 20/02/19 21:15, Joao Martins wrote:
2. PV Driver support (patches 17 - 39)
We start by redirecting hypercalls from the backend to routines
which emulate the behaviour that PV backends expect i.e. grant
table and interdomain events. Next, we add support for late
initialization of xenbus, followed by implementing
frontend/backend communication mechanisms (i.e. grant tables and
interdomain event channels). Finally, introduce xen-shim.ko,
which will setup a limited Xen environment. This uses the added
functionality of Xen specific shared memory (grant tables) and
notifications (event channels).
I am a bit worried by the last patches, they seem really brittle and
prone to breakage. I don't know Xen well enough to understand if the
lack of support for GNTMAP_host_map is fixable, but if not, you have to
define a completely different hypercall.
I assume you are aware of most of this, but just in case, here's the
flow when a backend driver wants to map a grant-reference in the
host: it allocates an empty struct page (via ballooning) and does a
map_grant_ref(GNTMAP_host_map) hypercall. In response, Xen validates the
grant-reference and maps it onto the address associated with the struct
page.
After this, from the POV of the underlying network/block drivers, these
struct pages can be used as just regular pages.
To support this in a KVM environment, where AFAICS no remapping of pages
is possible, the idea was to make minimal changes to the backend drivers
such that map_grant_ref() could just return the PFN from which the
backend could derive the struct page.
To ensure that backends -- when running in this environment -- have been
modified to deal with these new semantics, our map_grant_ref()
implementation explicitly disallows the GNTMAP_host_map flag.
Now if I'm reading you right, you would prefer something more
straightforward -- perhaps similar semantics but a new flag that
makes this behaviour explicit?
Of course, tests are missing. You should use the
tools/testing/selftests/kvm/ framework, and ideally each patch should
come with coverage for the newly-added code.
Agreed.
Thanks
Ankur
Thanks,
Paolo
