Re: [PATCH 4/6] KVM: in-kernel LAPIC save and restore support

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.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 <eddie.dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>     Signed-off-by: Qing He <qing.he-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
>  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/