Re: [PATCH 3/3] arm, arm64: KVM: handle potential incoherency of readonly memslots

[Paolo Bonzini <pbonzini@redhat.com>]

On 19/11/2014 09:51, Ard Biesheuvel wrote:
> On 17 November 2014 15:58, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> Readonly memslots are often used to implement emulation of ROMs and
>> NOR flashes, in which case the guest may legally map these regions as
>> uncached.
>> To deal with the incoherency associated with uncached guest mappings,
>> treat all readonly memslots as incoherent, and ensure that pages that
>> belong to regions tagged as such are flushed to DRAM before being passed
>> to the guest.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
> 
> Hello all,
> 
> I have another bug report (from Canonical this time) of essentially
> the same issue, and it is also fixed by these patches.
> Are you happy with these patches? Should I respin to add Laszlo's tested-by?

Christoffer can add it, together with...

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

It will be 3.19 only, though.

Paolo

> Cheers,
> Ard.
> 
> 
>>  arch/arm/kvm/mmu.c | 20 +++++++++++++++-----
>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>> index cb924c6d56a6..f2a9874ff5cb 100644
>> --- a/arch/arm/kvm/mmu.c
>> +++ b/arch/arm/kvm/mmu.c
>> @@ -919,7 +919,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>         if (!hugetlb && !force_pte)
>>                 hugetlb = transparent_hugepage_adjust(&pfn, &fault_ipa);
>>
>> -       fault_ipa_uncached = false;
>> +       fault_ipa_uncached = memslot->flags & KVM_MEMSLOT_INCOHERENT;
>>
>>         if (hugetlb) {
>>                 pmd_t new_pmd = pfn_pmd(pfn, mem_type);
>> @@ -1298,11 +1298,12 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
>>                 hva = vm_end;
>>         } while (hva < reg_end);
>>
>> -       if (ret) {
>> -               spin_lock(&kvm->mmu_lock);
>> +       spin_lock(&kvm->mmu_lock);
>> +       if (ret)
>>                 unmap_stage2_range(kvm, mem->guest_phys_addr, mem->memory_size);
>> -               spin_unlock(&kvm->mmu_lock);
>> -       }
>> +       else
>> +               stage2_flush_memslot(kvm, memslot);
>> +       spin_unlock(&kvm->mmu_lock);
>>         return ret;
>>  }
>>
>> @@ -1314,6 +1315,15 @@ void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free,
>>  int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
>>                             unsigned long npages)
>>  {
>> +       /*
>> +        * Readonly memslots are not incoherent with the caches by definition,
>> +        * but in practice, they are used mostly to emulate ROMs or NOR flashes
>> +        * that the guest may consider devices and hence map as uncached.
>> +        * To prevent incoherency issues in these cases, tag all readonly
>> +        * regions as incoherent.
>> +        */
>> +       if (slot->flags & KVM_MEM_READONLY)
>> +               slot->flags |= KVM_MEMSLOT_INCOHERENT;
>>         return 0;
>>  }
>>
>> --
>> 1.8.3.2
>>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>