From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>,
suravee.suthikulpanit@amd.com, Eddie Dong <eddie.dong@intel.com>,
Jacob Shin <jacob.shin@amd.com>,
Jun Nakajima <jun.nakajima@intel.com>,
Yang Z Zhang <yang.z.zhang@intel.com>,
xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v4 4/4] x86: make hvm_cpuid() tolerate NULL pointers
Date: Mon, 23 Sep 2013 09:42:44 -0400 [thread overview]
Message-ID: <52404554.1070200@oracle.com> (raw)
In-Reply-To: <5240304502000078000F570D@nat28.tlf.novell.com>
On 09/23/2013 06:12 AM, Jan Beulich wrote:
> Now that we other HVM code start making more extensive use of
Something is wrong with this sentence. I think "we" is not needed.
Other than that
Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
-boris
> hvm_cpuid(), let's not force every caller to declare dummy variables
> for output not cared about.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2757,7 +2757,17 @@ void hvm_cpuid(unsigned int input, unsig
> {
> struct vcpu *v = current;
> struct domain *d = v->domain;
> - unsigned int count = *ecx;
> + unsigned int count, dummy = 0;
> +
> + if ( !eax )
> + eax = &dummy;
> + if ( !ebx )
> + ebx = &dummy;
> + if ( !ecx )
> + ecx = &dummy;
> + count = *ecx;
> + if ( !edx )
> + edx = &dummy;
>
> if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )
> return;
> @@ -2765,7 +2775,7 @@ void hvm_cpuid(unsigned int input, unsig
> if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
> return;
>
> - domain_cpuid(d, input, *ecx, eax, ebx, ecx, edx);
> + domain_cpuid(d, input, count, eax, ebx, ecx, edx);
>
> switch ( input )
> {
> @@ -2853,15 +2863,15 @@ int hvm_msr_read_intercept(unsigned int
> {
> struct vcpu *v = current;
> uint64_t *var_range_base, *fixed_range_base;
> - int index, mtrr;
> - uint32_t cpuid[4];
> + bool_t mtrr;
> + unsigned int edx, index;
> int ret = X86EMUL_OKAY;
>
> var_range_base = (uint64_t *)v->arch.hvm_vcpu.mtrr.var_ranges;
> fixed_range_base = (uint64_t *)v->arch.hvm_vcpu.mtrr.fixed_ranges;
>
> - hvm_cpuid(1, &cpuid[0], &cpuid[1], &cpuid[2], &cpuid[3]);
> - mtrr = !!(cpuid[3] & cpufeat_mask(X86_FEATURE_MTRR));
> + hvm_cpuid(1, NULL, NULL, NULL, &edx);
> + mtrr = !!(edx & cpufeat_mask(X86_FEATURE_MTRR));
>
> switch ( msr )
> {
> @@ -2969,15 +2979,15 @@ int hvm_msr_read_intercept(unsigned int
> int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
> {
> struct vcpu *v = current;
> - int index, mtrr;
> - uint32_t cpuid[4];
> + bool_t mtrr;
> + unsigned int edx, index;
> int ret = X86EMUL_OKAY;
>
> HVMTRACE_3D(MSR_WRITE, msr,
> (uint32_t)msr_content, (uint32_t)(msr_content >> 32));
>
> - hvm_cpuid(1, &cpuid[0], &cpuid[1], &cpuid[2], &cpuid[3]);
> - mtrr = !!(cpuid[3] & cpufeat_mask(X86_FEATURE_MTRR));
> + hvm_cpuid(1, NULL, NULL, NULL, &edx);
> + mtrr = !!(edx & cpufeat_mask(X86_FEATURE_MTRR));
>
> hvm_memory_event_msr(msr, msr_content);
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -806,13 +806,13 @@ static inline void svm_lwp_load(struct v
> /* Update LWP_CFG MSR (0xc0000105). Return -1 if error; otherwise returns 0. */
> static int svm_update_lwp_cfg(struct vcpu *v, uint64_t msr_content)
> {
> - unsigned int eax, ebx, ecx, edx;
> + unsigned int edx;
> uint32_t msr_low;
> static uint8_t lwp_intr_vector;
>
> if ( xsave_enabled(v) && cpu_has_lwp )
> {
> - hvm_cpuid(0x8000001c, &eax, &ebx, &ecx, &edx);
> + hvm_cpuid(0x8000001c, NULL, NULL, NULL, &edx);
> msr_low = (uint32_t)msr_content;
>
> /* generate #GP if guest tries to turn on unsupported features. */
> @@ -1163,10 +1163,10 @@ static void svm_init_erratum_383(struct
>
> static int svm_handle_osvw(struct vcpu *v, uint32_t msr, uint64_t *val, bool_t read)
> {
> - uint eax, ebx, ecx, edx;
> + unsigned int ecx;
>
> /* Guest OSVW support */
> - hvm_cpuid(0x80000001, &eax, &ebx, &ecx, &edx);
> + hvm_cpuid(0x80000001, NULL, NULL, &ecx, NULL);
> if ( !test_bit((X86_FEATURE_OSVW & 31), &ecx) )
> return -1;
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1813,7 +1813,7 @@ int nvmx_handle_invvpid(struct cpu_user_
> int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
> {
> struct vcpu *v = current;
> - unsigned int eax, ebx, ecx, edx, dummy;
> + unsigned int eax, ebx, ecx, edx;
> u64 data = 0, host_data = 0;
> int r = 1;
>
> @@ -1821,7 +1821,7 @@ int nvmx_msr_read_intercept(unsigned int
> return 0;
>
> /* VMX capablity MSRs are available only when guest supports VMX. */
> - hvm_cpuid(0x1, &dummy, &dummy, &ecx, &edx);
> + hvm_cpuid(0x1, NULL, NULL, &ecx, &edx);
> if ( !(ecx & cpufeat_mask(X86_FEATURE_VMXE)) )
> return 0;
>
> @@ -1972,18 +1972,18 @@ int nvmx_msr_read_intercept(unsigned int
> if ( ecx & cpufeat_mask(X86_FEATURE_XSAVE) )
> data |= X86_CR4_OSXSAVE;
>
> - hvm_cpuid(0x0, &eax, &dummy, &dummy, &dummy);
> + hvm_cpuid(0x0, &eax, NULL, NULL, NULL);
> switch ( eax )
> {
> default:
> - hvm_cpuid(0xa, &eax, &dummy, &dummy, &dummy);
> + hvm_cpuid(0xa, &eax, NULL, NULL, NULL);
> /* Check whether guest has the perf monitor feature. */
> if ( (eax & 0xff) && (eax & 0xff00) )
> data |= X86_CR4_PCE;
> /* fall through */
> case 0x7 ... 0x9:
> ecx = 0;
> - hvm_cpuid(0x7, &dummy, &ebx, &ecx, &dummy);
> + hvm_cpuid(0x7, NULL, &ebx, &ecx, NULL);
> if ( ebx & cpufeat_mask(X86_FEATURE_FSGSBASE) )
> data |= X86_CR4_FSGSBASE;
> if ( ebx & cpufeat_mask(X86_FEATURE_SMEP) )
>
>
next prev parent reply other threads:[~2013-09-23 13:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-23 10:04 [PATCH v4 0/4] x86/HVM: miscellaneous improvements Jan Beulich
2013-09-23 10:10 ` [PATCH v4 1/4] Nested VMX: check VMX capability before read VMX related MSRs Jan Beulich
2013-09-23 14:50 ` Boris Ostrovsky
2013-09-23 14:52 ` Jan Beulich
2013-09-23 10:10 ` [PATCH v4 2/4] VMX: clean up capability checks Jan Beulich
2013-09-23 10:11 ` [PATCH v4 3/4] Nested VMX: fix IA32_VMX_CR4_FIXED1 msr emulation Jan Beulich
2013-09-23 10:12 ` [PATCH v4 4/4] x86: make hvm_cpuid() tolerate NULL pointers Jan Beulich
2013-09-23 10:34 ` Andrew Cooper
2013-09-23 13:42 ` Boris Ostrovsky [this message]
2013-09-23 13:45 ` Jan Beulich
2013-09-30 12:05 ` Ping: [PATCH v4 0/4] x86/HVM: miscellaneous improvements Jan Beulich
2013-10-02 15:48 ` Nakajima, Jun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52404554.1070200@oracle.com \
--to=boris.ostrovsky@oracle.com \
--cc=JBeulich@suse.com \
--cc=eddie.dong@intel.com \
--cc=jacob.shin@amd.com \
--cc=jun.nakajima@intel.com \
--cc=keir@xen.org \
--cc=suravee.suthikulpanit@amd.com \
--cc=xen-devel@lists.xenproject.org \
--cc=yang.z.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.