Il 07/05/2013 19:15, Jordan Justen ha scritto: > A slot that uses KVM_MEM_READONLY can be read from and code > can execute from the region, but writes will trap. > > For regions that are readonly and also not writeable, we > force the slot to be removed so reads or writes to the region > will trap. (A memory region in this state is not executable > within kvm.) > > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > Reviewed-by: Xiao Guangrong <xiaoguangr...@linux.vnet.ibm.com> > --- > kvm-all.c | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 1686adc..fffd2f4 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -201,12 +201,18 @@ static int kvm_set_user_memory_region(KVMState *s, > KVMSlot *slot) > > mem.slot = slot->slot; > mem.guest_phys_addr = slot->start_addr; > - mem.memory_size = slot->memory_size; > mem.userspace_addr = (unsigned long)slot->ram; > mem.flags = slot->flags; > if (s->migration_log) { > mem.flags |= KVM_MEM_LOG_DIRTY_PAGES; > } > + if (mem.flags & KVM_MEM_READONLY && mem.memory_size != 0) { > + /* Set the slot size to 0 before setting the slot to the desired > + * value. This is needed based on KVM commit 75d61fbc. */ > + mem.memory_size = 0; > + kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem); > + } > + mem.memory_size = slot->memory_size; > return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem); > } > > @@ -268,9 +274,14 @@ err: > * dirty pages logging control > */ > > -static int kvm_mem_flags(KVMState *s, bool log_dirty) > +static int kvm_mem_flags(KVMState *s, bool log_dirty, bool readonly) > { > - return log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0; > + int flags = 0; > + flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0; > + if (readonly && kvm_readonly_mem_allowed) { > + flags |= KVM_MEM_READONLY; > + } > + return flags; > } > > static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty) > @@ -281,7 +292,7 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, > bool log_dirty) > > old_flags = mem->flags; > > - flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty); > + flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false); > mem->flags = flags; > > /* If nothing changed effectively, no need to issue ioctl */ > @@ -638,7 +649,14 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > } > > if (!memory_region_is_ram(mr)) { > - return; > + if (!mr->readonly || !kvm_readonly_mem_allowed) { > + return; > + } else if (!mr->readable && add) { > + /* If the memory range is not readable, then we actually want > + * to remove the kvm memory slot so all accesses will trap. */ > + assert(mr->readonly && kvm_readonly_mem_allowed); > + add = false; > + }
mr->readable really means "read from ROM, write to device", hence it should always be read-only from KVM's point of view. I think this should be if (!memory_region_is_ram(mr)) { if (!mr->readable) { return; } assert(kvm_readonly_mem_allowed); } with occurrences below of mr->readonly, like mem->flags = kvm_mem_flags(s, log_dirty, mr->readonly); changed to mr->readonly || mr->readable. This should eliminate the need for patch 4, too. Should have pointed out this before. I'm just learning this stuff as well... Paolo > } > > ram = memory_region_get_ram_ptr(mr) + section->offset_within_region + > delta; > @@ -687,7 +705,7 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > mem->memory_size = old.memory_size; > mem->start_addr = old.start_addr; > mem->ram = old.ram; > - mem->flags = kvm_mem_flags(s, log_dirty); > + mem->flags = kvm_mem_flags(s, log_dirty, mr->readonly); > > err = kvm_set_user_memory_region(s, mem); > if (err) { > @@ -708,7 +726,7 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > mem->memory_size = start_addr - old.start_addr; > mem->start_addr = old.start_addr; > mem->ram = old.ram; > - mem->flags = kvm_mem_flags(s, log_dirty); > + mem->flags = kvm_mem_flags(s, log_dirty, mr->readonly); > > err = kvm_set_user_memory_region(s, mem); > if (err) { > @@ -732,7 +750,7 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > size_delta = mem->start_addr - old.start_addr; > mem->memory_size = old.memory_size - size_delta; > mem->ram = old.ram + size_delta; > - mem->flags = kvm_mem_flags(s, log_dirty); > + mem->flags = kvm_mem_flags(s, log_dirty, mr->readonly); > > err = kvm_set_user_memory_region(s, mem); > if (err) { > @@ -754,7 +772,7 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > mem->memory_size = size; > mem->start_addr = start_addr; > mem->ram = ram; > - mem->flags = kvm_mem_flags(s, log_dirty); > + mem->flags = kvm_mem_flags(s, log_dirty, mr->readonly); > > err = kvm_set_user_memory_region(s, mem); > if (err) { >