Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host
On 25/11/2015 18:26, Eduardo Habkost wrote:
>> > Yoda conditions?
>> >
>> > if (banks < MCE_BANKS_DEF) {
>> > error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM
>> > = %d)",
>> > MCE_BANKS_DEF, banks);
> This was on purpose, because MCE_BANKS_DEF is replaced by
> (env->mcg_caps & MCG_CAPS_COUNT_MASK) in the next patch.
Yeah, I noticed it later.
Paolo
Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host
On Wed, Nov 25, 2015 at 05:46:38PM +0100, Paolo Bonzini wrote:
>
>
> On 25/11/2015 16:49, Eduardo Habkost wrote:
> > Instead of silently changing the number of banks in mcg_cap based
> > on kvm_get_mce_cap_supported(), abort initialization if the host
> > doesn't support MCE_BANKS_DEF banks.
> >
> > Note that MCE_BANKS_DEF was always 10 since it was introduced in
> > QEMU, and Linux always returned 32 at KVM_CAP_MCE since
> > KVM_CAP_MCE was introduced, so no behavior is being changed and
> > the error can't be triggered by any Linux version. The point of
> > the new check is to ensure we won't silently change the bank
> > count if we change MCE_BANKS_DEF or make the bank count
> > configurable in the future.
> >
> > Signed-off-by: Eduardo Habkost
> > ---
> > target-i386/kvm.c | 9 ++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 2a9953b..ee7bc69 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > return ret;
> > }
> >
> > -if (banks > MCE_BANKS_DEF) {
> > -banks = MCE_BANKS_DEF;
> > +if (MCE_BANKS_DEF > banks) {
> > +error_report("kvm: Unsupported MCE bank count: %d > %d\n",
> > + MCE_BANKS_DEF, banks);
>
> Yoda conditions?
>
> if (banks < MCE_BANKS_DEF) {
> error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM =
> %d)",
> MCE_BANKS_DEF, banks);
This was on purpose, because MCE_BANKS_DEF is replaced by
(env->mcg_caps & MCG_CAPS_COUNT_MASK) in the next patch.
--
Eduardo
Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host
On 25/11/2015 16:49, Eduardo Habkost wrote:
> Instead of silently changing the number of banks in mcg_cap based
> on kvm_get_mce_cap_supported(), abort initialization if the host
> doesn't support MCE_BANKS_DEF banks.
>
> Note that MCE_BANKS_DEF was always 10 since it was introduced in
> QEMU, and Linux always returned 32 at KVM_CAP_MCE since
> KVM_CAP_MCE was introduced, so no behavior is being changed and
> the error can't be triggered by any Linux version. The point of
> the new check is to ensure we won't silently change the bank
> count if we change MCE_BANKS_DEF or make the bank count
> configurable in the future.
>
> Signed-off-by: Eduardo Habkost
> ---
> target-i386/kvm.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2a9953b..ee7bc69 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> return ret;
> }
>
> -if (banks > MCE_BANKS_DEF) {
> -banks = MCE_BANKS_DEF;
> +if (MCE_BANKS_DEF > banks) {
> +error_report("kvm: Unsupported MCE bank count: %d > %d\n",
> + MCE_BANKS_DEF, banks);
Yoda conditions?
if (banks < MCE_BANKS_DEF) {
error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM =
%d)",
MCE_BANKS_DEF, banks);
Paolo
> +return -ENOTSUP;
> }
> +
> mcg_cap &= MCE_CAP_DEF;
> -mcg_cap |= banks;
> +mcg_cap |= MCE_BANKS_DEF;
> ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, &mcg_cap);
> if (ret < 0) {
> fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
>
