From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [PATCH 5/9] Add VMLOAD and VMSAVE handlers v5
Date: Thu, 30 Oct 2008 14:14:22 -0500
Message-ID: <490A078E.10007@codemonkey.ws>
References: <1224522290-11740-1-git-send-email-agraf@suse.de> <1224522290-11740-2-git-send-email-agraf@suse.de> <1224522290-11740-3-git-send-email-agraf@suse.de> <1224522290-11740-4-git-send-email-agraf@suse.de> <1224522290-11740-5-git-send-email-agraf@suse.de> <1224522290-11740-6-git-send-email-agraf@suse.de> <4909F793.4080404@codemonkey.ws> <71E40A19-2D2B-4884-94F6-910363EEE0C4@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, joro@8bytes.org, avi@redhat.com
To: Alexander Graf <agraf@suse.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from yx-out-2324.google.com ([74.125.44.28]:9656 "EHLO
	yx-out-2324.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1759022AbYJ3TO3 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 30 Oct 2008 15:14:29 -0400
Received: by yx-out-2324.google.com with SMTP id 8so328796yxm.1
        for <kvm@vger.kernel.org>; Thu, 30 Oct 2008 12:14:27 -0700 (PDT)
In-Reply-To: <71E40A19-2D2B-4884-94F6-910363EEE0C4@suse.de>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Alexander Graf wrote:
>
> On 30.10.2008, at 19:06, Anthony Liguori wrote:
>
>> Alexander Graf wrote:
>>> This implements the VMLOAD and VMSAVE instructions, that usually 
>>> surround
>>> the VMRUN instructions. Both instructions load / restore the same 
>>> elements,
>>> so we only need to implement them once.
>>>
>>> v2 fixes CPL checking and replaces memcpy by assignments
>>> v3 makes use of the new permission checking
>>>
>
> snip
>
>>>
>>> +static int vmload_interception(struct vcpu_svm *svm, struct kvm_run 
>>> *kvm_run)
>>> +{
>>> +    if (nested_svm_check_permissions(svm))
>>> +        return 1;
>>> +
>>> +    svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
>>> +    skip_emulated_instruction(&svm->vcpu);
>>> +
>>> +    nested_svm_do(svm, svm->vmcb->save.rax, 0, NULL, 
>>> nested_svm_vmload);
>>>
>>
>> The whole trampoline thing seems awkward to me.  I think it would be 
>> more reasonable to just open code this routine and use helper 
>> functions when appropriate.
>
> What exactly do you mean? Do you prefer to duplicate the 
> guest-page-mapping code several times?

You do:

foo(x, y, cb);

where foo is:

// something with x
// something with y

cb();

// something else

And then do:

foo(1, 2, bar);

An alternative module would be:

foo_helper1(x)
// something with x

foo_helper2(y)
// something with y

foo_cleanup()
// something else

And then:

foo_helper1(1);
foo_helper2(2)
bar()
foo_cleanup();

And since you only sometimes pass both x, y parameters, I do think this 
later style would result in more readable code.

Regards,

Anthony Liguori

> Alex
>