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 E1DC8C433FE for ; Tue, 7 Dec 2021 10:30:50 +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=VkpXMHaL9vf/MR46E8eec3bUMDC2E97VRArs0m7wEGE=; b=0LG2FNTa+rSeV7ea5Iwqn7/KtK IXyqk9ifyJ7wTIKFbUyUGy7SMO3/FbhA/zsnZPbX1mrTlqDN0BFgo84GueF7HpB/RZAmJbLINTwH/ 61JOZuw65rp9qg3pooDth1OcIvPEyx4q7FGQjn8nKyQ8ozX52nDprsjiAv89A895krOCm6H44wOal 3LNosUQniZigB4Pj2ZGqj6tBm5SUWbJffDPXyopiWyWHtX/YD4ja/gWBw1TpMq7ygGl8nC45D7oVq /U5Vc/Q00H7FpZYLH/bB+DSGN1WiqfzvV5KJl2jRnpJHobny86c2lA1xiboY0eC6g/wzgwBqLOdYX VeW2vYCA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1muXhn-0081JO-EE; Tue, 07 Dec 2021 10:28:17 +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 1muWzI-007r7Y-Sy for linux-arm-kernel@lists.infradead.org; Tue, 07 Dec 2021 09:42:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1638870132; 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=IfdtTnAi2J8orsL+RMPenY5oP4wZsK7814Lhx3qxSSo=; b=CCkFvqgxZc8nTDBrnDLfFmcIogiVmS+oFKkIK5EEor6juKFtruxkYmrXuDL7XSDvDM5LKO +vURZ2NFUKn4I4vmng/ys+H+XEdwKNFuhyqt0eDPdNohzjKNDHjfKkXX9AXx6LznsOPYJ7 wVL52ZTdnuD9Zsvy1diMUwgM6cYJ1Fw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-370-T4QOT51fPlqVnLjNHQOa-Q-1; Tue, 07 Dec 2021 04:42:11 -0500 X-MC-Unique: T4QOT51fPlqVnLjNHQOa-Q-1 Received: by mail-wm1-f71.google.com with SMTP id v62-20020a1cac41000000b0033719a1a714so7457518wme.6 for ; Tue, 07 Dec 2021 01:42:11 -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=IfdtTnAi2J8orsL+RMPenY5oP4wZsK7814Lhx3qxSSo=; b=ANpBJU4qMSaElubnlxakkAsQ5QzdieRbqGSZa3Yd5aLtfEAhapi31OH9RvVGYmk/z0 B358wCvw6ySIHdusG4ap4wnf+IEZ7vnFKOzzq7Tw6n+0iPrjfsJtHbw9NZDPlvP8xykG X/TYyYR+12f6BeDsfvpsIpgY7LLDg+mqKXdS7T7rz+i/GoKYUEyExPeRTL2qXfDz3FVx gE6CA4gmMzuvEmA1sC3loBAcK+BW0P9U1qJtpr7rGiv4uOSP2DfSaGvyqxlUh25kFVOP g9o6XDYeqiDAxYyhOfHnDOWOlxea/gssWWAnx/QYWesgh6w8kUZf9WDd/bJh83TOpKOt Ujng== X-Gm-Message-State: AOAM533J25+R6hYczWMVg1JczVezGiHK30s76/Gi4f8YfdQ3rwecQjmE WFXAEfgQIEX85v6u/3xm9TN+2AOLSC4sDIiTMfq8R2GeihJ5Ckc4YXSgU09KEzz+c73r/xqKUXF XDRMiXxfTlbsxTQ2U7FxvzNtEcrY0lqS+kStlSNokqtv/vGSlIUEMcUoJuSLtK4K0l9s323KBj8 V6HyZJB8+J X-Received: by 2002:a05:600c:35c8:: with SMTP id r8mr5642771wmq.8.1638870129260; Tue, 07 Dec 2021 01:42:09 -0800 (PST) X-Google-Smtp-Source: ABdhPJzgiz4h3rob+EwyRswCrpRZGyj0kE1qIofZJRmkuP700RK+ws81XqcftlJ91CeLT3kVM2yBuA== X-Received: by 2002:a05:600c:35c8:: with SMTP id r8mr5642733wmq.8.1638870129012; Tue, 07 Dec 2021 01:42:09 -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 d15sm18436814wri.50.2021.12.07.01.42.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Dec 2021 01:42:08 -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: <728dae43-c97e-0982-b7d0-dd7d6eed6d6b@redhat.com> Date: Tue, 7 Dec 2021 10:42:07 +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-20211207_014217_079839_1DB088A2 X-CRM114-Status: GOOD ( 36.30 ) 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 12/4/21 8:59 AM, Reiji Watanabe wrote: > Hi Eric, > > On Thu, Dec 2, 2021 at 5:02 AM Eric Auger wrote: >> >> 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? > > Thank you for sharing your thoughts and providing the suggestion ! > > Does the "new vCPU targets" mean Armv8.0, armv8.1, and so on ? Yeah my suggestion probably is not a good idea, ie. introducing such VCPU targets. I was simply confused by the fact we introduce in this series quite intricate consistency checks but given the fact we miss the spec rev information we are not exhaustive in terms of checking. So it is sometimes difficult to review against the spec. > > The ID registers' consistency checking in the series is to not > promise more to userspace than what KVM (on the host) can provide, > and to not expose ID register values that are not supported on > any ARM v8 architecture for guests (I think those are what the > current KVM is trying to assure). I'm not trying to have KVM > provide full consistency checking of ID registers to completely > prevent userspace's bugs in setting ID registers. > > I agree that it's quite possible that userspace exposes such wrong > CPU models, and KVM's providing more consistency checking would be > nicer in general. But should it be KVM's responsibility to completely > prevent such ID register issues due to userspace bugs ? > > Honestly, I'm a bit reluctant to do that so far yet:) understood. I will look at the spec in more details on my next review cycle. Looking forward to reviewing the next version ;-) Thanks Eric > If that is something useful that userspace or we (KVM developers) > really want or need, or such userspace issue could affect KVM, > I would be happy to add such extra consistency checking though. > > Thanks, > Reiji > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel