Re: [PATCH v6 1/4] KVM: arm64: Introduce cache maintenance callbacks for guest stage-2

Marc Zyngier
2021-06-16T13:21:33Z

Hi Yanan,

On Wed, 16 Jun 2021 10:51:57 +0100,
Yanan Wang <wangyanan55@huawei.com> wrote:
>
> To prepare for performing guest CMOs in the fault handlers in pgtable.c,
> introduce two cache maintenance callbacks in struct kvm_pgtable_mm_ops.
>
> The new callbacks are specific for guest stage-2, so they will only be
> initialized in 'struct kvm_pgtable_mm_ops kvm_s2_mm_ops'.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
> arch/arm64/include/asm/kvm_pgtable.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index c3674c47d48c..302eca32e0af 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -44,6 +44,11 @@ typedef u64 kvm_pte_t;
> * in the current context.
> * @virt_to_phys: Convert a virtual address mapped in the current context
> * into a physical address.
> + * @flush_dcache: Clean data cache for a guest page address range before
> + * creating the corresponding stage-2 mapping.

Please don't reintroduce the word 'flush'. We are really trying to
move away from it as it doesn't describe what we want to do. Here this
should be 'clean_invalidate_dcache' which, despite being a mouthful,
describe accurately what we expect it to do.

The comment is also missing the invalidate part, and we shouldn't
assume that this is only used for S2 mapping.

> + * @flush_icache: Invalidate instruction cache for a guest page address
> + * range before creating or updating the corresponding
> + * stage-2 mapping.

Same thing here; this should be 'invalidate_icache', and the comment
cleaned up.

> */
> struct kvm_pgtable_mm_ops {
> void* (*zalloc_page)(void *arg);
> @@ -54,6 +59,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 (*flush_dcache)(void *addr, size_t size);
> + void (*flush_icache)(void *addr, size_t size);
> };
>
> /**

Thanks,

M.

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