Re: [Qemu-devel] [PATCH] kvm: Delete the slot only when KVM_MEM_READONLY flag is changed

[Shannon Zhao]

Ping?

On 2018/5/16 17:18, Shannon Zhao wrote:
> According to KVM commit 75d61fbc, it needs to delete the slot before
> changing the KVM_MEM_READONLY flag. But QEMU commit 235e8982 only check
> whether KVM_MEM_READONLY flag is set instead of changing. It doesn't
> need to delete the slot if the KVM_MEM_READONLY flag is not changed.
> 
> This fixes a issue that migrating a VM at the OVMF startup stage and
> VM is executing the codes in rom. Between the deleting and adding the
> slot in kvm_set_user_memory_region, there is a chance that guest access
> rom and trap to KVM, then KVM can't find the corresponding memslot.
> While KVM (on ARM) injects an abort to guest due to the broken hva, then
> guest will get stuck.
> 
> Signed-off-by: Shannon Zhao <address@hidden>
> ---
>  include/sysemu/kvm_int.h | 1 +
>  kvm-all.c                | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
> index 888557a..f838412 100644
> --- a/include/sysemu/kvm_int.h
> +++ b/include/sysemu/kvm_int.h
> @@ -20,6 +20,7 @@ typedef struct KVMSlot
>      void *ram;
>      int slot;
>      int flags;
> +    int old_flags;
>  } KVMSlot;
>  
>  typedef struct KVMMemoryListener {
> diff --git a/kvm-all.c b/kvm-all.c
> index 2515a23..de8250e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -252,7 +252,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener 
> *kml, KVMSlot *slot)
>      mem.userspace_addr = (unsigned long)slot->ram;
>      mem.flags = slot->flags;
>  
> -    if (slot->memory_size && mem.flags & KVM_MEM_READONLY) {
> +    if (slot->memory_size && (mem.flags ^ slot->old_flags) & 
> KVM_MEM_READONLY) {
>          /* 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;
> @@ -376,11 +376,11 @@ static int kvm_slot_update_flags(KVMMemoryListener 
> *kml, KVMSlot *mem,
>  {
>      int old_flags;
>  
> -    old_flags = mem->flags;
> +    mem->old_flags = mem->flags;
>      mem->flags = kvm_mem_flags(mr);
>  
>      /* If nothing changed effectively, no need to issue ioctl */
> -    if (mem->flags == old_flags) {
> +    if (mem->flags == mem->old_flags) {
>          return 0;
>      }
>  
> 

-- 
Shannon