Re: [PATCH v3 1/8] ARM: KVM: Initial skeleton to compile KVM support

[Avi Kivity <avi@redhat.com>]

On 06/03/2011 06:03 PM, Christoffer Dall wrote:
> Targets KVM support for Cortex A-15 processors.
>
> Contains no real functionality but all the framework components,
> make files, header files and some tracing functionality.
>
> +
> +struct kvm_regs {
> +	__u32 regs0_7[8];	/* Unbanked regs. (r0 - r7)	   */
> +	__u32 fiq_regs8_12[5];	/* Banked fiq regs. (r8 - r12)	   */
> +	__u32 usr_regs8_12[5];	/* Banked usr registers (r8 - r12) */
> +	__u32 reg13[6];		/* Banked r13, indexed by MODE_	   */
> +	__u32 reg14[6];		/* Banked r13, indexed by MODE_	   */
> +	__u32 reg15;
> +	__u32 cpsr;
> +	__u32 spsr[5];		/* Banked SPSR,  indexed by MODE_  */
> +	struct {
> +		__u32 c2_base0;
> +		__u32 c2_base1;
> +		__u32 c3_dacr;
> +	} cp15;
> +
> +};
> +
> +struct kvm_sregs {
> +};
> +
> +struct kvm_fpu {
> +};
> +
> +struct kvm_guest_debug_arch {
> +};
> +
> +struct kvm_debug_exit_arch {
> +};

Presumably, to be filled in later?

> +
> +/* Get vcpu register for current mode */
> +#define vcpu_reg(_vcpu, _reg_num) \
> +	(*kvm_vcpu_reg((_vcpu), _reg_num, vcpu_mode(_vcpu)))
> +
> +/* Get vcpu register for specific mode */
> +#define vcpu_reg_m(_vcpu, _reg_num, _mode) \
> +	(*kvm_vcpu_reg(_vcpu, _reg_num, _mode))
> +
> +#define vcpu_cpsr(_vcpu) \
> +	(_vcpu->arch.regs.cpsr)
> +
> +/* Get vcpu SPSR for current mode */
> +#define vcpu_spsr(_vcpu) \
> +	kvm_vcpu_spsr(_vcpu, vcpu_mode(_vcpu))
> +
> +/* Get vcpu SPSR for specific mode */
> +#define vcpu_spsr_m(_vcpu, _mode) \
> +	kvm_vcpu_spsr(_vcpu, _mode)
> +
> +#define MODE_HAS_SPSR(_vcpu) \
> +	 ((vcpu_mode(_vcpu))<  MODE_USR)
> +
> +#define VCPU_MODE_PRIV(_vcpu) \
> +	(((vcpu_mode(_vcpu)) == MODE_USR) ? 0 : 1)

Please use static inlines.  Yes, you'll need more helpers to set 
registers, but it's worth it, especially as some macros evaluate an 
argument multiple times.

> +if VIRTUALIZATION
> +
> +config KVM
> +	bool "Kernel-based Virtual Machine (KVM) support"
> +	select PREEMPT_NOTIFIERS
> +	select ANON_INODES
> +	select KVM_ARM_HOST
> +	select KVM_MMIO
> +	---help---
> +	  Support hosting virtualized guest machines. You will also
> +	  need to select one or more of the processor modules below.
> +
> +	  This module provides access to the hardware capabilities through
> +	  a character device node named /dev/kvm.
> +
> +	  If unsure, say N.

I see you can't support a modular build, which is a pity.

> +
> +static int k_show(struct seq_file *m, void *v)
> +{
> +	print_kvm_debug_info(&seq_printf, m);
> +	return 0;
> +}
> +
> +static void *k_start(struct seq_file *m, loff_t *pos)
> +{
> +	return *pos<  1 ? (void *)1 : NULL;
> +}
> +
> +static void *k_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> +	++*pos;
> +	return NULL;
> +}
> +
> +static void k_stop(struct seq_file *m, void *v)
> +{
> +}
> +
> +static const struct seq_operations kvmproc_op = {
> +	.start	= k_start,
> +	.next	= k_next,
> +	.stop	= k_stop,
> +	.show	= k_show
> +};
> +
> +static int kvm_open(struct inode *inode, struct file *file)
> +{
> +	return seq_open(file,&kvmproc_op);
> +}
> +
> +static const struct file_operations proc_kvm_operations = {
> +	.open		= kvm_open,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= seq_release,
> +};
> +
> +static int arm_init(void)
> +{
> +	int rc = kvm_init(NULL, sizeof(struct kvm_vcpu), 0, THIS_MODULE);
> +	if (rc == 0)
> +		proc_create("kvm", 0, NULL,&proc_kvm_operations);
> +	return rc;
> +}

/proc is frowned upon these days.  Is there no better place for this?+
> +/*
> + * Return a pointer to the register number valid in the specified mode of
> + * the virtual CPU.
> + */
> +u32* kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode)
> +{
> +	struct kvm_vcpu_regs *regs;
> +	u8 reg_idx;
> +	BUG_ON(reg_num>  15);
> +
> +	regs =&vcpu->arch.regs;
> +
> +	/* The PC is trivial */
> +	if (reg_num == 15)
> +		return&(regs->pc);
> +
> +	/* Non-banked registers */
> +	if (reg_num<  8)
> +		return&(regs->usr_regs[reg_num]);
> +
> +	/* Banked registers r13 and r14 */
> +	if (reg_num>= 13) {
> +		reg_idx = reg_num - 13; /* 0=r13 and 1=r14 */
> +		switch (mode) {
> +		case MODE_FIQ:
> +			return&(regs->fiq_regs[reg_idx + 5]);
> +		case MODE_IRQ:
> +			return&(regs->irq_regs[reg_idx]);
> +		case MODE_SVC:
> +			return&(regs->svc_regs[reg_idx]);
> +		case MODE_ABT:
> +			return&(regs->abt_regs[reg_idx]);
> +		case MODE_UND:
> +			return&(regs->und_regs[reg_idx]);
> +		case MODE_USR:
> +		case MODE_SYS:
> +			return&(regs->usr_regs[reg_idx]);
> +		}
> +	}
> +
> +	/* Banked FIQ registers r8-r12 */
> +	if (reg_num>= 8&&  reg_num<= 12) {
> +		if (mode == MODE_FIQ) {
> +			reg_idx = reg_num - 8; /* 0=r8, ..., 4=r12 */
> +			return&(regs->fiq_regs[reg_idx]);
> +		} else
> +			return&(regs->usr_regs[reg_num]);
> +	}

You could have a static 2D array indexed by mode and register number, 
returning an offsetof() into the vcpu structure.

> +
> +	BUG();
> +	return NULL;
> +}
>
> diff --git a/arch/arm/kvm/trace.c b/arch/arm/kvm/trace.c
> new file mode 100644
> index 0000000..8ea1155
> --- /dev/null
> +++ b/arch/arm/kvm/trace.c
> @@ -0,0 +1,436 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + */
> +#include<linux/types.h>
> +#include<linux/kvm_types.h>
> +#include<linux/kvm_host.h>
> +
> +#include<asm/kvm_emulate.h>
> +#include "trace.h"
> +
> +
> +/******************************************************************************
> + * Simple event counting
> + */
> +
> +struct kvm_event {
> +	unsigned long long cnt;
> +	char *descr;
> +};
> +
> +static struct kvm_event kvm_eventc_log[KVM_EVENTC_ITEMS] =
> +{
> +	{ 0, "switch to guest" },
> +	{ 0, "exit from guest" },
> +	{ 0, "Block VCPU" },
> +	{ 0, "Exit to QEMU for IRQ window" },
> +	{ 0, "Switch VCPU mode" },
> +	{ 0, "VCPU IRQs on" },
> +	{ 0, "VCPU IRQs off" },
> +	{ 0, "Wait-for-interrupts" },
> +	{ 0, "Flush shadow page table" },
> +	{ 0, "Virtual TTBR change" },
> +	{ 0, "Read guest page table entry" },
> +	{ 0, "Map GVA to GFN" },
> +	{ 0, "Virtual DACR change" },
> +	{ 0, "VCPU switch to privileged mode" },
> +	{ 0, "VCPU switch from privileged mode" },
> +	{ 0, "VCPU process ID registers change" },
> +	{ 0, "Emulate Load/Store with translation" },
> +	{ 0, "Emulate MRS" },
> +	{ 0, "Emulate MSR" },
> +	{ 0, "Emulate CPS" },
> +	{ 0, "Need reschedule in execution loop" },
> +	{ 0, "MCR 7,  5, 0 - Invalidate entire I-cache" },
> +	{ 0, "MCR 7,  5, 1 - Invalidate line in I-cache MVA" },
> +	{ 0, "MCR 7,  5, 2 - Invalidate line in I-cache set/way" },
> +	{ 0, "MCR 7,  5, 7 - Flush branch target cache - MVA" },
> +	{ 0, "MCR 7,  6, 0 - Invalidate entire data cache" },
> +	{ 0, "MCR 7,  6, 1 - Invalidate data cache line - MVA" },
> +	{ 0, "MCR 7,  6, 2 - Invalidate data cache line - set/way" },
> +	{ 0, "MCR 7,  7, 0 - Invalidate D- and I-cache" },
> +	{ 0, "MCR 7, 10, 0 - Clean entire data cache" },
> +	{ 0, "MCR 7, 10, 1 - Clean data cache line - MVA" },
> +	{ 0, "MCR 7, 10, 4 - Data Synchronization Barrier (DSB)" },
> +	{ 0, "MCR 7, 14, 0 - Clean and invalidate entire D-cache" },
> +	{ 0, "MCR 7, 14, 1 - Clean and invalidate D-cache line - MVA" },
> +	{ 0, "MCR 7, 15, 0 - Clean and invalidate unified cache" },
> +	{ 0, "MCR 8,  5, 0 - Invalidate instruction TLB" },
> +	{ 0, "MCR 8,  6, 0 - Invalidate data TLB" },
> +	{ 0, "MCR 8,  7, 0 - Invalidate unified TLB" },
> +	{ 0, "Emulate Load-Store multiple" },
> +};
> +
> +void kvm_arm_count_event(unsigned int event)
> +{
> +	if (event>= KVM_EVENTC_ITEMS)
> +		return;
> +
> +	kvm_eventc_log[event].cnt++;
> +}

We've switched to ftrace for this sort of thing.  Simply add a 
tracepoint for each interesting event, and the kernel can provide you with

- a count of events ('perf stat')
- a log of events ('trace-cmd record/report'), possibly with other 
kernel events interspersed
- a running histogram ('kvm_stat')

with near-zero impact when disabled.

See include/trace/events/kvm.h, arch/x86/kvm/trace.h.

-- 
error compiling committee.c: too many arguments to function