Re: [PATCH] kvm/ia64: Add printk support for kvm-intel modules.

[Avi Kivity <avi@qumranet.com>]

Zhang, Xiantao wrote:
> Resend. 
> Avi, 
> 	Please help to apply it, Thanks! 
> Xiantao
>
>   

(sorry for the late review)

> Since this module will be reloated to an isolated address
> space from host side, so kvm-intel can't call printk of host
> kernel. This patch implements the printk function for kvm-intel
> module, so it doesn't need to suffer no-printk pains.
>
>  
> +#define BYTES_PER_LOG 1024
> +
> +struct kvm_vmm_log {
> +	spinlock_t log_lock;
> +	unsigned long w_pointer;
> +	unsigned long r_pointer;
> +	char log_slot[VMM_LOG_SIZE/BYTES_PER_LOG][BYTES_PER_LOG - 1];
> +};
> +
>   

1024 bytes per line?  that's wasteful.

Why not variable size records?

>  
> +static void vcpu_print_vmm_log(void)
> +{
> +	unsigned int slot;
> +
> +	spin_lock(&vmm_log->log_lock);
>   

You're going to impact scalability with this.  Are per-vcpu logs workable?

> @@ -1011,6 +1030,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
> *vcpu,
>  
>  static int kvm_alloc_vmm_area(void)
>  {
> +
>   

blank line?

> diff --git a/arch/ia64/kvm/kvm_lib.c b/arch/ia64/kvm/kvm_lib.c
> new file mode 100644
> index 0000000..4c43efe
> --- /dev/null
> +++ b/arch/ia64/kvm/kvm_lib.c
> @@ -0,0 +1,13 @@
> +
> +/*
> + * vsprintf.c: Let kvm-intel module has the ability to use print
> functions.
> + *	Just include kernel's library, and disable symbols export.
> + * 	Copyright (C) 2008, Intel Corporation.
> + *  	Xiantao Zhang  (xiantao.zhang@intel.com)
> + *
> + */
> +
> +#undef CONFIG_MODULES
> +
> +#include "../../../lib/vsprintf.c"
> +#include "../../../lib/ctype.c"
>   

I suspect this will start breaking when people start using the new 
printk("%pBLAH") functionality, which will require linking additional files.

Is it possible to pass the format string and the arguments in the log 
record instead, and do the printk() in the kernel?

I can't think of a way on x86, but maybe ia64 varargs are different.

(worst case you can limit the number of arguments and just copy a bunch 
of stack).

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