| Message ID | 67b90d11eae2c88faab22d458e7e38db0f5aada4.1597854907.git.don.slutz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Xen VMware tools support | expand |
On 19.08.2020 18:51, Don Slutz wrote: > Since I need to change xen/arch/x86/hvm/Makefile; also add > a newline at end of file. Should this have been removed? Also please update / trim your Cc list. I've dropped / replaced a number of entries which I'm sure would have bounced. > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -597,6 +597,11 @@ int arch_domain_create(struct domain *d, > } > d->arch.emulation_flags = emflags; > > + if ( is_hvm_domain(d) ) > + { > + d->arch.hvm.vmware_hwver = config->arch.vmware_hwver; > + } As per the description it's not like any value is okay. Shouldn't you refuse bad values in arch_sanitise_domain_config()? Also please drop the unnecessary braces. > --- a/xen/arch/x86/hvm/Makefile > +++ b/xen/arch/x86/hvm/Makefile > @@ -1,6 +1,7 @@ > obj-y += svm/ > obj-y += vmx/ > obj-y += viridian/ > +obj-y += vmware/ Generally we try to sort such lists alphabetically. I realize a mistake was already made when Viridian gained its own subdir, but please don't widen the issue. > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -68,6 +68,7 @@ > #include <asm/hvm/viridian.h> > #include <asm/hvm/vm_event.h> > #include <asm/altp2m.h> > +#include <asm/hvm/vmware.h> Like above, please try to honor (partial) sorting in #include-s as well. > @@ -4109,6 +4110,13 @@ static int hvm_allow_set_param(struct domain *d, > { > /* The following parameters should only be changed once. */ > case HVM_PARAM_VIRIDIAN: > + /* Disallow if vmware_hwver is in use */ > + if ( d->arch.hvm.vmware_hwver ) > + { > + rc = -EOPNOTSUPP; > + break; > + } > + /* Fall through */ Afaic the comment is too redundant with the code. If at least it wouldn't name the field name, but say e.g. "VMware emulation", things would already be better. Using something like "can't coexist" instead of "disallow" may further improve usefulness. > --- /dev/null > +++ b/xen/arch/x86/hvm/vmware/vmware.c > @@ -0,0 +1,82 @@ > +/* > + * arch/x86/hvm/vmware/cpuid.c > + * > + * Copyright (C) 2012-2015 Verizon Corporation > + * > + * This file is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License Version 2 (GPLv2) > + * as published by the Free Software Foundation. > + * > + * This file is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. <http://www.gnu.org/licenses/>. > + */ > + > +#include <xen/sched.h> > +#include <xen/version.h> > +#include <xen/hypercall.h> > +#include <xen/domain_page.h> > +#include <xen/param.h> > +#include <asm/guest_access.h> > +#include <asm/guest/hyperv-tlfs.h> > +#include <asm/paging.h> > +#include <asm/p2m.h> > +#include <asm/apic.h> > +#include <asm/hvm/support.h> > +#include <public/sched.h> > +#include <public/hvm/hvm_op.h> Please sort each sub-section alphabetically, and please remove ones you don't really need - the list looks surprisingly long for just CPUID handling. > +/* > + * VMware hardware version 7 defines some of these cpuid levels, > + * below is a brief description about those. > + * > + * Leaf 0x40000000, Hypervisor CPUID information > + * # EAX: The maximum input value for hypervisor CPUID info (0x40000010). > + * # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware" > + * > + * Leaf 0x40000010, Timing information. > + * # EAX: (Virtual) TSC frequency in kHz. > + * # EBX: (Virtual) Bus (local apic timer) frequency in kHz. > + * # ECX, EDX: RESERVED > + */ > + > +void cpuid_vmware_leaves(const struct vcpu *v, uint32_t leaf, > + uint32_t subleaf, struct cpuid_leaf *res) > +{ > + struct domain *d = current->domain; Surely v->domain, and please add const. > + ASSERT(has_vmware_cpuid(d)); > + ASSERT(leaf >= 0x40000000 && leaf < 0x40000100); What earlier check guarantees this? > + leaf -= 0x40000000; > + > + switch ( leaf ) > + { > + case 0x0: > + res->a = 0x40000010; /* Maximum leaf */ > + memcpy(&res->b, "VMwa", 4); > + memcpy(&res->c, "reVM", 4); > + memcpy(&res->d, "ware", 4); > + break; > + > + case 0x10: > + /* (Virtual) TSC frequency in kHz. */ > + res->a = d->arch.tsc_khz; > + /* (Virtual) Bus (local apic timer) frequency in kHz. */ > + res->b = 1000000ull / APIC_BUS_CYCLE_NS; > + res->c = 0; /* Reserved */ > + res->d = 0; /* Reserved */ > + break; > + } No further dependency on the selected version? > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -885,8 +885,11 @@ static void do_trap(struct cpu_user_regs *regs) > int guest_rdmsr_xen(const struct vcpu *v, uint32_t idx, uint64_t *val) > { > const struct domain *d = v->domain; > - /* Optionally shift out of the way of Viridian architectural MSRs. */ > - uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; > + /* > + * Optionally shift out of the way of Viridian or VMware > + * architectural leaves. > + */ > + uint32_t base = is_viridian_or_vmware_cpuid(d) ? 0x40000200 : 0x40000000; > > switch ( idx - base ) > { > @@ -901,8 +904,11 @@ int guest_rdmsr_xen(const struct vcpu *v, uint32_t idx, uint64_t *val) > int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) > { > struct domain *d = v->domain; > - /* Optionally shift out of the way of Viridian architectural MSRs. */ > - uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; > + /* > + * Optionally shift out of the way of Viridian or VMware > + * architectural leaves. > + */ > + uint32_t base = is_viridian_or_vmware_cpuid(d) ? 0x40000200 : 0x40000000; > > switch ( idx - base ) > { How do these (MSR related) changes correspond to the subject of this change? (Mentioning why they're needed in the description would help.) Also your choice of name (is_viridian_or_vmware_cpuid()) wouldn't scale if there were one or more further hypervisor emulations added. I don't have a good suggestion for a name right away, but one should be found. > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -474,6 +474,18 @@ static inline bool hvm_get_guest_bndcfgs(struct vcpu *v, u64 *val) > #define has_viridian_synic(d) \ > (is_viridian_domain(d) && (viridian_feature_mask(d) & HVMPV_synic)) > > +#define vmware_feature_mask(d) \ > + ((d)->arch.hvm.vmware_hwver) Why "mask"? This is simply a numeric value, isn't it? Also why do this and ... > +#define is_vmware_domain(d) \ > + (is_hvm_domain(d) && vmware_feature_mask(d)) > + > +#define has_vmware_cpuid(d) \ > + (is_hvm_domain(d) && (vmware_feature_mask(d) >= 7)) ... these not live in the new vmware.h? > --- /dev/null > +++ b/xen/include/asm-x86/hvm/vmware.h > @@ -0,0 +1,33 @@ > +/* > + * asm-x86/hvm/vmware.h > + * > + * Copyright (C) 2012-2015 Verizon Corporation > + * > + * This file is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License Version 2 (GPLv2) > + * as published by the Free Software Foundation. > + * > + * This file is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef ASM_X86_HVM_VMWARE_H__ > +#define ASM_X86_HVM_VMWARE_H__ > + > +#include <xen/types.h> > + > +void cpuid_vmware_leaves(const struct vcpu *v, uint32_t leaf, > + uint32_t subleaf, struct cpuid_leaf *res); At the example of this, as per ./CODING_STYLE please avoid the use of uint<N>_t when more basic types (unsigned int here) are fine to use. With that you won't need xen/types.h anymore. You'll want to forward-declare the two struct-s the prototype uses in any event, though. > --- a/xen/include/public/arch-x86/xen.h > +++ b/xen/include/public/arch-x86/xen.h > @@ -304,6 +304,7 @@ struct xen_arch_domainconfig { > XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\ > XEN_X86_EMU_VPCI) > uint32_t emulation_flags; > + uint32_t vmware_hwver; > }; As per the comment above this struct XEN_DOMCTL_INTERFACE_VERSION would need bumping with such an addition, unless it already has been in the current release cycle. Also - is VMware really x86-only? Jan
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index e57f632..3418a81 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -21,6 +21,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, abort(); } + /* Note: will be changed in next patch (tools: Add ...). */ + config->arch.vmware_hwver = 0; return 0; } diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index 63a03ef..6cd3670 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -8,6 +8,7 @@ #include <asm/hvm/nestedhvm.h> #include <asm/hvm/svm/svm.h> #include <asm/hvm/viridian.h> +#include <asm/hvm/vmware.h> #include <asm/hvm/vmx/vmcs.h> #include <asm/paging.h> #include <asm/processor.h> @@ -791,6 +792,9 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf, if ( is_viridian_domain(d) ) return cpuid_viridian_leaves(v, leaf, subleaf, res); + if ( has_vmware_cpuid(d) ) + return cpuid_vmware_leaves(v, leaf, subleaf, res); + /* * Fallthrough. * diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index d1ecc7b..a317907 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -597,6 +597,11 @@ int arch_domain_create(struct domain *d, } d->arch.emulation_flags = emflags; + if ( is_hvm_domain(d) ) + { + d->arch.hvm.vmware_hwver = config->arch.vmware_hwver; + } + HYPERVISOR_COMPAT_VIRT_START(d) = is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u; diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile index 3464191..58de573 100644 --- a/xen/arch/x86/hvm/Makefile +++ b/xen/arch/x86/hvm/Makefile @@ -1,6 +1,7 @@ obj-y += svm/ obj-y += vmx/ obj-y += viridian/ +obj-y += vmware/ obj-y += asid.o obj-y += dm.o diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index a9d1685..e91169f 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -68,6 +68,7 @@ #include <asm/hvm/viridian.h> #include <asm/hvm/vm_event.h> #include <asm/altp2m.h> +#include <asm/hvm/vmware.h> #include <asm/mtrr.h> #include <asm/apic.h> #include <asm/vm_event.h> @@ -4109,6 +4110,13 @@ static int hvm_allow_set_param(struct domain *d, { /* The following parameters should only be changed once. */ case HVM_PARAM_VIRIDIAN: + /* Disallow if vmware_hwver is in use */ + if ( d->arch.hvm.vmware_hwver ) + { + rc = -EOPNOTSUPP; + break; + } + /* Fall through */ case HVM_PARAM_IOREQ_PFN: case HVM_PARAM_BUFIOREQ_PFN: case HVM_PARAM_IOREQ_SERVER_PFN: diff --git a/xen/arch/x86/hvm/vmware/Makefile b/xen/arch/x86/hvm/vmware/Makefile new file mode 100644 index 0000000..f864486 --- /dev/null +++ b/xen/arch/x86/hvm/vmware/Makefile @@ -0,0 +1 @@ +obj-y += vmware.o diff --git a/xen/arch/x86/hvm/vmware/vmware.c b/xen/arch/x86/hvm/vmware/vmware.c new file mode 100644 index 0000000..19e52c1 --- /dev/null +++ b/xen/arch/x86/hvm/vmware/vmware.c @@ -0,0 +1,82 @@ +/* + * arch/x86/hvm/vmware/cpuid.c + * + * Copyright (C) 2012-2015 Verizon Corporation + * + * This file is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License Version 2 (GPLv2) + * as published by the Free Software Foundation. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. <http://www.gnu.org/licenses/>. + */ + +#include <xen/sched.h> +#include <xen/version.h> +#include <xen/hypercall.h> +#include <xen/domain_page.h> +#include <xen/param.h> +#include <asm/guest_access.h> +#include <asm/guest/hyperv-tlfs.h> +#include <asm/paging.h> +#include <asm/p2m.h> +#include <asm/apic.h> +#include <asm/hvm/support.h> +#include <public/sched.h> +#include <public/hvm/hvm_op.h> + +/* + * VMware hardware version 7 defines some of these cpuid levels, + * below is a brief description about those. + * + * Leaf 0x40000000, Hypervisor CPUID information + * # EAX: The maximum input value for hypervisor CPUID info (0x40000010). + * # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware" + * + * Leaf 0x40000010, Timing information. + * # EAX: (Virtual) TSC frequency in kHz. + * # EBX: (Virtual) Bus (local apic timer) frequency in kHz. + * # ECX, EDX: RESERVED + */ + +void cpuid_vmware_leaves(const struct vcpu *v, uint32_t leaf, + uint32_t subleaf, struct cpuid_leaf *res) +{ + struct domain *d = current->domain; + + ASSERT(has_vmware_cpuid(d)); + ASSERT(leaf >= 0x40000000 && leaf < 0x40000100); + + leaf -= 0x40000000; + + switch ( leaf ) + { + case 0x0: + res->a = 0x40000010; /* Maximum leaf */ + memcpy(&res->b, "VMwa", 4); + memcpy(&res->c, "reVM", 4); + memcpy(&res->d, "ware", 4); + break; + + case 0x10: + /* (Virtual) TSC frequency in kHz. */ + res->a = d->arch.tsc_khz; + /* (Virtual) Bus (local apic timer) frequency in kHz. */ + res->b = 1000000ull / APIC_BUS_CYCLE_NS; + res->c = 0; /* Reserved */ + res->d = 0; /* Reserved */ + break; + } +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index bc5b8f8..b1e93bb 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -885,8 +885,11 @@ static void do_trap(struct cpu_user_regs *regs) int guest_rdmsr_xen(const struct vcpu *v, uint32_t idx, uint64_t *val) { const struct domain *d = v->domain; - /* Optionally shift out of the way of Viridian architectural MSRs. */ - uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; + /* + * Optionally shift out of the way of Viridian or VMware + * architectural leaves. + */ + uint32_t base = is_viridian_or_vmware_cpuid(d) ? 0x40000200 : 0x40000000; switch ( idx - base ) { @@ -901,8 +904,11 @@ int guest_rdmsr_xen(const struct vcpu *v, uint32_t idx, uint64_t *val) int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) { struct domain *d = v->domain; - /* Optionally shift out of the way of Viridian architectural MSRs. */ - uint32_t base = is_viridian_domain(d) ? 0x40000200 : 0x40000000; + /* + * Optionally shift out of the way of Viridian or VMware + * architectural leaves. + */ + uint32_t base = is_viridian_or_vmware_cpuid(d) ? 0x40000200 : 0x40000000; switch ( idx - base ) { @@ -959,9 +965,10 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, { const struct domain *d = v->domain; const struct cpuid_policy *p = d->arch.cpuid; - uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000; + uint32_t base = is_viridian_or_vmware_cpuid(d) ? 0x40000100 : 0x40000000; uint32_t idx = leaf - base; - unsigned int limit = is_viridian_domain(d) ? p->hv2_limit : p->hv_limit; + unsigned int limit = is_viridian_or_vmware_cpuid(d) ? + p->hv2_limit : p->hv_limit; if ( limit == 0 ) /* Default number of leaves */ @@ -989,7 +996,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, case 2: res->a = 1; /* Number of hypercall-transfer pages */ /* MSR base address */ - res->b = is_viridian_domain(d) ? 0x40000200 : 0x40000000; + res->b = is_viridian_or_vmware_cpuid(d) ? 0x40000200 : 0x40000000; if ( is_pv_domain(d) ) /* Features */ res->c |= XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD; break; diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h index 9d247ba..0f64a4b 100644 --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -147,6 +147,9 @@ struct hvm_domain { uint64_t *params; + /* emulated VMware Hardware Version */ + uint32_t vmware_hwver; + /* Memory ranges with pinned cache attributes. */ struct list_head pinned_cacheattr_ranges; diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index be0d8b0..cb55dfb 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -474,6 +474,18 @@ static inline bool hvm_get_guest_bndcfgs(struct vcpu *v, u64 *val) #define has_viridian_synic(d) \ (is_viridian_domain(d) && (viridian_feature_mask(d) & HVMPV_synic)) +#define vmware_feature_mask(d) \ + ((d)->arch.hvm.vmware_hwver) + +#define is_vmware_domain(d) \ + (is_hvm_domain(d) && vmware_feature_mask(d)) + +#define has_vmware_cpuid(d) \ + (is_hvm_domain(d) && (vmware_feature_mask(d) >= 7)) + +#define is_viridian_or_vmware_cpuid(d) \ + (is_viridian_domain(d) || has_vmware_cpuid(d)) + static inline void hvm_inject_exception( unsigned int vector, unsigned int type, unsigned int insn_len, int error_code) @@ -754,6 +766,9 @@ static inline bool hvm_has_set_descriptor_access_exiting(void) #define is_viridian_domain(d) ((void)(d), false) #define is_viridian_vcpu(v) ((void)(v), false) #define has_viridian_time_ref_count(d) ((void)(d), false) +#define is_vmware_domain(d) ((void)(d), false) +#define has_vmware_cpuid(d) ((void)(d), false) +#define is_viridian_or_vmware_cpuid(d) ((void)(d), false) #define hvm_long_mode_active(v) ((void)(v), false) #define hvm_get_guest_time(v) ((void)(v), 0) diff --git a/xen/include/asm-x86/hvm/vmware.h b/xen/include/asm-x86/hvm/vmware.h new file mode 100644 index 0000000..98248e7 --- /dev/null +++ b/xen/include/asm-x86/hvm/vmware.h @@ -0,0 +1,33 @@ +/* + * asm-x86/hvm/vmware.h + * + * Copyright (C) 2012-2015 Verizon Corporation + * + * This file is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License Version 2 (GPLv2) + * as published by the Free Software Foundation. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. <http://www.gnu.org/licenses/>. + */ + +#ifndef ASM_X86_HVM_VMWARE_H__ +#define ASM_X86_HVM_VMWARE_H__ + +#include <xen/types.h> + +void cpuid_vmware_leaves(const struct vcpu *v, uint32_t leaf, + uint32_t subleaf, struct cpuid_leaf *res); + +#endif /* ASM_X86_HVM_VMWARE_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h index 629cb2b..54b1c4d 100644 --- a/xen/include/public/arch-x86/xen.h +++ b/xen/include/public/arch-x86/xen.h @@ -304,6 +304,7 @@ struct xen_arch_domainconfig { XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\ XEN_X86_EMU_VPCI) uint32_t emulation_flags; + uint32_t vmware_hwver; }; /* Location of online VCPU bitmap. */