Re: [PATCH 0/4] KVM: Clean up and optimize __kvm_set_memory_region() - part1

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jan 17, 2013 at 11:20:43AM +0800, Xiao Guangrong wrote:
> On 01/17/2013 03:07 AM, Marcelo Tosatti wrote:
> > On Fri, Jan 11, 2013 at 06:25:18PM +0900, Takuya Yoshikawa wrote:
> >> Patches 1 to 3 are trivial.
> >>
> >> Patch 4 is the main cause of the increased lines, but I think the new
> >> code makes it easier to understand why each condition in
> >> __kvm_set_memory_region() is there.
> >>
> >> If you don't agree with patch 4, please consider taking the rest of the
> >> series at this time.
> >>
> >> Takuya Yoshikawa (4):
> >>   KVM: set_memory_region: Don't jump to out_free unnecessarily
> >>   KVM: set_memory_region: Don't check for overlaps unless we create or move a slot
> >>   KVM: set_memory_region: Remove unnecessary variable memslot
> >>   KVM: set_memory_region: Identify the requested change explicitly
> >>
> >>  virt/kvm/kvm_main.c |   94 ++++++++++++++++++++++++++++++++-------------------
> >>  1 files changed, 59 insertions(+), 35 deletions(-)
> >>
> >> -- 
> >> 1.7.5.4
> > 
> > Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > BTW, while at it, its probably worthwhile to restrict flags
> > modifications: change from flags = 0 to flags = read-only is 
> > incomplete. Xiao, should it be allowed only during creation?
> 
> Will Readonly memory be used for VM-mem-sharing in the future?
> I remember you mentioned about mem-sharing things before. ;)
> 
> Actually, It is safe on KVM MMU because all the gfns in the slot
> can become readonly after calling __kvm_set_memory_region. It is
> unsafe on IOMMU, what need to be fixed is unmapping gfns on IOMMU
> when the flag is changed.

Which means if we allow changing flags from 0 to read_only or back we
need to call kvm_iommu_map_pages() when flags change. BTW as far as I
see currently IOMMU maps everything as read/write not matter what flags
say. Looks like a bug. Alex?

--
			Gleb.