Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210621111716.37157-1-steven.price@arm.com> <20210621111716.37157-5-steven.price@arm.com>
In-Reply-To: <20210621111716.37157-5-steven.price@arm.com>
From:   Fuad Tabba <tabba@google.com>
Date:   Tue, 22 Jun 2021 09:07:51 +0100
Message-ID: <CA+EHjTzuduzTcJo+jjVzVAVUB4i3Nr3mki4jyiNW9K=pr-HPYA@mail.gmail.com>
Subject: Re: [PATCH v17 4/6] KVM: arm64: Expose KVM_ARM_CAP_MTE
To:     Steven Price <steven.price@arm.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
        "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
        qemu-devel@nongnu.org, Dave Martin <Dave.Martin@arm.com>,
        Juan Quintela <quintela@redhat.com>,
        Richard Henderson <richard.henderson@linaro.org>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Mon, Jun 21, 2021 at 12:18 PM Steven Price <steven.price@arm.com> wrote:
>
> It's now safe for the VMM to enable MTE in a guest, so expose the
> capability to user space.
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
>  arch/arm64/kvm/arm.c      | 9 +++++++++
>  arch/arm64/kvm/reset.c    | 4 ++++
>  arch/arm64/kvm/sys_regs.c | 3 +++
>  3 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index e720148232a0..28ce26a68f09 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -93,6 +93,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>                 r = 0;
>                 kvm->arch.return_nisv_io_abort_to_user = true;
>                 break;
> +       case KVM_CAP_ARM_MTE:
> +               if (!system_supports_mte() || kvm->created_vcpus)
> +                       return -EINVAL;
> +               r = 0;
> +               kvm->arch.mte_enabled = true;
> +               break;
>         default:
>                 r = -EINVAL;
>                 break;
> @@ -237,6 +243,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>                  */
>                 r = 1;
>                 break;
> +       case KVM_CAP_ARM_MTE:
> +               r = system_supports_mte();
> +               break;
>         case KVM_CAP_STEAL_TIME:
>                 r = kvm_arm_pvtime_supported();
>                 break;
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index d37ebee085cf..9e6922b9503a 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -244,6 +244,10 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>         switch (vcpu->arch.target) {
>         default:
>                 if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
> +                       if (vcpu->kvm->arch.mte_enabled) {
> +                               ret = -EINVAL;
> +                               goto out;
> +                       }
>                         pstate = VCPU_RESET_PSTATE_SVC;
>                 } else {
>                         pstate = VCPU_RESET_PSTATE_EL1;

nit: I was wondering whether this check would be better suited in
kvm_vcpu_set_target, rather than here (kvm_reset_vcpu). kvm_reset_vcpu
is called by kvm_vcpu_set_target, but kvm_vcpu_set_target is where
checking for supported features happens. It might be better to group
all such checks together. I don't think that there is any risk of this
feature being toggled by the other call path to kvm_reset_vcpu (via
check_vcpu_requests).

Cheers,
/fuad

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 5c75b24eae21..f6f126eb6ac1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1312,6 +1312,9 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  static unsigned int mte_visibility(const struct kvm_vcpu *vcpu,
>                                    const struct sys_reg_desc *rd)
>  {
> +       if (kvm_has_mte(vcpu->kvm))
> +               return 0;
> +
>         return REG_HIDDEN;
>  }
>
> --
> 2.20.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm