Re: [PATCH] kvm: rework remove-write-access for a slot
On 06/04/2010 11:14 AM, Lai Jiangshan wrote:
- I thought of a different approach to write protection: write protect
the L4 sptes, on write fault add write permission to the L4 spte and
write protect the L3 sptes that it points to, etc. This method can use
the slot bitmap to reduce the number of write faults. However we can
reintroduce the slot bitmap if/when we use the method, this shouldn't
block the patch.
It is very a good approach and it is blazing fast.
I have no time to implement it currently,
could you update it into the TODO list?
Done.
+static void rmapp_remove_write_access(struct kvm *kvm, unsigned long
*rmapp)
+{
+u64 *spte = rmap_next(kvm, rmapp, NULL);
+
+while (spte) {
+/* avoid RMW */
+if (is_writable_pte(*spte))
+*spte= ~PT_WRITABLE_MASK;
Must use an atomic operation here to avoid losing dirty or accessed bit.
Atomic operation is too expensive, I retained the comment /* avoid RMW */
and wait someone take a good approach for it.
You are right, it is an existing problem. I just posted a patchset
which fixes the problem, when it's merged please rebase on top.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: rework remove-write-access for a slot
Avi Kivity wrote:
On 06/02/2010 11:53 AM, Lai Jiangshan wrote:
Current code uses slot_bitmap to find ptes who map a page
from the memory slot, it is not precise: some ptes in the shadow page
are not map any page from the memory slot.
This patch uses rmap to find the ptes precisely, and remove
the unused slot_bitmap.
Patch looks good; a couple of comments:
- We might see a slowdown with !tdp, since we no longer have locality.
Each page will map to an spte in a different page. However, it's still
worth it in my opinion.
Yes, this patch hurts the cache since we no longer have locality.
And if most pages of the slot are not mapped(rmap_next(kvm, rmapp, NULL)==NULL),
this patch will worse than old method I think.
This patch do things straightly, precisely.
- I thought of a different approach to write protection: write protect
the L4 sptes, on write fault add write permission to the L4 spte and
write protect the L3 sptes that it points to, etc. This method can use
the slot bitmap to reduce the number of write faults. However we can
reintroduce the slot bitmap if/when we use the method, this shouldn't
block the patch.
It is very a good approach and it is blazing fast.
I have no time to implement it currently,
could you update it into the TODO list?
+static void rmapp_remove_write_access(struct kvm *kvm, unsigned long
*rmapp)
+{
+u64 *spte = rmap_next(kvm, rmapp, NULL);
+
+while (spte) {
+/* avoid RMW */
+if (is_writable_pte(*spte))
+*spte = ~PT_WRITABLE_MASK;
Must use an atomic operation here to avoid losing dirty or accessed bit.
Atomic operation is too expensive, I retained the comment /* avoid RMW */
and wait someone take a good approach for it.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: rework remove-write-access for a slot
On Fri, Jun 04, 2010 at 04:14:08PM +0800, Lai Jiangshan wrote: Avi Kivity wrote: On 06/02/2010 11:53 AM, Lai Jiangshan wrote: Current code uses slot_bitmap to find ptes who map a page from the memory slot, it is not precise: some ptes in the shadow page are not map any page from the memory slot. This patch uses rmap to find the ptes precisely, and remove the unused slot_bitmap. Note that the current code is precise: memslot_id does unalias_gfn. Patch looks good; a couple of comments: - We might see a slowdown with !tdp, since we no longer have locality. Each page will map to an spte in a different page. However, it's still worth it in my opinion. Yes, this patch hurts the cache since we no longer have locality. And if most pages of the slot are not mapped(rmap_next(kvm, rmapp, NULL)==NULL), this patch will worse than old method I think. Can you get some numbers before/after patch, with/without lots of shadow pages instantiated? Better with large amount of memory for the guest. Because shrinking kvm_mmu_page is good. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: rework remove-write-access for a slot
On 06/02/2010 11:53 AM, Lai Jiangshan wrote:
Current code uses slot_bitmap to find ptes who map a page
from the memory slot, it is not precise: some ptes in the shadow page
are not map any page from the memory slot.
This patch uses rmap to find the ptes precisely, and remove
the unused slot_bitmap.
Patch looks good; a couple of comments:
- We might see a slowdown with !tdp, since we no longer have locality.
Each page will map to an spte in a different page. However, it's still
worth it in my opinion.
- I thought of a different approach to write protection: write protect
the L4 sptes, on write fault add write permission to the L4 spte and
write protect the L3 sptes that it points to, etc. This method can use
the slot bitmap to reduce the number of write faults. However we can
reintroduce the slot bitmap if/when we use the method, this shouldn't
block the patch.
+static void rmapp_remove_write_access(struct kvm *kvm, unsigned long
*rmapp)
+{
+u64 *spte = rmap_next(kvm, rmapp, NULL);
+
+while (spte) {
+/* avoid RMW */
+if (is_writable_pte(*spte))
+*spte = ~PT_WRITABLE_MASK;
Must use an atomic operation here to avoid losing dirty or accessed bit.
+spte = rmap_next(kvm, rmapp, spte);
+}
+}
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
