[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 0 of 2] Add libxc API that sets mem_access type for an array of gfns
- To: Aravindh Puthiyaparambil <aravindh@xxxxxxxxxxxx>
- From: Christian Limpach <christian.limpach@xxxxxxxxx>
- Date: Thu, 26 Apr 2012 18:06:13 -0700
- Cc: tim@xxxxxxx, xen-devel@xxxxxxxxxxxxx
- Delivery-date: Fri, 27 Apr 2012 01:06:40 +0000
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:content-transfer-encoding; bh=5FPzyEnbVBXMagO30D8doF4g5Z4WAGGqClPU+9jwuWc=; b=f97QLhgQCWvGTzrzLasXJ2deAjjL0IbL4zDAkKPRQabX0fZxtFUHqKs47FsQhDEBEO udO2NSpt8vb0YaPEeuDd35aWWuhjCH2WdqdwyPRzhu2DWbwXqr+XebnFmoM5TZmckZN6 DIHq2zfNC3/Uy25ZHlgdkVg2rXQ0SCjC80lsWBGh6JW5ZMnpX45Yc+N0B5/cE9VOeD5W 6fffg/G/4ZEKzzcWhyBNBnDiInIjwGOPypLid/vY3UduoSZ8QuyqyZKW+1BjQP6DpZNX Qwa7wQjtXAwPBAYMEBz+ZIQK3Nur+56uWsUkNEE3wV6FyP/QsOP1jQ05PdEEXsjw7c67 VtzA==
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On Thu, Apr 26, 2012 at 5:15 PM, Aravindh Puthiyaparambil
> Does this look correct now?
It addresses the issues I've pointed out, but:
- you should leave the ASSERT where it is, or is there a reason to move it?
- this is wrong:
> - old_entry = *ept_entry;
> + old_entry->epte = ept_entry->epte;
You should follow the code and see what uses old_entry and you'll see
that within the function old_entry->mfn is used (your diff changes the
line that uses it) and ept_free_entry also accesses mfn.
- are you sure you can move the ept_sync_domain call past the iommu code?
I made a similar change a while ago, though it is for a more specific
case, updating the ept table to "clean" the vram tracking. My change
- clear needs_sync when setting the type to logdirty for a leaf entry
if ( !is_epte_present(ept_entry) ||
(!target && p2mt == p2m_ram_logdirty) )
needs_sync = 0;
- only call ept_free_entry in the non-leaf case
if ( target && is_epte_present(&old_entry) )
ept_free_entry(p2m, &old_entry, target);
- call ept_sync_domain from hap_clean_vram_tracking
Maybe you can do something similar, for example passing in a hint
whether ept_sync_domain needs to be done or not. In my case, the
reasoning is that all the entries changed from hap_clean_vram_tracking
are leaf entries, so ept_free_entry will never be called and thus
ept_sync_domain can be deferred. I didn't think through/consider the
iommu case since that code is commented out in my tree.
Xen-devel mailing list