[v15,10/17] RISC-V: KVM: Implement stage2 page table programming

Commit Message

Anup Patel Nov. 9, 2020, 11:32 a.m. UTC

This patch implements all required functions for programming
the stage2 page table for each Guest/VM.

At high-level, the flow of stage2 related functions is similar
from KVM ARM/ARM64 implementation but the stage2 page table
format is quite different for KVM RISC-V.

[jiangyifei: stage2 dirty log support]
Signed-off-by: Yifei Jiang <[e-mail]>
Signed-off-by: Anup Patel <[e-mail]>
Acked-by: Paolo Bonzini <[e-mail]>
Reviewed-by: Paolo Bonzini <[e-mail]>
---
 arch/riscv/include/asm/kvm_host.h     |  12 +
 arch/riscv/include/asm/pgtable-bits.h |   1 +
 arch/riscv/kvm/Kconfig                |   1 +
 arch/riscv/kvm/main.c                 |  19 +
 arch/riscv/kvm/mmu.c                  | 649 +++++++++++++++++++++++++-
 arch/riscv/kvm/vm.c                   |   6 -
 6 files changed, 672 insertions(+), 16 deletions(-)

Comments

Yifei Jiang Nov. 16, 2020, 9:29 a.m. UTC | #1

> -----Original Message-----
> From: Anup Patel [mailto:[e-mail]]
> Sent: Monday, November 9, 2020 7:33 PM
> To: Palmer Dabbelt <[e-mail]>; Palmer Dabbelt
> <[e-mail]>; Paul Walmsley <[e-mail]>;
> Albert Ou <[e-mail]>; Paolo Bonzini <[e-mail]>
> Cc: Alexander Graf <[e-mail]>; Atish Patra <[e-mail]>;
> Alistair Francis <[e-mail]>; Damien Le Moal
> <[e-mail]>; Anup Patel <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Anup Patel
> <[e-mail]>; Jiangyifei <[e-mail]>
> Subject: [PATCH v15 10/17] RISC-V: KVM: Implement stage2 page table
> programming
> 
> This patch implements all required functions for programming the stage2 page
> table for each Guest/VM.
> 
> At high-level, the flow of stage2 related functions is similar from KVM
> ARM/ARM64 implementation but the stage2 page table format is quite
> different for KVM RISC-V.
> 
> [jiangyifei: stage2 dirty log support]
> Signed-off-by: Yifei Jiang <[e-mail]>
> Signed-off-by: Anup Patel <[e-mail]>
> Acked-by: Paolo Bonzini <[e-mail]>
> Reviewed-by: Paolo Bonzini <[e-mail]>
> ---
>  arch/riscv/include/asm/kvm_host.h     |  12 +
>  arch/riscv/include/asm/pgtable-bits.h |   1 +
>  arch/riscv/kvm/Kconfig                |   1 +
>  arch/riscv/kvm/main.c                 |  19 +
>  arch/riscv/kvm/mmu.c                  | 649
> +++++++++++++++++++++++++-
>  arch/riscv/kvm/vm.c                   |   6 -
>  6 files changed, 672 insertions(+), 16 deletions(-)
> 

......

> 
>  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu, @@ -69,27 +562,163 @@
> int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
>  			 gpa_t gpa, unsigned long hva,
>  			 bool writeable, bool is_write)
>  {
> -	/* TODO: */
> -	return 0;
> +	int ret;
> +	kvm_pfn_t hfn;
> +	short vma_pageshift;
> +	gfn_t gfn = gpa >> PAGE_SHIFT;
> +	struct vm_area_struct *vma;
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_mmu_page_cache *pcache = &vcpu->arch.mmu_page_cache;
> +	bool logging = (memslot->dirty_bitmap &&
> +			!(memslot->flags & KVM_MEM_READONLY)) ? true : false;
> +	unsigned long vma_pagesize;
> +
> +	mmap_read_lock(current->mm);
> +
> +	vma = find_vma_intersection(current->mm, hva, hva + 1);
> +	if (unlikely(!vma)) {
> +		kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
> +		mmap_read_unlock(current->mm);
> +		return -EFAULT;
> +	}
> +
> +	if (is_vm_hugetlb_page(vma))
> +		vma_pageshift = huge_page_shift(hstate_vma(vma));
> +	else
> +		vma_pageshift = PAGE_SHIFT;
> +	vma_pagesize = 1ULL << vma_pageshift;
> +	if (logging || (vma->vm_flags & VM_PFNMAP))
> +		vma_pagesize = PAGE_SIZE;
> +
> +	if (vma_pagesize == PMD_SIZE || vma_pagesize == PGDIR_SIZE)
> +		gfn = (gpa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> +
> +	mmap_read_unlock(current->mm);
> +
> +	if (vma_pagesize != PGDIR_SIZE &&
> +	    vma_pagesize != PMD_SIZE &&
> +	    vma_pagesize != PAGE_SIZE) {
> +		kvm_err("Invalid VMA page size 0x%lx\n", vma_pagesize);
> +		return -EFAULT;
> +	}
> +
> +	/* We need minimum second+third level pages */
> +	ret = stage2_cache_topup(pcache, stage2_pgd_levels,
> +				 KVM_MMU_PAGE_CACHE_NR_OBJS);
> +	if (ret) {
> +		kvm_err("Failed to topup stage2 cache\n");
> +		return ret;
> +	}
> +
> +	hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> +	if (hfn == KVM_PFN_ERR_HWPOISON) {
> +		send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
> +				vma_pageshift, current);
> +		return 0;
> +	}
> +	if (is_error_noslot_pfn(hfn))
> +		return -EFAULT;
> +
> +	/*
> +	 * If logging is active then we allow writable pages only
> +	 * for write faults.
> +	 */
> +	if (logging && !is_write)
> +		writeable = false;
> +
> +	spin_lock(&kvm->mmu_lock);
> +
> +	if (writeable) {

Hi Anup,

What is the purpose of "writable = !memslot_is_readonly(slot)" in this series?

When mapping the HVA to HPA above, it doesn't know that the PTE writeable of stage2 is "!memslot_is_readonly(slot)".
This may causes the difference between the writability of HVA->HPA and GPA->HPA.
For example, GPA->HPA is writeable, but HVA->HPA is not writeable.

Is it better that the writability of HVA->HPA is also determined by whether the memslot is readonly in this change?
Like this:
-    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
+    hfn = gfn_to_pfn_prot(kvm, gfn, writeable, NULL);

Regards,
Yifei

> +		kvm_set_pfn_dirty(hfn);
> +		mark_page_dirty(kvm, gfn);
> +		ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
> +				      vma_pagesize, false, true);
> +	} else {
> +		ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
> +				      vma_pagesize, true, true);
> +	}
> +
> +	if (ret)
> +		kvm_err("Failed to map in stage2\n");
> +
> +	spin_unlock(&kvm->mmu_lock);
> +	kvm_set_pfn_accessed(hfn);
> +	kvm_release_pfn_clean(hfn);
> +	return ret;
>  }
> 

......

Anup Patel Nov. 24, 2020, 9:26 a.m. UTC | #2

On Mon, Nov 16, 2020 at 2:59 PM Jiangyifei <[e-mail]> wrote:
>
>
> > -----Original Message-----
> > From: Anup Patel [mailto:[e-mail]]
> > Sent: Monday, November 9, 2020 7:33 PM
> > To: Palmer Dabbelt <[e-mail]>; Palmer Dabbelt
> > <[e-mail]>; Paul Walmsley <[e-mail]>;
> > Albert Ou <[e-mail]>; Paolo Bonzini <[e-mail]>
> > Cc: Alexander Graf <[e-mail]>; Atish Patra <[e-mail]>;
> > Alistair Francis <[e-mail]>; Damien Le Moal
> > <[e-mail]>; Anup Patel <[e-mail]>;
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Anup Patel
> > <[e-mail]>; Jiangyifei <[e-mail]>
> > Subject: [PATCH v15 10/17] RISC-V: KVM: Implement stage2 page table
> > programming
> >
> > This patch implements all required functions for programming the stage2 page
> > table for each Guest/VM.
> >
> > At high-level, the flow of stage2 related functions is similar from KVM
> > ARM/ARM64 implementation but the stage2 page table format is quite
> > different for KVM RISC-V.
> >
> > [jiangyifei: stage2 dirty log support]
> > Signed-off-by: Yifei Jiang <[e-mail]>
> > Signed-off-by: Anup Patel <[e-mail]>
> > Acked-by: Paolo Bonzini <[e-mail]>
> > Reviewed-by: Paolo Bonzini <[e-mail]>
> > ---
> >  arch/riscv/include/asm/kvm_host.h     |  12 +
> >  arch/riscv/include/asm/pgtable-bits.h |   1 +
> >  arch/riscv/kvm/Kconfig                |   1 +
> >  arch/riscv/kvm/main.c                 |  19 +
> >  arch/riscv/kvm/mmu.c                  | 649
> > +++++++++++++++++++++++++-
> >  arch/riscv/kvm/vm.c                   |   6 -
> >  6 files changed, 672 insertions(+), 16 deletions(-)
> >
>
> ......
>
> >
> >  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu, @@ -69,27 +562,163 @@
> > int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
> >                        gpa_t gpa, unsigned long hva,
> >                        bool writeable, bool is_write)
> >  {
> > -     /* TODO: */
> > -     return 0;
> > +     int ret;
> > +     kvm_pfn_t hfn;
> > +     short vma_pageshift;
> > +     gfn_t gfn = gpa >> PAGE_SHIFT;
> > +     struct vm_area_struct *vma;
> > +     struct kvm *kvm = vcpu->kvm;
> > +     struct kvm_mmu_page_cache *pcache = &vcpu->arch.mmu_page_cache;
> > +     bool logging = (memslot->dirty_bitmap &&
> > +                     !(memslot->flags & KVM_MEM_READONLY)) ? true : false;
> > +     unsigned long vma_pagesize;
> > +
> > +     mmap_read_lock(current->mm);
> > +
> > +     vma = find_vma_intersection(current->mm, hva, hva + 1);
> > +     if (unlikely(!vma)) {
> > +             kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
> > +             mmap_read_unlock(current->mm);
> > +             return -EFAULT;
> > +     }
> > +
> > +     if (is_vm_hugetlb_page(vma))
> > +             vma_pageshift = huge_page_shift(hstate_vma(vma));
> > +     else
> > +             vma_pageshift = PAGE_SHIFT;
> > +     vma_pagesize = 1ULL << vma_pageshift;
> > +     if (logging || (vma->vm_flags & VM_PFNMAP))
> > +             vma_pagesize = PAGE_SIZE;
> > +
> > +     if (vma_pagesize == PMD_SIZE || vma_pagesize == PGDIR_SIZE)
> > +             gfn = (gpa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> > +
> > +     mmap_read_unlock(current->mm);
> > +
> > +     if (vma_pagesize != PGDIR_SIZE &&
> > +         vma_pagesize != PMD_SIZE &&
> > +         vma_pagesize != PAGE_SIZE) {
> > +             kvm_err("Invalid VMA page size 0x%lx\n", vma_pagesize);
> > +             return -EFAULT;
> > +     }
> > +
> > +     /* We need minimum second+third level pages */
> > +     ret = stage2_cache_topup(pcache, stage2_pgd_levels,
> > +                              KVM_MMU_PAGE_CACHE_NR_OBJS);
> > +     if (ret) {
> > +             kvm_err("Failed to topup stage2 cache\n");
> > +             return ret;
> > +     }
> > +
> > +     hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> > +     if (hfn == KVM_PFN_ERR_HWPOISON) {
> > +             send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
> > +                             vma_pageshift, current);
> > +             return 0;
> > +     }
> > +     if (is_error_noslot_pfn(hfn))
> > +             return -EFAULT;
> > +
> > +     /*
> > +      * If logging is active then we allow writable pages only
> > +      * for write faults.
> > +      */
> > +     if (logging && !is_write)
> > +             writeable = false;
> > +
> > +     spin_lock(&kvm->mmu_lock);
> > +
> > +     if (writeable) {
>
> Hi Anup,
>
> What is the purpose of "writable = !memslot_is_readonly(slot)" in this series?

Where ? I don't see this line in any of the patches.

>
> When mapping the HVA to HPA above, it doesn't know that the PTE writeable of stage2 is "!memslot_is_readonly(slot)".
> This may causes the difference between the writability of HVA->HPA and GPA->HPA.
> For example, GPA->HPA is writeable, but HVA->HPA is not writeable.

Yes, this is possible particularly when Host kernel is updating writability
of HVA->HPA mappings for swapping in/out pages.

>
> Is it better that the writability of HVA->HPA is also determined by whether the memslot is readonly in this change?
> Like this:
> -    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> +    hfn = gfn_to_pfn_prot(kvm, gfn, writeable, NULL);

The gfn_to_pfn_prot() needs to know what type of fault we
got (i.e read/write fault). Rest of the information (such as whether
slot is writable or not) is already available to gfn_to_pfn_prot().

The question here is should we pass "&writeable" or NULL as
last parameter to gfn_to_pfn_prot(). The recent JUMP label
support in Linux RISC-V causes problem on HW where PTE
'A' and 'D' bits are not updated by HW so I have to change
last parameter of gfn_to_pfn_prot() from "&writeable" to NULL.

I am still investigating this.

Regards,
Anup

>
> Regards,
> Yifei
>
> > +             kvm_set_pfn_dirty(hfn);
> > +             mark_page_dirty(kvm, gfn);
> > +             ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
> > +                                   vma_pagesize, false, true);
> > +     } else {
> > +             ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
> > +                                   vma_pagesize, true, true);
> > +     }
> > +
> > +     if (ret)
> > +             kvm_err("Failed to map in stage2\n");
> > +
> > +     spin_unlock(&kvm->mmu_lock);
> > +     kvm_set_pfn_accessed(hfn);
> > +     kvm_release_pfn_clean(hfn);
> > +     return ret;
> >  }
> >
>
> ......
>

Anup Patel Nov. 30, 2020, 10:21 a.m. UTC | #3

On Tue, Nov 24, 2020 at 2:56 PM Anup Patel <[e-mail]> wrote:
>
> On Mon, Nov 16, 2020 at 2:59 PM Jiangyifei <[e-mail]> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Anup Patel [mailto:[e-mail]]
> > > Sent: Monday, November 9, 2020 7:33 PM
> > > To: Palmer Dabbelt <[e-mail]>; Palmer Dabbelt
> > > <[e-mail]>; Paul Walmsley <[e-mail]>;
> > > Albert Ou <[e-mail]>; Paolo Bonzini <[e-mail]>
> > > Cc: Alexander Graf <[e-mail]>; Atish Patra <[e-mail]>;
> > > Alistair Francis <[e-mail]>; Damien Le Moal
> > > <[e-mail]>; Anup Patel <[e-mail]>;
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; Anup Patel
> > > <[e-mail]>; Jiangyifei <[e-mail]>
> > > Subject: [PATCH v15 10/17] RISC-V: KVM: Implement stage2 page table
> > > programming
> > >
> > > This patch implements all required functions for programming the stage2 page
> > > table for each Guest/VM.
> > >
> > > At high-level, the flow of stage2 related functions is similar from KVM
> > > ARM/ARM64 implementation but the stage2 page table format is quite
> > > different for KVM RISC-V.
> > >
> > > [jiangyifei: stage2 dirty log support]
> > > Signed-off-by: Yifei Jiang <[e-mail]>
> > > Signed-off-by: Anup Patel <[e-mail]>
> > > Acked-by: Paolo Bonzini <[e-mail]>
> > > Reviewed-by: Paolo Bonzini <[e-mail]>
> > > ---
> > >  arch/riscv/include/asm/kvm_host.h     |  12 +
> > >  arch/riscv/include/asm/pgtable-bits.h |   1 +
> > >  arch/riscv/kvm/Kconfig                |   1 +
> > >  arch/riscv/kvm/main.c                 |  19 +
> > >  arch/riscv/kvm/mmu.c                  | 649
> > > +++++++++++++++++++++++++-
> > >  arch/riscv/kvm/vm.c                   |   6 -
> > >  6 files changed, 672 insertions(+), 16 deletions(-)
> > >
> >
> > ......
> >
> > >
> > >  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu, @@ -69,27 +562,163 @@
> > > int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
> > >                        gpa_t gpa, unsigned long hva,
> > >                        bool writeable, bool is_write)
> > >  {
> > > -     /* TODO: */
> > > -     return 0;
> > > +     int ret;
> > > +     kvm_pfn_t hfn;
> > > +     short vma_pageshift;
> > > +     gfn_t gfn = gpa >> PAGE_SHIFT;
> > > +     struct vm_area_struct *vma;
> > > +     struct kvm *kvm = vcpu->kvm;
> > > +     struct kvm_mmu_page_cache *pcache = &vcpu->arch.mmu_page_cache;
> > > +     bool logging = (memslot->dirty_bitmap &&
> > > +                     !(memslot->flags & KVM_MEM_READONLY)) ? true : false;
> > > +     unsigned long vma_pagesize;
> > > +
> > > +     mmap_read_lock(current->mm);
> > > +
> > > +     vma = find_vma_intersection(current->mm, hva, hva + 1);
> > > +     if (unlikely(!vma)) {
> > > +             kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
> > > +             mmap_read_unlock(current->mm);
> > > +             return -EFAULT;
> > > +     }
> > > +
> > > +     if (is_vm_hugetlb_page(vma))
> > > +             vma_pageshift = huge_page_shift(hstate_vma(vma));
> > > +     else
> > > +             vma_pageshift = PAGE_SHIFT;
> > > +     vma_pagesize = 1ULL << vma_pageshift;
> > > +     if (logging || (vma->vm_flags & VM_PFNMAP))
> > > +             vma_pagesize = PAGE_SIZE;
> > > +
> > > +     if (vma_pagesize == PMD_SIZE || vma_pagesize == PGDIR_SIZE)
> > > +             gfn = (gpa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> > > +
> > > +     mmap_read_unlock(current->mm);
> > > +
> > > +     if (vma_pagesize != PGDIR_SIZE &&
> > > +         vma_pagesize != PMD_SIZE &&
> > > +         vma_pagesize != PAGE_SIZE) {
> > > +             kvm_err("Invalid VMA page size 0x%lx\n", vma_pagesize);
> > > +             return -EFAULT;
> > > +     }
> > > +
> > > +     /* We need minimum second+third level pages */
> > > +     ret = stage2_cache_topup(pcache, stage2_pgd_levels,
> > > +                              KVM_MMU_PAGE_CACHE_NR_OBJS);
> > > +     if (ret) {
> > > +             kvm_err("Failed to topup stage2 cache\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> > > +     if (hfn == KVM_PFN_ERR_HWPOISON) {
> > > +             send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
> > > +                             vma_pageshift, current);
> > > +             return 0;
> > > +     }
> > > +     if (is_error_noslot_pfn(hfn))
> > > +             return -EFAULT;
> > > +
> > > +     /*
> > > +      * If logging is active then we allow writable pages only
> > > +      * for write faults.
> > > +      */
> > > +     if (logging && !is_write)
> > > +             writeable = false;
> > > +
> > > +     spin_lock(&kvm->mmu_lock);
> > > +
> > > +     if (writeable) {
> >
> > Hi Anup,
> >
> > What is the purpose of "writable = !memslot_is_readonly(slot)" in this series?
>
> Where ? I don't see this line in any of the patches.
>
> >
> > When mapping the HVA to HPA above, it doesn't know that the PTE writeable of stage2 is "!memslot_is_readonly(slot)".
> > This may causes the difference between the writability of HVA->HPA and GPA->HPA.
> > For example, GPA->HPA is writeable, but HVA->HPA is not writeable.
>
> Yes, this is possible particularly when Host kernel is updating writability
> of HVA->HPA mappings for swapping in/out pages.
>
> >
> > Is it better that the writability of HVA->HPA is also determined by whether the memslot is readonly in this change?
> > Like this:
> > -    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> > +    hfn = gfn_to_pfn_prot(kvm, gfn, writeable, NULL);
>
> The gfn_to_pfn_prot() needs to know what type of fault we
> got (i.e read/write fault). Rest of the information (such as whether
> slot is writable or not) is already available to gfn_to_pfn_prot().
>
> The question here is should we pass "&writeable" or NULL as
> last parameter to gfn_to_pfn_prot(). The recent JUMP label
> support in Linux RISC-V causes problem on HW where PTE
> 'A' and 'D' bits are not updated by HW so I have to change
> last parameter of gfn_to_pfn_prot() from "&writeable" to NULL.
>
> I am still investigating this.

This turned-out to be a bug in Spike which is not fixed.

I will include following change in v16 patch series:


diff --git a/arch/riscv/include/asm/kvm_host.h
b/arch/riscv/include/asm/kvm_host.h
index 241030956d47..dc2666b4180b 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -232,8 +232,7 @@ void __kvm_riscv_hfence_gvma_all(void);

 int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
              struct kvm_memory_slot *memslot,
-             gpa_t gpa, unsigned long hva,
-             bool writeable, bool is_write);
+             gpa_t gpa, unsigned long hva, bool is_write);
 void kvm_riscv_stage2_flush_cache(struct kvm_vcpu *vcpu);
 int kvm_riscv_stage2_alloc_pgd(struct kvm *kvm);
 void kvm_riscv_stage2_free_pgd(struct kvm *kvm);
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index fcaeadc9b34d..56fda9ef70fd 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -689,11 +689,11 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)

 int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
              struct kvm_memory_slot *memslot,
-             gpa_t gpa, unsigned long hva,
-             bool writeable, bool is_write)
+             gpa_t gpa, unsigned long hva, bool is_write)
 {
     int ret;
     kvm_pfn_t hfn;
+    bool writeable;
     short vma_pageshift;
     gfn_t gfn = gpa >> PAGE_SHIFT;
     struct vm_area_struct *vma;
@@ -742,7 +742,7 @@ int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,

     mmu_seq = kvm->mmu_notifier_seq;

-    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
+    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, &writeable);
     if (hfn == KVM_PFN_ERR_HWPOISON) {
         send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
                 vma_pageshift, current);
diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
index f054406792a6..058cfa168abe 100644
--- a/arch/riscv/kvm/vcpu_exit.c
+++ b/arch/riscv/kvm/vcpu_exit.c
@@ -445,7 +445,7 @@ static int stage2_page_fault(struct kvm_vcpu
*vcpu, struct kvm_run *run,
         };
     }

-    ret = kvm_riscv_stage2_map(vcpu, memslot, fault_addr, hva, writeable,
+    ret = kvm_riscv_stage2_map(vcpu, memslot, fault_addr, hva,
         (trap->scause == EXC_STORE_GUEST_PAGE_FAULT) ? true : false);
     if (ret < 0)
         return ret;

Regards,
Anup

Yifei Jiang Dec. 1, 2020, 2:02 a.m. UTC | #4

> -----Original Message-----
> From: Anup Patel [mailto:[e-mail]]
> Sent: Monday, November 30, 2020 6:22 PM
> To: Jiangyifei <[e-mail]>
> Cc: Anup Patel <[e-mail]>; Palmer Dabbelt
> <[e-mail]>; Palmer Dabbelt <[e-mail]>; Paul
> Walmsley <[e-mail]>; Albert Ou <[e-mail]>;
> Paolo Bonzini <[e-mail]>; Alexander Graf <[e-mail]>;
> Atish Patra <[e-mail]>; Alistair Francis
> <[e-mail]>; Damien Le Moal <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Zhangxiaofeng (F)
> <[e-mail]>; Wubin (H) <[e-mail]>;
> dengkai (A) <[e-mail]>; yinyipeng <[e-mail]>
> Subject: Re: [PATCH v15 10/17] RISC-V: KVM: Implement stage2 page table
> programming
> 
> On Tue, Nov 24, 2020 at 2:56 PM Anup Patel <[e-mail]> wrote:
> >
> > On Mon, Nov 16, 2020 at 2:59 PM Jiangyifei <[e-mail]> wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Anup Patel [mailto:[e-mail]]
> > > > Sent: Monday, November 9, 2020 7:33 PM
> > > > To: Palmer Dabbelt <[e-mail]>; Palmer Dabbelt
> > > > <[e-mail]>; Paul Walmsley
> > > > <[e-mail]>; Albert Ou <[e-mail]>;
> > > > Paolo Bonzini <[e-mail]>
> > > > Cc: Alexander Graf <[e-mail]>; Atish Patra
> > > > <[e-mail]>; Alistair Francis
> > > > <[e-mail]>; Damien Le Moal
> > > > <[e-mail]>; Anup Patel <[e-mail]>;
> > > > [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail];
> > > > Anup Patel <[e-mail]>; Jiangyifei
> > > > <[e-mail]>
> > > > Subject: [PATCH v15 10/17] RISC-V: KVM: Implement stage2 page
> > > > table programming
> > > >
> > > > This patch implements all required functions for programming the
> > > > stage2 page table for each Guest/VM.
> > > >
> > > > At high-level, the flow of stage2 related functions is similar
> > > > from KVM
> > > > ARM/ARM64 implementation but the stage2 page table format is quite
> > > > different for KVM RISC-V.
> > > >
> > > > [jiangyifei: stage2 dirty log support]
> > > > Signed-off-by: Yifei Jiang <[e-mail]>
> > > > Signed-off-by: Anup Patel <[e-mail]>
> > > > Acked-by: Paolo Bonzini <[e-mail]>
> > > > Reviewed-by: Paolo Bonzini <[e-mail]>
> > > > ---
> > > >  arch/riscv/include/asm/kvm_host.h     |  12 +
> > > >  arch/riscv/include/asm/pgtable-bits.h |   1 +
> > > >  arch/riscv/kvm/Kconfig                |   1 +
> > > >  arch/riscv/kvm/main.c                 |  19 +
> > > >  arch/riscv/kvm/mmu.c                  | 649
> > > > +++++++++++++++++++++++++-
> > > >  arch/riscv/kvm/vm.c                   |   6 -
> > > >  6 files changed, 672 insertions(+), 16 deletions(-)
> > > >
> > >
> > > ......
> > >
> > > >
> > > >  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu, @@ -69,27
> > > > +562,163 @@ int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
> > > >                        gpa_t gpa, unsigned long hva,
> > > >                        bool writeable, bool is_write)  {
> > > > -     /* TODO: */
> > > > -     return 0;
> > > > +     int ret;
> > > > +     kvm_pfn_t hfn;
> > > > +     short vma_pageshift;
> > > > +     gfn_t gfn = gpa >> PAGE_SHIFT;
> > > > +     struct vm_area_struct *vma;
> > > > +     struct kvm *kvm = vcpu->kvm;
> > > > +     struct kvm_mmu_page_cache *pcache =
> &vcpu->arch.mmu_page_cache;
> > > > +     bool logging = (memslot->dirty_bitmap &&
> > > > +                     !(memslot->flags & KVM_MEM_READONLY)) ?
> true : false;
> > > > +     unsigned long vma_pagesize;
> > > > +
> > > > +     mmap_read_lock(current->mm);
> > > > +
> > > > +     vma = find_vma_intersection(current->mm, hva, hva + 1);
> > > > +     if (unlikely(!vma)) {
> > > > +             kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
> > > > +             mmap_read_unlock(current->mm);
> > > > +             return -EFAULT;
> > > > +     }
> > > > +
> > > > +     if (is_vm_hugetlb_page(vma))
> > > > +             vma_pageshift = huge_page_shift(hstate_vma(vma));
> > > > +     else
> > > > +             vma_pageshift = PAGE_SHIFT;
> > > > +     vma_pagesize = 1ULL << vma_pageshift;
> > > > +     if (logging || (vma->vm_flags & VM_PFNMAP))
> > > > +             vma_pagesize = PAGE_SIZE;
> > > > +
> > > > +     if (vma_pagesize == PMD_SIZE || vma_pagesize == PGDIR_SIZE)
> > > > +             gfn = (gpa & huge_page_mask(hstate_vma(vma))) >>
> > > > + PAGE_SHIFT;
> > > > +
> > > > +     mmap_read_unlock(current->mm);
> > > > +
> > > > +     if (vma_pagesize != PGDIR_SIZE &&
> > > > +         vma_pagesize != PMD_SIZE &&
> > > > +         vma_pagesize != PAGE_SIZE) {
> > > > +             kvm_err("Invalid VMA page size 0x%lx\n",
> vma_pagesize);
> > > > +             return -EFAULT;
> > > > +     }
> > > > +
> > > > +     /* We need minimum second+third level pages */
> > > > +     ret = stage2_cache_topup(pcache, stage2_pgd_levels,
> > > > +
> KVM_MMU_PAGE_CACHE_NR_OBJS);
> > > > +     if (ret) {
> > > > +             kvm_err("Failed to topup stage2 cache\n");
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> > > > +     if (hfn == KVM_PFN_ERR_HWPOISON) {
> > > > +             send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
> > > > +                             vma_pageshift, current);
> > > > +             return 0;
> > > > +     }
> > > > +     if (is_error_noslot_pfn(hfn))
> > > > +             return -EFAULT;
> > > > +
> > > > +     /*
> > > > +      * If logging is active then we allow writable pages only
> > > > +      * for write faults.
> > > > +      */
> > > > +     if (logging && !is_write)
> > > > +             writeable = false;
> > > > +
> > > > +     spin_lock(&kvm->mmu_lock);
> > > > +
> > > > +     if (writeable) {
> > >
> > > Hi Anup,
> > >
> > > What is the purpose of "writable = !memslot_is_readonly(slot)" in this
> series?
> >
> > Where ? I don't see this line in any of the patches.
> >
> > >
> > > When mapping the HVA to HPA above, it doesn't know that the PTE
> writeable of stage2 is "!memslot_is_readonly(slot)".
> > > This may causes the difference between the writability of HVA->HPA and
> GPA->HPA.
> > > For example, GPA->HPA is writeable, but HVA->HPA is not writeable.
> >
> > Yes, this is possible particularly when Host kernel is updating
> > writability of HVA->HPA mappings for swapping in/out pages.
> >
> > >
> > > Is it better that the writability of HVA->HPA is also determined by whether
> the memslot is readonly in this change?
> > > Like this:
> > > -    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> > > +    hfn = gfn_to_pfn_prot(kvm, gfn, writeable, NULL);
> >
> > The gfn_to_pfn_prot() needs to know what type of fault we got (i.e
> > read/write fault). Rest of the information (such as whether slot is
> > writable or not) is already available to gfn_to_pfn_prot().
> >
> > The question here is should we pass "&writeable" or NULL as last
> > parameter to gfn_to_pfn_prot(). The recent JUMP label support in Linux
> > RISC-V causes problem on HW where PTE 'A' and 'D' bits are not updated
> > by HW so I have to change last parameter of gfn_to_pfn_prot() from
> > "&writeable" to NULL.
> >
> > I am still investigating this.
> 
> This turned-out to be a bug in Spike which is not fixed.
> 
> I will include following change in v16 patch series:
> 
> 
> diff --git a/arch/riscv/include/asm/kvm_host.h
> b/arch/riscv/include/asm/kvm_host.h
> index 241030956d47..dc2666b4180b 100644
> --- a/arch/riscv/include/asm/kvm_host.h
> +++ b/arch/riscv/include/asm/kvm_host.h
> @@ -232,8 +232,7 @@ void __kvm_riscv_hfence_gvma_all(void);
> 
>  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
>               struct kvm_memory_slot *memslot,
> -             gpa_t gpa, unsigned long hva,
> -             bool writeable, bool is_write);
> +             gpa_t gpa, unsigned long hva, bool is_write);
>  void kvm_riscv_stage2_flush_cache(struct kvm_vcpu *vcpu);  int
> kvm_riscv_stage2_alloc_pgd(struct kvm *kvm);  void
> kvm_riscv_stage2_free_pgd(struct kvm *kvm); diff --git
> a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c index
> fcaeadc9b34d..56fda9ef70fd 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -689,11 +689,11 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned
> long hva)
> 
>  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
>               struct kvm_memory_slot *memslot,
> -             gpa_t gpa, unsigned long hva,
> -             bool writeable, bool is_write)
> +             gpa_t gpa, unsigned long hva, bool is_write)
>  {
>      int ret;
>      kvm_pfn_t hfn;
> +    bool writeable;
>      short vma_pageshift;
>      gfn_t gfn = gpa >> PAGE_SHIFT;
>      struct vm_area_struct *vma;
> @@ -742,7 +742,7 @@ int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
> 
>      mmu_seq = kvm->mmu_notifier_seq;
> 
> -    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
> +    hfn = gfn_to_pfn_prot(kvm, gfn, is_write, &writeable);
>      if (hfn == KVM_PFN_ERR_HWPOISON) {
>          send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
>                  vma_pageshift, current); diff --git
> a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c index
> f054406792a6..058cfa168abe 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c
> @@ -445,7 +445,7 @@ static int stage2_page_fault(struct kvm_vcpu *vcpu,
> struct kvm_run *run,
>          };
>      }
> 
> -    ret = kvm_riscv_stage2_map(vcpu, memslot, fault_addr, hva, writeable,
> +    ret = kvm_riscv_stage2_map(vcpu, memslot, fault_addr, hva,
>          (trap->scause == EXC_STORE_GUEST_PAGE_FAULT) ? true : false);
>      if (ret < 0)
>          return ret;
> 
> Regards,
> Anup

This change looks good.

Yifei

Patch

diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 08681e58c695..1c4c71e06e5b 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -76,6 +76,13 @@  struct kvm_mmio_decode {
 	int return_handled;
 };
 
+#define KVM_MMU_PAGE_CACHE_NR_OBJS	32
+
+struct kvm_mmu_page_cache {
+	int nobjs;
+	void *objects[KVM_MMU_PAGE_CACHE_NR_OBJS];
+};
+
 struct kvm_cpu_trap {
 	unsigned long sepc;
 	unsigned long scause;
@@ -177,6 +184,9 @@  struct kvm_vcpu_arch {
 	/* MMIO instruction details */
 	struct kvm_mmio_decode mmio_decode;
 
+	/* Cache pages needed to program page tables with spinlock held */
+	struct kvm_mmu_page_cache mmu_page_cache;
+
 	/* VCPU power-off state */
 	bool power_off;
 
@@ -206,6 +216,8 @@  void kvm_riscv_stage2_flush_cache(struct kvm_vcpu *vcpu);
 int kvm_riscv_stage2_alloc_pgd(struct kvm *kvm);
 void kvm_riscv_stage2_free_pgd(struct kvm *kvm);
 void kvm_riscv_stage2_update_hgatp(struct kvm_vcpu *vcpu);
+void kvm_riscv_stage2_mode_detect(void);
+unsigned long kvm_riscv_stage2_mode(void);
 
 void kvm_riscv_stage2_vmid_detect(void);
 unsigned long kvm_riscv_stage2_vmid_bits(void);
diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
index bbaeb5d35842..be49d62fcc2b 100644
--- a/arch/riscv/include/asm/pgtable-bits.h
+++ b/arch/riscv/include/asm/pgtable-bits.h
@@ -26,6 +26,7 @@ 
 
 #define _PAGE_SPECIAL   _PAGE_SOFT
 #define _PAGE_TABLE     _PAGE_PRESENT
+#define _PAGE_LEAF      (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC)
 
 /*
  * _PAGE_PROT_NONE is set on not-present pages (and ignored by the hardware) to
diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig
index b42979f84042..633063edaee8 100644
--- a/arch/riscv/kvm/Kconfig
+++ b/arch/riscv/kvm/Kconfig
@@ -23,6 +23,7 @@  config KVM
 	select PREEMPT_NOTIFIERS
 	select ANON_INODES
 	select KVM_MMIO
+	select KVM_GENERIC_DIRTYLOG_READ_PROTECT
 	select HAVE_KVM_VCPU_ASYNC_IOCTL
 	select HAVE_KVM_EVENTFD
 	select SRCU
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 49a4941e3838..421ecf4e6360 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -64,6 +64,8 @@  void kvm_arch_hardware_disable(void)
 
 int kvm_arch_init(void *opaque)
 {
+	const char *str;
+
 	if (!riscv_isa_extension_available(NULL, h)) {
 		kvm_info("hypervisor extension not available\n");
 		return -ENODEV;
@@ -79,10 +81,27 @@  int kvm_arch_init(void *opaque)
 		return -ENODEV;
 	}
 
+	kvm_riscv_stage2_mode_detect();
+
 	kvm_riscv_stage2_vmid_detect();
 
 	kvm_info("hypervisor extension available\n");
 
+	switch (kvm_riscv_stage2_mode()) {
+	case HGATP_MODE_SV32X4:
+		str = "Sv32x4";
+		break;
+	case HGATP_MODE_SV39X4:
+		str = "Sv39x4";
+		break;
+	case HGATP_MODE_SV48X4:
+		str = "Sv48x4";
+		break;
+	default:
+		return -ENODEV;
+	}
+	kvm_info("using %s G-stage page table format\n", str);
+
 	kvm_info("VMID %ld bits available\n", kvm_riscv_stage2_vmid_bits());
 
 	return 0;
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index f8153648a3bb..39063d1372cb 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -17,11 +17,415 @@ 
 #include <linux/sched/signal.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
+#include <asm/sbi.h>
+
+#ifdef CONFIG_64BIT
+static unsigned long stage2_mode = (HGATP_MODE_SV39X4 << HGATP_MODE_SHIFT);
+static unsigned long stage2_pgd_levels = 3;
+#define stage2_index_bits	9
+#else
+static unsigned long stage2_mode = (HGATP_MODE_SV32X4 << HGATP_MODE_SHIFT);
+static unsigned long stage2_pgd_levels = 2;
+#define stage2_index_bits	10
+#endif
+
+#define stage2_pgd_xbits	2
+#define stage2_pgd_size	(1UL << (HGATP_PAGE_SHIFT + stage2_pgd_xbits))
+#define stage2_gpa_bits	(HGATP_PAGE_SHIFT + \
+			 (stage2_pgd_levels * stage2_index_bits) + \
+			 stage2_pgd_xbits)
+#define stage2_gpa_size	((gpa_t)(1ULL << stage2_gpa_bits))
+
+static inline unsigned long stage2_pte_index(gpa_t addr, u32 level)
+{
+	unsigned long mask;
+	unsigned long shift = HGATP_PAGE_SHIFT + (stage2_index_bits * level);
+
+	if (level == (stage2_pgd_levels - 1))
+		mask = (PTRS_PER_PTE * (1UL << stage2_pgd_xbits)) - 1;
+	else
+		mask = PTRS_PER_PTE - 1;
+
+	return (addr >> shift) & mask;
+}
+
+static inline unsigned long stage2_pte_page_vaddr(pte_t pte)
+{
+	return (unsigned long)pfn_to_virt(pte_val(pte) >> _PAGE_PFN_SHIFT);
+}
+
+static int stage2_page_size_to_level(unsigned long page_size, u32 *out_level)
+{
+	u32 i;
+	unsigned long psz = 1UL << 12;
+
+	for (i = 0; i < stage2_pgd_levels; i++) {
+		if (page_size == (psz << (i * stage2_index_bits))) {
+			*out_level = i;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int stage2_level_to_page_size(u32 level, unsigned long *out_pgsize)
+{
+	if (stage2_pgd_levels < level)
+		return -EINVAL;
+
+	*out_pgsize = 1UL << (12 + (level * stage2_index_bits));
+
+	return 0;
+}
+
+static int stage2_cache_topup(struct kvm_mmu_page_cache *pcache,
+			      int min, int max)
+{
+	void *page;
+
+	BUG_ON(max > KVM_MMU_PAGE_CACHE_NR_OBJS);
+	if (pcache->nobjs >= min)
+		return 0;
+	while (pcache->nobjs < max) {
+		page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
+		if (!page)
+			return -ENOMEM;
+		pcache->objects[pcache->nobjs++] = page;
+	}
+
+	return 0;
+}
+
+static void stage2_cache_flush(struct kvm_mmu_page_cache *pcache)
+{
+	while (pcache && pcache->nobjs)
+		free_page((unsigned long)pcache->objects[--pcache->nobjs]);
+}
+
+static void *stage2_cache_alloc(struct kvm_mmu_page_cache *pcache)
+{
+	void *p;
+
+	if (!pcache)
+		return NULL;
+
+	BUG_ON(!pcache->nobjs);
+	p = pcache->objects[--pcache->nobjs];
+
+	return p;
+}
+
+static bool stage2_get_leaf_entry(struct kvm *kvm, gpa_t addr,
+				  pte_t **ptepp, u32 *ptep_level)
+{
+	pte_t *ptep;
+	u32 current_level = stage2_pgd_levels - 1;
+
+	*ptep_level = current_level;
+	ptep = (pte_t *)kvm->arch.pgd;
+	ptep = &ptep[stage2_pte_index(addr, current_level)];
+	while (ptep && pte_val(*ptep)) {
+		if (pte_val(*ptep) & _PAGE_LEAF) {
+			*ptep_level = current_level;
+			*ptepp = ptep;
+			return true;
+		}
+
+		if (current_level) {
+			current_level--;
+			*ptep_level = current_level;
+			ptep = (pte_t *)stage2_pte_page_vaddr(*ptep);
+			ptep = &ptep[stage2_pte_index(addr, current_level)];
+		} else {
+			ptep = NULL;
+		}
+	}
+
+	return false;
+}
+
+static void stage2_remote_tlb_flush(struct kvm *kvm, u32 level, gpa_t addr)
+{
+	struct cpumask hmask;
+	unsigned long size = PAGE_SIZE;
+	struct kvm_vmid *vmid = &kvm->arch.vmid;
+
+	if (stage2_level_to_page_size(level, &size))
+		return;
+	addr &= ~(size - 1);
+
+	/*
+	 * TODO: Instead of cpu_online_mask, we should only target CPUs
+	 * where the Guest/VM is running.
+	 */
+	preempt_disable();
+	riscv_cpuid_to_hartid_mask(cpu_online_mask, &hmask);
+	sbi_remote_hfence_gvma_vmid(cpumask_bits(&hmask), addr, size,
+				    READ_ONCE(vmid->vmid));
+	preempt_enable();
+}
+
+static int stage2_set_pte(struct kvm *kvm, u32 level,
+			   struct kvm_mmu_page_cache *pcache,
+			   gpa_t addr, const pte_t *new_pte)
+{
+	u32 current_level = stage2_pgd_levels - 1;
+	pte_t *next_ptep = (pte_t *)kvm->arch.pgd;
+	pte_t *ptep = &next_ptep[stage2_pte_index(addr, current_level)];
+
+	if (current_level < level)
+		return -EINVAL;
+
+	while (current_level != level) {
+		if (pte_val(*ptep) & _PAGE_LEAF)
+			return -EEXIST;
+
+		if (!pte_val(*ptep)) {
+			next_ptep = stage2_cache_alloc(pcache);
+			if (!next_ptep)
+				return -ENOMEM;
+			*ptep = pfn_pte(PFN_DOWN(__pa(next_ptep)),
+					__pgprot(_PAGE_TABLE));
+		} else {
+			if (pte_val(*ptep) & _PAGE_LEAF)
+				return -EEXIST;
+			next_ptep = (pte_t *)stage2_pte_page_vaddr(*ptep);
+		}
+
+		current_level--;
+		ptep = &next_ptep[stage2_pte_index(addr, current_level)];
+	}
+
+	*ptep = *new_pte;
+	if (pte_val(*ptep) & _PAGE_LEAF)
+		stage2_remote_tlb_flush(kvm, current_level, addr);
+
+	return 0;
+}
+
+static int stage2_map_page(struct kvm *kvm,
+			   struct kvm_mmu_page_cache *pcache,
+			   gpa_t gpa, phys_addr_t hpa,
+			   unsigned long page_size,
+			   bool page_rdonly, bool page_exec)
+{
+	int ret;
+	u32 level = 0;
+	pte_t new_pte;
+	pgprot_t prot;
+
+	ret = stage2_page_size_to_level(page_size, &level);
+	if (ret)
+		return ret;
+
+	/*
+	 * A RISC-V implementation can choose to either:
+	 * 1) Update 'A' and 'D' PTE bits in hardware
+	 * 2) Generate page fault when 'A' and/or 'D' bits are not set
+	 *    PTE so that software can update these bits.
+	 *
+	 * We support both options mentioned above. To achieve this, we
+	 * always set 'A' and 'D' PTE bits at time of creating stage2
+	 * mapping. To support KVM dirty page logging with both options
+	 * mentioned above, we will write-protect stage2 PTEs to track
+	 * dirty pages.
+	 */
+
+	if (page_exec) {
+		if (page_rdonly)
+			prot = PAGE_READ_EXEC;
+		else
+			prot = PAGE_WRITE_EXEC;
+	} else {
+		if (page_rdonly)
+			prot = PAGE_READ;
+		else
+			prot = PAGE_WRITE;
+	}
+	new_pte = pfn_pte(PFN_DOWN(hpa), prot);
+	new_pte = pte_mkdirty(new_pte);
+
+	return stage2_set_pte(kvm, level, pcache, gpa, &new_pte);
+}
+
+enum stage2_op {
+	STAGE2_OP_NOP = 0,	/* Nothing */
+	STAGE2_OP_CLEAR,	/* Clear/Unmap */
+	STAGE2_OP_WP,		/* Write-protect */
+};
+
+static void stage2_op_pte(struct kvm *kvm, gpa_t addr,
+			  pte_t *ptep, u32 ptep_level, enum stage2_op op)
+{
+	int i, ret;
+	pte_t *next_ptep;
+	u32 next_ptep_level;
+	unsigned long next_page_size, page_size;
+
+	ret = stage2_level_to_page_size(ptep_level, &page_size);
+	if (ret)
+		return;
+
+	BUG_ON(addr & (page_size - 1));
+
+	if (!pte_val(*ptep))
+		return;
+
+	if (ptep_level && !(pte_val(*ptep) & _PAGE_LEAF)) {
+		next_ptep = (pte_t *)stage2_pte_page_vaddr(*ptep);
+		next_ptep_level = ptep_level - 1;
+		ret = stage2_level_to_page_size(next_ptep_level,
+						&next_page_size);
+		if (ret)
+			return;
+
+		if (op == STAGE2_OP_CLEAR)
+			set_pte(ptep, __pte(0));
+		for (i = 0; i < PTRS_PER_PTE; i++)
+			stage2_op_pte(kvm, addr + i * next_page_size,
+					&next_ptep[i], next_ptep_level, op);
+		if (op == STAGE2_OP_CLEAR)
+			put_page(virt_to_page(next_ptep));
+	} else {
+		if (op == STAGE2_OP_CLEAR)
+			set_pte(ptep, __pte(0));
+		else if (op == STAGE2_OP_WP)
+			set_pte(ptep, __pte(pte_val(*ptep) & ~_PAGE_WRITE));
+		stage2_remote_tlb_flush(kvm, ptep_level, addr);
+	}
+}
+
+static void stage2_unmap_range(struct kvm *kvm, gpa_t start, gpa_t size)
+{
+	int ret;
+	pte_t *ptep;
+	u32 ptep_level;
+	bool found_leaf;
+	unsigned long page_size;
+	gpa_t addr = start, end = start + size;
+
+	while (addr < end) {
+		found_leaf = stage2_get_leaf_entry(kvm, addr,
+						   &ptep, &ptep_level);
+		ret = stage2_level_to_page_size(ptep_level, &page_size);
+		if (ret)
+			break;
+
+		if (!found_leaf)
+			goto next;
+
+		if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
+			stage2_op_pte(kvm, addr, ptep,
+				      ptep_level, STAGE2_OP_CLEAR);
+
+next:
+		addr += page_size;
+	}
+}
+
+static void stage2_wp_range(struct kvm *kvm, gpa_t start, gpa_t end)
+{
+	int ret;
+	pte_t *ptep;
+	u32 ptep_level;
+	bool found_leaf;
+	gpa_t addr = start;
+	unsigned long page_size;
+
+	while (addr < end) {
+		found_leaf = stage2_get_leaf_entry(kvm, addr,
+						   &ptep, &ptep_level);
+		ret = stage2_level_to_page_size(ptep_level, &page_size);
+		if (ret)
+			break;
+
+		if (!found_leaf)
+			goto next;
+
+		if (!(addr & (page_size - 1)) && ((end - addr) >= page_size))
+			stage2_op_pte(kvm, addr, ptep,
+				      ptep_level, STAGE2_OP_WP);
+
+next:
+		addr += page_size;
+	}
+}
+
+void stage2_wp_memory_region(struct kvm *kvm, int slot)
+{
+	struct kvm_memslots *slots = kvm_memslots(kvm);
+	struct kvm_memory_slot *memslot = id_to_memslot(slots, 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);
+	spin_unlock(&kvm->mmu_lock);
+	kvm_flush_remote_tlbs(kvm);
+}
+
+int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa,
+		   unsigned long size, bool writable)
+{
+	pte_t pte;
+	int ret = 0;
+	unsigned long pfn;
+	phys_addr_t addr, end;
+	struct kvm_mmu_page_cache pcache = { 0, };
+
+	end = (gpa + size + PAGE_SIZE - 1) & PAGE_MASK;
+	pfn = __phys_to_pfn(hpa);
+
+	for (addr = gpa; addr < end; addr += PAGE_SIZE) {
+		pte = pfn_pte(pfn, PAGE_KERNEL);
+
+		if (!writable)
+			pte = pte_wrprotect(pte);
+
+		ret = stage2_cache_topup(&pcache,
+					 stage2_pgd_levels,
+					 KVM_MMU_PAGE_CACHE_NR_OBJS);
+		if (ret)
+			goto out;
+
+		spin_lock(&kvm->mmu_lock);
+		ret = stage2_set_pte(kvm, 0, &pcache, addr, &pte);
+		spin_unlock(&kvm->mmu_lock);
+		if (ret)
+			goto out;
+
+		pfn++;
+	}
+
+out:
+	stage2_cache_flush(&pcache);
+	return ret;
+
+}
+
+void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
+					     struct kvm_memory_slot *slot,
+					     gfn_t gfn_offset,
+					     unsigned long mask)
+{
+	phys_addr_t base_gfn = slot->base_gfn + gfn_offset;
+	phys_addr_t start = (base_gfn +  __ffs(mask)) << PAGE_SHIFT;
+	phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT;
+
+	stage2_wp_range(kvm, start, end);
+}
 
 void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
 {
 }
 
+void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
+					struct kvm_memory_slot *memslot)
+{
+	kvm_flush_remote_tlbs(kvm);
+}
+
 void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free)
 {
 }
@@ -38,7 +442,7 @@  void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen)
 
 void kvm_arch_flush_shadow_all(struct kvm *kvm)
 {
-	/* TODO: */
+	kvm_riscv_stage2_free_pgd(kvm);
 }
 
 void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
@@ -52,7 +456,13 @@  void kvm_arch_commit_memory_region(struct kvm *kvm,
 				const struct kvm_memory_slot *new,
 				enum kvm_mr_change change)
 {
-	/* TODO: */
+	/*
+	 * At this point memslot has been committed and there is an
+	 * allocated dirty_bitmap[], dirty pages will be tracked while
+	 * the memory slot is write protected.
+	 */
+	if (change != KVM_MR_DELETE && mem->flags & KVM_MEM_LOG_DIRTY_PAGES)
+		stage2_wp_memory_region(kvm, mem->slot);
 }
 
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
@@ -60,8 +470,91 @@  int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				const struct kvm_userspace_memory_region *mem,
 				enum kvm_mr_change change)
 {
-	/* TODO: */
-	return 0;
+	hva_t hva = mem->userspace_addr;
+	hva_t reg_end = hva + mem->memory_size;
+	bool writable = !(mem->flags & KVM_MEM_READONLY);
+	int ret = 0;
+
+	if (change != KVM_MR_CREATE && change != KVM_MR_MOVE &&
+			change != KVM_MR_FLAGS_ONLY)
+		return 0;
+
+	/*
+	 * Prevent userspace from creating a memory region outside of the GPA
+	 * space addressable by the KVM guest GPA space.
+	 */
+	if ((memslot->base_gfn + memslot->npages) >=
+	    (stage2_gpa_size >> PAGE_SHIFT))
+		return -EFAULT;
+
+	mmap_read_lock(current->mm);
+
+	/*
+	 * A memory region could potentially cover multiple VMAs, and
+	 * any holes between them, so iterate over all of them to find
+	 * out if we can map any of them right now.
+	 *
+	 *     +--------------------------------------------+
+	 * +---------------+----------------+   +----------------+
+	 * |   : VMA 1     |      VMA 2     |   |    VMA 3  :    |
+	 * +---------------+----------------+   +----------------+
+	 *     |               memory region                |
+	 *     +--------------------------------------------+
+	 */
+	do {
+		struct vm_area_struct *vma = find_vma(current->mm, hva);
+		hva_t vm_start, vm_end;
+
+		if (!vma || vma->vm_start >= reg_end)
+			break;
+
+		/*
+		 * Mapping a read-only VMA is only allowed if the
+		 * memory region is configured as read-only.
+		 */
+		if (writable && !(vma->vm_flags & VM_WRITE)) {
+			ret = -EPERM;
+			break;
+		}
+
+		/* Take the intersection of this VMA with the memory region */
+		vm_start = max(hva, vma->vm_start);
+		vm_end = min(reg_end, vma->vm_end);
+
+		if (vma->vm_flags & VM_PFNMAP) {
+			gpa_t gpa = mem->guest_phys_addr +
+				    (vm_start - mem->userspace_addr);
+			phys_addr_t pa;
+
+			pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT;
+			pa += vm_start - vma->vm_start;
+
+			/* IO region dirty page logging not allowed */
+			if (memslot->flags & KVM_MEM_LOG_DIRTY_PAGES) {
+				ret = -EINVAL;
+				goto out;
+			}
+
+			ret = stage2_ioremap(kvm, gpa, pa,
+					     vm_end - vm_start, writable);
+			if (ret)
+				break;
+		}
+		hva = vm_end;
+	} while (hva < reg_end);
+
+	if (change == KVM_MR_FLAGS_ONLY)
+		goto out;
+
+	spin_lock(&kvm->mmu_lock);
+	if (ret)
+		stage2_unmap_range(kvm, mem->guest_phys_addr,
+				   mem->memory_size);
+	spin_unlock(&kvm->mmu_lock);
+
+out:
+	mmap_read_unlock(current->mm);
+	return ret;
 }
 
 int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
@@ -69,27 +562,163 @@  int kvm_riscv_stage2_map(struct kvm_vcpu *vcpu,
 			 gpa_t gpa, unsigned long hva,
 			 bool writeable, bool is_write)
 {
-	/* TODO: */
-	return 0;
+	int ret;
+	kvm_pfn_t hfn;
+	short vma_pageshift;
+	gfn_t gfn = gpa >> PAGE_SHIFT;
+	struct vm_area_struct *vma;
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_mmu_page_cache *pcache = &vcpu->arch.mmu_page_cache;
+	bool logging = (memslot->dirty_bitmap &&
+			!(memslot->flags & KVM_MEM_READONLY)) ? true : false;
+	unsigned long vma_pagesize;
+
+	mmap_read_lock(current->mm);
+
+	vma = find_vma_intersection(current->mm, hva, hva + 1);
+	if (unlikely(!vma)) {
+		kvm_err("Failed to find VMA for hva 0x%lx\n", hva);
+		mmap_read_unlock(current->mm);
+		return -EFAULT;
+	}
+
+	if (is_vm_hugetlb_page(vma))
+		vma_pageshift = huge_page_shift(hstate_vma(vma));
+	else
+		vma_pageshift = PAGE_SHIFT;
+	vma_pagesize = 1ULL << vma_pageshift;
+	if (logging || (vma->vm_flags & VM_PFNMAP))
+		vma_pagesize = PAGE_SIZE;
+
+	if (vma_pagesize == PMD_SIZE || vma_pagesize == PGDIR_SIZE)
+		gfn = (gpa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
+
+	mmap_read_unlock(current->mm);
+
+	if (vma_pagesize != PGDIR_SIZE &&
+	    vma_pagesize != PMD_SIZE &&
+	    vma_pagesize != PAGE_SIZE) {
+		kvm_err("Invalid VMA page size 0x%lx\n", vma_pagesize);
+		return -EFAULT;
+	}
+
+	/* We need minimum second+third level pages */
+	ret = stage2_cache_topup(pcache, stage2_pgd_levels,
+				 KVM_MMU_PAGE_CACHE_NR_OBJS);
+	if (ret) {
+		kvm_err("Failed to topup stage2 cache\n");
+		return ret;
+	}
+
+	hfn = gfn_to_pfn_prot(kvm, gfn, is_write, NULL);
+	if (hfn == KVM_PFN_ERR_HWPOISON) {
+		send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
+				vma_pageshift, current);
+		return 0;
+	}
+	if (is_error_noslot_pfn(hfn))
+		return -EFAULT;
+
+	/*
+	 * If logging is active then we allow writable pages only
+	 * for write faults.
+	 */
+	if (logging && !is_write)
+		writeable = false;
+
+	spin_lock(&kvm->mmu_lock);
+
+	if (writeable) {
+		kvm_set_pfn_dirty(hfn);
+		mark_page_dirty(kvm, gfn);
+		ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
+				      vma_pagesize, false, true);
+	} else {
+		ret = stage2_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
+				      vma_pagesize, true, true);
+	}
+
+	if (ret)
+		kvm_err("Failed to map in stage2\n");
+
+	spin_unlock(&kvm->mmu_lock);
+	kvm_set_pfn_accessed(hfn);
+	kvm_release_pfn_clean(hfn);
+	return ret;
 }
 
 void kvm_riscv_stage2_flush_cache(struct kvm_vcpu *vcpu)
 {
-	/* TODO: */
+	stage2_cache_flush(&vcpu->arch.mmu_page_cache);
 }
 
 int kvm_riscv_stage2_alloc_pgd(struct kvm *kvm)
 {
-	/* TODO: */
+	struct page *pgd_page;
+
+	if (kvm->arch.pgd != NULL) {
+		kvm_err("kvm_arch already initialized?\n");
+		return -EINVAL;
+	}
+
+	pgd_page = alloc_pages(GFP_KERNEL | __GFP_ZERO,
+				get_order(stage2_pgd_size));
+	if (!pgd_page)
+		return -ENOMEM;
+	kvm->arch.pgd = page_to_virt(pgd_page);
+	kvm->arch.pgd_phys = page_to_phys(pgd_page);
+
 	return 0;
 }
 
 void kvm_riscv_stage2_free_pgd(struct kvm *kvm)
 {
-	/* TODO: */
+	void *pgd = NULL;
+
+	spin_lock(&kvm->mmu_lock);
+	if (kvm->arch.pgd) {
+		stage2_unmap_range(kvm, 0UL, stage2_gpa_size);
+		pgd = READ_ONCE(kvm->arch.pgd);
+		kvm->arch.pgd = NULL;
+		kvm->arch.pgd_phys = 0;
+	}
+	spin_unlock(&kvm->mmu_lock);
+
+	if (pgd)
+		free_pages((unsigned long)pgd, get_order(stage2_pgd_size));
 }
 
 void kvm_riscv_stage2_update_hgatp(struct kvm_vcpu *vcpu)
 {
-	/* TODO: */
+	unsigned long hgatp = stage2_mode;
+	struct kvm_arch *k = &vcpu->kvm->arch;
+
+	hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
+		 HGATP_VMID_MASK;
+	hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
+
+	csr_write(CSR_HGATP, hgatp);
+
+	if (!kvm_riscv_stage2_vmid_bits())
+		__kvm_riscv_hfence_gvma_all();
+}
+
+void kvm_riscv_stage2_mode_detect(void)
+{
+#ifdef CONFIG_64BIT
+	/* Try Sv48x4 stage2 mode */
+	csr_write(CSR_HGATP, HGATP_MODE_SV48X4 << HGATP_MODE_SHIFT);
+	if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
+		stage2_mode = (HGATP_MODE_SV48X4 << HGATP_MODE_SHIFT);
+		stage2_pgd_levels = 4;
+	}
+	csr_write(CSR_HGATP, 0);
+
+	__kvm_riscv_hfence_gvma_all();
+#endif
+}
+
+unsigned long kvm_riscv_stage2_mode(void)
+{
+	return stage2_mode >> HGATP_MODE_SHIFT;
 }
diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
index 282d67617229..6cde69a82252 100644
--- a/arch/riscv/kvm/vm.c
+++ b/arch/riscv/kvm/vm.c
@@ -12,12 +12,6 @@ 
 #include <linux/uaccess.h>
 #include <linux/kvm_host.h>
 
-int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
-{
-	/* TODO: To be added later. */
-	return -EOPNOTSUPP;
-}
-
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	int r;