From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] kvm/ia64: Add printk support for kvm-intel modules. Date: Wed, 10 Sep 2008 18:19:56 +0300 Message-ID: <48C7E59C.2020000@qumranet.com> References: <42DFA526FC41B1429CE7279EF83C6BDC0196D6BB@pdsmsx415.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm-ia64@vger.kernel.org, kvm@vger.kernel.org To: "Zhang, Xiantao" Return-path: Received: from il.qumranet.com ([212.179.150.194]:17185 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752121AbYIJPT6 (ORCPT ); Wed, 10 Sep 2008 11:19:58 -0400 In-Reply-To: <42DFA526FC41B1429CE7279EF83C6BDC0196D6BB@pdsmsx415.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 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