All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oupton@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: kvm@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	Peter Shier <pshier@google.com>, Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 10/27] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest
Date: Thu, 17 Feb 2022 04:59:43 +0000	[thread overview]
Message-ID: <Yg3WP5HR9OJJMLj7@google.com> (raw)
In-Reply-To: <CAAeT=Fxvsniq4NW92LESqJ1ie6e+N1J793JrX0UBf2mq9B35dg@mail.gmail.com>

On Tue, Feb 15, 2022 at 06:52:27PM -0800, Reiji Watanabe wrote:
> Hi Oliver,
> 
> Thank you for the review!
> 
> On Tue, Feb 15, 2022 at 10:57 AM Oliver Upton <oupton@google.com> wrote:
> >
> > Hi Reiji,
> >
> > On Sun, Feb 13, 2022 at 10:57:29PM -0800, Reiji Watanabe wrote:
> > > When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which
> > > means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally
> > > expose the value for the guest as it is.  Since KVM doesn't support
> > > IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should
> > > expose 0x0 (PMU is not implemented) instead.
> > >
> > > Change cpuid_feature_cap_perfmon_field() to update the field value
> > > to 0x0 when it is 0xf.
> >
> > Definitely agree with the change in this patch. Do we need to tolerate
> > writes of 0xf for ABI compatibility (even if it is nonsensical)?
> > Otherwise a guest with IMP_DEF PMU cannot be migrated to a newer kernel.
> 
> Hmm, yes, I think KVM should tolerate writes of 0xf so that we can
> avoid the migration failure.  I will make this change in v6.
> 
> Since ID registers are immutable with the current KVM, I think a live
> migration failure to a newer kernel happens when the newer kernel/KVM
> supports more CPU features (or when an ID register field is newly
> masked or capped by KVM, etc).  So, I would assume such migration
> breakage on KVM/ARM has been introduced from time to time though.
>

Indeed it has, but IMO migration breakage should really be avoided at
all costs. End of the day, its ABI breakage.

Unless folks feel otherwise, I would be in favor of just ignoring the
IMP_DEF write and setting the field value to NI instead. Allows VMs to
migrate onto newer kernels and fixes the KVM bug at the same time.

--
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Will Deacon <will@kernel.org>, Andrew Jones <drjones@redhat.com>,
	Fuad Tabba <tabba@google.com>,
	Peng Liang <liangpeng10@huawei.com>,
	Peter Shier <pshier@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v5 10/27] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest
Date: Thu, 17 Feb 2022 04:59:43 +0000	[thread overview]
Message-ID: <Yg3WP5HR9OJJMLj7@google.com> (raw)
In-Reply-To: <CAAeT=Fxvsniq4NW92LESqJ1ie6e+N1J793JrX0UBf2mq9B35dg@mail.gmail.com>

On Tue, Feb 15, 2022 at 06:52:27PM -0800, Reiji Watanabe wrote:
> Hi Oliver,
> 
> Thank you for the review!
> 
> On Tue, Feb 15, 2022 at 10:57 AM Oliver Upton <oupton@google.com> wrote:
> >
> > Hi Reiji,
> >
> > On Sun, Feb 13, 2022 at 10:57:29PM -0800, Reiji Watanabe wrote:
> > > When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which
> > > means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally
> > > expose the value for the guest as it is.  Since KVM doesn't support
> > > IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should
> > > expose 0x0 (PMU is not implemented) instead.
> > >
> > > Change cpuid_feature_cap_perfmon_field() to update the field value
> > > to 0x0 when it is 0xf.
> >
> > Definitely agree with the change in this patch. Do we need to tolerate
> > writes of 0xf for ABI compatibility (even if it is nonsensical)?
> > Otherwise a guest with IMP_DEF PMU cannot be migrated to a newer kernel.
> 
> Hmm, yes, I think KVM should tolerate writes of 0xf so that we can
> avoid the migration failure.  I will make this change in v6.
> 
> Since ID registers are immutable with the current KVM, I think a live
> migration failure to a newer kernel happens when the newer kernel/KVM
> supports more CPU features (or when an ID register field is newly
> masked or capped by KVM, etc).  So, I would assume such migration
> breakage on KVM/ARM has been introduced from time to time though.
>

Indeed it has, but IMO migration breakage should really be avoided at
all costs. End of the day, its ABI breakage.

Unless folks feel otherwise, I would be in favor of just ignoring the
IMP_DEF write and setting the field value to NI instead. Allows VMs to
migrate onto newer kernels and fixes the KVM bug at the same time.

--
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Will Deacon <will@kernel.org>, Andrew Jones <drjones@redhat.com>,
	Fuad Tabba <tabba@google.com>,
	Peng Liang <liangpeng10@huawei.com>,
	Peter Shier <pshier@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v5 10/27] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest
Date: Thu, 17 Feb 2022 04:59:43 +0000	[thread overview]
Message-ID: <Yg3WP5HR9OJJMLj7@google.com> (raw)
In-Reply-To: <CAAeT=Fxvsniq4NW92LESqJ1ie6e+N1J793JrX0UBf2mq9B35dg@mail.gmail.com>

On Tue, Feb 15, 2022 at 06:52:27PM -0800, Reiji Watanabe wrote:
> Hi Oliver,
> 
> Thank you for the review!
> 
> On Tue, Feb 15, 2022 at 10:57 AM Oliver Upton <oupton@google.com> wrote:
> >
> > Hi Reiji,
> >
> > On Sun, Feb 13, 2022 at 10:57:29PM -0800, Reiji Watanabe wrote:
> > > When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which
> > > means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally
> > > expose the value for the guest as it is.  Since KVM doesn't support
> > > IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should
> > > expose 0x0 (PMU is not implemented) instead.
> > >
> > > Change cpuid_feature_cap_perfmon_field() to update the field value
> > > to 0x0 when it is 0xf.
> >
> > Definitely agree with the change in this patch. Do we need to tolerate
> > writes of 0xf for ABI compatibility (even if it is nonsensical)?
> > Otherwise a guest with IMP_DEF PMU cannot be migrated to a newer kernel.
> 
> Hmm, yes, I think KVM should tolerate writes of 0xf so that we can
> avoid the migration failure.  I will make this change in v6.
> 
> Since ID registers are immutable with the current KVM, I think a live
> migration failure to a newer kernel happens when the newer kernel/KVM
> supports more CPU features (or when an ID register field is newly
> masked or capped by KVM, etc).  So, I would assume such migration
> breakage on KVM/ARM has been introduced from time to time though.
>

Indeed it has, but IMO migration breakage should really be avoided at
all costs. End of the day, its ABI breakage.

Unless folks feel otherwise, I would be in favor of just ignoring the
IMP_DEF write and setting the field value to NI instead. Allows VMs to
migrate onto newer kernels and fixes the KVM bug at the same time.

--
Oliver

  reply	other threads:[~2022-02-17  4:59 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-14  6:57 [PATCH v5 00/27] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
2022-02-14  6:57 ` Reiji Watanabe
2022-02-14  6:57 ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 01/27] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 02/27] KVM: arm64: Save ID registers' sanitized value per guest Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 03/27] KVM: arm64: Introduce struct id_reg_info Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-17  5:14   ` Oliver Upton
2022-02-17  5:14     ` Oliver Upton
2022-02-17  5:14     ` Oliver Upton
2022-02-22  6:12     ` Reiji Watanabe
2022-02-22  6:12       ` Reiji Watanabe
2022-02-22  6:12       ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 04/27] KVM: arm64: Make ID_AA64PFR0_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 05/27] KVM: arm64: Make ID_AA64PFR1_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 06/27] KVM: arm64: Make ID_AA64ISAR0_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 07/27] KVM: arm64: Make ID_AA64ISAR1_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 08/27] KVM: arm64: Make ID_AA64MMFR0_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 09/27] KVM: arm64: Make ID_AA64MMFR1_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-15 18:53   ` Oliver Upton
2022-02-15 18:53     ` Oliver Upton
2022-02-15 18:53     ` Oliver Upton
2022-02-15 20:24     ` Reiji Watanabe
2022-02-15 20:24       ` Reiji Watanabe
2022-02-15 20:24       ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 10/27] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-15 18:57   ` Oliver Upton
2022-02-15 18:57     ` Oliver Upton
2022-02-15 18:57     ` Oliver Upton
2022-02-16  2:52     ` Reiji Watanabe
2022-02-16  2:52       ` Reiji Watanabe
2022-02-16  2:52       ` Reiji Watanabe
2022-02-17  4:59       ` Oliver Upton [this message]
2022-02-17  4:59         ` Oliver Upton
2022-02-17  4:59         ` Oliver Upton
2022-02-14  6:57 ` [PATCH v5 11/27] KVM: arm64: Make ID_AA64DFR0_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 12/27] KVM: arm64: Make ID_DFR0_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 13/27] KVM: arm64: Make MVFR1_EL1 writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 14/27] KVM: arm64: Make ID registers without id_reg_info writable Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 15/27] KVM: arm64: Add consistency checking for frac fields of ID registers Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 16/27] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_CONFIGURABLE capability Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 17/27] KVM: arm64: Add kunit test for ID register validation Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 18/27] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 19/27] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 20/27] KVM: arm64: Introduce framework to trap disabled features Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 21/27] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 22/27] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 23/27] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 24/27] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 25/27] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1 Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 26/27] KVM: arm64: Add kunit test for trap initialization Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57 ` [PATCH v5 27/27] KVM: arm64: selftests: Introduce id_reg_test Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe
2022-02-14  6:57   ` Reiji Watanabe

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=Yg3WP5HR9OJJMLj7@google.com \
    --to=oupton@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=pshier@google.com \
    --cc=reijiw@google.com \
    --cc=will@kernel.org \
    /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.