Messages in this thread | | | Date | Thu, 04 Mar 2010 10:24:18 -0800 | From | Jeremy Fitzhardinge <> | Subject | Re: [PATCH 4/7] xen/hvm: Xen PV extension of HVM initialization |
| |
On 03/04/2010 01:36 AM, Sheng Yang wrote: > diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig > index b83e119..af3c7a0 100644 > --- a/arch/x86/xen/Kconfig > +++ b/arch/x86/xen/Kconfig > @@ -36,3 +36,11 @@ config XEN_DEBUG_FS > help > Enable statistics output and various tuning options in debugfs. > Enabling this option may incur a significant performance overhead. > + > +config XEN_HVM_PV > + bool "PV extension of HVM" > + depends on XEN&& X86_64 > + default n > + help > + Enable the PV extension of HVM, so that HVM guest would benefit from > + PV features like PV timer and evtchn. >
Unless there's a strong reason to make this a user-visible setting, I'd make it a hidden option which is enabled by default when the dependencies are correct.
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile > index 3bb4fc2..73bd5db 100644 > --- a/arch/x86/xen/Makefile > +++ b/arch/x86/xen/Makefile > @@ -17,4 +17,5 @@ obj-y := enlighten.o setup.o multicalls.o mmu.o irq.o \ > obj-$(CONFIG_SMP) += smp.o > obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o > obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o > +obj-$(CONFIG_XEN_HVM_PV) += hvmpv.o > > diff --git a/arch/x86/xen/hvmpv.c b/arch/x86/xen/hvmpv.c > new file mode 100644 > index 0000000..7a741ed > --- /dev/null > +++ b/arch/x86/xen/hvmpv.c > @@ -0,0 +1,133 @@ > +/* > + * PV extension of HVM implementation. > + * > + * Sheng Yang<sheng@linux.intel.com>, Intel Corporation, 2010 > + * > + */ > +#include<linux/kernel.h> > +#include<linux/init.h> > +#include<linux/percpu.h> > +#include<linux/module.h> > + > +#include<xen/xen.h> > +#include<xen/features.h> > +#include<xen/events.h> > +#include<xen/hvm.h> > +#include<xen/interface/xen.h> > +#include<xen/interface/version.h> > +#include<xen/interface/memory.h> > + > +#include<asm/xen/cpuid.h> > +#include<asm/xen/hypercall.h> > +#include<asm/xen/hypervisor.h> > + > +#include "xen-ops.h" > + > +u32 xen_hvm_pv_features; > +EXPORT_SYMBOL_GPL(xen_hvm_pv_features); > + > +static const struct pv_info xen_hvm_pv_info __initdata = { > + .paravirt_enabled = 1, > + .shared_kernel_pmd = 0, > + .kernel_rpl = 0, > + .name = "Xen", > +}; > + > +static void __init xen_hvm_pv_banner(void) > +{ > + unsigned version = HYPERVISOR_xen_version(XENVER_version, NULL); > + struct xen_extraversion extra; > + HYPERVISOR_xen_version(XENVER_extraversion,&extra); > + > + printk(KERN_INFO "Booting PV extended HVM kernel on %s\n", > + pv_info.name); > + printk(KERN_INFO "Xen version: %d.%d%s\n", > + version>> 16, version& 0xffff, extra.extraversion); > +} > + > +static int __init xen_para_available(void) > +{ > + uint32_t eax, ebx, ecx, edx; > + cpuid(XEN_CPUID_LEAF(0),&eax,&ebx,&ecx,&edx); > + > + if (ebx == XEN_CPUID_SIGNATURE_EBX&& > + ecx == XEN_CPUID_SIGNATURE_ECX&& > + edx == XEN_CPUID_SIGNATURE_EDX&& > + ((eax - XEN_CPUID_LEAF(0))>= 2)) > + return 1; > + > + return 0; > +} > + > +static int __init enable_hvm_pv(u64 flags) > +{ > + struct xen_hvm_pv_type a; > + > + a.domid = DOMID_SELF; > + a.flags = flags; > + return HYPERVISOR_hvm_op(HVMOP_enable_pv,&a); > +} > + > +static int __init init_hvm_pv_info(void) > +{ > + uint32_t ecx, edx, pages, msr; > + u64 pfn; > + > + if (!xen_para_available()) > + return -EINVAL; > + > + cpuid(XEN_CPUID_LEAF(2),&pages,&msr,&ecx,&edx); > + > + /* Check if hvm_pv mode is supported */ > + if (!(edx& XEN_CPUID_FEAT2_HVM_PV)) > + return -ENODEV; > + > + /* We only support 1 page of hypercall for now */ > + if (pages != 1) > + return -ENOMEM; >
Why does it matter? If you're only using hypercalls in the first page, it doesn't matter whether Xen exports more pages beyond that.
> + > + pfn = __pa(hypercall_page); > + wrmsrl(msr, pfn); > wrmsr_safe() with error check, just in case it fails?
> + > + xen_setup_features(); > + > + x86_init.oem.banner = xen_hvm_pv_banner; > + pv_info = xen_hvm_pv_info; > + > + xen_domain_type = XEN_HVM_DOMAIN; >
Set this very last, once everything else has successfully set up.
> + > + return 0; > +} > + > +static struct shared_info shared_info_page __page_aligned_bss; > + > +static void __init init_shared_info(void) > +{ > + struct xen_add_to_physmap xatp; > + > + xatp.domid = DOMID_SELF; > + xatp.idx = 0; > + xatp.space = XENMAPSPACE_shared_info; > + xatp.gpfn = __pa(&shared_info_page)>> PAGE_SHIFT; > + if (HYPERVISOR_memory_op(XENMEM_add_to_physmap,&xatp)) > + BUG(); > It isn't too late to back out if it fails; just return an error code. > + > + HYPERVISOR_shared_info = (struct shared_info *)&shared_info_page; > + > + per_cpu(xen_vcpu, 0) =&HYPERVISOR_shared_info->vcpu_info[0]; > +} > + > +void __init xen_guest_init(void) > +{ > + int r; > + > + /* Ensure the we won't confused with others */ > + if (xen_domain_type != XEN_NATIVE) > Just use xen_domain()?
> + return; > + > + r = init_hvm_pv_info(); > + if (r< 0) > + return; > + > + init_shared_info(); > +} > diff --git a/include/xen/interface/hvm/hvm_op.h b/include/xen/interface/hvm/hvm_op.h > index 7c74ba4..3f0b118 100644 > --- a/include/xen/interface/hvm/hvm_op.h > +++ b/include/xen/interface/hvm/hvm_op.h > @@ -69,4 +69,12 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_hvm_set_pci_link_route); > /* Flushes all VCPU TLBs: @arg must be NULL. */ > #define HVMOP_flush_tlbs 5 > > +#define HVMOP_enable_pv 9 > +struct xen_hvm_pv_type { > + domid_t domid; > + uint32_t flags; > +#define HVM_PV_CLOCK (1ull<<0) > +#define HVM_PV_EVTCHN (1ull<<1) > +}; > + > #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */ > diff --git a/include/xen/xen.h b/include/xen/xen.h > index a164024..11f5ced 100644 > --- a/include/xen/xen.h > +++ b/include/xen/xen.h > @@ -19,6 +19,15 @@ extern enum xen_domain_type xen_domain_type; > #define xen_hvm_domain() (xen_domain()&& \ > xen_domain_type == XEN_HVM_DOMAIN) > > +#ifdef CONFIG_XEN_HVM_PV > + > +#define XEN_HVM_PV_EVTCHN_ENABLED (1u<< 1) > +extern u32 xen_hvm_pv_features; > + > +#define xen_hvm_pv_evtchn_enabled() \ > + (xen_hvm_pv_features& XEN_HVM_PV_EVTCHN_ENABLED) > +#endif /* CONFIG_XEN_HVM_PV */ >
Did you compile test this with !CONFIG_XEN_HVM_PV? I think it would fail with xen_hvm_pv_evtchn_enabled() undefined. Fortunately making xen_hvm_pv_evtchn_enabled() evaluate to constant 0 in the !configed case will clean things up.
J
| |