thr3ads.net - Xen devel - [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [Aug 2012]

If this information is useful, please help other people find it:
Share via:

Mukesh Rathor

2012-Aug-16 01:01 UTC

[[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

---
 arch/x86/xen/enlighten.c |   67 ++++++++++++++++++++++++++++++++++++++-------
 arch/x86/xen/irq.c       |   22 ++++++++++++++-
 2 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index bf4bda6..3a58c51 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -139,6 +139,8 @@ struct tls_descs {
  */
 static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
 
+static void __init xen_hvm_init_shared_info(void);
+
 static void clamp_max_cpus(void)
 {
 #ifdef CONFIG_SMP
@@ -217,8 +219,8 @@ static void __init xen_banner(void)
 	struct xen_extraversion extra;
 	HYPERVISOR_xen_version(XENVER_extraversion, &extra);
 
-	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
-	       pv_info.name);
+	printk(KERN_INFO "Booting paravirtualized kernel %son %s\n",
+		(xen_pvh_domain() ? "in HVM " : ""), pv_info.name);
 	printk(KERN_INFO "Xen version: %d.%d%s%s\n",
 	       version >> 16, version & 0xffff, extra.extraversion,
 	       xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? "
(preserve-AD)" : "");
@@ -271,12 +273,15 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 		break;
 	}
 
-	asm(XEN_EMULATE_PREFIX "cpuid"
-		: "=a" (*ax),
-		  "=b" (*bx),
-		  "=c" (*cx),
-		  "=d" (*dx)
-		: "0" (*ax), "2" (*cx));
+	if (xen_pvh_domain())
+		native_cpuid(ax, bx, cx, dx);
+	else
+		asm(XEN_EMULATE_PREFIX "cpuid"
+			: "=a" (*ax),
+			"=b" (*bx),
+			"=c" (*cx),
+			"=d" (*dx)
+			: "0" (*ax), "2" (*cx));
 
 	*bx &= maskebx;
 	*cx &= maskecx;
@@ -1034,6 +1039,10 @@ static int xen_write_msr_safe(unsigned int msr, unsigned
low, unsigned high)
 
 void xen_setup_shared_info(void)
 {
+	/* do later in xen_pvh_guest_init() when extend_brk is properly setup*/
+	if (xen_pvh_domain() && xen_initial_domain())
+		return;
+
 	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
 		set_fixmap(FIX_PARAVIRT_BOOTMAP,
 			   xen_start_info->shared_info);
@@ -1044,6 +1053,10 @@ void xen_setup_shared_info(void)
 		HYPERVISOR_shared_info  			(struct shared_info
*)__va(xen_start_info->shared_info);
 
+	/* PVH TBD/FIXME: vcpu info placement in phase 2 */
+	if (xen_pvh_domain())
+		return;
+
 #ifndef CONFIG_SMP
 	/* In UP this is as good a place as any to set up shared info */
 	xen_setup_vcpu_info_placement();
@@ -1274,6 +1287,10 @@ static const struct machine_ops xen_machine_ops
__initconst = {
  */
 static void __init xen_setup_stackprotector(void)
 {
+	if (xen_pvh_domain()) {
+		switch_to_new_gdt(0);
+		return;
+	}
 	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
 	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
 
@@ -1284,6 +1301,25 @@ static void __init xen_setup_stackprotector(void)
 	pv_cpu_ops.load_gdt = xen_load_gdt;
 }
 
+static void __init xen_pvh_guest_init(void)
+{
+#ifndef __HAVE_ARCH_PTE_SPECIAL
+	("__HAVE_ARCH_PTE_SPECIAL is required for PVH for now\n");
+	#error("__HAVE_ARCH_PTE_SPECIAL is required for PVH\n");
+#endif
+	/* PVH TBD/FIXME: for now just disable this. */
+	have_vcpu_info_placement = 0;
+
+	if (xen_feature(XENFEAT_hvm_callback_vector))
+		xen_have_vector_callback = 1;
+
+        /* for domU, the library sets start_info.shared_info to pfn, but for
+         * dom0, it contains mfn. we need to get the pfn for shared_info. PVH
+	 * uses HVM code in many places */
+	if (xen_initial_domain())
+		xen_hvm_init_shared_info();
+}
+
 /* First C function to be called on Xen boot */
 asmlinkage void __init xen_start_kernel(void)
 {
@@ -1294,15 +1330,23 @@ asmlinkage void __init xen_start_kernel(void)
 	if (!xen_start_info)
 		return;
 
+#ifdef CONFIG_X86_32
+	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
container\n");
+	return;
+#endif
 	xen_domain_type = XEN_PV_DOMAIN;
 
+	xen_setup_features();
 	xen_setup_machphys_mapping();
 
 	/* Install Xen paravirt ops */
 	pv_info = xen_info;
 	pv_init_ops = xen_init_ops;
-	pv_cpu_ops = xen_cpu_ops;
 	pv_apic_ops = xen_apic_ops;
+	if (xen_pvh_domain())
+		pv_cpu_ops.cpuid = xen_cpuid;
+	else
+		pv_cpu_ops = xen_cpu_ops;
 
 	x86_init.resources.memory_setup = xen_memory_setup;
 	x86_init.oem.arch_setup = xen_arch_setup;
@@ -1334,8 +1378,6 @@ asmlinkage void __init xen_start_kernel(void)
 	/* Work out if we support NX */
 	x86_configure_nx();
 
-	xen_setup_features();
-
 	/* Get mfn list */
 	if (!xen_feature(XENFEAT_auto_translated_physmap))
 		xen_build_dynamic_phys_to_machine();
@@ -1462,6 +1504,9 @@ asmlinkage void __init xen_start_kernel(void)
 
 	xen_setup_runstate_info(0);
 
+	if (xen_pvh_domain())
+		xen_pvh_guest_init();
+
 	/* Start the world */
 #ifdef CONFIG_X86_32
 	i386_start_kernel();
diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 1573376..7c7dfd1 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
 
 static void xen_safe_halt(void)
 {
+	/* so event channel can be delivered to us, since in HVM container */
+	if (xen_pvh_domain())
+		local_irq_enable();
+
 	/* Blocking includes an implicit local_irq_enable(). */
 	if (HYPERVISOR_sched_op(SCHEDOP_block, NULL) != 0)
 		BUG();
@@ -126,8 +130,24 @@ static const struct pv_irq_ops xen_irq_ops __initconst = {
 #endif
 };
 
+static const struct pv_irq_ops xen_pvh_irq_ops __initdata = {
+	.save_fl = __PV_IS_CALLEE_SAVE(native_save_fl),
+	.restore_fl = __PV_IS_CALLEE_SAVE(native_restore_fl),
+	.irq_disable = __PV_IS_CALLEE_SAVE(native_irq_disable),
+	.irq_enable = __PV_IS_CALLEE_SAVE(native_irq_enable),
+
+	.safe_halt = xen_safe_halt,
+	.halt = xen_halt,
+#ifdef CONFIG_X86_64
+	.adjust_exception_frame = paravirt_nop,
+#endif
+};
+
 void __init xen_init_irq_ops(void)
 {
-	pv_irq_ops = xen_irq_ops;
+	if (xen_pvh_domain())
+		pv_irq_ops = xen_pvh_irq_ops;
+	else
+		pv_irq_ops = xen_irq_ops;
 	x86_init.irqs.intr_init = xen_init_IRQ;
 }
-- 
1.7.2.3

Stefano Stabellini

2012-Aug-16 14:05 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, 16 Aug 2012, Mukesh Rathor wrote:> ---
>  arch/x86/xen/enlighten.c |   67
++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/irq.c       |   22 ++++++++++++++-
>  2 files changed, 77 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index bf4bda6..3a58c51 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -139,6 +139,8 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +static void __init xen_hvm_init_shared_info(void);
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -217,8 +219,8 @@ static void __init xen_banner(void)
>  	struct xen_extraversion extra;
>  	HYPERVISOR_xen_version(XENVER_extraversion, &extra);
>  
> -	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> -	       pv_info.name);
> +	printk(KERN_INFO "Booting paravirtualized kernel %son %s\n",
> +		(xen_pvh_domain() ? "in HVM " : ""), pv_info.name);
>  	printk(KERN_INFO "Xen version: %d.%d%s%s\n",
>  	       version >> 16, version & 0xffff, extra.extraversion,
>  	       xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? "
(preserve-AD)" : "");
> @@ -271,12 +273,15 @@ static void xen_cpuid(unsigned int *ax, unsigned int
*bx,
>  		break;
>  	}
>  
> -	asm(XEN_EMULATE_PREFIX "cpuid"
> -		: "=a" (*ax),
> -		  "=b" (*bx),
> -		  "=c" (*cx),
> -		  "=d" (*dx)
> -		: "0" (*ax), "2" (*cx));
> +	if (xen_pvh_domain())
> +		native_cpuid(ax, bx, cx, dx);
> +	else
> +		asm(XEN_EMULATE_PREFIX "cpuid"
> +			: "=a" (*ax),
> +			"=b" (*bx),
> +			"=c" (*cx),
> +			"=d" (*dx)
> +			: "0" (*ax), "2" (*cx));
>  
>  	*bx &= maskebx;
>  	*cx &= maskecx;
> @@ -1034,6 +1039,10 @@ static int xen_write_msr_safe(unsigned int msr,
unsigned low, unsigned high)
>  
>  void xen_setup_shared_info(void)
>  {
> +	/* do later in xen_pvh_guest_init() when extend_brk is properly setup*/
> +	if (xen_pvh_domain() && xen_initial_domain())
> +		return;
> +
>  	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
>  		set_fixmap(FIX_PARAVIRT_BOOTMAP,
>  			   xen_start_info->shared_info);
> @@ -1044,6 +1053,10 @@ void xen_setup_shared_info(void)
>  		HYPERVISOR_shared_info >  			(struct shared_info
*)__va(xen_start_info->shared_info);
>  
> +	/* PVH TBD/FIXME: vcpu info placement in phase 2 */
> +	if (xen_pvh_domain())
> +		return;
Is XENFEAT_auto_translated_physmap even believed to work?

If not, we could change the XENFEAT_auto_translated_physmap case and
make it point to xen_hvm_init_shared_info, that would work for pvh as
well.

>  #ifndef CONFIG_SMP
>  	/* In UP this is as good a place as any to set up shared info */
>  	xen_setup_vcpu_info_placement();
> @@ -1274,6 +1287,10 @@ static const struct machine_ops xen_machine_ops
__initconst = {
>   */
>  static void __init xen_setup_stackprotector(void)
>  {
> +	if (xen_pvh_domain()) {
> +		switch_to_new_gdt(0);
> +		return;
> +	}
>  	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
>  	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
>  
> @@ -1284,6 +1301,25 @@ static void __init xen_setup_stackprotector(void)
>  	pv_cpu_ops.load_gdt = xen_load_gdt;
>  }
>  
> +static void __init xen_pvh_guest_init(void)
> +{
> +#ifndef __HAVE_ARCH_PTE_SPECIAL
> +	("__HAVE_ARCH_PTE_SPECIAL is required for PVH for now\n");
> +	#error("__HAVE_ARCH_PTE_SPECIAL is required for PVH\n");
> +#endif
> +	/* PVH TBD/FIXME: for now just disable this. */
> +	have_vcpu_info_placement = 0;
> +
> +	if (xen_feature(XENFEAT_hvm_callback_vector))
> +		xen_have_vector_callback = 1;
> +
> +        /* for domU, the library sets start_info.shared_info to pfn, but
for
> +         * dom0, it contains mfn. we need to get the pfn for shared_info.
PVH
> +	 * uses HVM code in many places */
> +	if (xen_initial_domain())
> +		xen_hvm_init_shared_info();
> +}
> +
>  /* First C function to be called on Xen boot */
>  asmlinkage void __init xen_start_kernel(void)
>  {
> @@ -1294,15 +1330,23 @@ asmlinkage void __init xen_start_kernel(void)
>  	if (!xen_start_info)
>  		return;
>  
> +#ifdef CONFIG_X86_32
> +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
container\n");
> +	return;
> +#endif
this has to be wrong: shouldn''t the return be at least below the
following line?

>  	xen_domain_type = XEN_PV_DOMAIN;
>  
> +	xen_setup_features();
>  	xen_setup_machphys_mapping();
>  
>  	/* Install Xen paravirt ops */
>  	pv_info = xen_info;
>  	pv_init_ops = xen_init_ops;
> -	pv_cpu_ops = xen_cpu_ops;
>  	pv_apic_ops = xen_apic_ops;
> +	if (xen_pvh_domain())
> +		pv_cpu_ops.cpuid = xen_cpuid;
> +	else
> +		pv_cpu_ops = xen_cpu_ops;
>  
>  	x86_init.resources.memory_setup = xen_memory_setup;
>  	x86_init.oem.arch_setup = xen_arch_setup;
> @@ -1334,8 +1378,6 @@ asmlinkage void __init xen_start_kernel(void)
>  	/* Work out if we support NX */
>  	x86_configure_nx();
>  
> -	xen_setup_features();
> -
>  	/* Get mfn list */
>  	if (!xen_feature(XENFEAT_auto_translated_physmap))
>  		xen_build_dynamic_phys_to_machine();
> @@ -1462,6 +1504,9 @@ asmlinkage void __init xen_start_kernel(void)
>  
>  	xen_setup_runstate_info(0);
>  
> +	if (xen_pvh_domain())
> +		xen_pvh_guest_init();
> +
>  	/* Start the world */
>  #ifdef CONFIG_X86_32
>  	i386_start_kernel();
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 1573376..7c7dfd1 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
>  
>  static void xen_safe_halt(void)
>  {
> +	/* so event channel can be delivered to us, since in HVM container */
> +	if (xen_pvh_domain())
> +		local_irq_enable();
> +
>  	/* Blocking includes an implicit local_irq_enable(). */
>  	if (HYPERVISOR_sched_op(SCHEDOP_block, NULL) != 0)
>  		BUG();
> @@ -126,8 +130,24 @@ static const struct pv_irq_ops xen_irq_ops __initconst
= {
>  #endif
>  };
>  
> +static const struct pv_irq_ops xen_pvh_irq_ops __initdata = {
> +	.save_fl = __PV_IS_CALLEE_SAVE(native_save_fl),
> +	.restore_fl = __PV_IS_CALLEE_SAVE(native_restore_fl),
> +	.irq_disable = __PV_IS_CALLEE_SAVE(native_irq_disable),
> +	.irq_enable = __PV_IS_CALLEE_SAVE(native_irq_enable),
> +
> +	.safe_halt = xen_safe_halt,
> +	.halt = xen_halt,
> +#ifdef CONFIG_X86_64
> +	.adjust_exception_frame = paravirt_nop,
> +#endif
> +};
> +
>  void __init xen_init_irq_ops(void)
>  {
> -	pv_irq_ops = xen_irq_ops;
> +	if (xen_pvh_domain())
> +		pv_irq_ops = xen_pvh_irq_ops;
> +	else
> +		pv_irq_ops = xen_irq_ops;
>  	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

Konrad Rzeszutek Wilk

2012-Aug-16 14:09 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Wed, Aug 15, 2012 at 06:01:31PM -0700, Mukesh Rathor
wrote:> 
> ---
>  arch/x86/xen/enlighten.c |   67
++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/irq.c       |   22 ++++++++++++++-
>  2 files changed, 77 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index bf4bda6..3a58c51 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -139,6 +139,8 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +static void __init xen_hvm_init_shared_info(void);
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -217,8 +219,8 @@ static void __init xen_banner(void)
>  	struct xen_extraversion extra;
>  	HYPERVISOR_xen_version(XENVER_extraversion, &extra);
>  
> -	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> -	       pv_info.name);
> +	printk(KERN_INFO "Booting paravirtualized kernel %son %s\n",
> +		(xen_pvh_domain() ? "in HVM " : ""), pv_info.name);
>  	printk(KERN_INFO "Xen version: %d.%d%s%s\n",
>  	       version >> 16, version & 0xffff, extra.extraversion,
>  	       xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? "
(preserve-AD)" : "");
> @@ -271,12 +273,15 @@ static void xen_cpuid(unsigned int *ax, unsigned int
*bx,
>  		break;
>  	}
>  
> -	asm(XEN_EMULATE_PREFIX "cpuid"
> -		: "=a" (*ax),
> -		  "=b" (*bx),
> -		  "=c" (*cx),
> -		  "=d" (*dx)
> -		: "0" (*ax), "2" (*cx));
> +	if (xen_pvh_domain())
> +		native_cpuid(ax, bx, cx, dx);
> +	else
> +		asm(XEN_EMULATE_PREFIX "cpuid"
> +			: "=a" (*ax),
> +			"=b" (*bx),
> +			"=c" (*cx),
> +			"=d" (*dx)
> +			: "0" (*ax), "2" (*cx));
>  
>  	*bx &= maskebx;
>  	*cx &= maskecx;
> @@ -1034,6 +1039,10 @@ static int xen_write_msr_safe(unsigned int msr,
unsigned low, unsigned high)
>  
>  void xen_setup_shared_info(void)
>  {
> +	/* do later in xen_pvh_guest_init() when extend_brk is properly setup*/
> +	if (xen_pvh_domain() && xen_initial_domain())
> +		return;
> +
>  	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
>  		set_fixmap(FIX_PARAVIRT_BOOTMAP,
>  			   xen_start_info->shared_info);
> @@ -1044,6 +1053,10 @@ void xen_setup_shared_info(void)
>  		HYPERVISOR_shared_info >  			(struct shared_info
*)__va(xen_start_info->shared_info);
>  
> +	/* PVH TBD/FIXME: vcpu info placement in phase 2 */
> +	if (xen_pvh_domain())
> +		return;
> +
>  #ifndef CONFIG_SMP
>  	/* In UP this is as good a place as any to set up shared info */
>  	xen_setup_vcpu_info_placement();
> @@ -1274,6 +1287,10 @@ static const struct machine_ops xen_machine_ops
__initconst = {
>   */
>  static void __init xen_setup_stackprotector(void)
>  {
> +	if (xen_pvh_domain()) {
> +		switch_to_new_gdt(0);
> +		return;
> +	}
>  	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
>  	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
>  
> @@ -1284,6 +1301,25 @@ static void __init xen_setup_stackprotector(void)
>  	pv_cpu_ops.load_gdt = xen_load_gdt;
>  }
>  
> +static void __init xen_pvh_guest_init(void)
> +{
> +#ifndef __HAVE_ARCH_PTE_SPECIAL
> +	("__HAVE_ARCH_PTE_SPECIAL is required for PVH for now\n");
> +	#error("__HAVE_ARCH_PTE_SPECIAL is required for PVH\n");
> +#endif
Should this be just rolled in the Kconfig as part of enabling
this mode?
> +	/* PVH TBD/FIXME: for now just disable this. */
> +	have_vcpu_info_placement = 0;
> +
> +	if (xen_feature(XENFEAT_hvm_callback_vector))
> +		xen_have_vector_callback = 1;
> +
> +        /* for domU, the library sets start_info.shared_info to pfn, but
for
> +         * dom0, it contains mfn. we need to get the pfn for shared_info.
PVH
> +	 * uses HVM code in many places */
> +	if (xen_initial_domain())
> +		xen_hvm_init_shared_info();
> +}
> +
>  /* First C function to be called on Xen boot */
>  asmlinkage void __init xen_start_kernel(void)
>  {
> @@ -1294,15 +1330,23 @@ asmlinkage void __init xen_start_kernel(void)
>  	if (!xen_start_info)
>  		return;
>  
> +#ifdef CONFIG_X86_32
> +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
container\n");
> +	return;
Perhaps BUG() ?> +#endif
>  	xen_domain_type = XEN_PV_DOMAIN;
>  
> +	xen_setup_features();
>  	xen_setup_machphys_mapping();
>  
>  	/* Install Xen paravirt ops */
>  	pv_info = xen_info;
>  	pv_init_ops = xen_init_ops;
> -	pv_cpu_ops = xen_cpu_ops;
>  	pv_apic_ops = xen_apic_ops;
> +	if (xen_pvh_domain())
> +		pv_cpu_ops.cpuid = xen_cpuid;
> +	else
> +		pv_cpu_ops = xen_cpu_ops;
>  
>  	x86_init.resources.memory_setup = xen_memory_setup;
>  	x86_init.oem.arch_setup = xen_arch_setup;
> @@ -1334,8 +1378,6 @@ asmlinkage void __init xen_start_kernel(void)
>  	/* Work out if we support NX */
>  	x86_configure_nx();
>  
> -	xen_setup_features();
> -
>  	/* Get mfn list */
>  	if (!xen_feature(XENFEAT_auto_translated_physmap))
>  		xen_build_dynamic_phys_to_machine();
> @@ -1462,6 +1504,9 @@ asmlinkage void __init xen_start_kernel(void)
>  
>  	xen_setup_runstate_info(0);
>  
> +	if (xen_pvh_domain())
> +		xen_pvh_guest_init();
> +
>  	/* Start the world */
>  #ifdef CONFIG_X86_32
>  	i386_start_kernel();
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 1573376..7c7dfd1 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
>  
>  static void xen_safe_halt(void)
>  {
> +	/* so event channel can be delivered to us, since in HVM container */
> +	if (xen_pvh_domain())
> +		local_irq_enable();
> +
>  	/* Blocking includes an implicit local_irq_enable(). */
>  	if (HYPERVISOR_sched_op(SCHEDOP_block, NULL) != 0)
>  		BUG();
> @@ -126,8 +130,24 @@ static const struct pv_irq_ops xen_irq_ops __initconst
= {
>  #endif
>  };
>  
> +static const struct pv_irq_ops xen_pvh_irq_ops __initdata = {
> +	.save_fl = __PV_IS_CALLEE_SAVE(native_save_fl),
> +	.restore_fl = __PV_IS_CALLEE_SAVE(native_restore_fl),
> +	.irq_disable = __PV_IS_CALLEE_SAVE(native_irq_disable),
> +	.irq_enable = __PV_IS_CALLEE_SAVE(native_irq_enable),
> +
> +	.safe_halt = xen_safe_halt,
> +	.halt = xen_halt,
> +#ifdef CONFIG_X86_64
> +	.adjust_exception_frame = paravirt_nop,
> +#endif
> +};
> +
>  void __init xen_init_irq_ops(void)
>  {
> -	pv_irq_ops = xen_irq_ops;
> +	if (xen_pvh_domain())
> +		pv_irq_ops = xen_pvh_irq_ops;
> +	else
> +		pv_irq_ops = xen_irq_ops;
>  	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> -- 
> 1.7.2.3

Mukesh Rathor

2012-Aug-16 18:56 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, 16 Aug 2012 15:05:27 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 16 Aug 2012, Mukesh Rathor wrote:
> > ---
> >  arch/x86/xen/enlighten.c |   67
> > +
> >  	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> >  		set_fixmap(FIX_PARAVIRT_BOOTMAP,
> >  			   xen_start_info->shared_info);
> > @@ -1044,6 +1053,10 @@ void xen_setup_shared_info(void)
> >  		HYPERVISOR_shared_info > >  			(struct shared_info
> > *)__va(xen_start_info->shared_info); 
> > +	/* PVH TBD/FIXME: vcpu info placement in phase 2 */
> > +	if (xen_pvh_domain())
> > +		return;
> 
> Is XENFEAT_auto_translated_physmap even believed to work?
> 
> If not, we could change the XENFEAT_auto_translated_physmap case and
> make it point to xen_hvm_init_shared_info, that would work for pvh as
> well.
Ok, I''ll open an action item on this, and open a bug as soon as patch
is in to investigate and fix it.
> > +#ifdef CONFIG_X86_32
> > +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
> > container\n");
> > +	return;
> > +#endif
> 
> this has to be wrong: shouldn''t the return be at least below the
> following line?
Ooopsy, fixing it. Whic is why, my ex-manager at IBM used to say, "it
ain''t done till it''s tested!" :) :).

thanks,
Mukesh

Mukesh Rathor

2012-Aug-16 19:00 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, 16 Aug 2012 10:09:29 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > +#ifdef CONFIG_X86_32
> > +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
> > container\n");
> > +	return;
> 
> Perhaps BUG() ?
BUG() will prob not work because it''s way early in boot, which is why
I figured the prev if statement (right before it) has return.

thanks,
Mukesh

Konrad Rzeszutek Wilk

2012-Aug-16 19:09 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, Aug 16, 2012 at 12:00:41PM -0700, Mukesh Rathor
wrote:> On Thu, 16 Aug 2012 10:09:29 -0400
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > > +#ifdef CONFIG_X86_32
> > > +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
> > > container\n");
> > > +	return;
> > 
> > Perhaps BUG() ?
> 
> BUG() will prob not work because it''s way early in boot, which is
why
> I figured the prev if statement (right before it) has return.
It will crash with this:

XEN) domain_crash_sync called from entry.S
.. some more data.> 
> thanks,
> Mukesh

Mukesh Rathor

2012-Aug-16 21:45 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, 16 Aug 2012 15:09:35 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Thu, Aug 16, 2012 at 12:00:41PM -0700, Mukesh Rathor wrote:
> > On Thu, 16 Aug 2012 10:09:29 -0400
> > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > 
> > > > +#ifdef CONFIG_X86_32
> > > > +	xen_raw_printk("ERROR: 32bit PV guest can not run in
> > > > HVM container\n");
> > > > +	return;
> > > 
> > > Perhaps BUG() ?
> > 
> > BUG() will prob not work because it''s way early in boot,
which is
> > why I figured the prev if statement (right before it) has return.
> 
> It will crash with this:
> 
> XEN) domain_crash_sync called from entry.S
> .. some more data.

Do you want me to chnage the return in the prev if statement too in
that case?

        if (!xen_start_info)
                return;

Ian Campbell

2012-Aug-17 08:56 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Thu, 2012-08-16 at 02:01 +0100, Mukesh Rathor wrote:> ---
>  arch/x86/xen/enlighten.c |   67
++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/irq.c       |   22 ++++++++++++++-
>  2 files changed, 77 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index bf4bda6..3a58c51 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -139,6 +139,8 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +static void __init xen_hvm_init_shared_info(void);
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -217,8 +219,8 @@ static void __init xen_banner(void)
>  	struct xen_extraversion extra;
>  	HYPERVISOR_xen_version(XENVER_extraversion, &extra);
>  
> -	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> -	       pv_info.name);
> +	printk(KERN_INFO "Booting paravirtualized kernel %son %s\n",
> +		(xen_pvh_domain() ? "in HVM " : ""), pv_info.name);
Please can we avoid HVM  in the context of PVH here. "with PVH
extensions" or something.
>  	printk(KERN_INFO "Xen version: %d.%d%s%s\n",
>  	       version >> 16, version & 0xffff, extra.extraversion,
>  	       xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? "
(preserve-AD)" : "");
[...]> @@ -1034,6 +1039,10 @@ static int xen_write_msr_safe(unsigned int msr,
unsigned low, unsigned high)
>  
>  void xen_setup_shared_info(void)
>  {
> +	/* do later in xen_pvh_guest_init() when extend_brk is properly setup*/
> +	if (xen_pvh_domain() && xen_initial_domain())
> +		return;
Could we push this setup later for a pv guest too and reduce the
divergence?
> +
>  	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
>  		set_fixmap(FIX_PARAVIRT_BOOTMAP,
>  			   xen_start_info->shared_info);
[...]> @@ -1274,6 +1287,10 @@ static const struct machine_ops xen_machine_ops
__initconst = {
>   */
>  static void __init xen_setup_stackprotector(void)
>  {
> +	if (xen_pvh_domain()) {
> +		switch_to_new_gdt(0);
This seems to skip calling setup_stack_canary_segment too?

Assuming that''s not deliberate I''d be tempted to just put
"if
(xen_pv_domain())" around the updates of pv_cpus_ops and leave the main
flow of the code the same. If it was deliberate a comment might be in
order.

Unrelated to PVH, so I guess more a question for Konrad, but it seems
odd to me that "struct pv_cpu_ops xen_cpu_ops" starts of with
xen_write_gdt in it, gets overridden to xen_write_gdt_boot temporarily
here and then gets put back to xen_write_gdt immediately. Having the
struct start off with xen_write_gdt_boot in it would seem more natural
to me. Unless the _boot suffix is really supposed to mean
_while_setting_up_stack_protector?

(same for the load_gdt hook, of course).
> +		return;
> +	}
>  	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
>  	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
>  
> @@ -1284,6 +1301,25 @@ static void __init xen_setup_stackprotector(void)
>  	pv_cpu_ops.load_gdt = xen_load_gdt;
>  }
>  
> +static void __init xen_pvh_guest_init(void)
> +{
> +#ifndef __HAVE_ARCH_PTE_SPECIAL
> +	("__HAVE_ARCH_PTE_SPECIAL is required for PVH for now\n");
> +	#error("__HAVE_ARCH_PTE_SPECIAL is required for PVH\n");
> +#endif
Isn''t this an unconditional feature of arch/x86?

And if not then this check belongs in Kconfig.
> +	/* PVH TBD/FIXME: for now just disable this. */
> +	have_vcpu_info_placement = 0;
> +
> +	if (xen_feature(XENFEAT_hvm_callback_vector))
> +		xen_have_vector_callback = 1;
> +
> +        /* for domU, the library sets start_info.shared_info to pfn, but
for
> +         * dom0, it contains mfn. we need to get the pfn for shared_info.
PVH
> +	 * uses HVM code in many places */
> +	if (xen_initial_domain())
> +		xen_hvm_init_shared_info();
> +}
> +
>  /* First C function to be called on Xen boot */
>  asmlinkage void __init xen_start_kernel(void)
>  {
> @@ -1294,15 +1330,23 @@ asmlinkage void __init xen_start_kernel(void)
>  	if (!xen_start_info)
>  		return;
>  
> +#ifdef CONFIG_X86_32
> +	xen_raw_printk("ERROR: 32bit PV guest can not run in HVM
container\n");
"run with PVH extensions"

I think you also want a panic here somewhere instead/as well as the
printk.

Plus I haven''t got to it yet but I guess the kernels features are going
to declare something which the tools would use to error out when trying
to build such a thing? Not that this isn''t a good sanity check even so.

[...]> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 1573376..7c7dfd1 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
>  
>  static void xen_safe_halt(void)
>  {
> +	/* so event channel can be delivered to us, since in HVM container */
> +	if (xen_pvh_domain())
> +		local_irq_enable();
> +
>  	/* Blocking includes an implicit local_irq_enable(). */
So this comment isn''t true for a PVH guest? Why not? Should it be?

I''m half wondering if we couldn''t use native_safe_halt here,
IIRC both
SVN and VTd handle "sti; hlt" in a sensible way on the hypervisor side
by calling hvm_hlt

I suppose that''s more of a philosophical question about the nature of
PVH ;-)
>  	if (HYPERVISOR_sched_op(SCHEDOP_block, NULL) != 0)
>  		BUG();
Ian.

Mukesh Rathor

2012-Aug-17 23:37 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Fri, 17 Aug 2012 09:56:20 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-08-16 at 02:01 +0100, Mukesh Rathor wrote:
> [...]
> > @@ -1034,6 +1039,10 @@ static int xen_write_msr_safe(unsigned int
> > msr, unsigned low, unsigned high) 
> >  void xen_setup_shared_info(void)
> >  {
> > +	/* do later in xen_pvh_guest_init() when extend_brk is
> > properly setup*/
> > +	if (xen_pvh_domain() && xen_initial_domain())
> > +		return;
> 
> Could we push this setup later for a pv guest too and reduce the
> divergence?
A bit nervous changing PV paths until I''ve the bandwidth to test it
thoroughly with various mem configs. So, I''ll put a TBD for now.
> > +
> >  	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> >  		set_fixmap(FIX_PARAVIRT_BOOTMAP,
> >  			   xen_start_info->shared_info);
> [...]
> > @@ -1274,6 +1287,10 @@ static const struct machine_ops
> > xen_machine_ops __initconst = { */
> >  static void __init xen_setup_stackprotector(void)
> >  {
> > +	if (xen_pvh_domain()) {
> > +		switch_to_new_gdt(0);
> 
> This seems to skip calling setup_stack_canary_segment too?
> 
> Assuming that''s not deliberate I''d be tempted to just put
"if
> (xen_pv_domain())" around the updates of pv_cpus_ops and leave the
> main flow of the code the same. If it was deliberate a comment might
> be in order.
I meant to comment to do this phase II. I''m not very familiar with
setup_stack_canary_segment stuff and will need to learn it first.
  
> >  }
> >  
> > +static void __init xen_pvh_guest_init(void)
> > +{
> > +#ifndef __HAVE_ARCH_PTE_SPECIAL
> > +	("__HAVE_ARCH_PTE_SPECIAL is required for PVH for now\n");
> > +	#error("__HAVE_ARCH_PTE_SPECIAL is required for PVH\n");
> > +#endif
> 
> Isn''t this an unconditional feature of arch/x86?
Right. I can remove it now. I had started from much older linux.
> > diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> > index 1573376..7c7dfd1 100644
> > --- a/arch/x86/xen/irq.c
> > +++ b/arch/x86/xen/irq.c
> > @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
> >  
> >  static void xen_safe_halt(void)
> >  {
> > +	/* so event channel can be delivered to us, since in HVM
> > container */
> > +	if (xen_pvh_domain())
> > +		local_irq_enable();
> > +
> >  	/* Blocking includes an implicit local_irq_enable(). */
> 
> So this comment isn''t true for a PVH guest? Why not? Should it be? 
I need to make sure the EFLAGS.IF is enabled. IIRC, the comment is saying
that xen will clear event channel mask bit. For PVH, there''s the
additional
EFLAGS.IF flag.

Ian Campbell

2012-Aug-18 08:23 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

> > > diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> > > index 1573376..7c7dfd1 100644
> > > --- a/arch/x86/xen/irq.c
> > > +++ b/arch/x86/xen/irq.c
> > > @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
> > >  
> > >  static void xen_safe_halt(void)
> > >  {
> > > +	/* so event channel can be delivered to us, since in HVM
> > > container */
> > > +	if (xen_pvh_domain())
> > > +		local_irq_enable();
> > > +
> > >  	/* Blocking includes an implicit local_irq_enable(). */
> > 
> > So this comment isn''t true for a PVH guest? Why not? Should
it be?
>  
> I need to make sure the EFLAGS.IF is enabled. IIRC, the comment is saying
> that xen will clear event channel mask bit. For PVH, there''s the
additional
> EFLAGS.IF flag.
> 
My reading of the hypercall semantics would be that it reenables
whichever event delivery mechanism the guest is using and therefore it
should enable EFLAGS.IF for a PVH guest since manipulating the evtchn
mask in this case is pointless.

Ian.

Mukesh Rathor

2012-Sep-04 23:56 UTC

head link

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

On Fri, 17 Aug 2012 09:56:20 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> [...]
> > diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> > index 1573376..7c7dfd1 100644
> > --- a/arch/x86/xen/irq.c
> > +++ b/arch/x86/xen/irq.c
> > @@ -100,6 +100,10 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
> >  
> >  static void xen_safe_halt(void)
> >  {
> > +	/* so event channel can be delivered to us, since in HVM
> > container */
> > +	if (xen_pvh_domain())
> > +		local_irq_enable();
> > +
> >  	/* Blocking includes an implicit local_irq_enable(). */
> 
> So this comment isn''t true for a PVH guest? Why not? Should it be?
> 
> I''m half wondering if we couldn''t use native_safe_halt
here, IIRC both
> SVN and VTd handle "sti; hlt" in a sensible way on the hypervisor
side
> by calling hvm_hlt
I was able to change it to use native_safe_halt.

Xen devel - Aug 2012 - [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

[[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring

Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring