lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4BF2DB26.9090803@goop.org>
Date:	Tue, 18 May 2010 11:23:34 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Stefano Stabellini <stefano.stabellini@...citrix.com>
CC:	linux-kernel@...r.kernel.org, xen-devel@...ts.xensource.com,
	Don Dutile <ddutile@...hat.com>,
	Sheng Yang <sheng@...ux.intel.com>
Subject: Re: [PATCH 11/12] Support VIRQ_TIMER and pvclock on HVM

On 05/18/2010 03:23 AM, Stefano Stabellini wrote:

Please describe what you're doing, why its useful and how it works.

> Signed-off-by: Stefano Stabellini <stefano.stabellini@...citrix.com>
> ---
>  arch/x86/xen/enlighten.c         |   39 +++++++++++++++++++++++++++++++++++++-
>  arch/x86/xen/time.c              |    3 ++
>  drivers/xen/manage.c             |    1 +
>  include/xen/interface/features.h |    3 ++
>  4 files changed, 45 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 77ba321..41677fe 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1274,6 +1274,7 @@ void init_shared_info(void)
>  {
>  	struct xen_add_to_physmap xatp;
>  	static struct shared_info *shared_info_page = 0;
> +	int cpu;
>  
>  	if (!shared_info_page)
>  		shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
> @@ -1288,7 +1289,42 @@ void init_shared_info(void)
>  
>  	/* Don't do the full vcpu_info placement stuff until we have a
>  	   possible map and a non-dummy shared_info. */
> -	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> +	/* This code is run at resume time so make sure all the online cpus
> +	 * have xen_vcpu properly set */
>   

Why is this necessary? Can the vcpu structures move on resume?

> +	for_each_online_cpu(cpu)
> +		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
> +}
> +
> +static void xen_hvm_setup_cpu_clockevents(void)
> +{
> +	int cpu = smp_processor_id();
> +	xen_setup_timer(cpu);
> +	per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
> +	xen_setup_cpu_clockevents();
> +}
> +
> +static void init_hvm_time(void)
> +{
> +#ifdef CONFIG_SMP
> +	/* vector callback is needed otherwise we cannot receive interrupts
> +	 * on cpu > 0 */
> +	if (!xen_have_vector_callback)
> +		return;
>   

Putting this check in CONFIG_SMP is more or less pointless, since
effectively every kernel is built with SMP enabled now.  Can you either
check whether the max CPUs is 1, or just make it always depend on vector
callback, even on a UP kernel/domain?

> +#endif
> +	if (!xen_feature(XENFEAT_hvm_safe_pvclock)) {
> +		printk(KERN_WARNING "Xen doesn't support pvclock on HVM,"
> +				"disable pv timer\n");
>   

What does this mean?  Is it asking the user to do something?

> +		return;
> +	}
> +
> +	pv_time_ops = xen_time_ops;
> +	x86_init.timers.timer_init = xen_time_init;
> +	x86_init.timers.setup_percpu_clockev = x86_init_noop;
> +	x86_cpuinit.setup_percpu_clockev = xen_hvm_setup_cpu_clockevents;
> +
> +	x86_platform.calibrate_tsc = xen_tsc_khz;
> +	x86_platform.get_wallclock = xen_get_wallclock;
> +	x86_platform.set_wallclock = xen_set_wallclock;
>  }
>  
>  int xen_set_callback_via(uint64_t via)
> @@ -1373,6 +1409,7 @@ void __init xen_guest_init(void)
>  		outw(unplug, XEN_IOPORT_UNPLUG);
>  	have_vcpu_info_placement = 0;
>  	x86_init.irqs.intr_init = xen_init_IRQ;
> +	init_hvm_time();
>  }
>  
>  static int __init parse_unplug(char *arg)
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index 32764b8..620e68f 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -19,6 +19,7 @@
>  #include <asm/xen/hypervisor.h>
>  #include <asm/xen/hypercall.h>
>  
> +#include <xen/xen.h>
>  #include <xen/events.h>
>  #include <xen/interface/xen.h>
>  #include <xen/interface/vcpu.h>
> @@ -470,6 +471,8 @@ void xen_timer_resume(void)
>  	for_each_online_cpu(cpu) {
>  		if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL))
>  			BUG();
> +		if (xen_hvm_domain())
> +			xen_setup_runstate_info(cpu);
>  	}
>  }
>  
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 49ee52d..4a8af22 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -60,6 +60,7 @@ static int xen_hvm_suspend(void *data)
>  	if (!*cancelled) {
>  		xen_irq_resume();
>  		platform_pci_resume_hook();
> +		xen_timer_resume();
>  	}
>  
>  	return 0;
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index 8ab08b9..70d2563 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -44,6 +44,9 @@
>  /* x86: Does this Xen host support the HVM callback vector type? */
>  #define XENFEAT_hvm_callback_vector        8
>  
> +/* x86: pvclock algorithm is safe to use on HVM */
> +#define XENFEAT_hvm_safe_pvclock           9
>   

Why is this needed?  When is it not safe?

> +
>  #define XENFEAT_NR_SUBMAPS 1
>  
>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ