[PATCH 0/2] kvm: IOMMU read-only mapping support

[PATCH 0/2] kvm: IOMMU read-only mapping support

[Alex Williamson]

A couple patches to make KVM IOMMU support honor read-only mappings.
This causes an un-map, re-map when the read-only flag changes and
makes use of it when setting IOMMU attributes.  Thanks,

Alex

---

Alex Williamson (2):
      kvm: Force IOMMU remapping on memory slot read-only flag changes
      kvm: Obey read-only mappings in iommu


 virt/kvm/iommu.c    |    4 +++-
 virt/kvm/kvm_main.c |   28 ++++++++++++++++++++++++----
 2 files changed, 27 insertions(+), 5 deletions(-)

[PATCH 1/2] kvm: Force IOMMU remapping on memory slot read-only flag changes

[Alex Williamson]

Memory slot flags can be altered without changing other parameters of
the slot.  The read-only attribute is the only one the IOMMU cares
about, so generate an un-map, re-map when this occurs.  This also
avoid unnecessarily re-mapping the slot when no IOMMU visible changes
are made.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 virt/kvm/kvm_main.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5e709eb..3fec2cd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -731,6 +731,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	struct kvm_memory_slot *slot;
 	struct kvm_memory_slot old, new;
 	struct kvm_memslots *slots = NULL, *old_memslots;
+	bool old_iommu_mapped;
 
 	r = check_memory_region_flags(mem);
 	if (r)
@@ -772,6 +773,8 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	new.npages = npages;
 	new.flags = mem->flags;
 
+	old_iommu_mapped = old.npages;
+
 	/*
 	 * Disallow changing a memory slot's size or changing anything about
 	 * zero sized slots that doesn't involve making them non-zero.
@@ -835,6 +838,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 
 		/* slot was deleted or moved, clear iommu mapping */
 		kvm_iommu_unmap_pages(kvm, &old);
+		old_iommu_mapped = false;
 		/* From this point no new shadow pages pointing to a deleted,
 		 * or moved, memslot will be created.
 		 *
@@ -863,11 +867,27 @@ int __kvm_set_memory_region(struct kvm *kvm,
 			goto out_free;
 	}
 
-	/* map new memory slot into the iommu */
+	/*
+	 * IOMMU mapping:  New slots need to be mapped.  Old slots need to be
+	 * un-mapped and re-mapped if their base changes or if flags that the
+	 * iommu cares about change (read-only).  Base change unmapping is
+	 * handled above with slot deletion, so we only unmap incompatible
+	 * flags here.  Anything else the iommu might care about for existing
+	 * slots (size changes, userspace addr changes) is disallowed above,
+	 * so any other attribute changes getting here can be skipped.
+	 */
 	if (npages) {
-		r = kvm_iommu_map_pages(kvm, &new);
-		if (r)
-			goto out_slots;
+		if (old_iommu_mapped &&
+		    ((new.flags ^ old.flags) & KVM_MEM_READONLY)) {
+			kvm_iommu_unmap_pages(kvm, &old);
+			old_iommu_mapped = false;
+		}
+
+		if (!old_iommu_mapped) {
+			r = kvm_iommu_map_pages(kvm, &new);
+			if (r)
+				goto out_slots;
+		}
 	}
 
 	/* actual memory is freed via old in kvm_free_physmem_slot below */

[PATCH 2/2] kvm: Obey read-only mappings in iommu

[Alex Williamson]

We've been ignoring read-only mappings and programming everything
into the iommu as read-write.  Fix this to only include the write
access flag when read-only is not set.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 virt/kvm/iommu.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 4a340cb..72a130b 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -76,7 +76,9 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 	gfn     = slot->base_gfn;
 	end_gfn = gfn + slot->npages;
 
-	flags = IOMMU_READ | IOMMU_WRITE;
+	flags = IOMMU_READ;
+	if (!(slot->flags & KVM_MEM_READONLY))
+		flags |= IOMMU_WRITE;
 	if (kvm->arch.iommu_flags & KVM_IOMMU_CACHE_COHERENCY)
 		flags |= IOMMU_CACHE;
 

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Thu, 24 Jan 2013 15:03:57 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> A couple patches to make KVM IOMMU support honor read-only mappings.
> This causes an un-map, re-map when the read-only flag changes and
> makes use of it when setting IOMMU attributes.  Thanks,

Looks good to me.

I think I can naturally update my patch after this gets merged.

Thanks,
	Takuya

Re: [PATCH 1/2] kvm: Force IOMMU remapping on memory slot read-only flag changes

[Xiao Guangrong]

On 01/25/2013 06:04 AM, Alex Williamson wrote:
> Memory slot flags can be altered without changing other parameters of
> the slot.  The read-only attribute is the only one the IOMMU cares
> about, so generate an un-map, re-map when this occurs.  This also
> avoid unnecessarily re-mapping the slot when no IOMMU visible changes
> are made.
> 

Reviewed-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Xiao Guangrong]

On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
> On Thu, 24 Jan 2013 15:03:57 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
>> A couple patches to make KVM IOMMU support honor read-only mappings.
>> This causes an un-map, re-map when the read-only flag changes and
>> makes use of it when setting IOMMU attributes.  Thanks,
> 
> Looks good to me.
> 
> I think I can naturally update my patch after this gets merged.
> 

Please wait.

The commit c972f3b1 changed the write-protect behaviour - it does
wirte-protection only when dirty flag is set.
[ I did not see this commit when we discussed the problem before. ]

Further more, i notice that write-protect is not enough, when do sync
shadow page:

FNAME(sync_page):

		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;

		set_spte(vcpu, &sp->spt[i], pte_access,
			 PT_PAGE_TABLE_LEVEL, gfn,
			 spte_to_pfn(sp->spt[i]), true, false,
			 host_writable);

It sets spte based on the old value that means the readonly flag check
is missed. We need to call kvm_arch_flush_shadow_all under this case.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Fri, 25 Jan 2013 11:28:40 +0800
Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote:

> > I think I can naturally update my patch after this gets merged.
> > 
> 
> Please wait.

The patch I mentioned above won't change anything.  Just cleans up
set_memory_region().  The only possible change which we discussed
before was whether we call iommu_map() on a flags change.

> The commit c972f3b1 changed the write-protect behaviour - it does
> wirte-protection only when dirty flag is set.
> [ I did not see this commit when we discussed the problem before. ]

I'll look at the commit later, after the lunch break.

> Further more, i notice that write-protect is not enough, when do sync
> shadow page:
> 
> FNAME(sync_page):
> 
> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
> 
> 		set_spte(vcpu, &sp->spt[i], pte_access,
> 			 PT_PAGE_TABLE_LEVEL, gfn,
> 			 spte_to_pfn(sp->spt[i]), true, false,
> 			 host_writable);
> 
> It sets spte based on the old value that means the readonly flag check
> is missed. We need to call kvm_arch_flush_shadow_all under this case.

So the change needed will be in arch/x86.
	in arch_commit_* one.

Right?

Note: I'm not touching arch_* memory slot APIs now because ARM KVM
is coming now.  So no problem, the flags will be passed as before.

	Takuya

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Fri, 25 Jan 2013 12:59:12 +0900
Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> wrote:

> > The commit c972f3b1 changed the write-protect behaviour - it does
> > wirte-protection only when dirty flag is set.
> > [ I did not see this commit when we discussed the problem before. ]
> 
> I'll look at the commit later, after the lunch break.

I've done!

So we need to do:

	if (npages && (GET_DIRTY || RDONLY))
		write protect

About sync_page, I'm not sure what is the best fix.

	Takuya

> 
> > Further more, i notice that write-protect is not enough, when do sync
> > shadow page:
> > 
> > FNAME(sync_page):
> > 
> > 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
> > 
> > 		set_spte(vcpu, &sp->spt[i], pte_access,
> > 			 PT_PAGE_TABLE_LEVEL, gfn,
> > 			 spte_to_pfn(sp->spt[i]), true, false,
> > 			 host_writable);
> > 
> > It sets spte based on the old value that means the readonly flag check
> > is missed. We need to call kvm_arch_flush_shadow_all under this case.
> 
> So the change needed will be in arch/x86.
> 	in arch_commit_* one.
> 
> Right?
> 
> Note: I'm not touching arch_* memory slot APIs now because ARM KVM
> is coming now.  So no problem, the flags will be passed as before.
> 
> 	Takuya


-- 
Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Gleb Natapov]

On Thu, Jan 24, 2013 at 03:03:57PM -0700, Alex Williamson wrote:
> A couple patches to make KVM IOMMU support honor read-only mappings.
> This causes an un-map, re-map when the read-only flag changes and
> makes use of it when setting IOMMU attributes.  Thanks,
> 
> Alex
> 
Applied, thanks.

> ---
> 
> Alex Williamson (2):
>       kvm: Force IOMMU remapping on memory slot read-only flag changes
>       kvm: Obey read-only mappings in iommu
> 
> 
>  virt/kvm/iommu.c    |    4 +++-
>  virt/kvm/kvm_main.c |   28 ++++++++++++++++++++++++----
>  2 files changed, 27 insertions(+), 5 deletions(-)

--
			Gleb.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Gleb Natapov]

On Fri, Jan 25, 2013 at 11:28:40AM +0800, Xiao Guangrong wrote:
> On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
> > On Thu, 24 Jan 2013 15:03:57 -0700
> > Alex Williamson <alex.williamson@redhat.com> wrote:
> > 
> >> A couple patches to make KVM IOMMU support honor read-only mappings.
> >> This causes an un-map, re-map when the read-only flag changes and
> >> makes use of it when setting IOMMU attributes.  Thanks,
> > 
> > Looks good to me.
> > 
> > I think I can naturally update my patch after this gets merged.
> > 
> 
> Please wait.
> 
> The commit c972f3b1 changed the write-protect behaviour - it does
> wirte-protection only when dirty flag is set.
> [ I did not see this commit when we discussed the problem before. ]
> 
> Further more, i notice that write-protect is not enough, when do sync
> shadow page:
> 
> FNAME(sync_page):
> 
> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
> 
> 		set_spte(vcpu, &sp->spt[i], pte_access,
> 			 PT_PAGE_TABLE_LEVEL, gfn,
> 			 spte_to_pfn(sp->spt[i]), true, false,
> 			 host_writable);
> 
> It sets spte based on the old value that means the readonly flag check
> is missed. We need to call kvm_arch_flush_shadow_all under this case.
Why not just disallow changing memory region KVM_MEM_READONLY flag
without deleting the region?

--
			Gleb.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Mon, 28 Jan 2013 12:59:03 +0200
Gleb Natapov <gleb@redhat.com> wrote:

> > It sets spte based on the old value that means the readonly flag check
> > is missed. We need to call kvm_arch_flush_shadow_all under this case.
> Why not just disallow changing memory region KVM_MEM_READONLY flag
> without deleting the region?

Sounds good.

If you prefer, I can fold the required change into my patch.

	Takuya

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Alex Williamson]

On Mon, 2013-01-28 at 21:25 +0900, Takuya Yoshikawa wrote:
> On Mon, 28 Jan 2013 12:59:03 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > > It sets spte based on the old value that means the readonly flag check
> > > is missed. We need to call kvm_arch_flush_shadow_all under this case.
> > Why not just disallow changing memory region KVM_MEM_READONLY flag
> > without deleting the region?
> 
> Sounds good.
> 
> If you prefer, I can fold the required change into my patch.

That would seem to make my patch 1/2 unnecessary.  Thanks,

Alex

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Mon, 28 Jan 2013 08:36:56 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Mon, 2013-01-28 at 21:25 +0900, Takuya Yoshikawa wrote:
> > On Mon, 28 Jan 2013 12:59:03 +0200
> > Gleb Natapov <gleb@redhat.com> wrote:
> > 
> > > > It sets spte based on the old value that means the readonly flag check
> > > > is missed. We need to call kvm_arch_flush_shadow_all under this case.
> > > Why not just disallow changing memory region KVM_MEM_READONLY flag
> > > without deleting the region?
> > 
> > Sounds good.
> > 
> > If you prefer, I can fold the required change into my patch.
> 
> That would seem to make my patch 1/2 unnecessary.  Thanks,

I've decided not to fold that change into my
  KVM: set_memory_region: Identify the requested change explicitly
since this is a functional change, see v3 I posted today.

Since QEMU is not the only hypervisor using KVM, we should make the
change with a proper subject informing every user of the new restriction.

If the maintainers prefer to remove Alex's patch 1/2 first and then
rebase my v3 patch, I can do so.

	Takuya

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Xiao Guangrong]

On 01/28/2013 06:59 PM, Gleb Natapov wrote:
> On Fri, Jan 25, 2013 at 11:28:40AM +0800, Xiao Guangrong wrote:
>> On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
>>> On Thu, 24 Jan 2013 15:03:57 -0700
>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>
>>>> A couple patches to make KVM IOMMU support honor read-only mappings.
>>>> This causes an un-map, re-map when the read-only flag changes and
>>>> makes use of it when setting IOMMU attributes.  Thanks,
>>>
>>> Looks good to me.
>>>
>>> I think I can naturally update my patch after this gets merged.
>>>
>>
>> Please wait.
>>
>> The commit c972f3b1 changed the write-protect behaviour - it does
>> wirte-protection only when dirty flag is set.
>> [ I did not see this commit when we discussed the problem before. ]
>>
>> Further more, i notice that write-protect is not enough, when do sync
>> shadow page:
>>
>> FNAME(sync_page):
>>
>> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
>>
>> 		set_spte(vcpu, &sp->spt[i], pte_access,
>> 			 PT_PAGE_TABLE_LEVEL, gfn,
>> 			 spte_to_pfn(sp->spt[i]), true, false,
>> 			 host_writable);
>>
>> It sets spte based on the old value that means the readonly flag check
>> is missed. We need to call kvm_arch_flush_shadow_all under this case.
> Why not just disallow changing memory region KVM_MEM_READONLY flag
> without deleting the region?

It will introduce some restriction when VM-sharing-mem is being implemented,
but we need to do some optimization for it, at least, properly write-protect
readonly pages (fix sync_page()) instead of zap_all_page.

So, i guess we can do the simple fix first.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Gleb Natapov]

On Tue, Jan 29, 2013 at 11:06:43AM +0800, Xiao Guangrong wrote:
> On 01/28/2013 06:59 PM, Gleb Natapov wrote:
> > On Fri, Jan 25, 2013 at 11:28:40AM +0800, Xiao Guangrong wrote:
> >> On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
> >>> On Thu, 24 Jan 2013 15:03:57 -0700
> >>> Alex Williamson <alex.williamson@redhat.com> wrote:
> >>>
> >>>> A couple patches to make KVM IOMMU support honor read-only mappings.
> >>>> This causes an un-map, re-map when the read-only flag changes and
> >>>> makes use of it when setting IOMMU attributes.  Thanks,
> >>>
> >>> Looks good to me.
> >>>
> >>> I think I can naturally update my patch after this gets merged.
> >>>
> >>
> >> Please wait.
> >>
> >> The commit c972f3b1 changed the write-protect behaviour - it does
> >> wirte-protection only when dirty flag is set.
> >> [ I did not see this commit when we discussed the problem before. ]
> >>
> >> Further more, i notice that write-protect is not enough, when do sync
> >> shadow page:
> >>
> >> FNAME(sync_page):
> >>
> >> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
> >>
> >> 		set_spte(vcpu, &sp->spt[i], pte_access,
> >> 			 PT_PAGE_TABLE_LEVEL, gfn,
> >> 			 spte_to_pfn(sp->spt[i]), true, false,
> >> 			 host_writable);
> >>
> >> It sets spte based on the old value that means the readonly flag check
> >> is missed. We need to call kvm_arch_flush_shadow_all under this case.
> > Why not just disallow changing memory region KVM_MEM_READONLY flag
> > without deleting the region?
> 
> It will introduce some restriction when VM-sharing-mem is being implemented,
> but we need to do some optimization for it, at least, properly write-protect
> readonly pages (fix sync_page()) instead of zap_all_page.
> 
What is VM-sharing-mem?

> So, i guess we can do the simple fix first.
> 
By simple fix you mean calling kvm_arch_flush_shadow_all() on READONLY
flag change?

--
			Gleb.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Xiao Guangrong]

On 01/29/2013 02:50 PM, Gleb Natapov wrote:
> On Tue, Jan 29, 2013 at 11:06:43AM +0800, Xiao Guangrong wrote:
>> On 01/28/2013 06:59 PM, Gleb Natapov wrote:
>>> On Fri, Jan 25, 2013 at 11:28:40AM +0800, Xiao Guangrong wrote:
>>>> On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
>>>>> On Thu, 24 Jan 2013 15:03:57 -0700
>>>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>>>
>>>>>> A couple patches to make KVM IOMMU support honor read-only mappings.
>>>>>> This causes an un-map, re-map when the read-only flag changes and
>>>>>> makes use of it when setting IOMMU attributes.  Thanks,
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> I think I can naturally update my patch after this gets merged.
>>>>>
>>>>
>>>> Please wait.
>>>>
>>>> The commit c972f3b1 changed the write-protect behaviour - it does
>>>> wirte-protection only when dirty flag is set.
>>>> [ I did not see this commit when we discussed the problem before. ]
>>>>
>>>> Further more, i notice that write-protect is not enough, when do sync
>>>> shadow page:
>>>>
>>>> FNAME(sync_page):
>>>>
>>>> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
>>>>
>>>> 		set_spte(vcpu, &sp->spt[i], pte_access,
>>>> 			 PT_PAGE_TABLE_LEVEL, gfn,
>>>> 			 spte_to_pfn(sp->spt[i]), true, false,
>>>> 			 host_writable);
>>>>
>>>> It sets spte based on the old value that means the readonly flag check
>>>> is missed. We need to call kvm_arch_flush_shadow_all under this case.
>>> Why not just disallow changing memory region KVM_MEM_READONLY flag
>>> without deleting the region?
>>
>> It will introduce some restriction when VM-sharing-mem is being implemented,
>> but we need to do some optimization for it, at least, properly write-protect
>> readonly pages (fix sync_page()) instead of zap_all_page.
>>
> What is VM-sharing-mem?

Sharing memory between different guests.

> 
>> So, i guess we can do the simple fix first.
>>
> By simple fix you mean calling kvm_arch_flush_shadow_all() on READONLY
> flag change?

Simply disallow READONLY flag changing.

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Xiao Guangrong]

On 01/29/2013 03:48 PM, Gleb Natapov wrote:
> On Tue, Jan 29, 2013 at 03:37:23PM +0800, Xiao Guangrong wrote:
>> On 01/29/2013 02:50 PM, Gleb Natapov wrote:
>>> On Tue, Jan 29, 2013 at 11:06:43AM +0800, Xiao Guangrong wrote:
>>>> On 01/28/2013 06:59 PM, Gleb Natapov wrote:
>>>>> On Fri, Jan 25, 2013 at 11:28:40AM +0800, Xiao Guangrong wrote:
>>>>>> On 01/25/2013 09:17 AM, Takuya Yoshikawa wrote:
>>>>>>> On Thu, 24 Jan 2013 15:03:57 -0700
>>>>>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>>>>>
>>>>>>>> A couple patches to make KVM IOMMU support honor read-only mappings.
>>>>>>>> This causes an un-map, re-map when the read-only flag changes and
>>>>>>>> makes use of it when setting IOMMU attributes.  Thanks,
>>>>>>>
>>>>>>> Looks good to me.
>>>>>>>
>>>>>>> I think I can naturally update my patch after this gets merged.
>>>>>>>
>>>>>>
>>>>>> Please wait.
>>>>>>
>>>>>> The commit c972f3b1 changed the write-protect behaviour - it does
>>>>>> wirte-protection only when dirty flag is set.
>>>>>> [ I did not see this commit when we discussed the problem before. ]
>>>>>>
>>>>>> Further more, i notice that write-protect is not enough, when do sync
>>>>>> shadow page:
>>>>>>
>>>>>> FNAME(sync_page):
>>>>>>
>>>>>> 		host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
>>>>>>
>>>>>> 		set_spte(vcpu, &sp->spt[i], pte_access,
>>>>>> 			 PT_PAGE_TABLE_LEVEL, gfn,
>>>>>> 			 spte_to_pfn(sp->spt[i]), true, false,
>>>>>> 			 host_writable);
>>>>>>
>>>>>> It sets spte based on the old value that means the readonly flag check
>>>>>> is missed. We need to call kvm_arch_flush_shadow_all under this case.
>>>>> Why not just disallow changing memory region KVM_MEM_READONLY flag
>>>>> without deleting the region?
>>>>
>>>> It will introduce some restriction when VM-sharing-mem is being implemented,
>>>> but we need to do some optimization for it, at least, properly write-protect
>>>> readonly pages (fix sync_page()) instead of zap_all_page.
>>>>
>>> What is VM-sharing-mem?
>>
>> Sharing memory between different guests.
>>
> That much I can figure out for the name itself. My question is how this
> sharing will work? Why KVM_MEM_READONLY is needed for it? Why ability to
> change KVM_MEM_READONLY flag without destroying memory region will be
> important. What's wrong with nahanni, shared memory device we have today?

I'm not clear now or maybe my memory is wrong, i need to find the origin
discussion...

> 
>>>
>>>> So, i guess we can do the simple fix first.
>>>>
>>> By simple fix you mean calling kvm_arch_flush_shadow_all() on READONLY
>>> flag change?
>>
>> Simply disallow READONLY flag changing.
> Ok, can somebody craft a patch?

Takuya, will you? ;)

Re: [PATCH 0/2] kvm: IOMMU read-only mapping support

[Takuya Yoshikawa]

On Wed, 30 Jan 2013 12:06:32 +0800
Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote:

> >>>> So, i guess we can do the simple fix first.
> >>>>
> >>> By simple fix you mean calling kvm_arch_flush_shadow_all() on READONLY
> >>> flag change?
> >>
> >> Simply disallow READONLY flag changing.
> > Ok, can somebody craft a patch?
> 
> Takuya, will you? ;)

I will.
Thank you for pointing out the problem.

We can revisit the API later to determine what kind of changes
should be allowed.

	Takuya