From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 4/6] KVM: in-kernel LAPIC save and restore support Date: Fri, 03 Aug 2007 19:43:52 +0300 Message-ID: <46B35B48.9090907@qumranet.com> References: <37E52D09333DE2469A03574C88DBF40F048EE4@pdsmsx414.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: "He, Qing" Return-path: In-Reply-To: <37E52D09333DE2469A03574C88DBF40F048EE4-wq7ZOvIWXbM/UvCtAeCM4rfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org He, Qing wrote: > KVM: in-kernel LAPIC save and restore support > > This patch adds a new vcpu-based IOCTL to save and restore the local > apic > registers for a single vcpu. The kernel only copies the apic page as > a whole, > extraction of registers is left to userspace side. On restore, the > APIC timer > is restarted from the initial count, this introduces a little delay, > but works > fine > > Signed-off-by: Yaozu (Eddie) Dong > Signed-off-by: Qing He > > static long kvm_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -2805,6 +2826,31 @@ static long kvm_vcpu_ioctl(struct file *filp, > r = 0; > break; > } > + case KVM_GET_LAPIC: { > + struct kvm_lapic_state lapic; > + > + memset(&lapic, 0, sizeof lapic); > + r = kvm_vcpu_ioctl_get_lapic(vcpu, &lapic); > + if (r) > + goto out; > + r = -EFAULT; > + if (copy_to_user(argp, &lapic, sizeof lapic)) > + goto out; > + r = 0; > + break; > + } > > +/* for KVM_GET_LAPIC and KVM_SET_LAPIC */ > +#define KVM_APIC_REG_SIZE 0x400 > +struct kvm_lapic_state { > + char regs[KVM_APIC_REG_SIZE]; > +}; > + > kvm_lapic_state is 1KB in size, and you are allocating it on the stack. On i386, the stack can be 4KB, and you're allocating 25% of it... While it's true that this code path is short and can't be compounded with others like I/O code paths, still it's not a good idea to allocate so much stack space. I suggest defining kvm_lapic_state as #define KVM_LAPIC_NR_REGS 0x40 struct kvm_lapic_state { u32 regs[KVM_LAPIC_NR_REGS]; }; That reduces the state size to 256 bytes. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/