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

[Gregory Haskins <gregory.haskins@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2615 bytes --]

Avi Kivity wrote:
> Gregory Haskins wrote:
>> Avi Kivity wrote:
>>  
>>> Gregory Haskins wrote:
>>>    
>>>> (Applies to Linus' tree, b4348f32dae3cb6eb4bc21c7ed8f76c0b11e9d6a)
>>>>
>>>> Please see patch 1/3 for a description.  This has been tested with
>>>> a KVM
>>>> guest on x86_64 and appears to work properly.  Comments, please.
>>>>         
>>> What about the hypercalls in include/asm/kvm_para.h?
>>>
>>> In general, hypercalls cannot be generic since each hypervisor
>>> implements its own ABI.
>>>     
>> Please see the prologue to 1/3.  Its all described there, including a
>> use case which I think answers your questions.  If there is still
>> ambiguity, let me know.
>>
>>   
>
> Yeah, sorry.
>
>>>   The abstraction needs to be at a higher level (pv_ops is such a
>>> level).
>>>     
>> Yep, agreed.  Thats exactly what this series is doing, actually.
>>   
>
> No, it doesn't.  It makes "making hypercalls" a pv_op, but hypervisors
> don't implement the same ABI.
Yes, that is true, but I think the issue right now is more of
semantics.  I think we are on the same page.

So you would never have someone making a generic
hypercall(KVM_HC_MMU_OP).  I agree.  What I am proposing here is more
akin to PIO-BAR + iowrite()/ioread().  E.g. the infrastructure sets up
the "addressing" (where in PIO this is literally an address, and for
hypercalls this is a vector), but the "device" defines the ABI at that
address.  So its really the "device end-point" that is defining the ABI
here, not the hypervisor (per se) and thats why I thought its ok to
declare these "generic".  But to your point below...

>
> pv_ops all _use_ hypercalls to implement higher level operations, like
> set_pte (probably the only place set_pte can be considered a high
> level operation).
>
> In this case, the higher level event could be
> hypervisor_dynamic_event(number); each pv_ops implementation would use
> its own hypercalls to implement that.

I see.  I had designed it slightly different where KVM could assign any
top level vector it wanted and thus that drove the guest-side interface
you see here to be more "generic hypercall".  However, I think your
proposal is perfectly fine too and it makes sense to more narrowly focus
these calls as specifically "dynamic"...as thats the only vectors that
we could technically use like this anyway.

So rather than allocate a top-level vector, I will add "KVM_HC_DYNAMIC"
to kvm_para.h, and I will change the interface to follow suit (something
like s/hypercall/dynhc).  Sound good?

Thanks, Avi,
-Greg



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 266 bytes --]