On 09/02/2014 07:42 PM, wanghaibin wrote:
> On 2014/8/27 8:04, Mario Smarduch wrote:
>
>> Patch adds support for initial write protection of VM memlsot. This patch series
>> assumes that huge PUDs will not be used in 2nd stage tables, which is awlays
>> valid on ARMv7.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@xxxxxxxxxxx>
>> ---
>> arch/arm/include/asm/kvm_host.h | 1 +
>> arch/arm/include/asm/kvm_mmu.h | 20 ++++++
>> arch/arm/include/asm/pgtable-3level.h | 1 +
>> arch/arm/kvm/arm.c | 9 +++
>> arch/arm/kvm/mmu.c | 128 ++++++++++++++++++++++++++++++++++
>> 5 files changed, 159 insertions(+)
>>
[...]
>> +
>> +static inline void kvm_set_s2pmd_readonly(pmd_t *pmd)
>> +{
>> + pmd_val(*pmd) = (pmd_val(*pmd) & ~L_PMD_S2_RDWR) | L_PMD_S2_RDONLY;
>> +}
>> +
>> +static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
>> +{
>> + return (pmd_val(*pmd) & L_PMD_S2_RDWR) == L_PMD_S2_RDONLY;
>> +}
>> +
>> /* Open coded p*d_addr_end that can deal with 64bit addresses */
>> #define kvm_pgd_addr_end(addr, end) \
>> ({ u64 __boundary = ((addr) + PGDIR_SIZE) & PGDIR_MASK; \
>> diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
>> index 85c60ad..d8bb40b 100644
>> --- a/arch/arm/include/asm/pgtable-3level.h
>> +++ b/arch/arm/include/asm/pgtable-3level.h
>> @@ -129,6 +129,7 @@
>> #define L_PTE_S2_RDONLY (_AT(pteval_t, 1) << 6) /* HAP[1] */
>> #define L_PTE_S2_RDWR (_AT(pteval_t, 3) << 6) /* HAP[2:1] */
>>
>
>
> Hi, Mario, the L_PMD_S2_RDONLY defined, **pteval_t** type should be *pmdval_t* .
Yes it should thanks for catching it.
>
>> +#define L_PMD_S2_RDONLY (_AT(pteval_t, 1) << 6) /* HAP[1] */
>
>> #define L_PMD_S2_RDWR (_AT(pmdval_t, 3) << 6) /* HAP[2:1] */
>>
>> /*
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index c52b2bd..e1be6c7 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -242,6 +242,15 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>> const struct kvm_memory_slot *old,
>> enum kvm_mr_change change)
>> {
>> +#ifdef CONFIG_ARM
>
>
> About CONFIG_ARM , I suggest cancle this limmit..
> I just implement a few interfaces for ARM64 , and tested this patch using VNC, it's work.
>
Yes I was planing to do that once ARMv7 and generic/arch split for
dirty log and tlb flush functions are settled. I realize at this time
ARMv8 just needs it's own tlb flush and it's done. But I need a strong
test case before claiming ARMv8 support
I have some ideas on testing it but migration is probably most
reliable and the only use case really.
Can you elaborate on your test environment for ARMv8 (you could
email off-line if too long, also thanks for testing!).
>> + /*
>> + * At this point memslot has been committed and there is an
>> + * allocated dirty_bitmap[], dirty pages will be be tracked while the
>> + * memory slot is write protected.
>> + */
>> + if ((change != KVM_MR_DELETE) && (mem->flags & KVM_MEM_LOG_DIRTY_PAGES))
>> + kvm_mmu_wp_memory_region(kvm, mem->slot);
>> +#endif
>> }
>>
[...]
>> +/**
>> + * stage2_wp_pud_range - write protect PUD range
>> + * @kvm: pointer to kvm structure
>> + * @pud: pointer to pgd entry
>> + * @addr: range start address
>> + * @end: range end address
>> + *
>> + * While walking the PUD range huge PUD pages are ignored.
>> + */
>> +static void stage2_wp_pud_range(struct kvm *kvm, pgd_t *pgd,
>> + phys_addr_t addr, phys_addr_t end)
>> +{
>> + pud_t *pud;
>> + phys_addr_t next;
>> +
>> + pud = pud_offset(pgd, addr);
>> + do {
>> + next = kvm_pud_addr_end(addr, end);
>
>
> There may be a problem.
> I tested the patch set using VNC, the video ram which qemu emulated for CLCD, will be regist to KVM with
> the log dirty attribution.
'log dirty attribution' what do you mean by that?
>It is worth to mention that the S2 page for video ram is not created at that time.
> So, there must be BUG_ON(kvm_pud_huge(*pud));
'not created at that time'? What time?
The time when memory region is registered should have no
bearing on logging after it's committed logging
will start, if its log flag is set.
>
> I think this code should be :
>
> if (!pud_none(*pud)) { /* check the *pud valid first */
> BUG_ON(kvm_pud_huge(*pud));
> stage2_wp_pmd_range(pud, addr, next);
> }
I may be missing something here but not sure how the update improves
anything? Definitely follow-up if I'm off on this.
Thanks.
>
>
> Thanks !
>
>> + /* TODO: PUD not supported, revisit later if implemented */
>> + BUG_ON(kvm_pud_huge(*pud));
>> + if (!pud_none(*pud))
>> + stage2_wp_pmd_range(pud, addr, next);
>> + } while (pud++, addr = next, addr != end);
>> +}
>> +
>> +/**
>> + * stage2_wp_range() - write protect stage2 memory region range
>> + * @kvm: The KVM pointer
>> + * @start: Start address of range
>> + * &end: End address of range
>> + */
>> +static void stage2_wp_range(struct kvm *kvm, phys_addr_t addr, phys_addr_t end)
>> +{
>> + pgd_t *pgd;
>> + phys_addr_t next;
>> +
>> + pgd = kvm->arch.pgd + pgd_index(addr);
>> + do {
>> + /*
>> + * Release kvm_mmu_lock periodically if the memory region is
>> + * large. Otherwise, we may see kernel panics with
>> + * CONFIG_DETECT_HUNG_TASK, CONFIG_LOCK_DETECTOR,
>> + * CONFIG_LOCK_DEP. Additionally, holding the lock too long
>> + * will also starve other vCPUs.
>> + */
>> + if (need_resched() || spin_needbreak(&kvm->mmu_lock))
>> + cond_resched_lock(&kvm->mmu_lock);
>> +
>> + next = kvm_pgd_addr_end(addr, end);
>> + if (pgd_present(*pgd))
>> + stage2_wp_pud_range(kvm, pgd, addr, next);
>> + } while (pgd++, addr = next, addr != end);
>> +}
>> +
>> +/**
>> + * kvm_mmu_wp_memory_region() - write protect stage 2 entries for memory slot
>> + * @kvm: The KVM pointer
>> + * @slot: The memory slot to write protect
>> + *
>> + * Called to start logging dirty pages after memory region
>> + * KVM_MEM_LOG_DIRTY_PAGES operation is called. After this function returns
>> + * all present PMD and PTEs are write protected in the memory region.
>> + * Afterwards read of dirty page log can be called.
>> + *
>> + * Acquires kvm_mmu_lock. Called with kvm->slots_lock mutex acquired,
>> + * serializing operations for VM memory regions.
>> + */
>> +
>> +void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot)
>> +{
>> + struct kvm_memory_slot *memslot = id_to_memslot(kvm->memslots, slot);
>> + phys_addr_t start = memslot->base_gfn << PAGE_SHIFT;
>> + phys_addr_t end = (memslot->base_gfn + memslot->npages) << PAGE_SHIFT;
>> +
>> + spin_lock(&kvm->mmu_lock);
>> + stage2_wp_range(kvm, start, end);
>> + kvm_flush_remote_tlbs(kvm);
>> + spin_unlock(&kvm->mmu_lock);
>> +}
>> +#endif
>> +
>> static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>> struct kvm_memory_slot *memslot,
>> unsigned long fault_status)
>
>
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
