Re: [RFC PATCH 0/3] generic hypercall support

[Gregory Haskins]

Chris Wright wrote:
> * Gregory Haskins (gregory.haskins@xxxxxxxxx) wrote:
>   
>> So you would never have someone making a generic
>> hypercall(KVM_HC_MMU_OP).  I agree.
>>     
>
> Which is why I think the interface proposal you've made is wrong.

I respectfully disagree.  Its only wrong in that the name chosen for the
interface was perhaps too broad/vague.  I still believe the concept is
sound, and the general layering is appropriate. 

>   There's
> already hypercall interfaces w/ specific ABI and semantic meaning (which
> are typically called directly/indirectly from an existing pv op hook).
>   

Yes, these are different, thus the new interface.

> But a free-form hypercall(unsigned long nr, unsigned long *args, size_t count)
> means hypercall number and arg list must be the same in order for code
> to call hypercall() in a hypervisor agnostic way.
>   

Yes, and that is exactly the intention.  I think its perhaps the point
you are missing.

I am well aware that historically the things we do over a hypercall
interface would inherently have meaning only to a specific hypervisor
(e.g. KVM_HC_MMU_OPS (vector 2) via kvm_hypercall()).  However, this
doesn't in any way infer that it is the only use for the general
concept.  Its just the only way they have been exploited to date.

While I acknowledge that the hypervisor certainly must be coordinated
with their use, in their essence hypercalls are just another form of IO
joining the ranks of things like MMIO and PIO.  This is an attempt to
bring them out of the bowels of CONFIG_PARAVIRT to make them a first
class citizen. 

The thing I am building here is really not a general hypercall in the
broad sense.  Rather, its a subset of the hypercall vector namespace. 
It is designed specifically for dynamic binding a synchronous call()
interface to things like virtual devices, and it is therefore these
virtual device models that define the particular ABI within that
namespace.  Thus the ABI in question is explicitly independent of the
underlying hypervisor.  I therefore stand by the proposed design to have
this interface described above the hypervisor support layer (i.e.
pv_ops) (albeit with perhaps a better name like "dynamic hypercall" as
per my later discussion with Avi).

Consider PIO: The hypervisor (or hardware) and OS negotiate a port
address, but the two end-points are the driver and the device-model (or
real device).  The driver doesnt have to say:

if (kvm)
   kvm_iowrite32(addr, ..);
else if (lguest)
   lguest_iowrite32(addr, ...);
else
   native_iowrite32(addr, ...);

Instead, it just says "iowrite32(addr, ...);" and the address is used to
route the message appropriately by the platform.  The ABI of that
message, however, is specific to the driver/device and is not
interpreted by kvm/lguest/native-hw infrastructure on the way.

Today, there is no equivelent of a platform agnostic "iowrite32()" for
hypercalls so the driver would look like the pseudocode above except
substitute with kvm_hypercall(), lguest_hypercall(), etc.  The proposal
is to allow the hypervisor to assign a dynamic vector to resources in
the backend and convey this vector to the guest (such as in PCI
config-space as mentioned in my example use-case).  The provides the
"address negotiation" function that would normally be done for something
like a pio port-address.   The hypervisor agnostic driver can then use
this globally recognized address-token coupled with other device-private
ABI parameters to communicate with the device.  This can all occur
without the core hypervisor needing to understand the details beyond the
addressing.

What this means to our interface design is that the only thing the
hypervisor really cares about is the first "nr" parameter.  This acts as
our address-token.  The optional/variable list of args is just payload
as far as the core infrastructure is concerned and are coupled only to
our device ABI.  They were chosen to be an array of ulongs (vs something
like vargs) to reflect the fact that hypercalls are typically passed by
packing registers.

Hope this helps,
-Greg

Attachment: signature.asc
Description: OpenPGP digital signature