Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Xen: Devel

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

 

 

Xen devel RSS feed   Index | Next | Previous | View Threaded


mukesh.rathor at oracle

Aug 15, 2012, 6:01 PM

Post #1 of 11 (123 views)
Permalink
[[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


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


stefano.stabellini at eu

Aug 16, 2012, 7:05 AM

Post #2 of 11 (114 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

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 [at] lists
> http://lists.xen.org/xen-devel
>

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


konrad.wilk at oracle

Aug 16, 2012, 7:09 AM

Post #3 of 11 (114 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

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

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


mukesh.rathor at oracle

Aug 16, 2012, 11:56 AM

Post #4 of 11 (116 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

On Thu, 16 Aug 2012 15:05:27 +0100
Stefano Stabellini <stefano.stabellini [at] eu> 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


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


mukesh.rathor at oracle

Aug 16, 2012, 12:00 PM

Post #5 of 11 (116 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

On Thu, 16 Aug 2012 10:09:29 -0400
Konrad Rzeszutek Wilk <konrad.wilk [at] oracle> 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


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


konrad.wilk at oracle

Aug 16, 2012, 12:09 PM

Post #6 of 11 (116 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

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 [at] oracle> 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

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


mukesh.rathor at oracle

Aug 16, 2012, 2:45 PM

Post #7 of 11 (118 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

On Thu, 16 Aug 2012 15:09:35 -0400
Konrad Rzeszutek Wilk <konrad.wilk [at] oracle> 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 [at] oracle> 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;



_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Campbell at citrix

Aug 17, 2012, 1:56 AM

Post #8 of 11 (121 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

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.



_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


mukesh.rathor at oracle

Aug 17, 2012, 4:37 PM

Post #9 of 11 (121 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

On Fri, 17 Aug 2012 09:56:20 +0100
Ian Campbell <Ian.Campbell [at] citrix> 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.


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


Ian.Campbell at citrix

Aug 18, 2012, 1:23 AM

Post #10 of 11 (117 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

> > > 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.


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


mukesh.rathor at oracle

Sep 4, 2012, 4:56 PM

Post #11 of 11 (58 views)
Permalink
Re: [[RFC PATCH 2/8]: PVH: changes related to initial boot and irq rewiring [In reply to]

On Fri, 17 Aug 2012 09:56:20 +0100
Ian Campbell <Ian.Campbell [at] citrix> 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 mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel

Xen devel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.