From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 18E57C433EF for ; Tue, 30 Nov 2021 01:30:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236373AbhK3Bdc (ORCPT ); Mon, 29 Nov 2021 20:33:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46706 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236332AbhK3Bdc (ORCPT ); Mon, 29 Nov 2021 20:33:32 -0500 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2616C061574 for ; Mon, 29 Nov 2021 17:30:13 -0800 (PST) Received: by mail-pl1-x634.google.com with SMTP id b11so13596323pld.12 for ; Mon, 29 Nov 2021 17:30:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=e+pDAbhA3qgo5VrRLD3GioZhOJH88Y+ZobynMlP5+GQ3GX6zzk43B/1yG+rsEjTK2j jDOwaSNw3exODm+rdBqEbseuVL90SfGErs9xFCBOAFKDOnF2L47MddrQrzeBo49E67Ac xawvOZp1p9ika1lAvGxvLrtaSlCN/xAwB6Nl1eK33Iixtbjn3fJMew6EpmlxshrqqTww 5/sixT3Bp6t/lvEBrEIIuZnD0FvTLRX8glF8VI7PW4vkXKw/5gDN2XKrn5/l0rP5wF/x 5Tp0aZXrijks003EJUQX09dSEA3bhgVpqSOt36HfY699jD2sNR9secXTNrMHHrLVRa9D pODg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=DRti9o9DoOoUEzb5bzoAog2mjvQzqMHjw/B79xM+769ZlNHW22rCY7/NEGybS6H1H2 K6ArZyvS4OCuHfd4uCLbjHU+QtgcZQyIun3QcFzyfQrpxx949u//jckBXH0nh1RjS4uw LqOTQxWCpQrG0S3204GPYFfdeJ7BdBYmlm2JGUbAG25dpDoVU9adJTpZug+WgYbLygBY YuvwhWgcLCfa0ZSa+i55mbT5WloTAHl8F3lBXfw5SSJECYxhXaX9pcg8XniAx0gKnL5C qeTYoR0Mf+DL8F4e3o2E+tvUmy3vkA1XlJ6vtWut3Bn+TKEcjnL6ctPiKxVsG84NXXyQ z1BA== X-Gm-Message-State: AOAM532lL/lQLUN8M17eXc3xv4aJyRiPnIgA1J1Ujr3iw7704sJ/4MTz osovKRuEnfvVkXOF2mNNkz9czaXvysIDZ6HUnbfQcg== X-Google-Smtp-Source: ABdhPJzq1HYUtohDmdrT9AteQ7TOnwp5TltRApAUwblcjchFt88a9JJpwnFl4yiZ3v/V9q0A0N9AnzGuPZTdwUJXvTg= X-Received: by 2002:a17:902:aa43:b0:142:6919:73da with SMTP id c3-20020a170902aa4300b00142691973damr63733052plr.39.1638235813148; Mon, 29 Nov 2021 17:30:13 -0800 (PST) MIME-Version: 1.0 References: <20211117064359.2362060-1-reijiw@google.com> <20211117064359.2362060-5-reijiw@google.com> In-Reply-To: From: Reiji Watanabe Date: Mon, 29 Nov 2021 17:29:57 -0800 Message-ID: Subject: Re: [RFC PATCH v3 04/29] KVM: arm64: Make ID_AA64PFR0_EL1 writable To: Eric Auger Cc: Marc Zyngier , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Will Deacon , Peter Shier , Paolo Bonzini , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Eric, On Thu, Nov 25, 2021 at 7:35 AM Eric Auger wrote: > > Hi Reiji, > > On 11/17/21 7:43 AM, Reiji Watanabe wrote: > > This patch adds id_reg_info for ID_AA64PFR0_EL1 to make it writable by > > userspace. > > > > The CSV2/CSV3 fields of the register were already writable and values > > that were written for them affected all vCPUs before. Now they only > > affect the vCPU. > > Return an error if userspace tries to set SVE/GIC field of the register > > to a value that conflicts with SVE/GIC configuration for the guest. > > SIMD/FP/SVE fields of the requested value are validated according to > > Arm ARM. > > > > Signed-off-by: Reiji Watanabe > > --- > > arch/arm64/kvm/sys_regs.c | 159 ++++++++++++++++++++++++-------------- > > 1 file changed, 103 insertions(+), 56 deletions(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 1552cd5581b7..35400869067a 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -401,6 +401,92 @@ static void id_reg_info_init(struct id_reg_info *id_reg) > > id_reg->init(id_reg); > > } > > > > +#define kvm_has_gic3(kvm) \ > > + (irqchip_in_kernel(kvm) && \ > > + (kvm)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) > you may move this macro to kvm/arm_vgic.h as this may be used in > vgic/vgic-v3.c too Thank you for the suggestion. I will move that to kvm/arm_vgic.h. > > + > > +static int validate_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *id_reg, u64 val) > > +{ > > + int fp, simd; > > + bool vcpu_has_sve = vcpu_has_sve(vcpu); > > + bool pfr0_has_sve = id_aa64pfr0_sve(val); > > + int gic; > > + > > + simd = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_ASIMD_SHIFT); > > + fp = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_FP_SHIFT); > > + if (simd != fp) > > + return -EINVAL; > > + > > + /* fp must be supported when sve is supported */ > > + if (pfr0_has_sve && (fp < 0)) > > + return -EINVAL; > > + > > + /* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */ > > + if (vcpu_has_sve ^ pfr0_has_sve) > > + return -EPERM; > > + > > + gic = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_GIC_SHIFT); > > + if ((gic > 0) ^ kvm_has_gic3(vcpu->kvm)) > > + return -EPERM; > > Sometimes from a given architecture version, some lower values are not > allowed. For instance from ARMv8.5 onlt 1 is permitted for CSV3. > Shouldn't we handle that kind of check? As far as I know, there is no way for guests to identify the architecture revision (e.g. v8.1, v8.2, etc). It might be able to indirectly infer the revision though (from features that are available or etc). > > + > > + return 0; > > +} > > + > > +static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg) > > +{ > > + u64 limit = id_reg->vcpu_limit_val; > > + unsigned int gic; > > + > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_AMU); > > + if (!system_supports_sve()) > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + /* > > + * The default is to expose CSV2 == 1 and CSV3 == 1 if the HW > > + * isn't affected. Userspace can override this as long as it > > + * doesn't promise the impossible. > > + */ > > + limit &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2) | > > + ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3)); > > + > > + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), 1); > > + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), 1); > > + > > + gic = cpuid_feature_extract_unsigned_field(limit, ID_AA64PFR0_GIC_SHIFT); > > + if (gic > 1) { > > + /* Limit to GICv3.0/4.0 */ > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1); > > + } > > + id_reg->vcpu_limit_val = limit; > > +} > > + > > +static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *idr) > > +{ > > + u64 val = idr->vcpu_limit_val; > > + > > + if (!vcpu_has_sve(vcpu)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + if (!kvm_has_gic3(vcpu->kvm)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + > > + return val; > > +} > > + > > +static struct id_reg_info id_aa64pfr0_el1_info = { > > + .sys_reg = SYS_ID_AA64PFR0_EL1, > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? They are needed because they are signed fields (the default is unsigned and FCT_LOWER_SAFE). Having said that, ftr_check_types itself will be gone in the next version (as arm64_ftr_bits will be used instead). Thanks, Reiji From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F353C433F5 for ; Tue, 30 Nov 2021 01:30:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 0367C4B191; Mon, 29 Nov 2021 20:30:19 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zUUKVBDM-VOw; Mon, 29 Nov 2021 20:30:17 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 072F34B1D3; Mon, 29 Nov 2021 20:30:17 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 5CBE04B1B0 for ; Mon, 29 Nov 2021 20:30:16 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bhud5HQmb3rl for ; Mon, 29 Nov 2021 20:30:14 -0500 (EST) Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 5B6784B191 for ; Mon, 29 Nov 2021 20:30:14 -0500 (EST) Received: by mail-pj1-f50.google.com with SMTP id v23so14062348pjr.5 for ; Mon, 29 Nov 2021 17:30:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=e+pDAbhA3qgo5VrRLD3GioZhOJH88Y+ZobynMlP5+GQ3GX6zzk43B/1yG+rsEjTK2j jDOwaSNw3exODm+rdBqEbseuVL90SfGErs9xFCBOAFKDOnF2L47MddrQrzeBo49E67Ac xawvOZp1p9ika1lAvGxvLrtaSlCN/xAwB6Nl1eK33Iixtbjn3fJMew6EpmlxshrqqTww 5/sixT3Bp6t/lvEBrEIIuZnD0FvTLRX8glF8VI7PW4vkXKw/5gDN2XKrn5/l0rP5wF/x 5Tp0aZXrijks003EJUQX09dSEA3bhgVpqSOt36HfY699jD2sNR9secXTNrMHHrLVRa9D pODg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=XtBcc5H9Li1wurr4ZGcnaAbry+SY1UR3OkRg48I2wyab9vvKNHKyzY3SXGoj8eLayK y99ZHhHEjq4wzJ2hzPOi+FFIroOYuyFMAVvjpvALh1vHt11TCXnoZSfPsGqd0i/dVwgD mOMp7TsCeHt9CPwuB6g6TwzndLxwtXidrfJv2fjqbNWq2lwAUgKm3FgKDi1eM2uA05y2 fJxHQhzmHQzQIhEYzT15f2tBIWaAu7A+Dq48bLYCv3PPVZ6TuQAn5SO6T96vGXF1KJLN Qd8ZqXFK3feYPKMX31g9oIp7nR0RnDOnPwbfWpg5SUCx8ZZqW/xPdwC4l2lGPFCAqvVx 9wGg== X-Gm-Message-State: AOAM530dbgj8N+g0EfZxoreO/y+dB9pfnS0MJjibfFqJyGOJ7h4VRgxj yCmIEK06IXy1Eg5yU3JsOyct/N05lSNcJdsaE9j8hQ== X-Google-Smtp-Source: ABdhPJzq1HYUtohDmdrT9AteQ7TOnwp5TltRApAUwblcjchFt88a9JJpwnFl4yiZ3v/V9q0A0N9AnzGuPZTdwUJXvTg= X-Received: by 2002:a17:902:aa43:b0:142:6919:73da with SMTP id c3-20020a170902aa4300b00142691973damr63733052plr.39.1638235813148; Mon, 29 Nov 2021 17:30:13 -0800 (PST) MIME-Version: 1.0 References: <20211117064359.2362060-1-reijiw@google.com> <20211117064359.2362060-5-reijiw@google.com> In-Reply-To: From: Reiji Watanabe Date: Mon, 29 Nov 2021 17:29:57 -0800 Message-ID: Subject: Re: [RFC PATCH v3 04/29] KVM: arm64: Make ID_AA64PFR0_EL1 writable To: Eric Auger Cc: kvm@vger.kernel.org, Marc Zyngier , Peter Shier , Paolo Bonzini , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi Eric, On Thu, Nov 25, 2021 at 7:35 AM Eric Auger wrote: > > Hi Reiji, > > On 11/17/21 7:43 AM, Reiji Watanabe wrote: > > This patch adds id_reg_info for ID_AA64PFR0_EL1 to make it writable by > > userspace. > > > > The CSV2/CSV3 fields of the register were already writable and values > > that were written for them affected all vCPUs before. Now they only > > affect the vCPU. > > Return an error if userspace tries to set SVE/GIC field of the register > > to a value that conflicts with SVE/GIC configuration for the guest. > > SIMD/FP/SVE fields of the requested value are validated according to > > Arm ARM. > > > > Signed-off-by: Reiji Watanabe > > --- > > arch/arm64/kvm/sys_regs.c | 159 ++++++++++++++++++++++++-------------- > > 1 file changed, 103 insertions(+), 56 deletions(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 1552cd5581b7..35400869067a 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -401,6 +401,92 @@ static void id_reg_info_init(struct id_reg_info *id_reg) > > id_reg->init(id_reg); > > } > > > > +#define kvm_has_gic3(kvm) \ > > + (irqchip_in_kernel(kvm) && \ > > + (kvm)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) > you may move this macro to kvm/arm_vgic.h as this may be used in > vgic/vgic-v3.c too Thank you for the suggestion. I will move that to kvm/arm_vgic.h. > > + > > +static int validate_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *id_reg, u64 val) > > +{ > > + int fp, simd; > > + bool vcpu_has_sve = vcpu_has_sve(vcpu); > > + bool pfr0_has_sve = id_aa64pfr0_sve(val); > > + int gic; > > + > > + simd = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_ASIMD_SHIFT); > > + fp = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_FP_SHIFT); > > + if (simd != fp) > > + return -EINVAL; > > + > > + /* fp must be supported when sve is supported */ > > + if (pfr0_has_sve && (fp < 0)) > > + return -EINVAL; > > + > > + /* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */ > > + if (vcpu_has_sve ^ pfr0_has_sve) > > + return -EPERM; > > + > > + gic = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_GIC_SHIFT); > > + if ((gic > 0) ^ kvm_has_gic3(vcpu->kvm)) > > + return -EPERM; > > Sometimes from a given architecture version, some lower values are not > allowed. For instance from ARMv8.5 onlt 1 is permitted for CSV3. > Shouldn't we handle that kind of check? As far as I know, there is no way for guests to identify the architecture revision (e.g. v8.1, v8.2, etc). It might be able to indirectly infer the revision though (from features that are available or etc). > > + > > + return 0; > > +} > > + > > +static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg) > > +{ > > + u64 limit = id_reg->vcpu_limit_val; > > + unsigned int gic; > > + > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_AMU); > > + if (!system_supports_sve()) > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + /* > > + * The default is to expose CSV2 == 1 and CSV3 == 1 if the HW > > + * isn't affected. Userspace can override this as long as it > > + * doesn't promise the impossible. > > + */ > > + limit &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2) | > > + ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3)); > > + > > + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), 1); > > + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), 1); > > + > > + gic = cpuid_feature_extract_unsigned_field(limit, ID_AA64PFR0_GIC_SHIFT); > > + if (gic > 1) { > > + /* Limit to GICv3.0/4.0 */ > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1); > > + } > > + id_reg->vcpu_limit_val = limit; > > +} > > + > > +static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *idr) > > +{ > > + u64 val = idr->vcpu_limit_val; > > + > > + if (!vcpu_has_sve(vcpu)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + if (!kvm_has_gic3(vcpu->kvm)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + > > + return val; > > +} > > + > > +static struct id_reg_info id_aa64pfr0_el1_info = { > > + .sys_reg = SYS_ID_AA64PFR0_EL1, > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? They are needed because they are signed fields (the default is unsigned and FCT_LOWER_SAFE). Having said that, ftr_check_types itself will be gone in the next version (as arm64_ftr_bits will be used instead). Thanks, Reiji _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1E1F0C433F5 for ; Tue, 30 Nov 2021 01:32:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=anERekAnJwCNSXPUPruc4yQVwn0gwX7aB+GdrHQKKp8=; b=Q+FIQ30zWumE6T nOtEW6DxSG0sETsvq/oSlULfsvZtNBCLfAKbjahOLj3gDSzC6svqlvl9sYALwZGxDULtVfS/Ronu/ /L7gJOyfEJAzfaHxI4BdK5AGuN21jIYnm2CQyGVPnuvhw/gq9uzQxlTULIC68ruaGmvUGgko3yWZR ZaCzb8Q6y/L7OGIs/8twx7aOVrqkQ7I1sRKxhPqyTkrQm/RokF1SEioXYnA7QomIdXvS2UYI21If6 DZZFQUvC7W5FhBk/ixTasMsfUz1spiTef23zIKUCXdL/3Sy0aiKAiSISOsZGQXFF4tlBnSY9B2WMe qCgYgN/3W59l5onNtWjA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mrryQ-003G1d-LQ; Tue, 30 Nov 2021 01:30:22 +0000 Received: from mail-pj1-x1034.google.com ([2607:f8b0:4864:20::1034]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mrryM-003G0k-Fr for linux-arm-kernel@lists.infradead.org; Tue, 30 Nov 2021 01:30:20 +0000 Received: by mail-pj1-x1034.google.com with SMTP id h24so14093815pjq.2 for ; Mon, 29 Nov 2021 17:30:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=e+pDAbhA3qgo5VrRLD3GioZhOJH88Y+ZobynMlP5+GQ3GX6zzk43B/1yG+rsEjTK2j jDOwaSNw3exODm+rdBqEbseuVL90SfGErs9xFCBOAFKDOnF2L47MddrQrzeBo49E67Ac xawvOZp1p9ika1lAvGxvLrtaSlCN/xAwB6Nl1eK33Iixtbjn3fJMew6EpmlxshrqqTww 5/sixT3Bp6t/lvEBrEIIuZnD0FvTLRX8glF8VI7PW4vkXKw/5gDN2XKrn5/l0rP5wF/x 5Tp0aZXrijks003EJUQX09dSEA3bhgVpqSOt36HfY699jD2sNR9secXTNrMHHrLVRa9D pODg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=z6k0u76oLfrxFnRfCQqpgH3a7DGT8Svyiv/OozS9s8Q=; b=sdcIVSgSBNIvH5aiQkQseERzSnhtPBpQSHpQjsTjYqNNapuHOdgQNv6XHKaNCk7hPx 96dQm4UL/kzoKnAtFVxu9Co7knGVB2glRAfZr96Y48jqXrihwcjLkL91Jjj2cEP+un3j E8ScMhGtz01AV/RgXTRJlXJb66JGFmshBpoe6qDi54ZHV6hhZwuRHBA1dD5E/sKLhvtn SStacJDWnJfqBMrDE+YOv0sJ57vacV+vjdDrSd/pSZyO3d9jfQ0AaVfwilrw3+T7pgA0 ninWoZ9SjlZw4blu6NLGMhoomJdmno0J6+gAHfWZJMEZAyMzF/Xm7W+9Cguhh3VeInuw j6UA== X-Gm-Message-State: AOAM531D1RVkm6iSqTP/FjgPvTUQPYIp2D9QnVacCJYUGtKMEQ6CjtXT RB9hxcaINS3rxH8A3Pq3b86A7wK7zMepi1FAMCcEjQ== X-Google-Smtp-Source: ABdhPJzq1HYUtohDmdrT9AteQ7TOnwp5TltRApAUwblcjchFt88a9JJpwnFl4yiZ3v/V9q0A0N9AnzGuPZTdwUJXvTg= X-Received: by 2002:a17:902:aa43:b0:142:6919:73da with SMTP id c3-20020a170902aa4300b00142691973damr63733052plr.39.1638235813148; Mon, 29 Nov 2021 17:30:13 -0800 (PST) MIME-Version: 1.0 References: <20211117064359.2362060-1-reijiw@google.com> <20211117064359.2362060-5-reijiw@google.com> In-Reply-To: From: Reiji Watanabe Date: Mon, 29 Nov 2021 17:29:57 -0800 Message-ID: Subject: Re: [RFC PATCH v3 04/29] KVM: arm64: Make ID_AA64PFR0_EL1 writable To: Eric Auger Cc: Marc Zyngier , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Will Deacon , Peter Shier , Paolo Bonzini , linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211129_173018_568080_CD6D0080 X-CRM114-Status: GOOD ( 36.46 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Eric, On Thu, Nov 25, 2021 at 7:35 AM Eric Auger wrote: > > Hi Reiji, > > On 11/17/21 7:43 AM, Reiji Watanabe wrote: > > This patch adds id_reg_info for ID_AA64PFR0_EL1 to make it writable by > > userspace. > > > > The CSV2/CSV3 fields of the register were already writable and values > > that were written for them affected all vCPUs before. Now they only > > affect the vCPU. > > Return an error if userspace tries to set SVE/GIC field of the register > > to a value that conflicts with SVE/GIC configuration for the guest. > > SIMD/FP/SVE fields of the requested value are validated according to > > Arm ARM. > > > > Signed-off-by: Reiji Watanabe > > --- > > arch/arm64/kvm/sys_regs.c | 159 ++++++++++++++++++++++++-------------- > > 1 file changed, 103 insertions(+), 56 deletions(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 1552cd5581b7..35400869067a 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -401,6 +401,92 @@ static void id_reg_info_init(struct id_reg_info *id_reg) > > id_reg->init(id_reg); > > } > > > > +#define kvm_has_gic3(kvm) \ > > + (irqchip_in_kernel(kvm) && \ > > + (kvm)->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) > you may move this macro to kvm/arm_vgic.h as this may be used in > vgic/vgic-v3.c too Thank you for the suggestion. I will move that to kvm/arm_vgic.h. > > + > > +static int validate_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *id_reg, u64 val) > > +{ > > + int fp, simd; > > + bool vcpu_has_sve = vcpu_has_sve(vcpu); > > + bool pfr0_has_sve = id_aa64pfr0_sve(val); > > + int gic; > > + > > + simd = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_ASIMD_SHIFT); > > + fp = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_FP_SHIFT); > > + if (simd != fp) > > + return -EINVAL; > > + > > + /* fp must be supported when sve is supported */ > > + if (pfr0_has_sve && (fp < 0)) > > + return -EINVAL; > > + > > + /* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */ > > + if (vcpu_has_sve ^ pfr0_has_sve) > > + return -EPERM; > > + > > + gic = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_GIC_SHIFT); > > + if ((gic > 0) ^ kvm_has_gic3(vcpu->kvm)) > > + return -EPERM; > > Sometimes from a given architecture version, some lower values are not > allowed. For instance from ARMv8.5 onlt 1 is permitted for CSV3. > Shouldn't we handle that kind of check? As far as I know, there is no way for guests to identify the architecture revision (e.g. v8.1, v8.2, etc). It might be able to indirectly infer the revision though (from features that are available or etc). > > + > > + return 0; > > +} > > + > > +static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg) > > +{ > > + u64 limit = id_reg->vcpu_limit_val; > > + unsigned int gic; > > + > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_AMU); > > + if (!system_supports_sve()) > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + /* > > + * The default is to expose CSV2 == 1 and CSV3 == 1 if the HW > > + * isn't affected. Userspace can override this as long as it > > + * doesn't promise the impossible. > > + */ > > + limit &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2) | > > + ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3)); > > + > > + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), 1); > > + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), 1); > > + > > + gic = cpuid_feature_extract_unsigned_field(limit, ID_AA64PFR0_GIC_SHIFT); > > + if (gic > 1) { > > + /* Limit to GICv3.0/4.0 */ > > + limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1); > > + } > > + id_reg->vcpu_limit_val = limit; > > +} > > + > > +static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > + const struct id_reg_info *idr) > > +{ > > + u64 val = idr->vcpu_limit_val; > > + > > + if (!vcpu_has_sve(vcpu)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE); > > + > > + if (!kvm_has_gic3(vcpu->kvm)) > > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC); > > + > > + return val; > > +} > > + > > +static struct id_reg_info id_aa64pfr0_el1_info = { > > + .sys_reg = SYS_ID_AA64PFR0_EL1, > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? > > + .ftr_check_types = S_FCT(ID_AA64PFR0_ASIMD_SHIFT, FCT_LOWER_SAFE) | > > + S_FCT(ID_AA64PFR0_FP_SHIFT, FCT_LOWER_SAFE), > is it needed as it is the default? They are needed because they are signed fields (the default is unsigned and FCT_LOWER_SAFE). Having said that, ftr_check_types itself will be gone in the next version (as arm64_ftr_bits will be used instead). Thanks, Reiji _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel