linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Fuad Tabba <tabba@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	 Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	 Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Quentin Perret <qperret@google.com>,
	 Mark Brown <broonie@kernel.org>,
	kernel-team@android.com
Subject: Re: [PATCH 09/18] KVM: arm64: Move vcpu debug/SPE/TRBE flags to the input flag set
Date: Wed, 8 Jun 2022 16:16:16 +0100	[thread overview]
Message-ID: <CA+EHjTzy99Waj8P48pbzbR2+KfgUzbZX5oj_J+JKo70VOq6yiQ@mail.gmail.com> (raw)
In-Reply-To: <20220528113829.1043361-10-maz@kernel.org>

Hi Marc,

On Sat, May 28, 2022 at 12:38 PM Marc Zyngier <maz@kernel.org> wrote:
>
> The three debug flags (which deal with the debug registers, SPE and
> TRBE) all are input flags to the hypervisor code.
>
> Move them into the input set and convert them to the new accessors.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_host.h          |  9 ++++++---
>  arch/arm64/kvm/debug.c                     | 22 +++++++++++-----------
>  arch/arm64/kvm/hyp/include/hyp/debug-sr.h  |  6 +++---
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  4 ++--
>  arch/arm64/kvm/hyp/nvhe/debug-sr.c         |  8 ++++----
>  arch/arm64/kvm/sys_regs.c                  |  8 ++++----
>  6 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 078567f5709c..a426cd3aaa74 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -500,6 +500,12 @@ struct kvm_vcpu_arch {
>  #define EXCEPT_AA64_EL2_IRQ    __vcpu_except_flags(5)
>  #define EXCEPT_AA64_EL2_FIQ    __vcpu_except_flags(6)
>  #define EXCEPT_AA64_EL2_SERR   __vcpu_except_flags(7)
> +/* Guest debug is live */
> +#define DEBUG_DIRTY            __vcpu_single_flag(iflags, BIT(4))
> +/* Save SPE context if active  */
> +#define DEBUG_STATE_SAVE_SPE   __vcpu_single_flag(iflags, BIT(5))
> +/* Save TRBE context if active  */
> +#define DEBUG_STATE_SAVE_TRBE  __vcpu_single_flag(iflags, BIT(6))
>
>  /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
>  #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +     \
> @@ -522,10 +528,7 @@ struct kvm_vcpu_arch {
>  })
>
>  /* vcpu_arch flags field values: */
> -#define KVM_ARM64_DEBUG_DIRTY          (1 << 0)
>  #define KVM_ARM64_HOST_SVE_ENABLED     (1 << 4) /* SVE enabled for EL0 */
> -#define KVM_ARM64_DEBUG_STATE_SAVE_SPE (1 << 12) /* Save SPE context if active  */
> -#define KVM_ARM64_DEBUG_STATE_SAVE_TRBE        (1 << 13) /* Save TRBE context if active  */
>  #define KVM_ARM64_ON_UNSUPPORTED_CPU   (1 << 15) /* Physical CPU not in supported_cpus */
>  #define KVM_ARM64_HOST_SME_ENABLED     (1 << 16) /* SME enabled for EL0 */
>  #define KVM_ARM64_WFIT                 (1 << 17) /* WFIT instruction trapped */
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 4fd5c216c4bb..c5c4c1837bf3 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -104,11 +104,11 @@ static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
>          * Trap debug register access when one of the following is true:
>          *  - Userspace is using the hardware to debug the guest
>          *  (KVM_GUESTDBG_USE_HW is set).
> -        *  - The guest is not using debug (KVM_ARM64_DEBUG_DIRTY is clear).
> +        *  - The guest is not using debug (DEBUG_DIRTY clear).
>          *  - The guest has enabled the OS Lock (debug exceptions are blocked).
>          */
>         if ((vcpu->guest_debug & KVM_GUESTDBG_USE_HW) ||
> -           !(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY) ||
> +           !vcpu_get_flag(vcpu, DEBUG_DIRTY) ||
>             kvm_vcpu_os_lock_enabled(vcpu))
>                 vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
>
> @@ -147,8 +147,8 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
>   * debug related registers.
>   *
>   * Additionally, KVM only traps guest accesses to the debug registers if
> - * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
> - * flag on vcpu->arch.flags).  Since the guest must not interfere
> + * the guest is not actively using them (see the DEBUG_DIRTY
> + * flag on vcpu->arch.iflags).  Since the guest must not interfere
>   * with the hardware state when debugging the guest, we must ensure that
>   * trapping is enabled whenever we are debugging the guest using the
>   * debug registers.
> @@ -205,7 +205,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>                  *
>                  * We simply switch the debug_ptr to point to our new
>                  * external_debug_state which has been populated by the
> -                * debug ioctl. The existing KVM_ARM64_DEBUG_DIRTY
> +                * debug ioctl. The existing KVM_ARM64_IFLAG_DEBUG_DIRTY

This should be DEBUG_DIRTY.

Cheers,
/fuad


>                  * mechanism ensures the registers are updated on the
>                  * world switch.
>                  */
> @@ -216,7 +216,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>                         vcpu_write_sys_reg(vcpu, mdscr, MDSCR_EL1);
>
>                         vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
> -                       vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
> +                       vcpu_set_flag(vcpu, DEBUG_DIRTY);
>
>                         trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
>                                                 &vcpu->arch.debug_ptr->dbg_bcr[0],
> @@ -246,7 +246,7 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>
>         /* If KDE or MDE are set, perform a full save/restore cycle. */
>         if (vcpu_read_sys_reg(vcpu, MDSCR_EL1) & (DBG_MDSCR_KDE | DBG_MDSCR_MDE))
> -               vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
> +               vcpu_set_flag(vcpu, DEBUG_DIRTY);
>
>         /* Write mdcr_el2 changes since vcpu_load on VHE systems */
>         if (has_vhe() && orig_mdcr_el2 != vcpu->arch.mdcr_el2)
> @@ -298,16 +298,16 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
>          */
>         if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_PMSVER_SHIFT) &&
>             !(read_sysreg_s(SYS_PMBIDR_EL1) & BIT(SYS_PMBIDR_EL1_P_SHIFT)))
> -               vcpu->arch.flags |= KVM_ARM64_DEBUG_STATE_SAVE_SPE;
> +               vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_SPE);
>
>         /* Check if we have TRBE implemented and available at the host */
>         if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TRBE_SHIFT) &&
>             !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_PROG))
> -               vcpu->arch.flags |= KVM_ARM64_DEBUG_STATE_SAVE_TRBE;
> +               vcpu_set_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>  }
>
>  void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu)
>  {
> -       vcpu->arch.flags &= ~(KVM_ARM64_DEBUG_STATE_SAVE_SPE |
> -                             KVM_ARM64_DEBUG_STATE_SAVE_TRBE);
> +       vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_SPE);
> +       vcpu_clear_flag(vcpu, DEBUG_STATE_SAVE_TRBE);
>  }
> diff --git a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> index 4ebe9f558f3a..961bbef104a6 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
> @@ -132,7 +132,7 @@ static inline void __debug_switch_to_guest_common(struct kvm_vcpu *vcpu)
>         struct kvm_guest_debug_arch *host_dbg;
>         struct kvm_guest_debug_arch *guest_dbg;
>
> -       if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
> +       if (!vcpu_get_flag(vcpu, DEBUG_DIRTY))
>                 return;
>
>         host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> @@ -151,7 +151,7 @@ static inline void __debug_switch_to_host_common(struct kvm_vcpu *vcpu)
>         struct kvm_guest_debug_arch *host_dbg;
>         struct kvm_guest_debug_arch *guest_dbg;
>
> -       if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
> +       if (!vcpu_get_flag(vcpu, DEBUG_DIRTY))
>                 return;
>
>         host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> @@ -162,7 +162,7 @@ static inline void __debug_switch_to_host_common(struct kvm_vcpu *vcpu)
>         __debug_save_state(guest_dbg, guest_ctxt);
>         __debug_restore_state(host_dbg, host_ctxt);
>
> -       vcpu->arch.flags &= ~KVM_ARM64_DEBUG_DIRTY;
> +       vcpu_clear_flag(vcpu, DEBUG_DIRTY);
>  }
>
>  #endif /* __ARM64_KVM_HYP_DEBUG_SR_H__ */
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 7ecca8b07851..baa5b9b3dde5 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -195,7 +195,7 @@ static inline void __sysreg32_save_state(struct kvm_vcpu *vcpu)
>         __vcpu_sys_reg(vcpu, DACR32_EL2) = read_sysreg(dacr32_el2);
>         __vcpu_sys_reg(vcpu, IFSR32_EL2) = read_sysreg(ifsr32_el2);
>
> -       if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
> +       if (has_vhe() || vcpu_get_flag(vcpu, DEBUG_DIRTY))
>                 __vcpu_sys_reg(vcpu, DBGVCR32_EL2) = read_sysreg(dbgvcr32_el2);
>  }
>
> @@ -212,7 +212,7 @@ static inline void __sysreg32_restore_state(struct kvm_vcpu *vcpu)
>         write_sysreg(__vcpu_sys_reg(vcpu, DACR32_EL2), dacr32_el2);
>         write_sysreg(__vcpu_sys_reg(vcpu, IFSR32_EL2), ifsr32_el2);
>
> -       if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
> +       if (has_vhe() || vcpu_get_flag(vcpu, DEBUG_DIRTY))
>                 write_sysreg(__vcpu_sys_reg(vcpu, DBGVCR32_EL2), dbgvcr32_el2);
>  }
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> index df361d839902..e17455773b98 100644
> --- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
> @@ -84,10 +84,10 @@ static void __debug_restore_trace(u64 trfcr_el1)
>  void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
>  {
>         /* Disable and flush SPE data generation */
> -       if (vcpu->arch.flags & KVM_ARM64_DEBUG_STATE_SAVE_SPE)
> +       if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_SPE))
>                 __debug_save_spe(&vcpu->arch.host_debug_state.pmscr_el1);
>         /* Disable and flush Self-Hosted Trace generation */
> -       if (vcpu->arch.flags & KVM_ARM64_DEBUG_STATE_SAVE_TRBE)
> +       if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
>                 __debug_save_trace(&vcpu->arch.host_debug_state.trfcr_el1);
>  }
>
> @@ -98,9 +98,9 @@ void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
>
>  void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
>  {
> -       if (vcpu->arch.flags & KVM_ARM64_DEBUG_STATE_SAVE_SPE)
> +       if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_SPE))
>                 __debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
> -       if (vcpu->arch.flags & KVM_ARM64_DEBUG_STATE_SAVE_TRBE)
> +       if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
>                 __debug_restore_trace(vcpu->arch.host_debug_state.trfcr_el1);
>  }
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d77be152cbd5..d6a55ed9ff10 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -387,7 +387,7 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
>  {
>         if (p->is_write) {
>                 vcpu_write_sys_reg(vcpu, p->regval, r->reg);
> -               vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
> +               vcpu_set_flag(vcpu, DEBUG_DIRTY);
>         } else {
>                 p->regval = vcpu_read_sys_reg(vcpu, r->reg);
>         }
> @@ -403,8 +403,8 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
>   * A 32 bit write to a debug register leave top bits alone
>   * A 32 bit read from a debug register only returns the bottom bits
>   *
> - * All writes will set the KVM_ARM64_DEBUG_DIRTY flag to ensure the
> - * hyp.S code switches between host and guest values in future.
> + * All writes will set the DEBUG_DIRTY flag to ensure the hyp code
> + * switches between host and guest values in future.
>   */
>  static void reg_to_dbg(struct kvm_vcpu *vcpu,
>                        struct sys_reg_params *p,
> @@ -420,7 +420,7 @@ static void reg_to_dbg(struct kvm_vcpu *vcpu,
>         val |= (p->regval & (mask >> shift)) << shift;
>         *dbg_reg = val;
>
> -       vcpu->arch.flags |= KVM_ARM64_DEBUG_DIRTY;
> +       vcpu_set_flag(vcpu, DEBUG_DIRTY);
>  }
>
>  static void dbg_to_reg(struct kvm_vcpu *vcpu,
> --
> 2.34.1
>

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

  reply	other threads:[~2022-06-08 15:18 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-28 11:38 [PATCH 00/18] KVM/arm64: Refactoring the vcpu flags Marc Zyngier
2022-05-28 11:38 ` [PATCH 01/18] KVM: arm64: Always start with clearing SVE flag on load Marc Zyngier
2022-05-30 14:41   ` Mark Brown
2022-06-06 11:28     ` Marc Zyngier
2022-06-06 12:16       ` Mark Brown
2022-05-28 11:38 ` [PATCH 02/18] KVM: arm64: Always start with clearing SME " Marc Zyngier
2022-05-30 14:51   ` Mark Brown
2022-05-28 11:38 ` [PATCH 03/18] KVM: arm64: Drop FP_FOREIGN_STATE from the hypervisor code Marc Zyngier
2022-06-03  5:23   ` Reiji Watanabe
2022-06-04  8:10     ` Marc Zyngier
2022-06-07  4:47       ` Reiji Watanabe
2022-06-03  9:09   ` Mark Brown
2022-05-28 11:38 ` [PATCH 04/18] KVM: arm64: Move FP state ownership from flag to a tristate Marc Zyngier
2022-06-03  9:14   ` Mark Brown
2022-06-06  8:41     ` Marc Zyngier
2022-06-06 10:31       ` Mark Brown
2022-06-04  8:16   ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 05/18] KVM: arm64: Add helpers to manipulate vcpu flags among a set Marc Zyngier
2022-06-08  5:26   ` Reiji Watanabe
2022-06-08  6:51     ` Marc Zyngier
2022-06-09  2:25       ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 06/18] KVM: arm64: Add three sets of flags to the vcpu state Marc Zyngier
2022-06-08 15:23   ` Fuad Tabba
2022-06-09  6:10   ` Reiji Watanabe
2022-06-09  7:46     ` Marc Zyngier
2022-06-09 17:24       ` Reiji Watanabe
2022-06-10  7:48         ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 07/18] KVM: arm64: Move vcpu configuration flags into their own set Marc Zyngier
2022-06-09  6:15   ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 08/18] KVM: arm64: Move vcpu PC/Exception flags to the input flag set Marc Zyngier
2022-06-10  6:13   ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 09/18] KVM: arm64: Move vcpu debug/SPE/TRBE " Marc Zyngier
2022-06-08 15:16   ` Fuad Tabba [this message]
2022-06-08 16:01     ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 10/18] KVM: arm64: Move vcpu SVE/SME flags to the state " Marc Zyngier
2022-05-28 11:38 ` [PATCH 11/18] KVM: arm64: Move vcpu ON_UNSUPPORTED_CPU flag " Marc Zyngier
2022-05-28 11:38 ` [PATCH 12/18] KVM: arm64: Move vcpu WFIT " Marc Zyngier
2022-05-28 11:38 ` [PATCH 13/18] KVM: arm64: Kill unused vcpu flags field Marc Zyngier
2022-05-28 11:38 ` [PATCH 14/18] KVM: arm64: Convert vcpu sysregs_loaded_on_cpu to a state flag Marc Zyngier
2022-05-28 11:38 ` [PATCH 15/18] KVM: arm64: Warn when PENDING_EXCEPTION and INCREMENT_PC are set together Marc Zyngier
2022-06-08 15:16   ` Fuad Tabba
2022-06-08 16:42     ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 16/18] KVM: arm64: Add build-time sanity checks for flags Marc Zyngier
2022-05-28 11:38 ` [PATCH 17/18] KVM: arm64: Reduce the size of the vcpu flag members Marc Zyngier
2022-05-28 11:38 ` [PATCH 18/18] KVM: arm64: Document why pause cannot be turned into a flag Marc Zyngier
2022-05-30  8:28 ` [PATCH 00/18] KVM/arm64: Refactoring the vcpu flags Marc Zyngier
2022-06-07 13:43 ` Marc Zyngier

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=CA+EHjTzy99Waj8P48pbzbR2+KfgUzbZX5oj_J+JKo70VOq6yiQ@mail.gmail.com \
    --to=tabba@google.com \
    --cc=alexandru.elisei@arm.com \
    --cc=broonie@kernel.org \
    --cc=james.morse@arm.com \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=oupton@google.com \
    --cc=qperret@google.com \
    --cc=suzuki.poulose@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).