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 D2467C433F5 for ; Wed, 24 Nov 2021 21:09:12 +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:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=d7fjTWSyKUgpzfBEt8CXep82NpNuW/UOLZ0SzgnU4Bw=; b=MyNBaEoGZnZq9XCMJCuO8aZdPa 1qfoI8GdcLsgPsqsGX2aWoo6OtgEyZ61lpchGu2IR7pQQCiv4H5EiJBB8jm7zwxvcwAJ7c26yG4nD QYoR+4eW30wwO8ik5X9BU8W7wTS1ZjBGp6aLTjH0lH+IPKd6TywVPTUujQ7eKMjNjuXcBs12qJTTA X1h5KSj9+MbZN/o2iwCo5W1yIQi9vOpeZgkziGrbeyBa+jriYJzmYYZvTBzbhMNGCOdc+TgMPlGgl a57QWR1xeHwGhQV2er8msRhO4r4y/6OXl2VCS0AcmRZjf0ANYSwxNNK3SFEgbixkrJAwspAjbwbKn ty1xqN2g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpzUU-005r14-Sw; Wed, 24 Nov 2021 21:07:43 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpzUM-005qzH-LY for linux-arm-kernel@lists.infradead.org; Wed, 24 Nov 2021 21:07:37 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1637788053; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/RoszFU0XTK0aV/vu4I1mgmUsImsHFL9k+YboQn9sa0=; b=HRf71mCH3wf4P7GLpkSR0aea2yD70Sex9qg2goGli6bRyv/3DyWlKSCINzLYKnpPrxMJS/ eZMuZqbnjLrIlvSxrUHBC89RcRPcouAXd6OTvQthfclD9ZePqnIjmooNyBa1WlbAC7zV9H FfZBbNGbd2xhQYm8jYm22MeHKGZzWX4= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-26-bVPMJUaaM-uQKsRIr1Bv3w-1; Wed, 24 Nov 2021 16:07:32 -0500 X-MC-Unique: bVPMJUaaM-uQKsRIr1Bv3w-1 Received: by mail-wm1-f70.google.com with SMTP id bg20-20020a05600c3c9400b0033a9300b44bso2142476wmb.2 for ; Wed, 24 Nov 2021 13:07:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=/RoszFU0XTK0aV/vu4I1mgmUsImsHFL9k+YboQn9sa0=; b=AcGIcd+jTJl6xczlbVJI83kK1XNRvNfKb5Fj8BexQkREf+oQemfWW48G5Cfu76wmSD QGE2W3fpdiLZkefWLtWpKKVO0RMZ0bNqfpquCULJYAeYelJHQIVWsIZ/GlO7BPFGHJvN tMo0GT4orOMzi33k+O8NfsfTgDnfnsldOWsgO+u4Kc7Y9G3fMvcjay8T9PEbHkmH2ZJG 7aFfihRa3S5KnzeT5okJfY4QE+2e7h19lF8nco8uuC7rGicR4/PiLq1F0CTHv+C/w+uP Xqi9uzh1DiF4TED2N/I/ntCpa/RgXakm+t6WQ8IwI6nyHBJHH1ql7UWPnOyblRNUt5f+ tpaA== X-Gm-Message-State: AOAM532YlRLywd/G4wZUO5p0Gf3Jdy3mMfzkUuDPOlXFpmNkIJdkT4Qi qrEql/yVsSIU1qjmY9+Fgaycn3xSKWm64zwiyA5N6UoPeykx3j1OdfkKbGoin26f1vn01rt+9JM Gf12AnYrQzfT40Hx06q/QBMk9gGGEqhH55Leh/h4g/M/XOYJgiALnHCAZhJVGmvVB5TPJTblHZ+ D4WDGkroeY X-Received: by 2002:adf:ea51:: with SMTP id j17mr24095184wrn.421.1637788050181; Wed, 24 Nov 2021 13:07:30 -0800 (PST) X-Google-Smtp-Source: ABdhPJzvtmp1vYY4JvJvhU7UCQah9E5DPqS6e1GV5BHuqB9k2zEOpJ9nLjJLltsjLvp7xxvv78snYw== X-Received: by 2002:adf:ea51:: with SMTP id j17mr24095099wrn.421.1637788049731; Wed, 24 Nov 2021 13:07:29 -0800 (PST) Received: from ?IPv6:2a01:e0a:59e:9d80:527b:9dff:feef:3874? ([2a01:e0a:59e:9d80:527b:9dff:feef:3874]) by smtp.gmail.com with ESMTPSA id h17sm5823882wmb.44.2021.11.24.13.07.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 24 Nov 2021 13:07:29 -0800 (PST) Subject: Re: [RFC PATCH v3 03/29] KVM: arm64: Introduce struct id_reg_info To: Reiji Watanabe , Marc Zyngier , kvmarm@lists.cs.columbia.edu Cc: kvm@vger.kernel.org, Will Deacon , Peter Shier , Paolo Bonzini , linux-arm-kernel@lists.infradead.org References: <20211117064359.2362060-1-reijiw@google.com> <20211117064359.2362060-4-reijiw@google.com> From: Eric Auger Message-ID: <57519386-0a30-40a6-b46f-d20595df0b86@redhat.com> Date: Wed, 24 Nov 2021 22:07:27 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20211117064359.2362060-4-reijiw@google.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eauger@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211124_130734_947925_9E3EAF6C X-CRM114-Status: GOOD ( 48.14 ) 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 Reiji, On 11/17/21 7:43 AM, Reiji Watanabe wrote: > This patch lays the groundwork to make ID registers writable. > > Introduce struct id_reg_info for an ID register to manage the > register specific control of its value for the guest, and provide set > of functions commonly used for ID registers to make them writable. > > The id_reg_info is used to do register specific initialization, > validation of the ID register and etc. Not all ID registers must > have the id_reg_info. ID registers that don't have the id_reg_info > are handled in a common way that is applied to all ID registers. > > At present, changing an ID register from userspace is allowed only > if the ID register has the id_reg_info, but that will be changed > by the following patches. > > No ID register has the structure yet and the following patches > will add the id_reg_info for some ID registers. > > Signed-off-by: Reiji Watanabe > --- > arch/arm64/include/asm/sysreg.h | 1 + > arch/arm64/kvm/sys_regs.c | 226 ++++++++++++++++++++++++++++++-- > 2 files changed, 218 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 16b3f1a1d468..597609f26331 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -1197,6 +1197,7 @@ > #define ICH_VTR_TDS_MASK (1 << ICH_VTR_TDS_SHIFT) > > #define ARM64_FEATURE_FIELD_BITS 4 > +#define ARM64_FEATURE_FIELD_MASK ((1ull << ARM64_FEATURE_FIELD_BITS) - 1) > > /* Create a mask for the feature bits of the specified feature. */ > #define ARM64_FEATURE_MASK(x) (GENMASK_ULL(x##_SHIFT + ARM64_FEATURE_FIELD_BITS - 1, x##_SHIFT)) > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 5608d3410660..1552cd5581b7 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -265,6 +265,181 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu, > return read_zero(vcpu, p); > } > > +/* > + * A value for FCT_LOWER_SAFE must be zero and changing that will affect > + * ftr_check_types of id_reg_info. > + */ > +enum feature_check_type { > + FCT_LOWER_SAFE = 0, > + FCT_HIGHER_SAFE, > + FCT_HIGHER_OR_ZERO_SAFE, > + FCT_EXACT, > + FCT_EXACT_OR_ZERO_SAFE, > + FCT_IGNORE, /* Don't check (any value is fine) */ > +}; > + > +static int arm64_check_feature_one(enum feature_check_type type, int val, > + int limit) > +{ > + bool is_safe = false; > + > + if (val == limit) > + return 0; > + > + switch (type) { > + case FCT_LOWER_SAFE: > + is_safe = (val <= limit); > + break; > + case FCT_HIGHER_OR_ZERO_SAFE: > + if (val == 0) { > + is_safe = true; > + break; > + } > + fallthrough; > + case FCT_HIGHER_SAFE: > + is_safe = (val >= limit); > + break; > + case FCT_EXACT: > + break; > + case FCT_EXACT_OR_ZERO_SAFE: > + is_safe = (val == 0); > + break; > + case FCT_IGNORE: > + is_safe = true; > + break; > + default: > + WARN_ONCE(1, "Unexpected feature_check_type (%d)\n", type); > + break; > + } > + > + return is_safe ? 0 : -1; > +} > + > +#define FCT_TYPE_MASK 0x7 > +#define FCT_TYPE_SHIFT 1 > +#define FCT_SIGN_MASK 0x1 > +#define FCT_SIGN_SHIFT 0 > +#define FCT_TYPE(val) ((val >> FCT_TYPE_SHIFT) & FCT_TYPE_MASK) > +#define FCT_SIGN(val) ((val >> FCT_SIGN_SHIFT) & FCT_SIGN_MASK) > + > +#define MAKE_FCT(shift, type, sign) \ > + ((u64)((((type) & FCT_TYPE_MASK) << FCT_TYPE_SHIFT) | \ > + (((sign) & FCT_SIGN_MASK) << FCT_SIGN_SHIFT)) << (shift)) > + > +/* For signed field */ > +#define S_FCT(shift, type) MAKE_FCT(shift, type, 1) > +/* For unigned field */ > +#define U_FCT(shift, type) MAKE_FCT(shift, type, 0) > + > +/* > + * @val and @lim are both a value of the ID register. The function checks > + * if all features indicated in @val can be supported for guests on the host, > + * which supports features indicated in @lim. @check_types indicates how > + * features in the ID register needs to be checked. > + * See comments for id_reg_info's ftr_check_types field for more detail. > + */ > +static int arm64_check_features(u64 check_types, u64 val, u64 lim) > +{ > + int i; > + > + for (i = 0; i < 64; i += ARM64_FEATURE_FIELD_BITS) { > + u8 ftr_check = (check_types >> i) & ARM64_FEATURE_FIELD_MASK; > + bool is_sign = FCT_SIGN(ftr_check); > + enum feature_check_type fctype = FCT_TYPE(ftr_check); > + int fval, flim, ret; > + > + fval = cpuid_feature_extract_field(val, i, is_sign); > + flim = cpuid_feature_extract_field(lim, i, is_sign); > + > + ret = arm64_check_feature_one(fctype, fval, flim); > + if (ret) > + return -E2BIG; nit: -EINVAL may be better because depending on the check type this may not mean too big. Eric > + } > + return 0; > +} > + > +struct id_reg_info { > + u32 sys_reg; /* Register ID */ > + > + /* > + * Limit value of the register for a vcpu. The value is the sanitized > + * system value with bits cleared for unsupported features for the > + * guest. > + */ > + u64 vcpu_limit_val; > + > + /* > + * The ftr_check_types is comprised of a set of 4 bits fields. > + * Each 4 bits field is for a feature indicated by the same bits > + * field of the ID register and indicates how the feature support > + * for guests needs to be checked. > + * The bit 0 indicates that the corresponding ID register field > + * is signed(1) or unsigned(0). > + * The bits [3:1] hold feature_check_type for the field. > + * If all zero, all features in the ID register are treated as unsigned > + * fields and checked based on Principles of the ID scheme for fields > + * in ID registers (FCT_LOWER_SAFE of feature_check_type). > + */ > + u64 ftr_check_types; > + > + /* Initialization function of the id_reg_info */ > + void (*init)(struct id_reg_info *id_reg); > + > + /* Register specific validation function */ > + int (*validate)(struct kvm_vcpu *vcpu, const struct id_reg_info *id_reg, > + u64 val); > + > + /* Return the reset value of the register for the vCPU */ > + u64 (*get_reset_val)(struct kvm_vcpu *vcpu, > + const struct id_reg_info *id_reg); > +}; > + > +static void id_reg_info_init(struct id_reg_info *id_reg) > +{ > + id_reg->vcpu_limit_val = read_sanitised_ftr_reg(id_reg->sys_reg); > + if (id_reg->init) > + id_reg->init(id_reg); > +} > + > +/* > + * An ID register that needs special handling to control the value for the > + * guest must have its own id_reg_info in id_reg_info_table. > + * (i.e. the reset value is different from the host's sanitized value, > + * the value is affected by opt-in features, some fields needs specific > + * validation, etc.) > + */ > +#define GET_ID_REG_INFO(id) (id_reg_info_table[IDREG_IDX(id)]) > +static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {}; > + > +static int validate_id_reg(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd, u64 val) > +{ > + u32 id = reg_to_encoding(rd); > + const struct id_reg_info *id_reg = GET_ID_REG_INFO(id); > + u64 limit, check_types; > + int err; > + > + if (id_reg) { > + check_types = id_reg->ftr_check_types; > + limit = id_reg->vcpu_limit_val; > + } else { > + /* All fields are treated as unsigned and FCT_LOWER_SAFE */ > + check_types = 0; > + limit = read_sanitised_ftr_reg(id); > + } > + > + /* Check if the value indicates any feature that is not in the limit. */ > + err = arm64_check_features(check_types, val, limit); > + if (err) > + return err; > + > + if (id_reg && id_reg->validate) > + /* Run the ID register specific validity check. */ > + err = id_reg->validate(vcpu, id_reg, val); > + > + return err; > +} > + > /* > * ARMv8.1 mandates at least a trivial LORegion implementation, where all the > * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0 > @@ -1183,11 +1358,19 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu, > static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd) > { > u32 id = reg_to_encoding(rd); > + struct id_reg_info *id_reg; > + u64 val; > > if (vcpu_has_reset_once(vcpu)) > return; > > - __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id); > + id_reg = GET_ID_REG_INFO(id); > + if (id_reg && id_reg->get_reset_val) > + val = id_reg->get_reset_val(vcpu, id_reg); > + else > + val = read_sanitised_ftr_reg(id); > + > + __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = val; > } > > static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > @@ -1232,11 +1415,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > return 0; > } > > -/* > - * cpufeature ID register user accessors > - * > - * We don't allow the effective value to be changed. > - */ > +/* cpufeature ID register user accessors */ > static int __get_id_reg(const struct kvm_vcpu *vcpu, > const struct sys_reg_desc *rd, void __user *uaddr, > bool raz) > @@ -1247,11 +1426,12 @@ static int __get_id_reg(const struct kvm_vcpu *vcpu, > return reg_to_user(uaddr, &val, id); > } > > -static int __set_id_reg(const struct kvm_vcpu *vcpu, > +static int __set_id_reg(struct kvm_vcpu *vcpu, > const struct sys_reg_desc *rd, void __user *uaddr, > bool raz) > { > const u64 id = sys_reg_to_index(rd); > + u32 encoding = reg_to_encoding(rd); > int err; > u64 val; > > @@ -1259,10 +1439,22 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu, > if (err) > return err; > > - /* This is what we mean by invariant: you can't change it. */ > - if (val != read_id_reg(vcpu, rd, raz)) > + /* Don't allow to change the reg unless the reg has id_reg_info */ > + if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding)) > return -EINVAL; > > + if (raz) > + return 0; > + > + /* Don't allow to change the reg after the first KVM_RUN. */ > + if (vcpu->arch.has_run_once)> + return -EINVAL; > + > + err = validate_id_reg(vcpu, rd, val); > + if (err) > + return err; > + > + __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(encoding)) = val; > return 0; > } > > @@ -2826,6 +3018,20 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) > return write_demux_regids(uindices); > } > > +static void id_reg_info_init_all(void) > +{ > + int i; > + struct id_reg_info *id_reg; > + > + for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++) { > + id_reg = (struct id_reg_info *)id_reg_info_table[i]; > + if (!id_reg) > + continue; > + > + id_reg_info_init(id_reg); > + } > +} > + > void kvm_sys_reg_table_init(void) > { > unsigned int i; > @@ -2860,4 +3066,6 @@ void kvm_sys_reg_table_init(void) > break; > /* Clear all higher bits. */ > cache_levels &= (1 << (i*3))-1; > + > + id_reg_info_init_all(); > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel