On Sun, Jun 5, 2011 at 2:36 PM, Avi Kivity <avi at redhat.com> wrote:
> 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?
>
I simply didn't look at these yet and didn't need them yet. I will
look into this later on.
>> +
>> +/* 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.
ok.
>
>> +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.
>
My concern is that I map in code that needs to be in place for running
in Hyp mode. Of course I just need to pin these, so it should be
possible. I'll look into this as well.
>> +
>> +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?+
>>
Yeah, this is actually quite legacy and probably shouldn't have been
included. I will give this a thorough overhaul before next patch
series.
>> +/*
>> + * 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.
You think it's simpler or faster? I don't quite see the incentive.
It's not going to be called a whole lot given the Virt. Extensions.
>
>> +
>> + ? ? ? BUG();
>> + ? ? ? return NULL;
>> +}
>>
>> diff --git a/arch/arm/kvm/trace.c b/arch/arm/kvm/trace.c
>> new file mode 100644
>> index 0000000..8ea1155
[snip]
>> +
>> +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.
Will do, thanks.
>
> --
> error compiling committee.c: too many arguments to function
>
>
