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 71F20C433F5 for ; Thu, 2 Dec 2021 13:04:00 +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=f10eJu/j9c6btjeATW+O5zb7IGH04ANUvAKtUwV0lpc=; b=RggWEh5K4ETZz6QgjwfWm1wv7G pB/5jMaNjZBePyppAPWUOPq+iYAiLYepcXoPdC1cYVjpACHbG49FXLtxJUqfGMKVJAE2ZX2+oQqB9 Mu1jWanDzx6PoYWJL89lc20qrKl67mU8oZyw72v6gaXhyM21l8jmkthTuABH+PflVpP8dawHOdXNZ Le+x0eRZp3swb5n+DCvihRfO7vKwR2rM1GRp2pn5eO5qWLn7AMXr/u8AeTLOprFWVPUDxdOIHH36O OMngni/WzoHjtvrCflomg1F45bLpSIRDc4mtPKp90aA9EoHrb6IIFnkj/Xo8eD6KMwrsMv+XdhpY4 +0I5mjLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mslj6-00CPeG-AE; Thu, 02 Dec 2021 13:02:16 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mslj2-00CPdd-4m for linux-arm-kernel@lists.infradead.org; Thu, 02 Dec 2021 13:02:14 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1638450130; 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=x1DDUlEbW7+TEcKrOe/PwHbaKXvgftYv4zFADOMy298=; b=hw4PYT6fHsGmoarQwI50rEmf+gzAcNBtaLpfXBZw3Dqjxg+wV0vQkZCtCUioeQixIELKbY m+DHguE6avfcgYLQKzvORZ14pVXKcyQJlJrxEz9eI2HCow+xb5fJSn07kWjFg50bYb13jF TGb31IAzJa4LY44Rq1i9wBPdV0FqfVE= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-337-Q8VjCQtrO3ezYSK5LtZxVw-1; Thu, 02 Dec 2021 08:02:09 -0500 X-MC-Unique: Q8VjCQtrO3ezYSK5LtZxVw-1 Received: by mail-wr1-f70.google.com with SMTP id q7-20020adff507000000b0017d160d35a8so5050254wro.4 for ; Thu, 02 Dec 2021 05:02:09 -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=x1DDUlEbW7+TEcKrOe/PwHbaKXvgftYv4zFADOMy298=; b=OUhD9cuwgDzAlo+meONJsj0Xv4XoJl/DDo0xknpFOOXK8OOWnQmS4si+WSecy3Udnn uBjGCQmk2pbNStgy2w7XMnK2usjmQ2CVYZRJyQ5KRMXfwnjLZ1BQFSvUrWH53VeDYRTB vr9R3ec61gMZhe7L+JO3oySgfFwisefYY/H9Z53IUGaXfFTPLeOUXPvai9Gm2xA9UGmM UXwKKQuAnH3hAsr1HGVWC2EMx2GeEhutPop/IfPUPQkThX/Zsa6nOk2nJdX/h3J1rx6Z SWUmaFYy27z/c5Mg8n6GjXJHTrILi88zOaiWw/GSRpxsHaIuqi1OZH3PmiduyC3wJ9sl Vs8w== X-Gm-Message-State: AOAM532TIJd3BWyfU5QYt8v+xzuAxlwFQYmQFq0bAVKOyZ/urDhirvpi eScJpopZQ0asWpiyBiq7kdbtP5dgxJ7iQ2OCdRRCA3VdU0qNPk8oJCytEALtO9IxKUYLySnsgOe 17LLnqe/iR+hF3VVZEYv7P0o/UCeJZCD3IPB6Sxaod1yA6Xs00iaaL89xoa7RXasnuOQkB4kkpM dcLu4282+6 X-Received: by 2002:a05:6000:143:: with SMTP id r3mr14462714wrx.236.1638450127687; Thu, 02 Dec 2021 05:02:07 -0800 (PST) X-Google-Smtp-Source: ABdhPJzMvw1j4167feOTyg3bdIJl0KldZ/+JDB+tLVdpAglu6yx0xuOJYBL7gNenCbomHVVt1er+1Q== X-Received: by 2002:a05:6000:143:: with SMTP id r3mr14462621wrx.236.1638450126984; Thu, 02 Dec 2021 05:02:06 -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 a22sm2110828wme.19.2021.12.02.05.02.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Dec 2021 05:02:06 -0800 (PST) Subject: Re: [RFC PATCH v3 04/29] KVM: arm64: Make ID_AA64PFR0_EL1 writable To: Reiji Watanabe Cc: Marc Zyngier , kvmarm@lists.cs.columbia.edu, 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-5-reijiw@google.com> From: Eric Auger Message-ID: Date: Thu, 2 Dec 2021 14:02:04 +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: 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-20211202_050212_452387_6AF344D2 X-CRM114-Status: GOOD ( 33.83 ) 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/30/21 2:29 AM, Reiji Watanabe wrote: > 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). OK. That sounds weird to me as we do many checks accross different IDREG settings but we may eventually have a wrong "CPU model" exposed by the user space violating those spec revision minima. Shouldn't we introduce some way for the userspace to provide his requirements? via new VCPU targets for instance? Thanks Eric > > >>> + >>> + 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 Ah OK, I did not catch it at first glance while looking at the ARM ARM. Thanks Eric > 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