Re: [KVM PATCH] KVM: introduce "xinterface" API for external interaction with guests

[Gregory Haskins <ghaskins@novell.com>]

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

Anthony Liguori wrote:
> Gregory Haskins wrote:
>> +/*
>> + * ------------
>> + * XINTERFACE (External Interface)
>> + * -------------
>> + */
>> +
>> +static struct kvm *
>> +intf_to_kvm(struct kvm_xinterface *intf)
>> +{
>> +    return container_of(intf, struct kvm, xinterface);
>> +}
>> +
>> +static unsigned long
>> +xinterface_gpa_to_hva(struct kvm_xinterface *intf, unsigned long gpa)
>> +{
>> +    struct kvm *kvm = intf_to_kvm(intf);
>> +    unsigned long addr;
>> +
>> +    addr = gfn_to_hva(kvm, gpa >> PAGE_SHIFT);
>> +    if (kvm_is_error_hva(addr))
>> +        return 0;
>> +
>> +    return addr + offset_in_page(gpa);
>> +}
>> +
>> +static struct page *
>> +xinterface_gpa_to_page(struct kvm_xinterface *intf, unsigned long gpa)
>> +{
>> +    struct kvm *kvm = intf_to_kvm(intf);
>> +    struct page *page;
>> +
>> +    page = gfn_to_page(kvm, gpa >> PAGE_SHIFT);
>> +    if (page == bad_page)
>> +        return ERR_PTR(-EINVAL);
>> +
>> +    return page;
>> +}
>> +
>> +static void
>> +xinterface_release(struct kvm_xinterface *intf)
>> +{
>> +    struct kvm *kvm = intf_to_kvm(intf);
>> +
>> +    kvm_put_kvm(kvm);
>> +}
>> +
>> +struct kvm_xinterface_ops _kvm_xinterface_ops = {
>> +    .gpa_to_hva  = xinterface_gpa_to_hva,
>> +    .gpa_to_page = xinterface_gpa_to_page,
>> +    .release     = xinterface_release,
>> +};
>>   
>
> How do you deal with locking?
>
> The mappings (gpa_to_page) are not fixed.  They can and do change very
> often.  The interface doesn't seem to attempt to cope with this.

Hmm, well I used to need gpa_to_page() in the older version of vbus that
did explicit hypercalls, but I don't actually use it today in v4.  I
left it in because it seems like it might be useful in general (perhaps
for Michael).  However, if this ends up being a real problem I suppose
we can just drop that vtable entry and let the guy that actually needs
it deal with the issues ;)  I really only require gpa_to_hva() in the
short-term.

That said, I think the assumption that was made when I was using this
was that a proper ref for the page was acquired by the gfn_to_page() and
dropped by the caller.  This was always used in the context of a
hypercall/vmexit so presumably the gpa should be considered stable
across that call.  Is that not true?

Regards,
-Greg



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