[PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Yanan Wang]

Hello,
This series makes some efficiency improvement of guest stage-2 page
table code, and there are some test results to quantify the benefit.

Description for this series:
We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

The following are test results originally from v3 [1] to represent how
much benefit was introduced by movement of CMOs. We can use KVM selftest
to simulate a scenario of concurrent guest memory access and test the
execution time that KVM uses to create new stage-2 mappings, update the
existing mappings, split/rebuild huge mappings during/after dirty logging.

hardware platform: HiSilicon Kunpeng920 Server
host kernel: Linux mainline v5.12-rc2
test tools: KVM selftest [2]
[1] https://lore.kernel.org/lkml/20210326031654.3716-1-wangyanan55@huawei.com/
[2] https://lore.kernel.org/lkml/20210302125751.19080-1-wangyanan55@huawei.com/

cmdline: ./kvm_page_table_test -m 4 -s anonymous -b 1G -v 80
           (80 vcpus, 1G memory, page mappings(normal 4K))
KVM_CREATE_MAPPINGS: before 104.35s -> after  90.42s  +13.35%
KVM_UPDATE_MAPPINGS: before  78.64s -> after  75.45s  + 4.06%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_thp -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(THP 2M))
KVM_CREATE_MAPPINGS: before  15.66s -> after   6.92s  +55.80%
KVM_UPDATE_MAPPINGS: before 178.80s -> after 123.35s  +31.00%
KVM_REBUILD_BLOCKS:  before 187.34s -> after 131.76s  +30.65%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_hugetlb_1gb -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(HUGETLB 1G))
KVM_CREATE_MAPPINGS: before 104.54s -> after   3.70s  +96.46%
KVM_UPDATE_MAPPINGS: before 174.20s -> after 115.94s  +33.44%
KVM_REBUILD_BLOCKS:  before 103.95s -> after   2.96s  +97.15%

---

Changelogs:
v6->v7:
- refine the new callback names and the related comments (Marc)
- refine the patch subject and commit messages
- v6: https://lore.kernel.org/lkml/20210616095200.38008-1-wangyanan55@huawei.com/

v5->v6:
- convert the guest CMO functions into callbacks in kvm_pgtable_mm_ops (Marc)
- drop patch #6 in v5 since we are stuffing topup into mmu_lock section (Quentin)
- rebased on latest kvmarm/tree
- v5: https://lore.kernel.org/lkml/20210415115032.35760-1-wangyanan55@huawei.com/

v4->v5:
- rebased on the latest kvmarm/tree to adapt to the new stage-2 page-table code
- v4: https://lore.kernel.org/lkml/20210409033652.28316-1-wangyanan55@huawei.com

---

Yanan Wang (4):
  KVM: arm64: Introduce two cache maintenance callbacks
  KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
  KVM: arm64: Tweak parameters of guest cache maintenance functions
  KVM: arm64: Move guest CMOs to the fault handlers

 arch/arm64/include/asm/kvm_mmu.h     |  9 +----
 arch/arm64/include/asm/kvm_pgtable.h | 42 ++++++++++++---------
 arch/arm64/kvm/hyp/pgtable.c         | 48 ++++++++++++++++++------
 arch/arm64/kvm/mmu.c                 | 55 +++++++++++++---------------
 4 files changed, 89 insertions(+), 65 deletions(-)

-- 
2.23.0

[PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Yanan Wang]

Hello,
This series makes some efficiency improvement of guest stage-2 page
table code, and there are some test results to quantify the benefit.

Description for this series:
We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

The following are test results originally from v3 [1] to represent how
much benefit was introduced by movement of CMOs. We can use KVM selftest
to simulate a scenario of concurrent guest memory access and test the
execution time that KVM uses to create new stage-2 mappings, update the
existing mappings, split/rebuild huge mappings during/after dirty logging.

hardware platform: HiSilicon Kunpeng920 Server
host kernel: Linux mainline v5.12-rc2
test tools: KVM selftest [2]
[1] https://lore.kernel.org/lkml/20210326031654.3716-1-wangyanan55@huawei.com/
[2] https://lore.kernel.org/lkml/20210302125751.19080-1-wangyanan55@huawei.com/

cmdline: ./kvm_page_table_test -m 4 -s anonymous -b 1G -v 80
           (80 vcpus, 1G memory, page mappings(normal 4K))
KVM_CREATE_MAPPINGS: before 104.35s -> after  90.42s  +13.35%
KVM_UPDATE_MAPPINGS: before  78.64s -> after  75.45s  + 4.06%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_thp -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(THP 2M))
KVM_CREATE_MAPPINGS: before  15.66s -> after   6.92s  +55.80%
KVM_UPDATE_MAPPINGS: before 178.80s -> after 123.35s  +31.00%
KVM_REBUILD_BLOCKS:  before 187.34s -> after 131.76s  +30.65%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_hugetlb_1gb -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(HUGETLB 1G))
KVM_CREATE_MAPPINGS: before 104.54s -> after   3.70s  +96.46%
KVM_UPDATE_MAPPINGS: before 174.20s -> after 115.94s  +33.44%
KVM_REBUILD_BLOCKS:  before 103.95s -> after   2.96s  +97.15%

---

Changelogs:
v6->v7:
- refine the new callback names and the related comments (Marc)
- refine the patch subject and commit messages
- v6: https://lore.kernel.org/lkml/20210616095200.38008-1-wangyanan55@huawei.com/

v5->v6:
- convert the guest CMO functions into callbacks in kvm_pgtable_mm_ops (Marc)
- drop patch #6 in v5 since we are stuffing topup into mmu_lock section (Quentin)
- rebased on latest kvmarm/tree
- v5: https://lore.kernel.org/lkml/20210415115032.35760-1-wangyanan55@huawei.com/

v4->v5:
- rebased on the latest kvmarm/tree to adapt to the new stage-2 page-table code
- v4: https://lore.kernel.org/lkml/20210409033652.28316-1-wangyanan55@huawei.com

---

Yanan Wang (4):
  KVM: arm64: Introduce two cache maintenance callbacks
  KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
  KVM: arm64: Tweak parameters of guest cache maintenance functions
  KVM: arm64: Move guest CMOs to the fault handlers

 arch/arm64/include/asm/kvm_mmu.h     |  9 +----
 arch/arm64/include/asm/kvm_pgtable.h | 42 ++++++++++++---------
 arch/arm64/kvm/hyp/pgtable.c         | 48 ++++++++++++++++++------
 arch/arm64/kvm/mmu.c                 | 55 +++++++++++++---------------
 4 files changed, 89 insertions(+), 65 deletions(-)

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Yanan Wang]

Hello,
This series makes some efficiency improvement of guest stage-2 page
table code, and there are some test results to quantify the benefit.

Description for this series:
We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

The following are test results originally from v3 [1] to represent how
much benefit was introduced by movement of CMOs. We can use KVM selftest
to simulate a scenario of concurrent guest memory access and test the
execution time that KVM uses to create new stage-2 mappings, update the
existing mappings, split/rebuild huge mappings during/after dirty logging.

hardware platform: HiSilicon Kunpeng920 Server
host kernel: Linux mainline v5.12-rc2
test tools: KVM selftest [2]
[1] https://lore.kernel.org/lkml/20210326031654.3716-1-wangyanan55@huawei.com/
[2] https://lore.kernel.org/lkml/20210302125751.19080-1-wangyanan55@huawei.com/

cmdline: ./kvm_page_table_test -m 4 -s anonymous -b 1G -v 80
           (80 vcpus, 1G memory, page mappings(normal 4K))
KVM_CREATE_MAPPINGS: before 104.35s -> after  90.42s  +13.35%
KVM_UPDATE_MAPPINGS: before  78.64s -> after  75.45s  + 4.06%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_thp -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(THP 2M))
KVM_CREATE_MAPPINGS: before  15.66s -> after   6.92s  +55.80%
KVM_UPDATE_MAPPINGS: before 178.80s -> after 123.35s  +31.00%
KVM_REBUILD_BLOCKS:  before 187.34s -> after 131.76s  +30.65%

cmdline: ./kvm_page_table_test -m 4 -s anonymous_hugetlb_1gb -b 20G -v 40
           (40 vcpus, 20G memory, block mappings(HUGETLB 1G))
KVM_CREATE_MAPPINGS: before 104.54s -> after   3.70s  +96.46%
KVM_UPDATE_MAPPINGS: before 174.20s -> after 115.94s  +33.44%
KVM_REBUILD_BLOCKS:  before 103.95s -> after   2.96s  +97.15%

---

Changelogs:
v6->v7:
- refine the new callback names and the related comments (Marc)
- refine the patch subject and commit messages
- v6: https://lore.kernel.org/lkml/20210616095200.38008-1-wangyanan55@huawei.com/

v5->v6:
- convert the guest CMO functions into callbacks in kvm_pgtable_mm_ops (Marc)
- drop patch #6 in v5 since we are stuffing topup into mmu_lock section (Quentin)
- rebased on latest kvmarm/tree
- v5: https://lore.kernel.org/lkml/20210415115032.35760-1-wangyanan55@huawei.com/

v4->v5:
- rebased on the latest kvmarm/tree to adapt to the new stage-2 page-table code
- v4: https://lore.kernel.org/lkml/20210409033652.28316-1-wangyanan55@huawei.com

---

Yanan Wang (4):
  KVM: arm64: Introduce two cache maintenance callbacks
  KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
  KVM: arm64: Tweak parameters of guest cache maintenance functions
  KVM: arm64: Move guest CMOs to the fault handlers

 arch/arm64/include/asm/kvm_mmu.h     |  9 +----
 arch/arm64/include/asm/kvm_pgtable.h | 42 ++++++++++++---------
 arch/arm64/kvm/hyp/pgtable.c         | 48 ++++++++++++++++++------
 arch/arm64/kvm/mmu.c                 | 55 +++++++++++++---------------
 4 files changed, 89 insertions(+), 65 deletions(-)

-- 
2.23.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Yanan Wang]

To prepare for performing CMOs for guest stage-2 in the fault handlers
in pgtable.c, here introduce two cache maintenance callbacks in struct
kvm_pgtable_mm_ops. We also adjust the comment alignment for the
existing part but make no real content change at all.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index c3674c47d48c..b6ce34aa44bb 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
 
 /**
  * struct kvm_pgtable_mm_ops - Memory management callbacks.
- * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
- *			can be used by the walker to pass a memcache. The
- *			initial refcount of the page is 1.
- * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
- *			@size parameter is in bytes, and is rounded-up to the
- *			next page boundary. The resulting allocation is
- *			physically contiguous.
- * @free_pages_exact:	Free an exact number of memory pages previously
- *			allocated by zalloc_pages_exact.
- * @get_page:		Increment the refcount on a page.
- * @put_page:		Decrement the refcount on a page. When the refcount
- *			reaches 0 the page is automatically freed.
- * @page_count:		Return the refcount of a page.
- * @phys_to_virt:	Convert a physical address into a virtual address mapped
- *			in the current context.
- * @virt_to_phys:	Convert a virtual address mapped in the current context
- *			into a physical address.
+ * @zalloc_page:		Allocate a single zeroed memory page.
+ *				The @arg parameter can be used by the walker
+ *				to pass a memcache. The initial refcount of
+ *				the page is 1.
+ * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
+ *				The @size parameter is in bytes, and is rounded
+ *				up to the next page boundary. The resulting
+ *				allocation is physically contiguous.
+ * @free_pages_exact:		Free an exact number of memory pages previously
+ *				allocated by zalloc_pages_exact.
+ * @get_page:			Increment the refcount on a page.
+ * @put_page:			Decrement the refcount on a page. When the
+ *				refcount reaches 0 the page is automatically
+ *				freed.
+ * @page_count:			Return the refcount of a page.
+ * @phys_to_virt:		Convert a physical address into a virtual address
+ *				mapped in the current context.
+ * @virt_to_phys:		Convert a virtual address mapped in the current
+ *				context into a physical address.
+ * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
+ *				specified memory address range.
+ * @invalidate_icache:		Invalidate the instruction cache for the
+ *				specified memory address range.
  */
 struct kvm_pgtable_mm_ops {
 	void*		(*zalloc_page)(void *arg);
@@ -54,6 +60,8 @@ struct kvm_pgtable_mm_ops {
 	int		(*page_count)(void *addr);
 	void*		(*phys_to_virt)(phys_addr_t phys);
 	phys_addr_t	(*virt_to_phys)(void *addr);
+	void		(*clean_invalidate_dcache)(void *addr, size_t size);
+	void		(*invalidate_icache)(void *addr, size_t size);
 };
 
 /**
-- 
2.23.0

[PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Yanan Wang]

To prepare for performing CMOs for guest stage-2 in the fault handlers
in pgtable.c, here introduce two cache maintenance callbacks in struct
kvm_pgtable_mm_ops. We also adjust the comment alignment for the
existing part but make no real content change at all.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index c3674c47d48c..b6ce34aa44bb 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
 
 /**
  * struct kvm_pgtable_mm_ops - Memory management callbacks.
- * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
- *			can be used by the walker to pass a memcache. The
- *			initial refcount of the page is 1.
- * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
- *			@size parameter is in bytes, and is rounded-up to the
- *			next page boundary. The resulting allocation is
- *			physically contiguous.
- * @free_pages_exact:	Free an exact number of memory pages previously
- *			allocated by zalloc_pages_exact.
- * @get_page:		Increment the refcount on a page.
- * @put_page:		Decrement the refcount on a page. When the refcount
- *			reaches 0 the page is automatically freed.
- * @page_count:		Return the refcount of a page.
- * @phys_to_virt:	Convert a physical address into a virtual address mapped
- *			in the current context.
- * @virt_to_phys:	Convert a virtual address mapped in the current context
- *			into a physical address.
+ * @zalloc_page:		Allocate a single zeroed memory page.
+ *				The @arg parameter can be used by the walker
+ *				to pass a memcache. The initial refcount of
+ *				the page is 1.
+ * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
+ *				The @size parameter is in bytes, and is rounded
+ *				up to the next page boundary. The resulting
+ *				allocation is physically contiguous.
+ * @free_pages_exact:		Free an exact number of memory pages previously
+ *				allocated by zalloc_pages_exact.
+ * @get_page:			Increment the refcount on a page.
+ * @put_page:			Decrement the refcount on a page. When the
+ *				refcount reaches 0 the page is automatically
+ *				freed.
+ * @page_count:			Return the refcount of a page.
+ * @phys_to_virt:		Convert a physical address into a virtual address
+ *				mapped in the current context.
+ * @virt_to_phys:		Convert a virtual address mapped in the current
+ *				context into a physical address.
+ * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
+ *				specified memory address range.
+ * @invalidate_icache:		Invalidate the instruction cache for the
+ *				specified memory address range.
  */
 struct kvm_pgtable_mm_ops {
 	void*		(*zalloc_page)(void *arg);
@@ -54,6 +60,8 @@ struct kvm_pgtable_mm_ops {
 	int		(*page_count)(void *addr);
 	void*		(*phys_to_virt)(phys_addr_t phys);
 	phys_addr_t	(*virt_to_phys)(void *addr);
+	void		(*clean_invalidate_dcache)(void *addr, size_t size);
+	void		(*invalidate_icache)(void *addr, size_t size);
 };
 
 /**
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Yanan Wang]

To prepare for performing CMOs for guest stage-2 in the fault handlers
in pgtable.c, here introduce two cache maintenance callbacks in struct
kvm_pgtable_mm_ops. We also adjust the comment alignment for the
existing part but make no real content change at all.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index c3674c47d48c..b6ce34aa44bb 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
 
 /**
  * struct kvm_pgtable_mm_ops - Memory management callbacks.
- * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
- *			can be used by the walker to pass a memcache. The
- *			initial refcount of the page is 1.
- * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
- *			@size parameter is in bytes, and is rounded-up to the
- *			next page boundary. The resulting allocation is
- *			physically contiguous.
- * @free_pages_exact:	Free an exact number of memory pages previously
- *			allocated by zalloc_pages_exact.
- * @get_page:		Increment the refcount on a page.
- * @put_page:		Decrement the refcount on a page. When the refcount
- *			reaches 0 the page is automatically freed.
- * @page_count:		Return the refcount of a page.
- * @phys_to_virt:	Convert a physical address into a virtual address mapped
- *			in the current context.
- * @virt_to_phys:	Convert a virtual address mapped in the current context
- *			into a physical address.
+ * @zalloc_page:		Allocate a single zeroed memory page.
+ *				The @arg parameter can be used by the walker
+ *				to pass a memcache. The initial refcount of
+ *				the page is 1.
+ * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
+ *				The @size parameter is in bytes, and is rounded
+ *				up to the next page boundary. The resulting
+ *				allocation is physically contiguous.
+ * @free_pages_exact:		Free an exact number of memory pages previously
+ *				allocated by zalloc_pages_exact.
+ * @get_page:			Increment the refcount on a page.
+ * @put_page:			Decrement the refcount on a page. When the
+ *				refcount reaches 0 the page is automatically
+ *				freed.
+ * @page_count:			Return the refcount of a page.
+ * @phys_to_virt:		Convert a physical address into a virtual address
+ *				mapped in the current context.
+ * @virt_to_phys:		Convert a virtual address mapped in the current
+ *				context into a physical address.
+ * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
+ *				specified memory address range.
+ * @invalidate_icache:		Invalidate the instruction cache for the
+ *				specified memory address range.
  */
 struct kvm_pgtable_mm_ops {
 	void*		(*zalloc_page)(void *arg);
@@ -54,6 +60,8 @@ struct kvm_pgtable_mm_ops {
 	int		(*page_count)(void *addr);
 	void*		(*phys_to_virt)(phys_addr_t phys);
 	phys_addr_t	(*virt_to_phys)(void *addr);
+	void		(*clean_invalidate_dcache)(void *addr, size_t size);
+	void		(*invalidate_icache)(void *addr, size_t size);
 };
 
 /**
-- 
2.23.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> To prepare for performing CMOs for guest stage-2 in the fault handlers
> in pgtable.c, here introduce two cache maintenance callbacks in struct
> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> existing part but make no real content change at all.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index c3674c47d48c..b6ce34aa44bb 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>  
>  /**
>   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> - *			can be used by the walker to pass a memcache. The
> - *			initial refcount of the page is 1.
> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> - *			@size parameter is in bytes, and is rounded-up to the
> - *			next page boundary. The resulting allocation is
> - *			physically contiguous.
> - * @free_pages_exact:	Free an exact number of memory pages previously
> - *			allocated by zalloc_pages_exact.
> - * @get_page:		Increment the refcount on a page.
> - * @put_page:		Decrement the refcount on a page. When the refcount
> - *			reaches 0 the page is automatically freed.
> - * @page_count:		Return the refcount of a page.
> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> - *			in the current context.
> - * @virt_to_phys:	Convert a virtual address mapped in the current context
> - *			into a physical address.
> + * @zalloc_page:		Allocate a single zeroed memory page.
> + *				The @arg parameter can be used by the walker
> + *				to pass a memcache. The initial refcount of
> + *				the page is 1.
> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> + *				The @size parameter is in bytes, and is rounded
> + *				up to the next page boundary. The resulting
> + *				allocation is physically contiguous.
> + * @free_pages_exact:		Free an exact number of memory pages previously
> + *				allocated by zalloc_pages_exact.
> + * @get_page:			Increment the refcount on a page.
> + * @put_page:			Decrement the refcount on a page. When the
> + *				refcount reaches 0 the page is automatically
> + *				freed.
> + * @page_count:			Return the refcount of a page.
> + * @phys_to_virt:		Convert a physical address into a virtual address
> + *				mapped in the current context.
> + * @virt_to_phys:		Convert a virtual address mapped in the current
> + *				context into a physical address.
> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> + *				specified memory address range.

This should probably be explicit about whether this to the PoU/PoC/PoP.

Will

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> To prepare for performing CMOs for guest stage-2 in the fault handlers
> in pgtable.c, here introduce two cache maintenance callbacks in struct
> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> existing part but make no real content change at all.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index c3674c47d48c..b6ce34aa44bb 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>  
>  /**
>   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> - *			can be used by the walker to pass a memcache. The
> - *			initial refcount of the page is 1.
> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> - *			@size parameter is in bytes, and is rounded-up to the
> - *			next page boundary. The resulting allocation is
> - *			physically contiguous.
> - * @free_pages_exact:	Free an exact number of memory pages previously
> - *			allocated by zalloc_pages_exact.
> - * @get_page:		Increment the refcount on a page.
> - * @put_page:		Decrement the refcount on a page. When the refcount
> - *			reaches 0 the page is automatically freed.
> - * @page_count:		Return the refcount of a page.
> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> - *			in the current context.
> - * @virt_to_phys:	Convert a virtual address mapped in the current context
> - *			into a physical address.
> + * @zalloc_page:		Allocate a single zeroed memory page.
> + *				The @arg parameter can be used by the walker
> + *				to pass a memcache. The initial refcount of
> + *				the page is 1.
> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> + *				The @size parameter is in bytes, and is rounded
> + *				up to the next page boundary. The resulting
> + *				allocation is physically contiguous.
> + * @free_pages_exact:		Free an exact number of memory pages previously
> + *				allocated by zalloc_pages_exact.
> + * @get_page:			Increment the refcount on a page.
> + * @put_page:			Decrement the refcount on a page. When the
> + *				refcount reaches 0 the page is automatically
> + *				freed.
> + * @page_count:			Return the refcount of a page.
> + * @phys_to_virt:		Convert a physical address into a virtual address
> + *				mapped in the current context.
> + * @virt_to_phys:		Convert a virtual address mapped in the current
> + *				context into a physical address.
> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> + *				specified memory address range.

This should probably be explicit about whether this to the PoU/PoC/PoP.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> To prepare for performing CMOs for guest stage-2 in the fault handlers
> in pgtable.c, here introduce two cache maintenance callbacks in struct
> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> existing part but make no real content change at all.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index c3674c47d48c..b6ce34aa44bb 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>  
>  /**
>   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> - *			can be used by the walker to pass a memcache. The
> - *			initial refcount of the page is 1.
> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> - *			@size parameter is in bytes, and is rounded-up to the
> - *			next page boundary. The resulting allocation is
> - *			physically contiguous.
> - * @free_pages_exact:	Free an exact number of memory pages previously
> - *			allocated by zalloc_pages_exact.
> - * @get_page:		Increment the refcount on a page.
> - * @put_page:		Decrement the refcount on a page. When the refcount
> - *			reaches 0 the page is automatically freed.
> - * @page_count:		Return the refcount of a page.
> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> - *			in the current context.
> - * @virt_to_phys:	Convert a virtual address mapped in the current context
> - *			into a physical address.
> + * @zalloc_page:		Allocate a single zeroed memory page.
> + *				The @arg parameter can be used by the walker
> + *				to pass a memcache. The initial refcount of
> + *				the page is 1.
> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> + *				The @size parameter is in bytes, and is rounded
> + *				up to the next page boundary. The resulting
> + *				allocation is physically contiguous.
> + * @free_pages_exact:		Free an exact number of memory pages previously
> + *				allocated by zalloc_pages_exact.
> + * @get_page:			Increment the refcount on a page.
> + * @put_page:			Decrement the refcount on a page. When the
> + *				refcount reaches 0 the page is automatically
> + *				freed.
> + * @page_count:			Return the refcount of a page.
> + * @phys_to_virt:		Convert a physical address into a virtual address
> + *				mapped in the current context.
> + * @virt_to_phys:		Convert a virtual address mapped in the current
> + *				context into a physical address.
> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> + *				specified memory address range.

This should probably be explicit about whether this to the PoU/PoC/PoP.

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On Thu, 17 Jun 2021 13:38:37 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> > To prepare for performing CMOs for guest stage-2 in the fault handlers
> > in pgtable.c, here introduce two cache maintenance callbacks in struct
> > kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> > existing part but make no real content change at all.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index c3674c47d48c..b6ce34aa44bb 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >  
> >  /**
> >   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> > - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> > - *			can be used by the walker to pass a memcache. The
> > - *			initial refcount of the page is 1.
> > - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> > - *			@size parameter is in bytes, and is rounded-up to the
> > - *			next page boundary. The resulting allocation is
> > - *			physically contiguous.
> > - * @free_pages_exact:	Free an exact number of memory pages previously
> > - *			allocated by zalloc_pages_exact.
> > - * @get_page:		Increment the refcount on a page.
> > - * @put_page:		Decrement the refcount on a page. When the refcount
> > - *			reaches 0 the page is automatically freed.
> > - * @page_count:		Return the refcount of a page.
> > - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> > - *			in the current context.
> > - * @virt_to_phys:	Convert a virtual address mapped in the current context
> > - *			into a physical address.
> > + * @zalloc_page:		Allocate a single zeroed memory page.
> > + *				The @arg parameter can be used by the walker
> > + *				to pass a memcache. The initial refcount of
> > + *				the page is 1.
> > + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> > + *				The @size parameter is in bytes, and is rounded
> > + *				up to the next page boundary. The resulting
> > + *				allocation is physically contiguous.
> > + * @free_pages_exact:		Free an exact number of memory pages previously
> > + *				allocated by zalloc_pages_exact.
> > + * @get_page:			Increment the refcount on a page.
> > + * @put_page:			Decrement the refcount on a page. When the
> > + *				refcount reaches 0 the page is automatically
> > + *				freed.
> > + * @page_count:			Return the refcount of a page.
> > + * @phys_to_virt:		Convert a physical address into a virtual address
> > + *				mapped in the current context.
> > + * @virt_to_phys:		Convert a virtual address mapped in the current
> > + *				context into a physical address.
> > + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> > + *				specified memory address range.
> 
> This should probably be explicit about whether this to the PoU/PoC/PoP.

Indeed. I can fix that locally if there is nothing else that requires
adjusting.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On Thu, 17 Jun 2021 13:38:37 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> > To prepare for performing CMOs for guest stage-2 in the fault handlers
> > in pgtable.c, here introduce two cache maintenance callbacks in struct
> > kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> > existing part but make no real content change at all.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index c3674c47d48c..b6ce34aa44bb 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >  
> >  /**
> >   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> > - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> > - *			can be used by the walker to pass a memcache. The
> > - *			initial refcount of the page is 1.
> > - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> > - *			@size parameter is in bytes, and is rounded-up to the
> > - *			next page boundary. The resulting allocation is
> > - *			physically contiguous.
> > - * @free_pages_exact:	Free an exact number of memory pages previously
> > - *			allocated by zalloc_pages_exact.
> > - * @get_page:		Increment the refcount on a page.
> > - * @put_page:		Decrement the refcount on a page. When the refcount
> > - *			reaches 0 the page is automatically freed.
> > - * @page_count:		Return the refcount of a page.
> > - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> > - *			in the current context.
> > - * @virt_to_phys:	Convert a virtual address mapped in the current context
> > - *			into a physical address.
> > + * @zalloc_page:		Allocate a single zeroed memory page.
> > + *				The @arg parameter can be used by the walker
> > + *				to pass a memcache. The initial refcount of
> > + *				the page is 1.
> > + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> > + *				The @size parameter is in bytes, and is rounded
> > + *				up to the next page boundary. The resulting
> > + *				allocation is physically contiguous.
> > + * @free_pages_exact:		Free an exact number of memory pages previously
> > + *				allocated by zalloc_pages_exact.
> > + * @get_page:			Increment the refcount on a page.
> > + * @put_page:			Decrement the refcount on a page. When the
> > + *				refcount reaches 0 the page is automatically
> > + *				freed.
> > + * @page_count:			Return the refcount of a page.
> > + * @phys_to_virt:		Convert a physical address into a virtual address
> > + *				mapped in the current context.
> > + * @virt_to_phys:		Convert a virtual address mapped in the current
> > + *				context into a physical address.
> > + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> > + *				specified memory address range.
> 
> This should probably be explicit about whether this to the PoU/PoC/PoP.

Indeed. I can fix that locally if there is nothing else that requires
adjusting.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On Thu, 17 Jun 2021 13:38:37 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> > To prepare for performing CMOs for guest stage-2 in the fault handlers
> > in pgtable.c, here introduce two cache maintenance callbacks in struct
> > kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> > existing part but make no real content change at all.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index c3674c47d48c..b6ce34aa44bb 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >  
> >  /**
> >   * struct kvm_pgtable_mm_ops - Memory management callbacks.
> > - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
> > - *			can be used by the walker to pass a memcache. The
> > - *			initial refcount of the page is 1.
> > - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
> > - *			@size parameter is in bytes, and is rounded-up to the
> > - *			next page boundary. The resulting allocation is
> > - *			physically contiguous.
> > - * @free_pages_exact:	Free an exact number of memory pages previously
> > - *			allocated by zalloc_pages_exact.
> > - * @get_page:		Increment the refcount on a page.
> > - * @put_page:		Decrement the refcount on a page. When the refcount
> > - *			reaches 0 the page is automatically freed.
> > - * @page_count:		Return the refcount of a page.
> > - * @phys_to_virt:	Convert a physical address into a virtual address mapped
> > - *			in the current context.
> > - * @virt_to_phys:	Convert a virtual address mapped in the current context
> > - *			into a physical address.
> > + * @zalloc_page:		Allocate a single zeroed memory page.
> > + *				The @arg parameter can be used by the walker
> > + *				to pass a memcache. The initial refcount of
> > + *				the page is 1.
> > + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
> > + *				The @size parameter is in bytes, and is rounded
> > + *				up to the next page boundary. The resulting
> > + *				allocation is physically contiguous.
> > + * @free_pages_exact:		Free an exact number of memory pages previously
> > + *				allocated by zalloc_pages_exact.
> > + * @get_page:			Increment the refcount on a page.
> > + * @put_page:			Decrement the refcount on a page. When the
> > + *				refcount reaches 0 the page is automatically
> > + *				freed.
> > + * @page_count:			Return the refcount of a page.
> > + * @phys_to_virt:		Convert a physical address into a virtual address
> > + *				mapped in the current context.
> > + * @virt_to_phys:		Convert a virtual address mapped in the current
> > + *				context into a physical address.
> > + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
> > + *				specified memory address range.
> 
> This should probably be explicit about whether this to the PoU/PoC/PoP.

Indeed. I can fix that locally if there is nothing else that requires
adjusting.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[wangyanan (Y)]

On 2021/6/17 22:20, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:38:37 +0100,
> Will Deacon <will@kernel.org> wrote:
>> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>>> existing part but make no real content change at all.
>>>
>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>>> ---
>>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>>>   1 file changed, 25 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>>> index c3674c47d48c..b6ce34aa44bb 100644
>>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>>>   
>>>   /**
>>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>>> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
>>> - *			can be used by the walker to pass a memcache. The
>>> - *			initial refcount of the page is 1.
>>> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
>>> - *			@size parameter is in bytes, and is rounded-up to the
>>> - *			next page boundary. The resulting allocation is
>>> - *			physically contiguous.
>>> - * @free_pages_exact:	Free an exact number of memory pages previously
>>> - *			allocated by zalloc_pages_exact.
>>> - * @get_page:		Increment the refcount on a page.
>>> - * @put_page:		Decrement the refcount on a page. When the refcount
>>> - *			reaches 0 the page is automatically freed.
>>> - * @page_count:		Return the refcount of a page.
>>> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
>>> - *			in the current context.
>>> - * @virt_to_phys:	Convert a virtual address mapped in the current context
>>> - *			into a physical address.
>>> + * @zalloc_page:		Allocate a single zeroed memory page.
>>> + *				The @arg parameter can be used by the walker
>>> + *				to pass a memcache. The initial refcount of
>>> + *				the page is 1.
>>> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
>>> + *				The @size parameter is in bytes, and is rounded
>>> + *				up to the next page boundary. The resulting
>>> + *				allocation is physically contiguous.
>>> + * @free_pages_exact:		Free an exact number of memory pages previously
>>> + *				allocated by zalloc_pages_exact.
>>> + * @get_page:			Increment the refcount on a page.
>>> + * @put_page:			Decrement the refcount on a page. When the
>>> + *				refcount reaches 0 the page is automatically
>>> + *				freed.
>>> + * @page_count:			Return the refcount of a page.
>>> + * @phys_to_virt:		Convert a physical address into a virtual address
>>> + *				mapped in the current context.
>>> + * @virt_to_phys:		Convert a virtual address mapped in the current
>>> + *				context into a physical address.
>>> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
>>> + *				specified memory address range.
>> This should probably be explicit about whether this to the PoU/PoC/PoP.
> Indeed. I can fix that locally if there is nothing else that requires
> adjusting.
Will be grateful !

Thanks,
Yanan
.
>
> 	M.
>

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[wangyanan (Y)]

On 2021/6/17 22:20, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:38:37 +0100,
> Will Deacon <will@kernel.org> wrote:
>> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>>> existing part but make no real content change at all.
>>>
>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>>> ---
>>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>>>   1 file changed, 25 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>>> index c3674c47d48c..b6ce34aa44bb 100644
>>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>>>   
>>>   /**
>>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>>> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
>>> - *			can be used by the walker to pass a memcache. The
>>> - *			initial refcount of the page is 1.
>>> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
>>> - *			@size parameter is in bytes, and is rounded-up to the
>>> - *			next page boundary. The resulting allocation is
>>> - *			physically contiguous.
>>> - * @free_pages_exact:	Free an exact number of memory pages previously
>>> - *			allocated by zalloc_pages_exact.
>>> - * @get_page:		Increment the refcount on a page.
>>> - * @put_page:		Decrement the refcount on a page. When the refcount
>>> - *			reaches 0 the page is automatically freed.
>>> - * @page_count:		Return the refcount of a page.
>>> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
>>> - *			in the current context.
>>> - * @virt_to_phys:	Convert a virtual address mapped in the current context
>>> - *			into a physical address.
>>> + * @zalloc_page:		Allocate a single zeroed memory page.
>>> + *				The @arg parameter can be used by the walker
>>> + *				to pass a memcache. The initial refcount of
>>> + *				the page is 1.
>>> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
>>> + *				The @size parameter is in bytes, and is rounded
>>> + *				up to the next page boundary. The resulting
>>> + *				allocation is physically contiguous.
>>> + * @free_pages_exact:		Free an exact number of memory pages previously
>>> + *				allocated by zalloc_pages_exact.
>>> + * @get_page:			Increment the refcount on a page.
>>> + * @put_page:			Decrement the refcount on a page. When the
>>> + *				refcount reaches 0 the page is automatically
>>> + *				freed.
>>> + * @page_count:			Return the refcount of a page.
>>> + * @phys_to_virt:		Convert a physical address into a virtual address
>>> + *				mapped in the current context.
>>> + * @virt_to_phys:		Convert a virtual address mapped in the current
>>> + *				context into a physical address.
>>> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
>>> + *				specified memory address range.
>> This should probably be explicit about whether this to the PoU/PoC/PoP.
> Indeed. I can fix that locally if there is nothing else that requires
> adjusting.
Will be grateful !

Thanks,
Yanan
.
>
> 	M.
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[wangyanan (Y)]

On 2021/6/17 22:20, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:38:37 +0100,
> Will Deacon <will@kernel.org> wrote:
>> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>>> existing part but make no real content change at all.
>>>
>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>>> ---
>>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>>>   1 file changed, 25 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>>> index c3674c47d48c..b6ce34aa44bb 100644
>>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>>>   
>>>   /**
>>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>>> - * @zalloc_page:	Allocate a single zeroed memory page. The @arg parameter
>>> - *			can be used by the walker to pass a memcache. The
>>> - *			initial refcount of the page is 1.
>>> - * @zalloc_pages_exact:	Allocate an exact number of zeroed memory pages. The
>>> - *			@size parameter is in bytes, and is rounded-up to the
>>> - *			next page boundary. The resulting allocation is
>>> - *			physically contiguous.
>>> - * @free_pages_exact:	Free an exact number of memory pages previously
>>> - *			allocated by zalloc_pages_exact.
>>> - * @get_page:		Increment the refcount on a page.
>>> - * @put_page:		Decrement the refcount on a page. When the refcount
>>> - *			reaches 0 the page is automatically freed.
>>> - * @page_count:		Return the refcount of a page.
>>> - * @phys_to_virt:	Convert a physical address into a virtual address mapped
>>> - *			in the current context.
>>> - * @virt_to_phys:	Convert a virtual address mapped in the current context
>>> - *			into a physical address.
>>> + * @zalloc_page:		Allocate a single zeroed memory page.
>>> + *				The @arg parameter can be used by the walker
>>> + *				to pass a memcache. The initial refcount of
>>> + *				the page is 1.
>>> + * @zalloc_pages_exact:		Allocate an exact number of zeroed memory pages.
>>> + *				The @size parameter is in bytes, and is rounded
>>> + *				up to the next page boundary. The resulting
>>> + *				allocation is physically contiguous.
>>> + * @free_pages_exact:		Free an exact number of memory pages previously
>>> + *				allocated by zalloc_pages_exact.
>>> + * @get_page:			Increment the refcount on a page.
>>> + * @put_page:			Decrement the refcount on a page. When the
>>> + *				refcount reaches 0 the page is automatically
>>> + *				freed.
>>> + * @page_count:			Return the refcount of a page.
>>> + * @phys_to_virt:		Convert a physical address into a virtual address
>>> + *				mapped in the current context.
>>> + * @virt_to_phys:		Convert a virtual address mapped in the current
>>> + *				context into a physical address.
>>> + * @clean_invalidate_dcache:	Clean and invalidate the data cache for the
>>> + *				specified memory address range.
>> This should probably be explicit about whether this to the PoU/PoC/PoP.
> Indeed. I can fix that locally if there is nothing else that requires
> adjusting.
Will be grateful !

Thanks,
Yanan
.
>
> 	M.
>

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Fuad Tabba]

Hi,

On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> wrote:
>
>
>
> On 2021/6/17 22:20, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:38:37 +0100,
> > Will Deacon <will@kernel.org> wrote:
> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> >>> existing part but make no real content change at all.
> >>>
> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >>> ---
> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> >>> index c3674c47d48c..b6ce34aa44bb 100644
> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >>>
> >>>   /**
> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
> >>> - *                 can be used by the walker to pass a memcache. The
> >>> - *                 initial refcount of the page is 1.
> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
> >>> - *                 @size parameter is in bytes, and is rounded-up to the
> >>> - *                 next page boundary. The resulting allocation is
> >>> - *                 physically contiguous.
> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
> >>> - *                 allocated by zalloc_pages_exact.
> >>> - * @get_page:              Increment the refcount on a page.
> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
> >>> - *                 reaches 0 the page is automatically freed.
> >>> - * @page_count:            Return the refcount of a page.
> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
> >>> - *                 in the current context.
> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
> >>> - *                 into a physical address.
> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
> >>> + *                         The @arg parameter can be used by the walker
> >>> + *                         to pass a memcache. The initial refcount of
> >>> + *                         the page is 1.
> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
> >>> + *                         The @size parameter is in bytes, and is rounded
> >>> + *                         up to the next page boundary. The resulting
> >>> + *                         allocation is physically contiguous.
> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
> >>> + *                         allocated by zalloc_pages_exact.
> >>> + * @get_page:                      Increment the refcount on a page.
> >>> + * @put_page:                      Decrement the refcount on a page. When the
> >>> + *                         refcount reaches 0 the page is automatically
> >>> + *                         freed.
> >>> + * @page_count:                    Return the refcount of a page.
> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
> >>> + *                         mapped in the current context.
> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
> >>> + *                         context into a physical address.
> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
> >>> + *                         specified memory address range.
> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
> > Indeed. I can fix that locally if there is nothing else that requires
> > adjusting.
> Will be grateful !

Sorry, I missed the v7 update. One comment here is that the naming
used in the patch series I mentioned shortens invalidate to inval (if
you want it to be less of a mouthful):
https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/

Otherwise:
Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks!
/fuad



>
> Thanks,
> Yanan
> .
> >
> >       M.
> >
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Fuad Tabba]

Hi,

On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> wrote:
>
>
>
> On 2021/6/17 22:20, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:38:37 +0100,
> > Will Deacon <will@kernel.org> wrote:
> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> >>> existing part but make no real content change at all.
> >>>
> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >>> ---
> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> >>> index c3674c47d48c..b6ce34aa44bb 100644
> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >>>
> >>>   /**
> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
> >>> - *                 can be used by the walker to pass a memcache. The
> >>> - *                 initial refcount of the page is 1.
> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
> >>> - *                 @size parameter is in bytes, and is rounded-up to the
> >>> - *                 next page boundary. The resulting allocation is
> >>> - *                 physically contiguous.
> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
> >>> - *                 allocated by zalloc_pages_exact.
> >>> - * @get_page:              Increment the refcount on a page.
> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
> >>> - *                 reaches 0 the page is automatically freed.
> >>> - * @page_count:            Return the refcount of a page.
> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
> >>> - *                 in the current context.
> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
> >>> - *                 into a physical address.
> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
> >>> + *                         The @arg parameter can be used by the walker
> >>> + *                         to pass a memcache. The initial refcount of
> >>> + *                         the page is 1.
> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
> >>> + *                         The @size parameter is in bytes, and is rounded
> >>> + *                         up to the next page boundary. The resulting
> >>> + *                         allocation is physically contiguous.
> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
> >>> + *                         allocated by zalloc_pages_exact.
> >>> + * @get_page:                      Increment the refcount on a page.
> >>> + * @put_page:                      Decrement the refcount on a page. When the
> >>> + *                         refcount reaches 0 the page is automatically
> >>> + *                         freed.
> >>> + * @page_count:                    Return the refcount of a page.
> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
> >>> + *                         mapped in the current context.
> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
> >>> + *                         context into a physical address.
> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
> >>> + *                         specified memory address range.
> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
> > Indeed. I can fix that locally if there is nothing else that requires
> > adjusting.
> Will be grateful !

Sorry, I missed the v7 update. One comment here is that the naming
used in the patch series I mentioned shortens invalidate to inval (if
you want it to be less of a mouthful):
https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/

Otherwise:
Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks!
/fuad



>
> Thanks,
> Yanan
> .
> >
> >       M.
> >
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Fuad Tabba]

Hi,

On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> wrote:
>
>
>
> On 2021/6/17 22:20, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:38:37 +0100,
> > Will Deacon <will@kernel.org> wrote:
> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
> >>> existing part but make no real content change at all.
> >>>
> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >>> ---
> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> >>> index c3674c47d48c..b6ce34aa44bb 100644
> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
> >>>
> >>>   /**
> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
> >>> - *                 can be used by the walker to pass a memcache. The
> >>> - *                 initial refcount of the page is 1.
> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
> >>> - *                 @size parameter is in bytes, and is rounded-up to the
> >>> - *                 next page boundary. The resulting allocation is
> >>> - *                 physically contiguous.
> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
> >>> - *                 allocated by zalloc_pages_exact.
> >>> - * @get_page:              Increment the refcount on a page.
> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
> >>> - *                 reaches 0 the page is automatically freed.
> >>> - * @page_count:            Return the refcount of a page.
> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
> >>> - *                 in the current context.
> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
> >>> - *                 into a physical address.
> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
> >>> + *                         The @arg parameter can be used by the walker
> >>> + *                         to pass a memcache. The initial refcount of
> >>> + *                         the page is 1.
> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
> >>> + *                         The @size parameter is in bytes, and is rounded
> >>> + *                         up to the next page boundary. The resulting
> >>> + *                         allocation is physically contiguous.
> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
> >>> + *                         allocated by zalloc_pages_exact.
> >>> + * @get_page:                      Increment the refcount on a page.
> >>> + * @put_page:                      Decrement the refcount on a page. When the
> >>> + *                         refcount reaches 0 the page is automatically
> >>> + *                         freed.
> >>> + * @page_count:                    Return the refcount of a page.
> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
> >>> + *                         mapped in the current context.
> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
> >>> + *                         context into a physical address.
> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
> >>> + *                         specified memory address range.
> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
> > Indeed. I can fix that locally if there is nothing else that requires
> > adjusting.
> Will be grateful !

Sorry, I missed the v7 update. One comment here is that the naming
used in the patch series I mentioned shortens invalidate to inval (if
you want it to be less of a mouthful):
https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/

Otherwise:
Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks!
/fuad



>
> Thanks,
> Yanan
> .
> >
> >       M.
> >
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On 2021-06-18 09:59, Fuad Tabba wrote:
> Hi,
> 
> On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> 
> wrote:
>> 
>> 
>> 
>> On 2021/6/17 22:20, Marc Zyngier wrote:
>> > On Thu, 17 Jun 2021 13:38:37 +0100,
>> > Will Deacon <will@kernel.org> wrote:
>> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>> >>> existing part but make no real content change at all.
>> >>>
>> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> >>> ---
>> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
>> >>>
>> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> index c3674c47d48c..b6ce34aa44bb 100644
>> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>> >>>
>> >>>   /**
>> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
>> >>> - *                 can be used by the walker to pass a memcache. The
>> >>> - *                 initial refcount of the page is 1.
>> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
>> >>> - *                 @size parameter is in bytes, and is rounded-up to the
>> >>> - *                 next page boundary. The resulting allocation is
>> >>> - *                 physically contiguous.
>> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
>> >>> - *                 allocated by zalloc_pages_exact.
>> >>> - * @get_page:              Increment the refcount on a page.
>> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
>> >>> - *                 reaches 0 the page is automatically freed.
>> >>> - * @page_count:            Return the refcount of a page.
>> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
>> >>> - *                 in the current context.
>> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
>> >>> - *                 into a physical address.
>> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
>> >>> + *                         The @arg parameter can be used by the walker
>> >>> + *                         to pass a memcache. The initial refcount of
>> >>> + *                         the page is 1.
>> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
>> >>> + *                         The @size parameter is in bytes, and is rounded
>> >>> + *                         up to the next page boundary. The resulting
>> >>> + *                         allocation is physically contiguous.
>> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
>> >>> + *                         allocated by zalloc_pages_exact.
>> >>> + * @get_page:                      Increment the refcount on a page.
>> >>> + * @put_page:                      Decrement the refcount on a page. When the
>> >>> + *                         refcount reaches 0 the page is automatically
>> >>> + *                         freed.
>> >>> + * @page_count:                    Return the refcount of a page.
>> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
>> >>> + *                         mapped in the current context.
>> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
>> >>> + *                         context into a physical address.
>> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
>> >>> + *                         specified memory address range.
>> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
>> > Indeed. I can fix that locally if there is nothing else that requires
>> > adjusting.
>> Will be grateful !
> 
> Sorry, I missed the v7 update. One comment here is that the naming
> used in the patch series I mentioned shortens invalidate to inval (if
> you want it to be less of a mouthful):
> https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/
> 

OK, I've now aligned these callbacks to Fuad's naming:

[...]

  * @dcache_clean_inval_poc:	Clean and invalidate the data cache to the 
PoC
  *				for the	specified memory address range.
  * @icache_inval_pou:		Invalidate the instruction cache to the PoU
  *				for the specified memory address range.
  */
struct kvm_pgtable_mm_ops {
	void*		(*zalloc_page)(void *arg);
	void*		(*zalloc_pages_exact)(size_t size);
	void		(*free_pages_exact)(void *addr, size_t size);
	void		(*get_page)(void *addr);
	void		(*put_page)(void *addr);
	int		(*page_count)(void *addr);
	void*		(*phys_to_virt)(phys_addr_t phys);
	phys_addr_t	(*virt_to_phys)(void *addr);
	void		(*dcache_clean_inval_poc)(void *addr, size_t size);
	void		(*icache_inval_pou)(void *addr, size_t size);
};

and repainted everything else.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On 2021-06-18 09:59, Fuad Tabba wrote:
> Hi,
> 
> On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> 
> wrote:
>> 
>> 
>> 
>> On 2021/6/17 22:20, Marc Zyngier wrote:
>> > On Thu, 17 Jun 2021 13:38:37 +0100,
>> > Will Deacon <will@kernel.org> wrote:
>> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>> >>> existing part but make no real content change at all.
>> >>>
>> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> >>> ---
>> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
>> >>>
>> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> index c3674c47d48c..b6ce34aa44bb 100644
>> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>> >>>
>> >>>   /**
>> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
>> >>> - *                 can be used by the walker to pass a memcache. The
>> >>> - *                 initial refcount of the page is 1.
>> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
>> >>> - *                 @size parameter is in bytes, and is rounded-up to the
>> >>> - *                 next page boundary. The resulting allocation is
>> >>> - *                 physically contiguous.
>> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
>> >>> - *                 allocated by zalloc_pages_exact.
>> >>> - * @get_page:              Increment the refcount on a page.
>> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
>> >>> - *                 reaches 0 the page is automatically freed.
>> >>> - * @page_count:            Return the refcount of a page.
>> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
>> >>> - *                 in the current context.
>> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
>> >>> - *                 into a physical address.
>> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
>> >>> + *                         The @arg parameter can be used by the walker
>> >>> + *                         to pass a memcache. The initial refcount of
>> >>> + *                         the page is 1.
>> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
>> >>> + *                         The @size parameter is in bytes, and is rounded
>> >>> + *                         up to the next page boundary. The resulting
>> >>> + *                         allocation is physically contiguous.
>> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
>> >>> + *                         allocated by zalloc_pages_exact.
>> >>> + * @get_page:                      Increment the refcount on a page.
>> >>> + * @put_page:                      Decrement the refcount on a page. When the
>> >>> + *                         refcount reaches 0 the page is automatically
>> >>> + *                         freed.
>> >>> + * @page_count:                    Return the refcount of a page.
>> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
>> >>> + *                         mapped in the current context.
>> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
>> >>> + *                         context into a physical address.
>> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
>> >>> + *                         specified memory address range.
>> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
>> > Indeed. I can fix that locally if there is nothing else that requires
>> > adjusting.
>> Will be grateful !
> 
> Sorry, I missed the v7 update. One comment here is that the naming
> used in the patch series I mentioned shortens invalidate to inval (if
> you want it to be less of a mouthful):
> https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/
> 

OK, I've now aligned these callbacks to Fuad's naming:

[...]

  * @dcache_clean_inval_poc:	Clean and invalidate the data cache to the 
PoC
  *				for the	specified memory address range.
  * @icache_inval_pou:		Invalidate the instruction cache to the PoU
  *				for the specified memory address range.
  */
struct kvm_pgtable_mm_ops {
	void*		(*zalloc_page)(void *arg);
	void*		(*zalloc_pages_exact)(size_t size);
	void		(*free_pages_exact)(void *addr, size_t size);
	void		(*get_page)(void *addr);
	void		(*put_page)(void *addr);
	int		(*page_count)(void *addr);
	void*		(*phys_to_virt)(phys_addr_t phys);
	phys_addr_t	(*virt_to_phys)(void *addr);
	void		(*dcache_clean_inval_poc)(void *addr, size_t size);
	void		(*icache_inval_pou)(void *addr, size_t size);
};

and repainted everything else.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 1/4] KVM: arm64: Introduce two cache maintenance callbacks

[Marc Zyngier]

On 2021-06-18 09:59, Fuad Tabba wrote:
> Hi,
> 
> On Fri, Jun 18, 2021 at 2:52 AM wangyanan (Y) <wangyanan55@huawei.com> 
> wrote:
>> 
>> 
>> 
>> On 2021/6/17 22:20, Marc Zyngier wrote:
>> > On Thu, 17 Jun 2021 13:38:37 +0100,
>> > Will Deacon <will@kernel.org> wrote:
>> >> On Thu, Jun 17, 2021 at 06:58:21PM +0800, Yanan Wang wrote:
>> >>> To prepare for performing CMOs for guest stage-2 in the fault handlers
>> >>> in pgtable.c, here introduce two cache maintenance callbacks in struct
>> >>> kvm_pgtable_mm_ops. We also adjust the comment alignment for the
>> >>> existing part but make no real content change at all.
>> >>>
>> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> >>> ---
>> >>>   arch/arm64/include/asm/kvm_pgtable.h | 42 +++++++++++++++++-----------
>> >>>   1 file changed, 25 insertions(+), 17 deletions(-)
>> >>>
>> >>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> index c3674c47d48c..b6ce34aa44bb 100644
>> >>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>> >>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>> >>> @@ -27,23 +27,29 @@ typedef u64 kvm_pte_t;
>> >>>
>> >>>   /**
>> >>>    * struct kvm_pgtable_mm_ops - Memory management callbacks.
>> >>> - * @zalloc_page:   Allocate a single zeroed memory page. The @arg parameter
>> >>> - *                 can be used by the walker to pass a memcache. The
>> >>> - *                 initial refcount of the page is 1.
>> >>> - * @zalloc_pages_exact:    Allocate an exact number of zeroed memory pages. The
>> >>> - *                 @size parameter is in bytes, and is rounded-up to the
>> >>> - *                 next page boundary. The resulting allocation is
>> >>> - *                 physically contiguous.
>> >>> - * @free_pages_exact:      Free an exact number of memory pages previously
>> >>> - *                 allocated by zalloc_pages_exact.
>> >>> - * @get_page:              Increment the refcount on a page.
>> >>> - * @put_page:              Decrement the refcount on a page. When the refcount
>> >>> - *                 reaches 0 the page is automatically freed.
>> >>> - * @page_count:            Return the refcount of a page.
>> >>> - * @phys_to_virt:  Convert a physical address into a virtual address mapped
>> >>> - *                 in the current context.
>> >>> - * @virt_to_phys:  Convert a virtual address mapped in the current context
>> >>> - *                 into a physical address.
>> >>> + * @zalloc_page:           Allocate a single zeroed memory page.
>> >>> + *                         The @arg parameter can be used by the walker
>> >>> + *                         to pass a memcache. The initial refcount of
>> >>> + *                         the page is 1.
>> >>> + * @zalloc_pages_exact:            Allocate an exact number of zeroed memory pages.
>> >>> + *                         The @size parameter is in bytes, and is rounded
>> >>> + *                         up to the next page boundary. The resulting
>> >>> + *                         allocation is physically contiguous.
>> >>> + * @free_pages_exact:              Free an exact number of memory pages previously
>> >>> + *                         allocated by zalloc_pages_exact.
>> >>> + * @get_page:                      Increment the refcount on a page.
>> >>> + * @put_page:                      Decrement the refcount on a page. When the
>> >>> + *                         refcount reaches 0 the page is automatically
>> >>> + *                         freed.
>> >>> + * @page_count:                    Return the refcount of a page.
>> >>> + * @phys_to_virt:          Convert a physical address into a virtual address
>> >>> + *                         mapped in the current context.
>> >>> + * @virt_to_phys:          Convert a virtual address mapped in the current
>> >>> + *                         context into a physical address.
>> >>> + * @clean_invalidate_dcache:       Clean and invalidate the data cache for the
>> >>> + *                         specified memory address range.
>> >> This should probably be explicit about whether this to the PoU/PoC/PoP.
>> > Indeed. I can fix that locally if there is nothing else that requires
>> > adjusting.
>> Will be grateful !
> 
> Sorry, I missed the v7 update. One comment here is that the naming
> used in the patch series I mentioned shortens invalidate to inval (if
> you want it to be less of a mouthful):
> https://lore.kernel.org/linux-arm-kernel/20210524083001.2586635-19-tabba@google.com/
> 

OK, I've now aligned these callbacks to Fuad's naming:

[...]

  * @dcache_clean_inval_poc:	Clean and invalidate the data cache to the 
PoC
  *				for the	specified memory address range.
  * @icache_inval_pou:		Invalidate the instruction cache to the PoU
  *				for the specified memory address range.
  */
struct kvm_pgtable_mm_ops {
	void*		(*zalloc_page)(void *arg);
	void*		(*zalloc_pages_exact)(size_t size);
	void		(*free_pages_exact)(void *addr, size_t size);
	void		(*get_page)(void *addr);
	void		(*put_page)(void *addr);
	int		(*page_count)(void *addr);
	void*		(*phys_to_virt)(phys_addr_t phys);
	phys_addr_t	(*virt_to_phys)(void *addr);
	void		(*dcache_clean_inval_poc)(void *addr, size_t size);
	void		(*icache_inval_pou)(void *addr, size_t size);
};

and repainted everything else.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Yanan Wang]

Also add a mm_ops member for structure stage2_attr_data, since we
will move I-cache maintenance for guest stage-2 to the permission
path and as a result will need mm_ops for some callbacks.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index c37c1dc4feaf..d99789432b05 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
 }
 
 struct stage2_attr_data {
-	kvm_pte_t	attr_set;
-	kvm_pte_t	attr_clr;
-	kvm_pte_t	pte;
-	u32		level;
+	kvm_pte_t			attr_set;
+	kvm_pte_t			attr_clr;
+	kvm_pte_t			pte;
+	u32				level;
+	struct kvm_pgtable_mm_ops	*mm_ops;
 };
 
 static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
@@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 	struct stage2_attr_data data = {
 		.attr_set	= attr_set & attr_mask,
 		.attr_clr	= attr_clr & attr_mask,
+		.mm_ops		= pgt->mm_ops,
 	};
 	struct kvm_pgtable_walker walker = {
 		.cb		= stage2_attr_walker,
-- 
2.23.0

[PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Yanan Wang]

Also add a mm_ops member for structure stage2_attr_data, since we
will move I-cache maintenance for guest stage-2 to the permission
path and as a result will need mm_ops for some callbacks.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index c37c1dc4feaf..d99789432b05 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
 }
 
 struct stage2_attr_data {
-	kvm_pte_t	attr_set;
-	kvm_pte_t	attr_clr;
-	kvm_pte_t	pte;
-	u32		level;
+	kvm_pte_t			attr_set;
+	kvm_pte_t			attr_clr;
+	kvm_pte_t			pte;
+	u32				level;
+	struct kvm_pgtable_mm_ops	*mm_ops;
 };
 
 static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
@@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 	struct stage2_attr_data data = {
 		.attr_set	= attr_set & attr_mask,
 		.attr_clr	= attr_clr & attr_mask,
+		.mm_ops		= pgt->mm_ops,
 	};
 	struct kvm_pgtable_walker walker = {
 		.cb		= stage2_attr_walker,
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Yanan Wang]

Also add a mm_ops member for structure stage2_attr_data, since we
will move I-cache maintenance for guest stage-2 to the permission
path and as a result will need mm_ops for some callbacks.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index c37c1dc4feaf..d99789432b05 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
 }
 
 struct stage2_attr_data {
-	kvm_pte_t	attr_set;
-	kvm_pte_t	attr_clr;
-	kvm_pte_t	pte;
-	u32		level;
+	kvm_pte_t			attr_set;
+	kvm_pte_t			attr_clr;
+	kvm_pte_t			pte;
+	u32				level;
+	struct kvm_pgtable_mm_ops	*mm_ops;
 };
 
 static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
@@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 	struct stage2_attr_data data = {
 		.attr_set	= attr_set & attr_mask,
 		.attr_clr	= attr_clr & attr_mask,
+		.mm_ops		= pgt->mm_ops,
 	};
 	struct kvm_pgtable_walker walker = {
 		.cb		= stage2_attr_walker,
-- 
2.23.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Also add a mm_ops member for structure stage2_attr_data, since we
> will move I-cache maintenance for guest stage-2 to the permission
> path and as a result will need mm_ops for some callbacks.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index c37c1dc4feaf..d99789432b05 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
>  }
>
>  struct stage2_attr_data {
> -       kvm_pte_t       attr_set;
> -       kvm_pte_t       attr_clr;
> -       kvm_pte_t       pte;
> -       u32             level;
> +       kvm_pte_t                       attr_set;
> +       kvm_pte_t                       attr_clr;
> +       kvm_pte_t                       pte;
> +       u32                             level;
> +       struct kvm_pgtable_mm_ops       *mm_ops;
>  };
>
>  static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
> @@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
>         struct stage2_attr_data data = {
>                 .attr_set       = attr_set & attr_mask,
>                 .attr_clr       = attr_clr & attr_mask,
> +               .mm_ops         = pgt->mm_ops,
>         };
>         struct kvm_pgtable_walker walker = {
>                 .cb             = stage2_attr_walker,
> --
> 2.23.0

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Also add a mm_ops member for structure stage2_attr_data, since we
> will move I-cache maintenance for guest stage-2 to the permission
> path and as a result will need mm_ops for some callbacks.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index c37c1dc4feaf..d99789432b05 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
>  }
>
>  struct stage2_attr_data {
> -       kvm_pte_t       attr_set;
> -       kvm_pte_t       attr_clr;
> -       kvm_pte_t       pte;
> -       u32             level;
> +       kvm_pte_t                       attr_set;
> +       kvm_pte_t                       attr_clr;
> +       kvm_pte_t                       pte;
> +       u32                             level;
> +       struct kvm_pgtable_mm_ops       *mm_ops;
>  };
>
>  static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
> @@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
>         struct stage2_attr_data data = {
>                 .attr_set       = attr_set & attr_mask,
>                 .attr_clr       = attr_clr & attr_mask,
> +               .mm_ops         = pgt->mm_ops,
>         };
>         struct kvm_pgtable_walker walker = {
>                 .cb             = stage2_attr_walker,
> --
> 2.23.0

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Also add a mm_ops member for structure stage2_attr_data, since we
> will move I-cache maintenance for guest stage-2 to the permission
> path and as a result will need mm_ops for some callbacks.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index c37c1dc4feaf..d99789432b05 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -861,10 +861,11 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
>  }
>
>  struct stage2_attr_data {
> -       kvm_pte_t       attr_set;
> -       kvm_pte_t       attr_clr;
> -       kvm_pte_t       pte;
> -       u32             level;
> +       kvm_pte_t                       attr_set;
> +       kvm_pte_t                       attr_clr;
> +       kvm_pte_t                       pte;
> +       u32                             level;
> +       struct kvm_pgtable_mm_ops       *mm_ops;
>  };
>
>  static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
> @@ -903,6 +904,7 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
>         struct stage2_attr_data data = {
>                 .attr_set       = attr_set & attr_mask,
>                 .attr_clr       = attr_clr & attr_mask,
> +               .mm_ops         = pgt->mm_ops,
>         };
>         struct kvm_pgtable_walker walker = {
>                 .cb             = stage2_attr_walker,
> --
> 2.23.0

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Yanan Wang]

Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
and __invalidate_icache_guest_page to "void *va", which paves the
way for converting these two guest CMO functions into callbacks in
structure kvm_pgtable_mm_ops. No functional change.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
 arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 25ed956f9af1..6844a7550392 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 	return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
 }
 
-static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
+static inline void __clean_dcache_guest_page(void *va, size_t size)
 {
-	void *va = page_address(pfn_to_page(pfn));
-
 	/*
 	 * With FWB, we ensure that the guest always accesses memory using
 	 * cacheable attributes, and we don't have to clean to PoC when
@@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
 	kvm_flush_dcache_to_poc(va, size);
 }
 
-static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
-						  unsigned long size)
+static inline void __invalidate_icache_guest_page(void *va, size_t size)
 {
 	if (icache_is_aliasing()) {
 		/* any kind of VIPT cache */
 		__flush_icache_all();
 	} else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
 		/* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
-		void *va = page_address(pfn_to_page(pfn));
-
 		invalidate_icache_range((unsigned long)va,
 					(unsigned long)va + size);
 	}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 5742ba765ff9..b980f8a47cbb 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
 	return __va(phys);
 }
 
+static void clean_dcache_guest_page(void *va, size_t size)
+{
+	__clean_dcache_guest_page(va, size);
+}
+
+static void invalidate_icache_guest_page(void *va, size_t size)
+{
+	__invalidate_icache_guest_page(va, size);
+}
+
 /*
  * Unmapping vs dcache management:
  *
@@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 	kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
-static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__clean_dcache_guest_page(pfn, size);
-}
-
-static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__invalidate_icache_guest_page(pfn, size);
-}
-
 static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
 {
 	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
@@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		prot |= KVM_PGTABLE_PROT_W;
 
 	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(pfn, vma_pagesize);
+		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
+					vma_pagesize);
 
 	if (exec_fault) {
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(pfn, vma_pagesize);
+		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
+					     vma_pagesize);
 	}
 
 	if (device)
@@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	 * We've moved a page around, probably through CoW, so let's treat it
 	 * just like a translation fault and clean the cache to the PoC.
 	 */
-	clean_dcache_guest_page(pfn, PAGE_SIZE);
+	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
 
 	/*
 	 * The MMU notifiers will have unmapped a huge PMD before calling
-- 
2.23.0

[PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Yanan Wang]

Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
and __invalidate_icache_guest_page to "void *va", which paves the
way for converting these two guest CMO functions into callbacks in
structure kvm_pgtable_mm_ops. No functional change.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
 arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 25ed956f9af1..6844a7550392 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 	return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
 }
 
-static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
+static inline void __clean_dcache_guest_page(void *va, size_t size)
 {
-	void *va = page_address(pfn_to_page(pfn));
-
 	/*
 	 * With FWB, we ensure that the guest always accesses memory using
 	 * cacheable attributes, and we don't have to clean to PoC when
@@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
 	kvm_flush_dcache_to_poc(va, size);
 }
 
-static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
-						  unsigned long size)
+static inline void __invalidate_icache_guest_page(void *va, size_t size)
 {
 	if (icache_is_aliasing()) {
 		/* any kind of VIPT cache */
 		__flush_icache_all();
 	} else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
 		/* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
-		void *va = page_address(pfn_to_page(pfn));
-
 		invalidate_icache_range((unsigned long)va,
 					(unsigned long)va + size);
 	}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 5742ba765ff9..b980f8a47cbb 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
 	return __va(phys);
 }
 
+static void clean_dcache_guest_page(void *va, size_t size)
+{
+	__clean_dcache_guest_page(va, size);
+}
+
+static void invalidate_icache_guest_page(void *va, size_t size)
+{
+	__invalidate_icache_guest_page(va, size);
+}
+
 /*
  * Unmapping vs dcache management:
  *
@@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 	kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
-static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__clean_dcache_guest_page(pfn, size);
-}
-
-static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__invalidate_icache_guest_page(pfn, size);
-}
-
 static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
 {
 	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
@@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		prot |= KVM_PGTABLE_PROT_W;
 
 	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(pfn, vma_pagesize);
+		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
+					vma_pagesize);
 
 	if (exec_fault) {
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(pfn, vma_pagesize);
+		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
+					     vma_pagesize);
 	}
 
 	if (device)
@@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	 * We've moved a page around, probably through CoW, so let's treat it
 	 * just like a translation fault and clean the cache to the PoC.
 	 */
-	clean_dcache_guest_page(pfn, PAGE_SIZE);
+	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
 
 	/*
 	 * The MMU notifiers will have unmapped a huge PMD before calling
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Yanan Wang]

Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
and __invalidate_icache_guest_page to "void *va", which paves the
way for converting these two guest CMO functions into callbacks in
structure kvm_pgtable_mm_ops. No functional change.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
 arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 25ed956f9af1..6844a7550392 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 	return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
 }
 
-static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
+static inline void __clean_dcache_guest_page(void *va, size_t size)
 {
-	void *va = page_address(pfn_to_page(pfn));
-
 	/*
 	 * With FWB, we ensure that the guest always accesses memory using
 	 * cacheable attributes, and we don't have to clean to PoC when
@@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
 	kvm_flush_dcache_to_poc(va, size);
 }
 
-static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
-						  unsigned long size)
+static inline void __invalidate_icache_guest_page(void *va, size_t size)
 {
 	if (icache_is_aliasing()) {
 		/* any kind of VIPT cache */
 		__flush_icache_all();
 	} else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
 		/* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
-		void *va = page_address(pfn_to_page(pfn));
-
 		invalidate_icache_range((unsigned long)va,
 					(unsigned long)va + size);
 	}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 5742ba765ff9..b980f8a47cbb 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
 	return __va(phys);
 }
 
+static void clean_dcache_guest_page(void *va, size_t size)
+{
+	__clean_dcache_guest_page(va, size);
+}
+
+static void invalidate_icache_guest_page(void *va, size_t size)
+{
+	__invalidate_icache_guest_page(va, size);
+}
+
 /*
  * Unmapping vs dcache management:
  *
@@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 	kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
-static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__clean_dcache_guest_page(pfn, size);
-}
-
-static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
-{
-	__invalidate_icache_guest_page(pfn, size);
-}
-
 static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
 {
 	send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
@@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		prot |= KVM_PGTABLE_PROT_W;
 
 	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(pfn, vma_pagesize);
+		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
+					vma_pagesize);
 
 	if (exec_fault) {
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(pfn, vma_pagesize);
+		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
+					     vma_pagesize);
 	}
 
 	if (device)
@@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	 * We've moved a page around, probably through CoW, so let's treat it
 	 * just like a translation fault and clean the cache to the PoC.
 	 */
-	clean_dcache_guest_page(pfn, PAGE_SIZE);
+	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
 
 	/*
 	 * The MMU notifiers will have unmapped a huge PMD before calling
-- 
2.23.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
> and __invalidate_icache_guest_page to "void *va", which paves the
> way for converting these two guest CMO functions into callbacks in
> structure kvm_pgtable_mm_ops. No functional change.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>  arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>  2 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 25ed956f9af1..6844a7550392 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
>         return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
>  }
>
> -static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> +static inline void __clean_dcache_guest_page(void *va, size_t size)
>  {
> -       void *va = page_address(pfn_to_page(pfn));
> -
>         /*
>          * With FWB, we ensure that the guest always accesses memory using
>          * cacheable attributes, and we don't have to clean to PoC when
> @@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
>         kvm_flush_dcache_to_poc(va, size);
>  }
>
> -static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
> -                                                 unsigned long size)
> +static inline void __invalidate_icache_guest_page(void *va, size_t size)
>  {
>         if (icache_is_aliasing()) {
>                 /* any kind of VIPT cache */
>                 __flush_icache_all();
>         } else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
>                 /* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
> -               void *va = page_address(pfn_to_page(pfn));
> -
>                 invalidate_icache_range((unsigned long)va,
>                                         (unsigned long)va + size);
>         }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 5742ba765ff9..b980f8a47cbb 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
>         return __va(phys);
>  }
>
> +static void clean_dcache_guest_page(void *va, size_t size)
> +{
> +       __clean_dcache_guest_page(va, size);
> +}
> +
> +static void invalidate_icache_guest_page(void *va, size_t size)
> +{
> +       __invalidate_icache_guest_page(va, size);
> +}
> +
>  /*
>   * Unmapping vs dcache management:
>   *
> @@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
>         kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
>  }
>
> -static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __clean_dcache_guest_page(pfn, size);
> -}
> -
> -static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __invalidate_icache_guest_page(pfn, size);
> -}
> -
>  static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
>  {
>         send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
> @@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>                 prot |= KVM_PGTABLE_PROT_W;
>
>         if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(pfn, vma_pagesize);
> +               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> +                                       vma_pagesize);
>
>         if (exec_fault) {
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(pfn, vma_pagesize);
> +               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> +                                            vma_pagesize);
>         }
>
>         if (device)
> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>          * We've moved a page around, probably through CoW, so let's treat it
>          * just like a translation fault and clean the cache to the PoC.
>          */
> -       clean_dcache_guest_page(pfn, PAGE_SIZE);
> +       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
>
>         /*
>          * The MMU notifiers will have unmapped a huge PMD before calling
> --
> 2.23.0


Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
> and __invalidate_icache_guest_page to "void *va", which paves the
> way for converting these two guest CMO functions into callbacks in
> structure kvm_pgtable_mm_ops. No functional change.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>  arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>  2 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 25ed956f9af1..6844a7550392 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
>         return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
>  }
>
> -static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> +static inline void __clean_dcache_guest_page(void *va, size_t size)
>  {
> -       void *va = page_address(pfn_to_page(pfn));
> -
>         /*
>          * With FWB, we ensure that the guest always accesses memory using
>          * cacheable attributes, and we don't have to clean to PoC when
> @@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
>         kvm_flush_dcache_to_poc(va, size);
>  }
>
> -static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
> -                                                 unsigned long size)
> +static inline void __invalidate_icache_guest_page(void *va, size_t size)
>  {
>         if (icache_is_aliasing()) {
>                 /* any kind of VIPT cache */
>                 __flush_icache_all();
>         } else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
>                 /* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
> -               void *va = page_address(pfn_to_page(pfn));
> -
>                 invalidate_icache_range((unsigned long)va,
>                                         (unsigned long)va + size);
>         }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 5742ba765ff9..b980f8a47cbb 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
>         return __va(phys);
>  }
>
> +static void clean_dcache_guest_page(void *va, size_t size)
> +{
> +       __clean_dcache_guest_page(va, size);
> +}
> +
> +static void invalidate_icache_guest_page(void *va, size_t size)
> +{
> +       __invalidate_icache_guest_page(va, size);
> +}
> +
>  /*
>   * Unmapping vs dcache management:
>   *
> @@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
>         kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
>  }
>
> -static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __clean_dcache_guest_page(pfn, size);
> -}
> -
> -static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __invalidate_icache_guest_page(pfn, size);
> -}
> -
>  static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
>  {
>         send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
> @@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>                 prot |= KVM_PGTABLE_PROT_W;
>
>         if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(pfn, vma_pagesize);
> +               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> +                                       vma_pagesize);
>
>         if (exec_fault) {
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(pfn, vma_pagesize);
> +               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> +                                            vma_pagesize);
>         }
>
>         if (device)
> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>          * We've moved a page around, probably through CoW, so let's treat it
>          * just like a translation fault and clean the cache to the PoC.
>          */
> -       clean_dcache_guest_page(pfn, PAGE_SIZE);
> +       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
>
>         /*
>          * The MMU notifiers will have unmapped a huge PMD before calling
> --
> 2.23.0


Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
> and __invalidate_icache_guest_page to "void *va", which paves the
> way for converting these two guest CMO functions into callbacks in
> structure kvm_pgtable_mm_ops. No functional change.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>  arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>  2 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 25ed956f9af1..6844a7550392 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -187,10 +187,8 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
>         return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
>  }
>
> -static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> +static inline void __clean_dcache_guest_page(void *va, size_t size)
>  {
> -       void *va = page_address(pfn_to_page(pfn));
> -
>         /*
>          * With FWB, we ensure that the guest always accesses memory using
>          * cacheable attributes, and we don't have to clean to PoC when
> @@ -203,16 +201,13 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
>         kvm_flush_dcache_to_poc(va, size);
>  }
>
> -static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
> -                                                 unsigned long size)
> +static inline void __invalidate_icache_guest_page(void *va, size_t size)
>  {
>         if (icache_is_aliasing()) {
>                 /* any kind of VIPT cache */
>                 __flush_icache_all();
>         } else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
>                 /* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
> -               void *va = page_address(pfn_to_page(pfn));
> -
>                 invalidate_icache_range((unsigned long)va,
>                                         (unsigned long)va + size);
>         }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 5742ba765ff9..b980f8a47cbb 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -126,6 +126,16 @@ static void *kvm_host_va(phys_addr_t phys)
>         return __va(phys);
>  }
>
> +static void clean_dcache_guest_page(void *va, size_t size)
> +{
> +       __clean_dcache_guest_page(va, size);
> +}
> +
> +static void invalidate_icache_guest_page(void *va, size_t size)
> +{
> +       __invalidate_icache_guest_page(va, size);
> +}
> +
>  /*
>   * Unmapping vs dcache management:
>   *
> @@ -693,16 +703,6 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
>         kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
>  }
>
> -static void clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __clean_dcache_guest_page(pfn, size);
> -}
> -
> -static void invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned long size)
> -{
> -       __invalidate_icache_guest_page(pfn, size);
> -}
> -
>  static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
>  {
>         send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
> @@ -1013,11 +1013,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>                 prot |= KVM_PGTABLE_PROT_W;
>
>         if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(pfn, vma_pagesize);
> +               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> +                                       vma_pagesize);
>
>         if (exec_fault) {
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(pfn, vma_pagesize);
> +               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> +                                            vma_pagesize);
>         }
>
>         if (device)
> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>          * We've moved a page around, probably through CoW, so let's treat it
>          * just like a translation fault and clean the cache to the PoC.
>          */
> -       clean_dcache_guest_page(pfn, PAGE_SIZE);
> +       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
>
>         /*
>          * The MMU notifiers will have unmapped a huge PMD before calling
> --
> 2.23.0


Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Marc Zyngier]

On Thu, 17 Jun 2021 11:58:23 +0100,
Yanan Wang <wangyanan55@huawei.com> wrote:
> 
> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
> and __invalidate_icache_guest_page to "void *va", which paves the
> way for converting these two guest CMO functions into callbacks in
> structure kvm_pgtable_mm_ops. No functional change.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>  arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>  2 files changed, 17 insertions(+), 20 deletions(-)
>

[...]

> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>  	 * We've moved a page around, probably through CoW, so let's treat it
>  	 * just like a translation fault and clean the cache to the PoC.
>  	 */
> -	clean_dcache_guest_page(pfn, PAGE_SIZE);
> +	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);

This obviously doesn't compile. I have fixed it locally, but in the
future please make sure that patch series can be bisected correctly.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[Marc Zyngier]

On Thu, 17 Jun 2021 11:58:23 +0100,
Yanan Wang <wangyanan55@huawei.com> wrote:
> 
> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
> and __invalidate_icache_guest_page to "void *va", which paves the
> way for converting these two guest CMO functions into callbacks in
> structure kvm_pgtable_mm_ops. No functional change.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>  arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>  2 files changed, 17 insertions(+), 20 deletions(-)
>

[...]

> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>  	 * We've moved a page around, probably through CoW, so let's treat it
>  	 * just like a translation fault and clean the cache to the PoC.
>  	 */
> -	clean_dcache_guest_page(pfn, PAGE_SIZE);
> +	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);

This obviously doesn't compile. I have fixed it locally, but in the
future please make sure that patch series can be bisected correctly.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[wangyanan (Y)]

On 2021/6/18 19:30, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 11:58:23 +0100,
> Yanan Wang <wangyanan55@huawei.com> wrote:
>> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
>> and __invalidate_icache_guest_page to "void *va", which paves the
>> way for converting these two guest CMO functions into callbacks in
>> structure kvm_pgtable_mm_ops. No functional change.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>>   arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>>   2 files changed, 17 insertions(+), 20 deletions(-)
>>
> [...]
>
>> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>>   	 * We've moved a page around, probably through CoW, so let's treat it
>>   	 * just like a translation fault and clean the cache to the PoC.
>>   	 */
>> -	clean_dcache_guest_page(pfn, PAGE_SIZE);
>> +	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> This obviously doesn't compile. I have fixed it locally, but in the
> future please make sure that patch series can be bisected correctly.
Ah, yes, I figure out what I have missed by mistake now, and this should
have never happened... Much thanks for the local fixes for this series.
Also thank Fuad for the naming reference and review.

Regards,
Yanan
.
> Thanks,
>
> 	M.
>

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[wangyanan (Y)]

On 2021/6/18 19:30, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 11:58:23 +0100,
> Yanan Wang <wangyanan55@huawei.com> wrote:
>> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
>> and __invalidate_icache_guest_page to "void *va", which paves the
>> way for converting these two guest CMO functions into callbacks in
>> structure kvm_pgtable_mm_ops. No functional change.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>>   arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>>   2 files changed, 17 insertions(+), 20 deletions(-)
>>
> [...]
>
>> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>>   	 * We've moved a page around, probably through CoW, so let's treat it
>>   	 * just like a translation fault and clean the cache to the PoC.
>>   	 */
>> -	clean_dcache_guest_page(pfn, PAGE_SIZE);
>> +	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> This obviously doesn't compile. I have fixed it locally, but in the
> future please make sure that patch series can be bisected correctly.
Ah, yes, I figure out what I have missed by mistake now, and this should
have never happened... Much thanks for the local fixes for this series.
Also thank Fuad for the naming reference and review.

Regards,
Yanan
.
> Thanks,
>
> 	M.
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions

[wangyanan (Y)]

On 2021/6/18 19:30, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 11:58:23 +0100,
> Yanan Wang <wangyanan55@huawei.com> wrote:
>> Adjust the parameter "kvm_pfn_t pfn" of __clean_dcache_guest_page
>> and __invalidate_icache_guest_page to "void *va", which paves the
>> way for converting these two guest CMO functions into callbacks in
>> structure kvm_pgtable_mm_ops. No functional change.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   arch/arm64/include/asm/kvm_mmu.h |  9 ++-------
>>   arch/arm64/kvm/mmu.c             | 28 +++++++++++++++-------------
>>   2 files changed, 17 insertions(+), 20 deletions(-)
>>
> [...]
>
>> @@ -1219,7 +1221,7 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>>   	 * We've moved a page around, probably through CoW, so let's treat it
>>   	 * just like a translation fault and clean the cache to the PoC.
>>   	 */
>> -	clean_dcache_guest_page(pfn, PAGE_SIZE);
>> +	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> This obviously doesn't compile. I have fixed it locally, but in the
> future please make sure that patch series can be bisected correctly.
Ah, yes, I figure out what I have missed by mistake now, and this should
have never happened... Much thanks for the local fixes for this series.
Also thank Fuad for the naming reference and review.

Regards,
Yanan
.
> Thanks,
>
> 	M.
>

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Yanan Wang]

We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
 arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index d99789432b05..760c551f61da 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
 	mm_ops->put_page(ptep);
 }
 
+static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
+{
+	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
+	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
+}
+
+static bool stage2_pte_executable(kvm_pte_t pte)
+{
+	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
+}
+
 static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 				      kvm_pte_t *ptep,
 				      struct stage2_map_data *data)
 {
 	kvm_pte_t new, old = *ptep;
 	u64 granule = kvm_granule_size(level), phys = data->phys;
+	struct kvm_pgtable *pgt = data->mmu->pgt;
 	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_block_mapping_supported(addr, end, phys, level))
@@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
 	}
 
+	/* Perform CMOs before installation of the guest stage-2 PTE */
+	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
+		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
+						granule);
+
+	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
+		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
+
 	smp_store_release(ptep, new);
 	if (stage2_pte_is_counted(new))
 		mm_ops->get_page(ptep);
@@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
 	return ret;
 }
 
-static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
-{
-	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
-	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
-}
-
 static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 			       enum kvm_pgtable_walk_flags flag,
 			       void * const arg)
@@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 {
 	kvm_pte_t pte = *ptep;
 	struct stage2_attr_data *data = arg;
+	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_pte_valid(pte))
 		return 0;
@@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 	 * but worst-case the access flag update gets lost and will be
 	 * set on the next access instead.
 	 */
-	if (data->pte != pte)
+	if (data->pte != pte) {
+		/*
+		 * Invalidate instruction cache before updating the guest
+		 * stage-2 PTE if we are going to add executable permission.
+		 */
+		if (mm_ops->invalidate_icache &&
+		    stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
+			mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
+						  kvm_granule_size(level));
 		WRITE_ONCE(*ptep, pte);
+	}
 
 	return 0;
 }
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index b980f8a47cbb..c9f002d74ab4 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
 }
 
 static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
-	.zalloc_page		= stage2_memcache_zalloc_page,
-	.zalloc_pages_exact	= kvm_host_zalloc_pages_exact,
-	.free_pages_exact	= free_pages_exact,
-	.get_page		= kvm_host_get_page,
-	.put_page		= kvm_host_put_page,
-	.page_count		= kvm_host_page_count,
-	.phys_to_virt		= kvm_host_va,
-	.virt_to_phys		= kvm_host_pa,
+	.zalloc_page			= stage2_memcache_zalloc_page,
+	.zalloc_pages_exact		= kvm_host_zalloc_pages_exact,
+	.free_pages_exact		= free_pages_exact,
+	.get_page			= kvm_host_get_page,
+	.put_page			= kvm_host_put_page,
+	.page_count			= kvm_host_page_count,
+	.phys_to_virt			= kvm_host_va,
+	.virt_to_phys			= kvm_host_pa,
+	.clean_invalidate_dcache	= clean_dcache_guest_page,
+	.invalidate_icache		= invalidate_icache_guest_page,
 };
 
 /**
@@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (writable)
 		prot |= KVM_PGTABLE_PROT_W;
 
-	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
-					vma_pagesize);
-
-	if (exec_fault) {
+	if (exec_fault)
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
-					     vma_pagesize);
-	}
 
 	if (device)
 		prot |= KVM_PGTABLE_PROT_DEVICE;
@@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	WARN_ON(range->end - range->start != 1);
 
 	/*
-	 * We've moved a page around, probably through CoW, so let's treat it
-	 * just like a translation fault and clean the cache to the PoC.
-	 */
-	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
-
-	/*
+	 * We've moved a page around, probably through CoW, so let's treat
+	 * it just like a translation fault and the map handler will clean
+	 * the cache to the PoC.
+	 *
 	 * The MMU notifiers will have unmapped a huge PMD before calling
 	 * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
 	 * therefore we never need to clear out a huge PMD through this
-- 
2.23.0

[PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Yanan Wang]

We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
 arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index d99789432b05..760c551f61da 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
 	mm_ops->put_page(ptep);
 }
 
+static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
+{
+	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
+	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
+}
+
+static bool stage2_pte_executable(kvm_pte_t pte)
+{
+	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
+}
+
 static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 				      kvm_pte_t *ptep,
 				      struct stage2_map_data *data)
 {
 	kvm_pte_t new, old = *ptep;
 	u64 granule = kvm_granule_size(level), phys = data->phys;
+	struct kvm_pgtable *pgt = data->mmu->pgt;
 	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_block_mapping_supported(addr, end, phys, level))
@@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
 	}
 
+	/* Perform CMOs before installation of the guest stage-2 PTE */
+	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
+		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
+						granule);
+
+	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
+		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
+
 	smp_store_release(ptep, new);
 	if (stage2_pte_is_counted(new))
 		mm_ops->get_page(ptep);
@@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
 	return ret;
 }
 
-static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
-{
-	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
-	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
-}
-
 static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 			       enum kvm_pgtable_walk_flags flag,
 			       void * const arg)
@@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 {
 	kvm_pte_t pte = *ptep;
 	struct stage2_attr_data *data = arg;
+	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_pte_valid(pte))
 		return 0;
@@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 	 * but worst-case the access flag update gets lost and will be
 	 * set on the next access instead.
 	 */
-	if (data->pte != pte)
+	if (data->pte != pte) {
+		/*
+		 * Invalidate instruction cache before updating the guest
+		 * stage-2 PTE if we are going to add executable permission.
+		 */
+		if (mm_ops->invalidate_icache &&
+		    stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
+			mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
+						  kvm_granule_size(level));
 		WRITE_ONCE(*ptep, pte);
+	}
 
 	return 0;
 }
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index b980f8a47cbb..c9f002d74ab4 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
 }
 
 static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
-	.zalloc_page		= stage2_memcache_zalloc_page,
-	.zalloc_pages_exact	= kvm_host_zalloc_pages_exact,
-	.free_pages_exact	= free_pages_exact,
-	.get_page		= kvm_host_get_page,
-	.put_page		= kvm_host_put_page,
-	.page_count		= kvm_host_page_count,
-	.phys_to_virt		= kvm_host_va,
-	.virt_to_phys		= kvm_host_pa,
+	.zalloc_page			= stage2_memcache_zalloc_page,
+	.zalloc_pages_exact		= kvm_host_zalloc_pages_exact,
+	.free_pages_exact		= free_pages_exact,
+	.get_page			= kvm_host_get_page,
+	.put_page			= kvm_host_put_page,
+	.page_count			= kvm_host_page_count,
+	.phys_to_virt			= kvm_host_va,
+	.virt_to_phys			= kvm_host_pa,
+	.clean_invalidate_dcache	= clean_dcache_guest_page,
+	.invalidate_icache		= invalidate_icache_guest_page,
 };
 
 /**
@@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (writable)
 		prot |= KVM_PGTABLE_PROT_W;
 
-	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
-					vma_pagesize);
-
-	if (exec_fault) {
+	if (exec_fault)
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
-					     vma_pagesize);
-	}
 
 	if (device)
 		prot |= KVM_PGTABLE_PROT_DEVICE;
@@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	WARN_ON(range->end - range->start != 1);
 
 	/*
-	 * We've moved a page around, probably through CoW, so let's treat it
-	 * just like a translation fault and clean the cache to the PoC.
-	 */
-	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
-
-	/*
+	 * We've moved a page around, probably through CoW, so let's treat
+	 * it just like a translation fault and the map handler will clean
+	 * the cache to the PoC.
+	 *
 	 * The MMU notifiers will have unmapped a huge PMD before calling
 	 * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
 	 * therefore we never need to clear out a huge PMD through this
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Yanan Wang]

We currently uniformly permorm CMOs of D-cache and I-cache in function
user_mem_abort before calling the fault handlers. If we get concurrent
guest faults(e.g. translation faults, permission faults) or some really
unnecessary guest faults caused by BBM, CMOs for the first vcpu are
necessary while the others later are not.

By moving CMOs to the fault handlers, we can easily identify conditions
where they are really needed and avoid the unnecessary ones. As it's a
time consuming process to perform CMOs especially when flushing a block
range, so this solution reduces much load of kvm and improve efficiency
of the stage-2 page table code.

We can imagine two specific scenarios which will gain much benefit:
1) In a normal VM startup, this solution will improve the efficiency of
handling guest page faults incurred by vCPUs, when initially populating
stage-2 page tables.
2) After live migration, the heavy workload will be resumed on the
destination VM, however all the stage-2 page tables need to be rebuilt
at the moment. So this solution will ease the performance drop during
resuming stage.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
 arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index d99789432b05..760c551f61da 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
 	mm_ops->put_page(ptep);
 }
 
+static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
+{
+	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
+	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
+}
+
+static bool stage2_pte_executable(kvm_pte_t pte)
+{
+	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
+}
+
 static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 				      kvm_pte_t *ptep,
 				      struct stage2_map_data *data)
 {
 	kvm_pte_t new, old = *ptep;
 	u64 granule = kvm_granule_size(level), phys = data->phys;
+	struct kvm_pgtable *pgt = data->mmu->pgt;
 	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_block_mapping_supported(addr, end, phys, level))
@@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
 		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
 	}
 
+	/* Perform CMOs before installation of the guest stage-2 PTE */
+	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
+		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
+						granule);
+
+	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
+		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
+
 	smp_store_release(ptep, new);
 	if (stage2_pte_is_counted(new))
 		mm_ops->get_page(ptep);
@@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
 	return ret;
 }
 
-static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
-{
-	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
-	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
-}
-
 static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 			       enum kvm_pgtable_walk_flags flag,
 			       void * const arg)
@@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 {
 	kvm_pte_t pte = *ptep;
 	struct stage2_attr_data *data = arg;
+	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
 
 	if (!kvm_pte_valid(pte))
 		return 0;
@@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
 	 * but worst-case the access flag update gets lost and will be
 	 * set on the next access instead.
 	 */
-	if (data->pte != pte)
+	if (data->pte != pte) {
+		/*
+		 * Invalidate instruction cache before updating the guest
+		 * stage-2 PTE if we are going to add executable permission.
+		 */
+		if (mm_ops->invalidate_icache &&
+		    stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
+			mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
+						  kvm_granule_size(level));
 		WRITE_ONCE(*ptep, pte);
+	}
 
 	return 0;
 }
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index b980f8a47cbb..c9f002d74ab4 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
 }
 
 static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
-	.zalloc_page		= stage2_memcache_zalloc_page,
-	.zalloc_pages_exact	= kvm_host_zalloc_pages_exact,
-	.free_pages_exact	= free_pages_exact,
-	.get_page		= kvm_host_get_page,
-	.put_page		= kvm_host_put_page,
-	.page_count		= kvm_host_page_count,
-	.phys_to_virt		= kvm_host_va,
-	.virt_to_phys		= kvm_host_pa,
+	.zalloc_page			= stage2_memcache_zalloc_page,
+	.zalloc_pages_exact		= kvm_host_zalloc_pages_exact,
+	.free_pages_exact		= free_pages_exact,
+	.get_page			= kvm_host_get_page,
+	.put_page			= kvm_host_put_page,
+	.page_count			= kvm_host_page_count,
+	.phys_to_virt			= kvm_host_va,
+	.virt_to_phys			= kvm_host_pa,
+	.clean_invalidate_dcache	= clean_dcache_guest_page,
+	.invalidate_icache		= invalidate_icache_guest_page,
 };
 
 /**
@@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (writable)
 		prot |= KVM_PGTABLE_PROT_W;
 
-	if (fault_status != FSC_PERM && !device)
-		clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
-					vma_pagesize);
-
-	if (exec_fault) {
+	if (exec_fault)
 		prot |= KVM_PGTABLE_PROT_X;
-		invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
-					     vma_pagesize);
-	}
 
 	if (device)
 		prot |= KVM_PGTABLE_PROT_DEVICE;
@@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
 	WARN_ON(range->end - range->start != 1);
 
 	/*
-	 * We've moved a page around, probably through CoW, so let's treat it
-	 * just like a translation fault and clean the cache to the PoC.
-	 */
-	clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
-
-	/*
+	 * We've moved a page around, probably through CoW, so let's treat
+	 * it just like a translation fault and the map handler will clean
+	 * the cache to the PoC.
+	 *
 	 * The MMU notifiers will have unmapped a huge PMD before calling
 	 * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
 	 * therefore we never need to clear out a huge PMD through this
-- 
2.23.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
> 
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>  	mm_ops->put_page(ptep);
>  }
>  
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  				      kvm_pte_t *ptep,
>  				      struct stage2_map_data *data)
>  {
>  	kvm_pte_t new, old = *ptep;
>  	u64 granule = kvm_granule_size(level), phys = data->phys;
> +	struct kvm_pgtable *pgt = data->mmu->pgt;
>  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>  
>  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>  	}
>  
> +	/* Perform CMOs before installation of the guest stage-2 PTE */
> +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +						granule);
> +
> +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);

One thing I'm missing here is why we need the indirection via mm_ops. Are
there cases where we would want to pass a different function pointer for
invalidating the icache? If not, why not just call the function directly?

Same for the D side.

Will

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
> 
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>  	mm_ops->put_page(ptep);
>  }
>  
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  				      kvm_pte_t *ptep,
>  				      struct stage2_map_data *data)
>  {
>  	kvm_pte_t new, old = *ptep;
>  	u64 granule = kvm_granule_size(level), phys = data->phys;
> +	struct kvm_pgtable *pgt = data->mmu->pgt;
>  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>  
>  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>  	}
>  
> +	/* Perform CMOs before installation of the guest stage-2 PTE */
> +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +						granule);
> +
> +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);

One thing I'm missing here is why we need the indirection via mm_ops. Are
there cases where we would want to pass a different function pointer for
invalidating the icache? If not, why not just call the function directly?

Same for the D side.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
> 
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>  	mm_ops->put_page(ptep);
>  }
>  
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  				      kvm_pte_t *ptep,
>  				      struct stage2_map_data *data)
>  {
>  	kvm_pte_t new, old = *ptep;
>  	u64 granule = kvm_granule_size(level), phys = data->phys;
> +	struct kvm_pgtable *pgt = data->mmu->pgt;
>  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>  
>  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>  	}
>  
> +	/* Perform CMOs before installation of the guest stage-2 PTE */
> +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +						granule);
> +
> +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);

One thing I'm missing here is why we need the indirection via mm_ops. Are
there cases where we would want to pass a different function pointer for
invalidating the icache? If not, why not just call the function directly?

Same for the D side.

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 13:45:57 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > We currently uniformly permorm CMOs of D-cache and I-cache in function
> > user_mem_abort before calling the fault handlers. If we get concurrent
> > guest faults(e.g. translation faults, permission faults) or some really
> > unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> > necessary while the others later are not.
> > 
> > By moving CMOs to the fault handlers, we can easily identify conditions
> > where they are really needed and avoid the unnecessary ones. As it's a
> > time consuming process to perform CMOs especially when flushing a block
> > range, so this solution reduces much load of kvm and improve efficiency
> > of the stage-2 page table code.
> > 
> > We can imagine two specific scenarios which will gain much benefit:
> > 1) In a normal VM startup, this solution will improve the efficiency of
> > handling guest page faults incurred by vCPUs, when initially populating
> > stage-2 page tables.
> > 2) After live migration, the heavy workload will be resumed on the
> > destination VM, however all the stage-2 page tables need to be rebuilt
> > at the moment. So this solution will ease the performance drop during
> > resuming stage.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
> >  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
> >  2 files changed, 46 insertions(+), 29 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index d99789432b05..760c551f61da 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
> >  	mm_ops->put_page(ptep);
> >  }
> >  
> > +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> > +{
> > +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> > +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> > +}
> > +
> > +static bool stage2_pte_executable(kvm_pte_t pte)
> > +{
> > +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> > +}
> > +
> >  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  				      kvm_pte_t *ptep,
> >  				      struct stage2_map_data *data)
> >  {
> >  	kvm_pte_t new, old = *ptep;
> >  	u64 granule = kvm_granule_size(level), phys = data->phys;
> > +	struct kvm_pgtable *pgt = data->mmu->pgt;
> >  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
> >  
> >  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> >  	}
> >  
> > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > +						granule);
> > +
> > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> 
> One thing I'm missing here is why we need the indirection via mm_ops. Are
> there cases where we would want to pass a different function pointer for
> invalidating the icache? If not, why not just call the function directly?
> 
> Same for the D side.

If we didn't do that, we'd end-up having to track whether the guest
context requires CMOs with additional flags, which is pretty ugly (see
v5 of this series for reference [1]).

It also means that we would have to drag the CM functions into the EL2
object, something that we don't need with this approach.

	M.

[1] https://lore.kernel.org/r/20210415115032.35760-1-wangyanan55@huawei.com

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 13:45:57 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > We currently uniformly permorm CMOs of D-cache and I-cache in function
> > user_mem_abort before calling the fault handlers. If we get concurrent
> > guest faults(e.g. translation faults, permission faults) or some really
> > unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> > necessary while the others later are not.
> > 
> > By moving CMOs to the fault handlers, we can easily identify conditions
> > where they are really needed and avoid the unnecessary ones. As it's a
> > time consuming process to perform CMOs especially when flushing a block
> > range, so this solution reduces much load of kvm and improve efficiency
> > of the stage-2 page table code.
> > 
> > We can imagine two specific scenarios which will gain much benefit:
> > 1) In a normal VM startup, this solution will improve the efficiency of
> > handling guest page faults incurred by vCPUs, when initially populating
> > stage-2 page tables.
> > 2) After live migration, the heavy workload will be resumed on the
> > destination VM, however all the stage-2 page tables need to be rebuilt
> > at the moment. So this solution will ease the performance drop during
> > resuming stage.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
> >  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
> >  2 files changed, 46 insertions(+), 29 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index d99789432b05..760c551f61da 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
> >  	mm_ops->put_page(ptep);
> >  }
> >  
> > +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> > +{
> > +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> > +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> > +}
> > +
> > +static bool stage2_pte_executable(kvm_pte_t pte)
> > +{
> > +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> > +}
> > +
> >  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  				      kvm_pte_t *ptep,
> >  				      struct stage2_map_data *data)
> >  {
> >  	kvm_pte_t new, old = *ptep;
> >  	u64 granule = kvm_granule_size(level), phys = data->phys;
> > +	struct kvm_pgtable *pgt = data->mmu->pgt;
> >  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
> >  
> >  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> >  	}
> >  
> > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > +						granule);
> > +
> > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> 
> One thing I'm missing here is why we need the indirection via mm_ops. Are
> there cases where we would want to pass a different function pointer for
> invalidating the icache? If not, why not just call the function directly?
> 
> Same for the D side.

If we didn't do that, we'd end-up having to track whether the guest
context requires CMOs with additional flags, which is pretty ugly (see
v5 of this series for reference [1]).

It also means that we would have to drag the CM functions into the EL2
object, something that we don't need with this approach.

	M.

[1] https://lore.kernel.org/r/20210415115032.35760-1-wangyanan55@huawei.com

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 13:45:57 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > We currently uniformly permorm CMOs of D-cache and I-cache in function
> > user_mem_abort before calling the fault handlers. If we get concurrent
> > guest faults(e.g. translation faults, permission faults) or some really
> > unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> > necessary while the others later are not.
> > 
> > By moving CMOs to the fault handlers, we can easily identify conditions
> > where they are really needed and avoid the unnecessary ones. As it's a
> > time consuming process to perform CMOs especially when flushing a block
> > range, so this solution reduces much load of kvm and improve efficiency
> > of the stage-2 page table code.
> > 
> > We can imagine two specific scenarios which will gain much benefit:
> > 1) In a normal VM startup, this solution will improve the efficiency of
> > handling guest page faults incurred by vCPUs, when initially populating
> > stage-2 page tables.
> > 2) After live migration, the heavy workload will be resumed on the
> > destination VM, however all the stage-2 page tables need to be rebuilt
> > at the moment. So this solution will ease the performance drop during
> > resuming stage.
> > 
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
> >  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
> >  2 files changed, 46 insertions(+), 29 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index d99789432b05..760c551f61da 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
> >  	mm_ops->put_page(ptep);
> >  }
> >  
> > +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> > +{
> > +	u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> > +	return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> > +}
> > +
> > +static bool stage2_pte_executable(kvm_pte_t pte)
> > +{
> > +	return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> > +}
> > +
> >  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  				      kvm_pte_t *ptep,
> >  				      struct stage2_map_data *data)
> >  {
> >  	kvm_pte_t new, old = *ptep;
> >  	u64 granule = kvm_granule_size(level), phys = data->phys;
> > +	struct kvm_pgtable *pgt = data->mmu->pgt;
> >  	struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
> >  
> >  	if (!kvm_block_mapping_supported(addr, end, phys, level))
> > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> >  	}
> >  
> > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > +						granule);
> > +
> > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> 
> One thing I'm missing here is why we need the indirection via mm_ops. Are
> there cases where we would want to pass a different function pointer for
> invalidating the icache? If not, why not just call the function directly?
> 
> Same for the D side.

If we didn't do that, we'd end-up having to track whether the guest
context requires CMOs with additional flags, which is pretty ugly (see
v5 of this series for reference [1]).

It also means that we would have to drag the CM functions into the EL2
object, something that we don't need with this approach.

	M.

[1] https://lore.kernel.org/r/20210415115032.35760-1-wangyanan55@huawei.com

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:45:57 +0100,
> Will Deacon <will@kernel.org> wrote:
> > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > >  	}
> > >  
> > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > +						granule);
> > > +
> > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > 
> > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > there cases where we would want to pass a different function pointer for
> > invalidating the icache? If not, why not just call the function directly?
> > 
> > Same for the D side.
> 
> If we didn't do that, we'd end-up having to track whether the guest
> context requires CMOs with additional flags, which is pretty ugly (see
> v5 of this series for reference [1]).

Fair enough, although the function pointers here _are_ being used as flags,
as they only ever have one of two possible values (NULL or the CMO function),
so it's a shame to bring in the indirect branch as well.

> It also means that we would have to drag the CM functions into the EL2
> object, something that we don't need with this approach.

I think it won't be long before we end up with CMO functions at EL2 and
you'd hope we'd be able to use the same code as EL1 for something like
that. But I also wouldn't want to put money on it...

Anyway, no strong opinion on this, it just jumped out when I skimmed the
patches.

Will

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:45:57 +0100,
> Will Deacon <will@kernel.org> wrote:
> > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > >  	}
> > >  
> > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > +						granule);
> > > +
> > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > 
> > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > there cases where we would want to pass a different function pointer for
> > invalidating the icache? If not, why not just call the function directly?
> > 
> > Same for the D side.
> 
> If we didn't do that, we'd end-up having to track whether the guest
> context requires CMOs with additional flags, which is pretty ugly (see
> v5 of this series for reference [1]).

Fair enough, although the function pointers here _are_ being used as flags,
as they only ever have one of two possible values (NULL or the CMO function),
so it's a shame to bring in the indirect branch as well.

> It also means that we would have to drag the CM functions into the EL2
> object, something that we don't need with this approach.

I think it won't be long before we end up with CMO functions at EL2 and
you'd hope we'd be able to use the same code as EL1 for something like
that. But I also wouldn't want to put money on it...

Anyway, no strong opinion on this, it just jumped out when I skimmed the
patches.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Will Deacon]

On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> On Thu, 17 Jun 2021 13:45:57 +0100,
> Will Deacon <will@kernel.org> wrote:
> > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > >  	}
> > >  
> > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > +						granule);
> > > +
> > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > 
> > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > there cases where we would want to pass a different function pointer for
> > invalidating the icache? If not, why not just call the function directly?
> > 
> > Same for the D side.
> 
> If we didn't do that, we'd end-up having to track whether the guest
> context requires CMOs with additional flags, which is pretty ugly (see
> v5 of this series for reference [1]).

Fair enough, although the function pointers here _are_ being used as flags,
as they only ever have one of two possible values (NULL or the CMO function),
so it's a shame to bring in the indirect branch as well.

> It also means that we would have to drag the CM functions into the EL2
> object, something that we don't need with this approach.

I think it won't be long before we end up with CMO functions at EL2 and
you'd hope we'd be able to use the same code as EL1 for something like
that. But I also wouldn't want to put money on it...

Anyway, no strong opinion on this, it just jumped out when I skimmed the
patches.

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 14:21:16 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:45:57 +0100,
> > Will Deacon <will@kernel.org> wrote:
> > > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > > >  	}
> > > >  
> > > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > > +						granule);
> > > > +
> > > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > > 
> > > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > > there cases where we would want to pass a different function pointer for
> > > invalidating the icache? If not, why not just call the function directly?
> > > 
> > > Same for the D side.
> > 
> > If we didn't do that, we'd end-up having to track whether the guest
> > context requires CMOs with additional flags, which is pretty ugly (see
> > v5 of this series for reference [1]).
> 
> Fair enough, although the function pointers here _are_ being used as
> flags, as they only ever have one of two possible values (NULL or
> the CMO function), so it's a shame to bring in the indirect branch
> as well.

What I hope eventually is to get rid of some of the FWB tracking we
have for the host in the protected case, and use the same abstraction.

> 
> > It also means that we would have to drag the CM functions into the EL2
> > object, something that we don't need with this approach.
> 
> I think it won't be long before we end up with CMO functions at EL2 and
> you'd hope we'd be able to use the same code as EL1 for something like
> that. But I also wouldn't want to put money on it...

It we reach that stage, I'll be happy to try and move these function
into some shared location.

> Anyway, no strong opinion on this, it just jumped out when I skimmed the
> patches.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 14:21:16 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:45:57 +0100,
> > Will Deacon <will@kernel.org> wrote:
> > > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > > >  	}
> > > >  
> > > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > > +						granule);
> > > > +
> > > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > > 
> > > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > > there cases where we would want to pass a different function pointer for
> > > invalidating the icache? If not, why not just call the function directly?
> > > 
> > > Same for the D side.
> > 
> > If we didn't do that, we'd end-up having to track whether the guest
> > context requires CMOs with additional flags, which is pretty ugly (see
> > v5 of this series for reference [1]).
> 
> Fair enough, although the function pointers here _are_ being used as
> flags, as they only ever have one of two possible values (NULL or
> the CMO function), so it's a shame to bring in the indirect branch
> as well.

What I hope eventually is to get rid of some of the FWB tracking we
have for the host in the protected case, and use the same abstraction.

> 
> > It also means that we would have to drag the CM functions into the EL2
> > object, something that we don't need with this approach.
> 
> I think it won't be long before we end up with CMO functions at EL2 and
> you'd hope we'd be able to use the same code as EL1 for something like
> that. But I also wouldn't want to put money on it...

It we reach that stage, I'll be happy to try and move these function
into some shared location.

> Anyway, no strong opinion on this, it just jumped out when I skimmed the
> patches.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Marc Zyngier]

On Thu, 17 Jun 2021 14:21:16 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jun 17, 2021 at 01:59:37PM +0100, Marc Zyngier wrote:
> > On Thu, 17 Jun 2021 13:45:57 +0100,
> > Will Deacon <will@kernel.org> wrote:
> > > On Thu, Jun 17, 2021 at 06:58:24PM +0800, Yanan Wang wrote:
> > > > @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
> > > >  		stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
> > > >  	}
> > > >  
> > > > +	/* Perform CMOs before installation of the guest stage-2 PTE */
> > > > +	if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> > > > +		mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> > > > +						granule);
> > > > +
> > > > +	if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> > > > +		mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> > > 
> > > One thing I'm missing here is why we need the indirection via mm_ops. Are
> > > there cases where we would want to pass a different function pointer for
> > > invalidating the icache? If not, why not just call the function directly?
> > > 
> > > Same for the D side.
> > 
> > If we didn't do that, we'd end-up having to track whether the guest
> > context requires CMOs with additional flags, which is pretty ugly (see
> > v5 of this series for reference [1]).
> 
> Fair enough, although the function pointers here _are_ being used as
> flags, as they only ever have one of two possible values (NULL or
> the CMO function), so it's a shame to bring in the indirect branch
> as well.

What I hope eventually is to get rid of some of the FWB tracking we
have for the host in the protected case, and use the same abstraction.

> 
> > It also means that we would have to drag the CM functions into the EL2
> > object, something that we don't need with this approach.
> 
> I think it won't be long before we end up with CMO functions at EL2 and
> you'd hope we'd be able to use the same code as EL1 for something like
> that. But I also wouldn't want to put money on it...

It we reach that stage, I'll be happy to try and move these function
into some shared location.

> Anyway, no strong opinion on this, it just jumped out when I skimmed the
> patches.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> We currently uniformly permorm CMOs of D-cache and I-cache in function

Nit: permorm -> perform

> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
>
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
>
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>         mm_ops->put_page(ptep);
>  }
>
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +       return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                                       kvm_pte_t *ptep,
>                                       struct stage2_map_data *data)
>  {
>         kvm_pte_t new, old = *ptep;
>         u64 granule = kvm_granule_size(level), phys = data->phys;
> +       struct kvm_pgtable *pgt = data->mmu->pgt;
>         struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                 stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>         }
>
> +       /* Perform CMOs before installation of the guest stage-2 PTE */
> +       if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +               mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +                                               granule);
> +
> +       if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +               mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> +
>         smp_store_release(ptep, new);
>         if (stage2_pte_is_counted(new))
>                 mm_ops->get_page(ptep);
> @@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
>         return ret;
>  }
>
> -static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> -{
> -       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> -       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> -}
> -
>  static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>                                enum kvm_pgtable_walk_flags flag,
>                                void * const arg)
> @@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>  {
>         kvm_pte_t pte = *ptep;
>         struct stage2_attr_data *data = arg;
> +       struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_pte_valid(pte))
>                 return 0;
> @@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>          * but worst-case the access flag update gets lost and will be
>          * set on the next access instead.
>          */
> -       if (data->pte != pte)
> +       if (data->pte != pte) {
> +               /*
> +                * Invalidate instruction cache before updating the guest
> +                * stage-2 PTE if we are going to add executable permission.
> +                */
> +               if (mm_ops->invalidate_icache &&
> +                   stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
> +                       mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
> +                                                 kvm_granule_size(level));
>                 WRITE_ONCE(*ptep, pte);
> +       }
>
>         return 0;
>  }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index b980f8a47cbb..c9f002d74ab4 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
>  }
>
>  static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> -       .zalloc_page            = stage2_memcache_zalloc_page,
> -       .zalloc_pages_exact     = kvm_host_zalloc_pages_exact,
> -       .free_pages_exact       = free_pages_exact,
> -       .get_page               = kvm_host_get_page,
> -       .put_page               = kvm_host_put_page,
> -       .page_count             = kvm_host_page_count,
> -       .phys_to_virt           = kvm_host_va,
> -       .virt_to_phys           = kvm_host_pa,
> +       .zalloc_page                    = stage2_memcache_zalloc_page,
> +       .zalloc_pages_exact             = kvm_host_zalloc_pages_exact,
> +       .free_pages_exact               = free_pages_exact,
> +       .get_page                       = kvm_host_get_page,
> +       .put_page                       = kvm_host_put_page,
> +       .page_count                     = kvm_host_page_count,
> +       .phys_to_virt                   = kvm_host_va,
> +       .virt_to_phys                   = kvm_host_pa,
> +       .clean_invalidate_dcache        = clean_dcache_guest_page,
> +       .invalidate_icache              = invalidate_icache_guest_page,
>  };
>
>  /**
> @@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>         if (writable)
>                 prot |= KVM_PGTABLE_PROT_W;
>
> -       if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> -                                       vma_pagesize);
> -
> -       if (exec_fault) {
> +       if (exec_fault)
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> -                                            vma_pagesize);
> -       }
>
>         if (device)
>                 prot |= KVM_PGTABLE_PROT_DEVICE;
> @@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>         WARN_ON(range->end - range->start != 1);
>
>         /*
> -        * We've moved a page around, probably through CoW, so let's treat it
> -        * just like a translation fault and clean the cache to the PoC.
> -        */
> -       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> -
> -       /*
> +        * We've moved a page around, probably through CoW, so let's treat
> +        * it just like a translation fault and the map handler will clean
> +        * the cache to the PoC.
> +        *
>          * The MMU notifiers will have unmapped a huge PMD before calling
>          * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
>          * therefore we never need to clear out a huge PMD through this

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> --
> 2.23.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> We currently uniformly permorm CMOs of D-cache and I-cache in function

Nit: permorm -> perform

> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
>
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
>
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>         mm_ops->put_page(ptep);
>  }
>
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +       return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                                       kvm_pte_t *ptep,
>                                       struct stage2_map_data *data)
>  {
>         kvm_pte_t new, old = *ptep;
>         u64 granule = kvm_granule_size(level), phys = data->phys;
> +       struct kvm_pgtable *pgt = data->mmu->pgt;
>         struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                 stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>         }
>
> +       /* Perform CMOs before installation of the guest stage-2 PTE */
> +       if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +               mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +                                               granule);
> +
> +       if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +               mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> +
>         smp_store_release(ptep, new);
>         if (stage2_pte_is_counted(new))
>                 mm_ops->get_page(ptep);
> @@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
>         return ret;
>  }
>
> -static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> -{
> -       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> -       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> -}
> -
>  static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>                                enum kvm_pgtable_walk_flags flag,
>                                void * const arg)
> @@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>  {
>         kvm_pte_t pte = *ptep;
>         struct stage2_attr_data *data = arg;
> +       struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_pte_valid(pte))
>                 return 0;
> @@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>          * but worst-case the access flag update gets lost and will be
>          * set on the next access instead.
>          */
> -       if (data->pte != pte)
> +       if (data->pte != pte) {
> +               /*
> +                * Invalidate instruction cache before updating the guest
> +                * stage-2 PTE if we are going to add executable permission.
> +                */
> +               if (mm_ops->invalidate_icache &&
> +                   stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
> +                       mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
> +                                                 kvm_granule_size(level));
>                 WRITE_ONCE(*ptep, pte);
> +       }
>
>         return 0;
>  }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index b980f8a47cbb..c9f002d74ab4 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
>  }
>
>  static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> -       .zalloc_page            = stage2_memcache_zalloc_page,
> -       .zalloc_pages_exact     = kvm_host_zalloc_pages_exact,
> -       .free_pages_exact       = free_pages_exact,
> -       .get_page               = kvm_host_get_page,
> -       .put_page               = kvm_host_put_page,
> -       .page_count             = kvm_host_page_count,
> -       .phys_to_virt           = kvm_host_va,
> -       .virt_to_phys           = kvm_host_pa,
> +       .zalloc_page                    = stage2_memcache_zalloc_page,
> +       .zalloc_pages_exact             = kvm_host_zalloc_pages_exact,
> +       .free_pages_exact               = free_pages_exact,
> +       .get_page                       = kvm_host_get_page,
> +       .put_page                       = kvm_host_put_page,
> +       .page_count                     = kvm_host_page_count,
> +       .phys_to_virt                   = kvm_host_va,
> +       .virt_to_phys                   = kvm_host_pa,
> +       .clean_invalidate_dcache        = clean_dcache_guest_page,
> +       .invalidate_icache              = invalidate_icache_guest_page,
>  };
>
>  /**
> @@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>         if (writable)
>                 prot |= KVM_PGTABLE_PROT_W;
>
> -       if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> -                                       vma_pagesize);
> -
> -       if (exec_fault) {
> +       if (exec_fault)
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> -                                            vma_pagesize);
> -       }
>
>         if (device)
>                 prot |= KVM_PGTABLE_PROT_DEVICE;
> @@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>         WARN_ON(range->end - range->start != 1);
>
>         /*
> -        * We've moved a page around, probably through CoW, so let's treat it
> -        * just like a translation fault and clean the cache to the PoC.
> -        */
> -       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> -
> -       /*
> +        * We've moved a page around, probably through CoW, so let's treat
> +        * it just like a translation fault and the map handler will clean
> +        * the cache to the PoC.
> +        *
>          * The MMU notifiers will have unmapped a huge PMD before calling
>          * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
>          * therefore we never need to clear out a huge PMD through this

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> --
> 2.23.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 4/4] KVM: arm64: Move guest CMOs to the fault handlers

[Fuad Tabba]

Hi Yanan,

On Thu, Jun 17, 2021 at 11:58 AM Yanan Wang <wangyanan55@huawei.com> wrote:
>
> We currently uniformly permorm CMOs of D-cache and I-cache in function

Nit: permorm -> perform

> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
>
> By moving CMOs to the fault handlers, we can easily identify conditions
> where they are really needed and avoid the unnecessary ones. As it's a
> time consuming process to perform CMOs especially when flushing a block
> range, so this solution reduces much load of kvm and improve efficiency
> of the stage-2 page table code.
>
> We can imagine two specific scenarios which will gain much benefit:
> 1) In a normal VM startup, this solution will improve the efficiency of
> handling guest page faults incurred by vCPUs, when initially populating
> stage-2 page tables.
> 2) After live migration, the heavy workload will be resumed on the
> destination VM, however all the stage-2 page tables need to be rebuilt
> at the moment. So this solution will ease the performance drop during
> resuming stage.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  arch/arm64/kvm/hyp/pgtable.c | 38 +++++++++++++++++++++++++++++-------
>  arch/arm64/kvm/mmu.c         | 37 ++++++++++++++---------------------
>  2 files changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index d99789432b05..760c551f61da 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -577,12 +577,24 @@ static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
>         mm_ops->put_page(ptep);
>  }
>
> +static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> +{
> +       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> +       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> +}
> +
> +static bool stage2_pte_executable(kvm_pte_t pte)
> +{
> +       return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
> +}
> +
>  static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                                       kvm_pte_t *ptep,
>                                       struct stage2_map_data *data)
>  {
>         kvm_pte_t new, old = *ptep;
>         u64 granule = kvm_granule_size(level), phys = data->phys;
> +       struct kvm_pgtable *pgt = data->mmu->pgt;
>         struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_block_mapping_supported(addr, end, phys, level))
> @@ -606,6 +618,14 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>                 stage2_put_pte(ptep, data->mmu, addr, level, mm_ops);
>         }
>
> +       /* Perform CMOs before installation of the guest stage-2 PTE */
> +       if (mm_ops->clean_invalidate_dcache && stage2_pte_cacheable(pgt, new))
> +               mm_ops->clean_invalidate_dcache(kvm_pte_follow(new, mm_ops),
> +                                               granule);
> +
> +       if (mm_ops->invalidate_icache && stage2_pte_executable(new))
> +               mm_ops->invalidate_icache(kvm_pte_follow(new, mm_ops), granule);
> +
>         smp_store_release(ptep, new);
>         if (stage2_pte_is_counted(new))
>                 mm_ops->get_page(ptep);
> @@ -798,12 +818,6 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
>         return ret;
>  }
>
> -static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
> -{
> -       u64 memattr = pte & KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR;
> -       return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
> -}
> -
>  static int stage2_unmap_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>                                enum kvm_pgtable_walk_flags flag,
>                                void * const arg)
> @@ -874,6 +888,7 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>  {
>         kvm_pte_t pte = *ptep;
>         struct stage2_attr_data *data = arg;
> +       struct kvm_pgtable_mm_ops *mm_ops = data->mm_ops;
>
>         if (!kvm_pte_valid(pte))
>                 return 0;
> @@ -888,8 +903,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
>          * but worst-case the access flag update gets lost and will be
>          * set on the next access instead.
>          */
> -       if (data->pte != pte)
> +       if (data->pte != pte) {
> +               /*
> +                * Invalidate instruction cache before updating the guest
> +                * stage-2 PTE if we are going to add executable permission.
> +                */
> +               if (mm_ops->invalidate_icache &&
> +                   stage2_pte_executable(pte) && !stage2_pte_executable(*ptep))
> +                       mm_ops->invalidate_icache(kvm_pte_follow(pte, mm_ops),
> +                                                 kvm_granule_size(level));
>                 WRITE_ONCE(*ptep, pte);
> +       }
>
>         return 0;
>  }
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index b980f8a47cbb..c9f002d74ab4 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -434,14 +434,16 @@ int create_hyp_exec_mappings(phys_addr_t phys_addr, size_t size,
>  }
>
>  static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> -       .zalloc_page            = stage2_memcache_zalloc_page,
> -       .zalloc_pages_exact     = kvm_host_zalloc_pages_exact,
> -       .free_pages_exact       = free_pages_exact,
> -       .get_page               = kvm_host_get_page,
> -       .put_page               = kvm_host_put_page,
> -       .page_count             = kvm_host_page_count,
> -       .phys_to_virt           = kvm_host_va,
> -       .virt_to_phys           = kvm_host_pa,
> +       .zalloc_page                    = stage2_memcache_zalloc_page,
> +       .zalloc_pages_exact             = kvm_host_zalloc_pages_exact,
> +       .free_pages_exact               = free_pages_exact,
> +       .get_page                       = kvm_host_get_page,
> +       .put_page                       = kvm_host_put_page,
> +       .page_count                     = kvm_host_page_count,
> +       .phys_to_virt                   = kvm_host_va,
> +       .virt_to_phys                   = kvm_host_pa,
> +       .clean_invalidate_dcache        = clean_dcache_guest_page,
> +       .invalidate_icache              = invalidate_icache_guest_page,
>  };
>
>  /**
> @@ -1012,15 +1014,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>         if (writable)
>                 prot |= KVM_PGTABLE_PROT_W;
>
> -       if (fault_status != FSC_PERM && !device)
> -               clean_dcache_guest_page(page_address(pfn_to_page(pfn)),
> -                                       vma_pagesize);
> -
> -       if (exec_fault) {
> +       if (exec_fault)
>                 prot |= KVM_PGTABLE_PROT_X;
> -               invalidate_icache_guest_page(page_address(pfn_to_page(pfn)),
> -                                            vma_pagesize);
> -       }
>
>         if (device)
>                 prot |= KVM_PGTABLE_PROT_DEVICE;
> @@ -1218,12 +1213,10 @@ bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
>         WARN_ON(range->end - range->start != 1);
>
>         /*
> -        * We've moved a page around, probably through CoW, so let's treat it
> -        * just like a translation fault and clean the cache to the PoC.
> -        */
> -       clean_dcache_guest_page(page_address(pfn_to_page(pfn), PAGE_SIZE);
> -
> -       /*
> +        * We've moved a page around, probably through CoW, so let's treat
> +        * it just like a translation fault and the map handler will clean
> +        * the cache to the PoC.
> +        *
>          * The MMU notifiers will have unmapped a huge PMD before calling
>          * ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
>          * therefore we never need to clear out a huge PMD through this

Reviewed-by: Fuad Tabba <tabba@google.com>

Thanks,
/fuad

> --
> 2.23.0
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Marc Zyngier]

On Thu, 17 Jun 2021 18:58:20 +0800, Yanan Wang wrote:
> This series makes some efficiency improvement of guest stage-2 page
> table code, and there are some test results to quantify the benefit.
> 
> Description for this series:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Introduce two cache maintenance callbacks
      commit: 6204004de3160900435bdb4b9a2fb8749a9277d2
[2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
      commit: a4d5ca5c7cd8fe85056b8cb838fbcb7e5a05f356
[3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions
      commit: 378e6a9c78a02b4b609846aa0afccf34d3038977
[4/4] KVM: arm64: Move guest CMOs to the fault handlers
      commit: 25aa28691bb960a76f0cffd8862144a29487f6ff

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Marc Zyngier]

On Thu, 17 Jun 2021 18:58:20 +0800, Yanan Wang wrote:
> This series makes some efficiency improvement of guest stage-2 page
> table code, and there are some test results to quantify the benefit.
> 
> Description for this series:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Introduce two cache maintenance callbacks
      commit: 6204004de3160900435bdb4b9a2fb8749a9277d2
[2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
      commit: a4d5ca5c7cd8fe85056b8cb838fbcb7e5a05f356
[3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions
      commit: 378e6a9c78a02b4b609846aa0afccf34d3038977
[4/4] KVM: arm64: Move guest CMOs to the fault handlers
      commit: 25aa28691bb960a76f0cffd8862144a29487f6ff

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 0/4] KVM: arm64: Improve efficiency of stage2 page table

[Marc Zyngier]

On Thu, 17 Jun 2021 18:58:20 +0800, Yanan Wang wrote:
> This series makes some efficiency improvement of guest stage-2 page
> table code, and there are some test results to quantify the benefit.
> 
> Description for this series:
> We currently uniformly permorm CMOs of D-cache and I-cache in function
> user_mem_abort before calling the fault handlers. If we get concurrent
> guest faults(e.g. translation faults, permission faults) or some really
> unnecessary guest faults caused by BBM, CMOs for the first vcpu are
> necessary while the others later are not.
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Introduce two cache maintenance callbacks
      commit: 6204004de3160900435bdb4b9a2fb8749a9277d2
[2/4] KVM: arm64: Introduce mm_ops member for structure stage2_attr_data
      commit: a4d5ca5c7cd8fe85056b8cb838fbcb7e5a05f356
[3/4] KVM: arm64: Tweak parameters of guest cache maintenance functions
      commit: 378e6a9c78a02b4b609846aa0afccf34d3038977
[4/4] KVM: arm64: Move guest CMOs to the fault handlers
      commit: 25aa28691bb960a76f0cffd8862144a29487f6ff

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm