Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Borislav Petkov]

On Wed, Nov 25, 2015 at 01:49:49PM -0200, Eduardo Habkost wrote:
> Instead of silently clearing mcg_cap bits when the host doesn't
> support them, print a warning when doing that.

Why the host? Why would we want there to be any relation between the MCA
capabilities of the host and what qemu is emulating?

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Eduardo Habkost]

On Wed, Nov 25, 2015 at 05:45:20PM +0100, Paolo Bonzini wrote:
> On 25/11/2015 16:49, Eduardo Habkost wrote:
> > Instead of silently clearing mcg_cap bits when the host doesn't
> > support them, print a warning when doing that.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  target-i386/kvm.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index d63a85b..446bdfc 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >  && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
> > (CPUID_MCE | CPUID_MCA)
> >  && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
> > -uint64_t mcg_cap;
> > +uint64_t mcg_cap, unsupported_caps;
> >  int banks;
> >  int ret;
> >  
> > @@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >  return -ENOTSUP;
> >  }
> >  
> > +unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
> > +if (unsupported_caps) {
> > +error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 
> > "\n",
> 
> \n should not be at end of error_report.
> 
> Fixed and applied.

MCG_CAP_BANKS_MASK is defined by patch 2/3. Have you applied the
whole series?

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 18:29, Eduardo Habkost wrote:
>>> > >  
>>> > > +unsupported_caps = env->mcg_cap & ~(mcg_cap | 
>>> > > MCG_CAP_BANKS_MASK);
>>> > > +if (unsupported_caps) {
>>> > > +error_report("warning: Unsupported MCG_CAP bits: 0x%" 
>>> > > PRIx64 "\n",
>> > 
>> > \n should not be at end of error_report.
>> > 
>> > Fixed and applied.
> MCG_CAP_BANKS_MASK is defined by patch 2/3. Have you applied the
> whole series?

Yes, of course.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 16:49, Eduardo Habkost wrote:
> Instead of silently clearing mcg_cap bits when the host doesn't
> support them, print a warning when doing that.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/kvm.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index d63a85b..446bdfc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
> (CPUID_MCE | CPUID_MCA)
>  && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
> -uint64_t mcg_cap;
> +uint64_t mcg_cap, unsupported_caps;
>  int banks;
>  int ret;
>  
> @@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  return -ENOTSUP;
>  }
>  
> +unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
> +if (unsupported_caps) {
> +error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 
> "\n",

\n should not be at end of error_report.

Fixed and applied.

Paolo

> + unsupported_caps);
> +}
> +
>  env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
>  ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
>  if (ret < 0) {
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 18:21, Borislav Petkov wrote:
>> Instead of silently clearing mcg_cap bits when the host doesn't
>> > support them, print a warning when doing that.
> Why the host? Why would we want there to be any relation between the MCA
> capabilities of the host and what qemu is emulating?

He means the hypervisor. :)

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Eduardo Habkost]

Instead of silently clearing mcg_cap bits when the host doesn't
support them, print a warning when doing that.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d63a85b..446bdfc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
(CPUID_MCE | CPUID_MCA)
 && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
-uint64_t mcg_cap;
+uint64_t mcg_cap, unsupported_caps;
 int banks;
 int ret;
 
@@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return -ENOTSUP;
 }
 
+unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
+if (unsupported_caps) {
+error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 "\n",
+ unsupported_caps);
+}
+
 env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
 ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
 if (ret < 0) {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Borislav Petkov]

On Wed, Nov 25, 2015 at 06:29:25PM +0100, Paolo Bonzini wrote:
> On 25/11/2015 18:21, Borislav Petkov wrote:
> >> Instead of silently clearing mcg_cap bits when the host doesn't
> >> > support them, print a warning when doing that.
> > Why the host? Why would we want there to be any relation between the MCA
> > capabilities of the host and what qemu is emulating?
> 
> He means the hypervisor. :)

Ah, ok. :)

Then they look good to me, a step in the right direction.

Acked-by: Borislav Petkov 

Thanks!

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html