Re: [PATCH 0 of 2] Add libxc API that sets mem_access type for an array of gfns

[Aravindh Puthiyaparambil <aravindh@virtuata.com>]

On Thu, Apr 26, 2012 at 1:20 PM, Christian Limpach
<christian.limpach@gmail.com> wrote:
> On Thu, Apr 26, 2012 at 11:33 AM, Aravindh Puthiyaparambil
> <aravindh@virtuata.com> wrote:
>> When the mem_access type needs to be changed for multiple discontiguous gfns, calling xc_hvm_set_mem_access() multiple times does not perform very well. The main pain points are the multiple libxc calls themselves plus the multiple map_domain_page() / unmap_domain_page() and ept_sync_domain() calls for each ept_set_entry(gfn). The following patches adds a new mem_access API that sets the mem_access type for an array of guest physical addresses in one libxc call with minimal map_domain_page() / unmap_domain_page() and ept_sync_domain() calls.
>
>
> Are you sure that your bulk code actually works?  It seems to me that
> your __ept_set_entry function assumes that table still points to the
> top of the p2m.  The "for ( i = ept_get_wl(d); i > target; i-- )" loop
> will walk the table, and so in the subsequent calls from your bulk
> loop, this won't work?
>
> I think you need something like an iterator, the context of which can
> be passed around...

Duh! You are right. My test code has been giving me a false positive.
I completely misread how ept_next_level() works.

> Also, the call to ept_get_entry in your bulk loop will do a walk in
> every iteration, it seems a bit arbitrary to only (try to) avoid one
> and not the other.  But I guess the "win" is from reducing the number
> of ept_sync_domain calls.

You are right. I was mainly focusing on removing the multiple libxc
calls and reducing the ept_sync_domain calls. I thought removing the
map and unmap of the p2m top page was an extra optimization which I
obviously messed up. I will rework the patch to only stick with the
original optimization I had in mind.

Thanks,
Aravindh