[v7,1/4] KVM: stats: Separate generic stats from architecture specific ones

Put all generic statistics in a separate structure to ease
statistics handling for the incoming new statistics API.

No functional change intended.

Reviewed-by: David Matlack <dmatlack@google.com>
Reviewed-by: Ricardo Koller <ricarkol@google.com>
Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
 arch/arm64/include/asm/kvm_host.h   |  9 ++-------
 arch/arm64/kvm/guest.c              | 12 ++++++------
 arch/mips/include/asm/kvm_host.h    |  9 ++-------
 arch/mips/kvm/mips.c                | 12 ++++++------
 arch/powerpc/include/asm/kvm_host.h |  9 ++-------
 arch/powerpc/kvm/book3s.c           | 12 ++++++------
 arch/powerpc/kvm/book3s_hv.c        | 12 ++++++------
 arch/powerpc/kvm/book3s_pr.c        |  2 +-
 arch/powerpc/kvm/book3s_pr_papr.c   |  2 +-
 arch/powerpc/kvm/booke.c            | 14 +++++++-------
 arch/s390/include/asm/kvm_host.h    |  9 ++-------
 arch/s390/kvm/kvm-s390.c            | 12 ++++++------
 arch/x86/include/asm/kvm_host.h     |  9 ++-------
 arch/x86/kvm/x86.c                  | 14 +++++++-------
 include/linux/kvm_host.h            |  9 +++++++--
 include/linux/kvm_types.h           | 12 ++++++++++++
 virt/kvm/kvm_main.c                 | 14 +++++++-------
 17 files changed, 82 insertions(+), 90 deletions(-)

On 6/3/21 2:14 PM, Jing Zhang wrote:
> Put all generic statistics in a separate structure to ease
> statistics handling for the incoming new statistics API.
>
> No functional change intended.
>
> Reviewed-by: David Matlack <dmatlack@google.com>
> Reviewed-by: Ricardo Koller <ricarkol@google.com>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
>   arch/arm64/include/asm/kvm_host.h   |  9 ++-------
>   arch/arm64/kvm/guest.c              | 12 ++++++------
>   arch/mips/include/asm/kvm_host.h    |  9 ++-------
>   arch/mips/kvm/mips.c                | 12 ++++++------
>   arch/powerpc/include/asm/kvm_host.h |  9 ++-------
>   arch/powerpc/kvm/book3s.c           | 12 ++++++------
>   arch/powerpc/kvm/book3s_hv.c        | 12 ++++++------
>   arch/powerpc/kvm/book3s_pr.c        |  2 +-
>   arch/powerpc/kvm/book3s_pr_papr.c   |  2 +-
>   arch/powerpc/kvm/booke.c            | 14 +++++++-------
>   arch/s390/include/asm/kvm_host.h    |  9 ++-------
>   arch/s390/kvm/kvm-s390.c            | 12 ++++++------
>   arch/x86/include/asm/kvm_host.h     |  9 ++-------
>   arch/x86/kvm/x86.c                  | 14 +++++++-------
>   include/linux/kvm_host.h            |  9 +++++++--
>   include/linux/kvm_types.h           | 12 ++++++++++++
>   virt/kvm/kvm_main.c                 | 14 +++++++-------
>   17 files changed, 82 insertions(+), 90 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7cd7d5c8c4bc..5a2c82f63baa 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -556,16 +556,11 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
>   }
>   
>   struct kvm_vm_stat {
> -	ulong remote_tlb_flush;
> +	struct kvm_vm_stat_generic generic;
>   };
>   
>   struct kvm_vcpu_stat {
> -	u64 halt_successful_poll;
> -	u64 halt_attempted_poll;
> -	u64 halt_poll_success_ns;
> -	u64 halt_poll_fail_ns;
> -	u64 halt_poll_invalid;
> -	u64 halt_wakeup;
> +	struct kvm_vcpu_stat_generic generic;
>   	u64 hvc_exit_stat;
>   	u64 wfe_exit_stat;
>   	u64 wfi_exit_stat;
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 5cb4a1cd5603..4962331d01e6 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -29,18 +29,18 @@
>   #include "trace.h"
>   
>   struct kvm_stats_debugfs_item debugfs_entries[] = {
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
>   	VCPU_STAT("hvc_exit_stat", hvc_exit_stat),
>   	VCPU_STAT("wfe_exit_stat", wfe_exit_stat),
>   	VCPU_STAT("wfi_exit_stat", wfi_exit_stat),
>   	VCPU_STAT("mmio_exit_user", mmio_exit_user),
>   	VCPU_STAT("mmio_exit_kernel", mmio_exit_kernel),
>   	VCPU_STAT("exits", exits),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
>   	{ NULL }
>   };
>   
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index fca4547d580f..696f6b009377 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -109,10 +109,11 @@ static inline bool kvm_is_error_hva(unsigned long addr)
>   }
>   
>   struct kvm_vm_stat {
> -	ulong remote_tlb_flush;
> +	struct kvm_vm_stat_generic generic;
>   };
>   
>   struct kvm_vcpu_stat {
> +	struct kvm_vcpu_stat_generic generic;
>   	u64 wait_exits;
>   	u64 cache_exits;
>   	u64 signal_exits;
> @@ -142,12 +143,6 @@ struct kvm_vcpu_stat {
>   #ifdef CONFIG_CPU_LOONGSON64
>   	u64 vz_cpucfg_exits;
>   #endif
> -	u64 halt_successful_poll;
> -	u64 halt_attempted_poll;
> -	u64 halt_poll_success_ns;
> -	u64 halt_poll_fail_ns;
> -	u64 halt_poll_invalid;
> -	u64 halt_wakeup;
>   };
>   
>   struct kvm_arch_memory_slot {
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 4d4af97dcc88..ff205b35719b 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -68,12 +68,12 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   #ifdef CONFIG_CPU_LOONGSON64
>   	VCPU_STAT("vz_cpucfg", vz_cpucfg_exits),
>   #endif
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
>   	{NULL}
>   };
>   
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 1e83359f286b..6e41064858a1 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -80,12 +80,13 @@ struct kvmppc_book3s_shadow_vcpu;
>   struct kvm_nested_guest;
>   
>   struct kvm_vm_stat {
> -	ulong remote_tlb_flush;
> +	struct kvm_vm_stat_generic generic;
>   	ulong num_2M_pages;
>   	ulong num_1G_pages;
>   };
>   
>   struct kvm_vcpu_stat {
> +	struct kvm_vcpu_stat_generic generic;
>   	u64 sum_exits;
>   	u64 mmio_exits;
>   	u64 signal_exits;
> @@ -101,14 +102,8 @@ struct kvm_vcpu_stat {
>   	u64 emulated_inst_exits;
>   	u64 dec_exits;
>   	u64 ext_intr_exits;
> -	u64 halt_poll_success_ns;
> -	u64 halt_poll_fail_ns;
>   	u64 halt_wait_ns;
> -	u64 halt_successful_poll;
> -	u64 halt_attempted_poll;
>   	u64 halt_successful_wait;
> -	u64 halt_poll_invalid;
> -	u64 halt_wakeup;
>   	u64 dbell_exits;
>   	u64 gdbell_exits;
>   	u64 ld;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 2b691f4d1f26..92cdb4175945 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -47,14 +47,14 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VCPU_STAT("dec", dec_exits),
>   	VCPU_STAT("ext_intr", ext_intr_exits),
>   	VCPU_STAT("queue_intr", queue_intr),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
>   	VCPU_STAT("halt_wait_ns", halt_wait_ns),
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
>   	VCPU_STAT("halt_successful_wait", halt_successful_wait),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
>   	VCPU_STAT("pf_storage", pf_storage),
>   	VCPU_STAT("sp_storage", sp_storage),
>   	VCPU_STAT("pf_instruc", pf_instruc),
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 28a80d240b76..2a1cf29ba3fd 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -236,7 +236,7 @@ static void kvmppc_fast_vcpu_kick_hv(struct kvm_vcpu *vcpu)
>   
>   	waitp = kvm_arch_vcpu_get_wait(vcpu);
>   	if (rcuwait_wake_up(waitp))
> -		++vcpu->stat.halt_wakeup;
> +		++vcpu->stat.generic.halt_wakeup;
>   
>   	cpu = READ_ONCE(vcpu->arch.thread_cpu);
>   	if (cpu >= 0 && kvmppc_ipi_thread(cpu))
> @@ -3925,7 +3925,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
>   	cur = start_poll = ktime_get();
>   	if (vc->halt_poll_ns) {
>   		ktime_t stop = ktime_add_ns(start_poll, vc->halt_poll_ns);
> -		++vc->runner->stat.halt_attempted_poll;
> +		++vc->runner->stat.generic.halt_attempted_poll;
>   
>   		vc->vcore_state = VCORE_POLLING;
>   		spin_unlock(&vc->lock);
> @@ -3942,7 +3942,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
>   		vc->vcore_state = VCORE_INACTIVE;
>   
>   		if (!do_sleep) {
> -			++vc->runner->stat.halt_successful_poll;
> +			++vc->runner->stat.generic.halt_successful_poll;
>   			goto out;
>   		}
>   	}
> @@ -3954,7 +3954,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
>   		do_sleep = 0;
>   		/* If we polled, count this as a successful poll */
>   		if (vc->halt_poll_ns)
> -			++vc->runner->stat.halt_successful_poll;
> +			++vc->runner->stat.generic.halt_successful_poll;
>   		goto out;
>   	}
>   
> @@ -3981,13 +3981,13 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
>   			ktime_to_ns(cur) - ktime_to_ns(start_wait);
>   		/* Attribute failed poll time */
>   		if (vc->halt_poll_ns)
> -			vc->runner->stat.halt_poll_fail_ns +=
> +			vc->runner->stat.generic.halt_poll_fail_ns +=
>   				ktime_to_ns(start_wait) -
>   				ktime_to_ns(start_poll);
>   	} else {
>   		/* Attribute successful poll time */
>   		if (vc->halt_poll_ns)
> -			vc->runner->stat.halt_poll_success_ns +=
> +			vc->runner->stat.generic.halt_poll_success_ns +=
>   				ktime_to_ns(cur) -
>   				ktime_to_ns(start_poll);
>   	}
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index d7733b07f489..71bcb0140461 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -493,7 +493,7 @@ static void kvmppc_set_msr_pr(struct kvm_vcpu *vcpu, u64 msr)
>   		if (!vcpu->arch.pending_exceptions) {
>   			kvm_vcpu_block(vcpu);
>   			kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> -			vcpu->stat.halt_wakeup++;
> +			vcpu->stat.generic.halt_wakeup++;
>   
>   			/* Unset POW bit after we woke up */
>   			msr &= ~MSR_POW;
> diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
> index 031c8015864a..ac14239f3424 100644
> --- a/arch/powerpc/kvm/book3s_pr_papr.c
> +++ b/arch/powerpc/kvm/book3s_pr_papr.c
> @@ -378,7 +378,7 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
>   		kvmppc_set_msr_fast(vcpu, kvmppc_get_msr(vcpu) | MSR_EE);
>   		kvm_vcpu_block(vcpu);
>   		kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> -		vcpu->stat.halt_wakeup++;
> +		vcpu->stat.generic.halt_wakeup++;
>   		return EMULATE_DONE;
>   	case H_LOGICAL_CI_LOAD:
>   		return kvmppc_h_pr_logical_ci_load(vcpu);
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 7d5fe43f85c4..80d3b39aa7ac 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -49,15 +49,15 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VCPU_STAT("inst_emu", emulated_inst_exits),
>   	VCPU_STAT("dec", dec_exits),
>   	VCPU_STAT("ext_intr", ext_intr_exits),
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
>   	VCPU_STAT("doorbell", dbell_exits),
>   	VCPU_STAT("guest doorbell", gdbell_exits),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> -	VM_STAT("remote_tlb_flush", remote_tlb_flush),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VM_STAT_GENERIC("remote_tlb_flush", remote_tlb_flush),
>   	{ NULL }
>   };
>   
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 8925f3969478..9b4473f76e56 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -361,6 +361,7 @@ struct sie_page {
>   };
>   
>   struct kvm_vcpu_stat {
> +	struct kvm_vcpu_stat_generic generic;
>   	u64 exit_userspace;
>   	u64 exit_null;
>   	u64 exit_external_request;
> @@ -370,13 +371,7 @@ struct kvm_vcpu_stat {
>   	u64 exit_validity;
>   	u64 exit_instruction;
>   	u64 exit_pei;
> -	u64 halt_successful_poll;
> -	u64 halt_attempted_poll;
> -	u64 halt_poll_invalid;
>   	u64 halt_no_poll_steal;
> -	u64 halt_wakeup;
> -	u64 halt_poll_success_ns;
> -	u64 halt_poll_fail_ns;
>   	u64 instruction_lctl;
>   	u64 instruction_lctlg;
>   	u64 instruction_stctl;
> @@ -755,12 +750,12 @@ struct kvm_vcpu_arch {
>   };
>   
>   struct kvm_vm_stat {
> +	struct kvm_vm_stat_generic generic;
>   	u64 inject_io;
>   	u64 inject_float_mchk;
>   	u64 inject_pfault_done;
>   	u64 inject_service_signal;
>   	u64 inject_virtio;
> -	u64 remote_tlb_flush;
>   };
>   
>   struct kvm_arch_memory_slot {
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1296fc10f80c..e8bc7cd06794 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -72,13 +72,13 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VCPU_STAT("exit_program_interruption", exit_program_interruption),
>   	VCPU_STAT("exit_instr_and_program_int", exit_instr_and_program),
>   	VCPU_STAT("exit_operation_exception", exit_operation_exception),
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
>   	VCPU_STAT("halt_no_poll_steal", halt_no_poll_steal),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
>   	VCPU_STAT("instruction_lctlg", instruction_lctlg),
>   	VCPU_STAT("instruction_lctl", instruction_lctl),
>   	VCPU_STAT("instruction_stctl", instruction_stctl),
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 55efbacfc244..db8eb3dead7e 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1127,6 +1127,7 @@ struct kvm_arch {
>   };
>   
>   struct kvm_vm_stat {
> +	struct kvm_vm_stat_generic generic;
>   	ulong mmu_shadow_zapped;
>   	ulong mmu_pte_write;
>   	ulong mmu_pde_zapped;
> @@ -1134,13 +1135,13 @@ struct kvm_vm_stat {
>   	ulong mmu_recycled;
>   	ulong mmu_cache_miss;
>   	ulong mmu_unsync;
> -	ulong remote_tlb_flush;
>   	ulong lpages;
>   	ulong nx_lpage_splits;
>   	ulong max_mmu_page_hash_collisions;
>   };
>   
>   struct kvm_vcpu_stat {
> +	struct kvm_vcpu_stat_generic generic;
>   	u64 pf_fixed;
>   	u64 pf_guest;
>   	u64 tlb_flush;
> @@ -1154,10 +1155,6 @@ struct kvm_vcpu_stat {
>   	u64 nmi_window_exits;
>   	u64 l1d_flush;
>   	u64 halt_exits;
> -	u64 halt_successful_poll;
> -	u64 halt_attempted_poll;
> -	u64 halt_poll_invalid;
> -	u64 halt_wakeup;
>   	u64 request_irq_exits;
>   	u64 irq_exits;
>   	u64 host_state_reload;
> @@ -1168,8 +1165,6 @@ struct kvm_vcpu_stat {
>   	u64 irq_injections;
>   	u64 nmi_injections;
>   	u64 req_event;
> -	u64 halt_poll_success_ns;
> -	u64 halt_poll_fail_ns;
>   	u64 nested_run;
>   	u64 directed_yield_attempted;
>   	u64 directed_yield_successful;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9b6bca616929..96d10253218a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -226,10 +226,10 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VCPU_STAT("irq_window", irq_window_exits),
>   	VCPU_STAT("nmi_window", nmi_window_exits),
>   	VCPU_STAT("halt_exits", halt_exits),
> -	VCPU_STAT("halt_successful_poll", halt_successful_poll),
> -	VCPU_STAT("halt_attempted_poll", halt_attempted_poll),
> -	VCPU_STAT("halt_poll_invalid", halt_poll_invalid),
> -	VCPU_STAT("halt_wakeup", halt_wakeup),
> +	VCPU_STAT_GENERIC("halt_successful_poll", halt_successful_poll),
> +	VCPU_STAT_GENERIC("halt_attempted_poll", halt_attempted_poll),
> +	VCPU_STAT_GENERIC("halt_poll_invalid", halt_poll_invalid),
> +	VCPU_STAT_GENERIC("halt_wakeup", halt_wakeup),
>   	VCPU_STAT("hypercalls", hypercalls),
>   	VCPU_STAT("request_irq", request_irq_exits),
>   	VCPU_STAT("irq_exits", irq_exits),
> @@ -241,8 +241,8 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VCPU_STAT("nmi_injections", nmi_injections),
>   	VCPU_STAT("req_event", req_event),
>   	VCPU_STAT("l1d_flush", l1d_flush),
> -	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
> -	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
> +	VCPU_STAT_GENERIC("halt_poll_success_ns", halt_poll_success_ns),
> +	VCPU_STAT_GENERIC("halt_poll_fail_ns", halt_poll_fail_ns),
>   	VCPU_STAT("nested_run", nested_run),
>   	VCPU_STAT("directed_yield_attempted", directed_yield_attempted),
>   	VCPU_STAT("directed_yield_successful", directed_yield_successful),
> @@ -253,7 +253,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>   	VM_STAT("mmu_recycled", mmu_recycled),
>   	VM_STAT("mmu_cache_miss", mmu_cache_miss),
>   	VM_STAT("mmu_unsync", mmu_unsync),
> -	VM_STAT("remote_tlb_flush", remote_tlb_flush),
> +	VM_STAT_GENERIC("remote_tlb_flush", remote_tlb_flush),
>   	VM_STAT("largepages", lpages, .mode = 0444),
>   	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
>   	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 2f34487e21f2..1870fa928762 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1243,10 +1243,15 @@ struct kvm_stats_debugfs_item {
>   #define KVM_DBGFS_GET_MODE(dbgfs_item)                                         \
>   	((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644)
>   
> -#define VM_STAT(n, x, ...) 							\
> +#define VM_STAT(n, x, ...)						       \
>   	{ n, offsetof(struct kvm, stat.x), KVM_STAT_VM, ## __VA_ARGS__ }
> -#define VCPU_STAT(n, x, ...)							\
> +#define VCPU_STAT(n, x, ...)						       \
>   	{ n, offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU, ## __VA_ARGS__ }
> +#define VM_STAT_GENERIC(n, x, ...)					       \
> +	{ n, offsetof(struct kvm, stat.generic.x), KVM_STAT_VM, ## __VA_ARGS__ }
> +#define VCPU_STAT_GENERIC(n, x, ...)					       \
> +	{ n, offsetof(struct kvm_vcpu, stat.generic.x),			       \
> +	  KVM_STAT_VCPU, ## __VA_ARGS__ }
>   
>   extern struct kvm_stats_debugfs_item debugfs_entries[];
>   extern struct dentry *kvm_debugfs_dir;
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index a7580f69dda0..7c39489f9953 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -76,5 +76,17 @@ struct kvm_mmu_memory_cache {
>   };
>   #endif
>   
> +struct kvm_vm_stat_generic {
> +	ulong remote_tlb_flush;
> +};
> +
> +struct kvm_vcpu_stat_generic {
> +	u64 halt_successful_poll;
> +	u64 halt_attempted_poll;
> +	u64 halt_poll_invalid;
> +	u64 halt_wakeup;
> +	u64 halt_poll_success_ns;
> +	u64 halt_poll_fail_ns;
> +};
>   
>   #endif /* __KVM_TYPES_H__ */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6b4feb92dc79..f6ad5b080994 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -330,7 +330,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
>   	 */
>   	if (!kvm_arch_flush_remote_tlb(kvm)
>   	    || kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
> -		++kvm->stat.remote_tlb_flush;
> +		++kvm->stat.generic.remote_tlb_flush;
>   	cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
>   }
>   EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
> @@ -2940,9 +2940,9 @@ static inline void
>   update_halt_poll_stats(struct kvm_vcpu *vcpu, u64 poll_ns, bool waited)
>   {
>   	if (waited)
> -		vcpu->stat.halt_poll_fail_ns += poll_ns;
> +		vcpu->stat.generic.halt_poll_fail_ns += poll_ns;
>   	else
> -		vcpu->stat.halt_poll_success_ns += poll_ns;
> +		vcpu->stat.generic.halt_poll_success_ns += poll_ns;
>   }
>   
>   /*
> @@ -2960,16 +2960,16 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
>   	if (vcpu->halt_poll_ns && !kvm_arch_no_poll(vcpu)) {
>   		ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns);
>   
> -		++vcpu->stat.halt_attempted_poll;
> +		++vcpu->stat.generic.halt_attempted_poll;
>   		do {
>   			/*
>   			 * This sets KVM_REQ_UNHALT if an interrupt
>   			 * arrives.
>   			 */
>   			if (kvm_vcpu_check_block(vcpu) < 0) {
> -				++vcpu->stat.halt_successful_poll;
> +				++vcpu->stat.generic.halt_successful_poll;
>   				if (!vcpu_valid_wakeup(vcpu))
> -					++vcpu->stat.halt_poll_invalid;
> +					++vcpu->stat.generic.halt_poll_invalid;
>   				goto out;
>   			}
>   			poll_end = cur = ktime_get();
> @@ -3027,7 +3027,7 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
>   	waitp = kvm_arch_vcpu_get_wait(vcpu);
>   	if (rcuwait_wake_up(waitp)) {
>   		WRITE_ONCE(vcpu->ready, true);
> -		++vcpu->stat.halt_wakeup;
> +		++vcpu->stat.generic.halt_wakeup;
>   		return true;
>   	}
>   

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

On 03.06.21 23:14, Jing Zhang wrote:
> Put all generic statistics in a separate structure to ease
> statistics handling for the incoming new statistics API.
> 
> No functional change intended.
> 
> Reviewed-by: David Matlack <dmatlack@google.com>
> Reviewed-by: Ricardo Koller <ricarkol@google.com>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
[...]
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 8925f3969478..9b4473f76e56 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
[...]
> @@ -755,12 +750,12 @@ struct kvm_vcpu_arch {
>   };
>   
>   struct kvm_vm_stat {
> +	struct kvm_vm_stat_generic generic;

s390 does not have remote_tlb_flush. I guess this does not hurt?

>   	u64 inject_io;
>   	u64 inject_float_mchk;
>   	u64 inject_pfault_done;
>   	u64 inject_service_signal;
>   	u64 inject_virtio;
> -	u64 remote_tlb_flush;
>   };
[...]
> +struct kvm_vm_stat_generic {
> +	ulong remote_tlb_flush;
> +};

On 11/06/21 08:57, Christian Borntraeger wrote:
>> @@ -755,12 +750,12 @@ struct kvm_vcpu_arch {
>>   };
>>   struct kvm_vm_stat {
>> +    struct kvm_vm_stat_generic generic;
> 
> s390 does not have remote_tlb_flush. I guess this does not hurt?

It would have to be accounted in gmap_flush_tlb, but there is no struct 
kvm in there.  A slightly hackish possibility would be to include the 
gmap by value (instead of by pointer) in struct kvm, and then use 
container_of.

This reminds me that I have never asked you why the gmap code is not in 
arch/s390/kvm, and also that there is no code in QEMU that uses 
KVM_VM_S390_UCONTROL or KVM_S390_VCPU_FAULT.  It would be nice to have 
some testcases for that, and also for KVM_S390_VCPU_FAULT with regular 
virtual machines... or to remove the code if it's unused.

Thanks,

Paolo

On 11.06.21 12:51, Paolo Bonzini wrote:
> On 11/06/21 08:57, Christian Borntraeger wrote:
>>> @@ -755,12 +750,12 @@ struct kvm_vcpu_arch {
>>>   };
>>>   struct kvm_vm_stat {
>>> +    struct kvm_vm_stat_generic generic;
>>
>> s390 does not have remote_tlb_flush. I guess this does not hurt?
> 
> It would have to be accounted in gmap_flush_tlb, but there is no struct kvm in there.  A slightly hackish possibility would be to include the gmap by value (instead of by pointer) in struct kvm, and then use container_of.

What is the semantics of remote_tlb_flush?
For the host:
We usually do not do remote TLB flushes in the "IPI with a flush executed on the remote CPU" sense.
We do have instructions that invalidates table entries and it will take care of remote TLBs as well (IPTE and IDTE and CRDTE).
This is nice, but on the other side an operating system MUST use these instruction if the page table might be in use by any CPU. If not, you can get a delayed access exception machine check if the hardware detect a TLB/page table incosistency.
Only if you can guarantee that nobody has this page table attached you can also use a normal store + tlb flush instruction.

For the guest (and I guess thats what we care about here?) TLB flushes are almost completely handled by hardware. There is only the set prefix instruction that is handled by KVM and this flushes the cpu local cache.
> 
> This reminds me that I have never asked you why the gmap code is not in arch/s390/kvm, 

Because we share the last level of the page tables with userspace so the KVM address space is somewhat tied to the user address space.
This is partly because Martin wanted to have control over this due to some oddities about our page tables and partly because of the rule from above. Using a IPTE of such a page table entry will take care of the TLB entries for both (user and guest) mappings in an atomic fashion when the page table changes.


and also that there is no code in QEMU that uses KVM_VM_S390_UCONTROL or KVM_S390_VCPU_FAULT.  It would be nice to have some testcases for that, and also for KVM_S390_VCPU_FAULT with regular virtual machines... or to remove the code if it's unused.

This is used by an internal firmware test tool that uses KVM to speed up simulation of hardware instructions.
Search for CECSIM to get an idea (the existing papers still talk about the same approach using z/VM).
I will check what we can do regarding regression tests.

On 11/06/21 14:00, Christian Borntraeger wrote:
> What is the semantics of remote_tlb_flush?

I always interpreted it as "number of times the KVM page table 
management code needed other CPUs to learn about new page tables". 
Whether the broadcast is done in software or hardware shouldn't matter; 
either way I suppose there is still some traffic on the bus involved.

ARM is also not updating the stat, I'll send a patch for that.

Paolo

> For the host:
> We usually do not do remote TLB flushes in the "IPI with a flush 
> executed on the remote CPU" sense.
> We do have instructions that invalidates table entries and it will take 
> care of remote TLBs as well (IPTE and IDTE and CRDTE).
> This is nice, but on the other side an operating system MUST use these 
> instruction if the page table might be in use by any CPU. If not, you 
> can get a delayed access exception machine check if the hardware detect 
> a TLB/page table incosistency.
> Only if you can guarantee that nobody has this page table attached you 
> can also use a normal store + tlb flush instruction.
> 
> For the guest (and I guess thats what we care about here?) TLB flushes 
> are almost completely handled by hardware. There is only the set prefix 
> instruction that is handled by KVM and this flushes the cpu local cache.

On 11.06.21 14:03, Paolo Bonzini wrote:
> On 11/06/21 14:00, Christian Borntraeger wrote:
>> What is the semantics of remote_tlb_flush?
> 
> I always interpreted it as "number of times the KVM page table management code needed other CPUs to learn about new page tables". Whether the broadcast is done in software or hardware shouldn't matter; either way I suppose there is still some traffic on the bus involved.


My point is that KVM page table management on s390x completely piggy-backs on the qemu address space page table management from common code for the last level.
And due to the way we handle page tables we also do not teach "other CPUs". We always teach the whole system with things like IPTE.

> 
> ARM is also not updating the stat, I'll send a patch for that.
> 
> Paolo
> 
>> For the host:
>> We usually do not do remote TLB flushes in the "IPI with a flush executed on the remote CPU" sense.
>> We do have instructions that invalidates table entries and it will take care of remote TLBs as well (IPTE and IDTE and CRDTE).
>> This is nice, but on the other side an operating system MUST use these instruction if the page table might be in use by any CPU. If not, you can get a delayed access exception machine check if the hardware detect a TLB/page table incosistency.
>> Only if you can guarantee that nobody has this page table attached you can also use a normal store + tlb flush instruction.
>>
>> For the guest (and I guess thats what we care about here?) TLB flushes are almost completely handled by hardware. There is only the set prefix instruction that is handled by KVM and this flushes the cpu local cache.
>

On 11/06/21 14:08, Christian Borntraeger wrote:
>>
>> I always interpreted it as "number of times the KVM page table 
>> management code needed other CPUs to learn about new page tables". 
>> Whether the broadcast is done in software or hardware shouldn't 
>> matter; either way I suppose there is still some traffic on the bus 
>> involved.
> 
> 
> My point is that KVM page table management on s390x completely 
> piggy-backs on the qemu address space page table management from common 
> code for the last level.
> And due to the way we handle page tables we also do not teach "other 
> CPUs". We always teach the whole system with things like IPTE.

But that just means that you'll have fewer KVM-exclusive and thus nicer 
numbers than x86 or ARM. :)  It still makes sense to count 
gmap_flush_tlb calls.

Paolo