Re: [PATCH 09/18] KVM: arm64: Move vcpu debug/SPE/TRBE flags to the input flag set

[Marc Zyngier <maz@kernel.org>]

On Wed, 08 Jun 2022 16:16:16 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> 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.

Ah, nice catch. That's a left-over from a previous implementation that
didn't have the notion of flag-set built-in.

There is also another one of these in kvm_host.h, which I will fix as
well.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm