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 80530C433F5 for ; Sat, 4 Dec 2021 07:59:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1355004AbhLDIDH (ORCPT ); Sat, 4 Dec 2021 03:03:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33082 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354886AbhLDIDH (ORCPT ); Sat, 4 Dec 2021 03:03:07 -0500 Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 50C82C061751 for ; Fri, 3 Dec 2021 23:59:42 -0800 (PST) Received: by mail-pj1-x1034.google.com with SMTP id x7so4060160pjn.0 for ; Fri, 03 Dec 2021 23:59:42 -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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=TeXbYPX+C4uPkxaaFLcgtGGJEujSGWZEHwDDrNsBd0VadDAJvtlEj0Iag/3oEQobME mSCi0i0sZxUT82ObDMEopmUc5fT1bocwKCmhlzBsTpDBbCM/IqCMysRkVQSU+hIZ7W7T LTv4Eu4l4MZEJbIFAIRPLCZ4Oft79rV92CR8Xiiu6hhYA3+cI+eJSANmXynO6hNagk+X PsCdOG46OFzs4b6BiZDMZmsQq2WBmlBcTLm2zeMSzGeWSsiUNSVYe8vzONKfJp3U3FZM b5P+jP/tiAN+/CZkQbUBIlshH4OQzIoWIa9LTfHBBeOTC7XD3aQW/gMTBYmwZQrugEgI PE7A== 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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=7qVEA9NscW4WLQxNaI64fQSAaz+QJIYsq5cUT+q2TbAdQ57/6Ns2LX05Zz8In+u8at fUG+zE5ljbFGVBaXrijTfqimfYJkiPuE8ElC00tjHZD1l8v+lu6e9kZ+t4521nMPd2E8 G0PYQlpZXhrK62wIp/f3nmkfV3AhSE/iwP+2Wp0r6vhJwA87/Bbpwil0Q07ffxtrG+jF LTan8lQVVTe0nGTRfy/cTW+BfTNDeDYV+jjSl3qYVWWGXeFDXX0z8hxTcOJ+WCuLIkty o/MnZMJx1wqG4lyUALoCSVt6F5LqWwjY6fE6copAfd3meuGjkFCVVPMejvMEXrIYVUB0 /MtQ== X-Gm-Message-State: AOAM532H7s0TTVJ98yKoMvtaZ4OvlNeX4ZPsMvJRf9PJs3xp4bG6Q+A8 /EEbtxvlquhX65/qJtKaqRenkrQUm89Xqg5HydGpnw== X-Google-Smtp-Source: ABdhPJz/LZkSzORmcnKM+1dzW2Nc3IsEy0R3q6edufSEOKTPTDCILwf40IG7uWOovBicDJ7eg3FHc1anE6oqqnkjUB0= X-Received: by 2002:a17:90b:4acd:: with SMTP id mh13mr20584196pjb.230.1638604781530; Fri, 03 Dec 2021 23:59:41 -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: Fri, 3 Dec 2021 23:59:25 -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, 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 ? 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:) 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 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 17FDEC433F5 for ; Sat, 4 Dec 2021 07:59:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 687C34A7FD; Sat, 4 Dec 2021 02:59:47 -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 4jXCJ3fk9JuJ; Sat, 4 Dec 2021 02:59:46 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 0AC2049F8F; Sat, 4 Dec 2021 02:59:46 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 489E7407ED for ; Sat, 4 Dec 2021 02:59:44 -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 KBjISey-5T79 for ; Sat, 4 Dec 2021 02:59:42 -0500 (EST) Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 9476540642 for ; Sat, 4 Dec 2021 02:59:42 -0500 (EST) Received: by mail-pl1-f170.google.com with SMTP id k4so3651489plx.8 for ; Fri, 03 Dec 2021 23:59:42 -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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=TeXbYPX+C4uPkxaaFLcgtGGJEujSGWZEHwDDrNsBd0VadDAJvtlEj0Iag/3oEQobME mSCi0i0sZxUT82ObDMEopmUc5fT1bocwKCmhlzBsTpDBbCM/IqCMysRkVQSU+hIZ7W7T LTv4Eu4l4MZEJbIFAIRPLCZ4Oft79rV92CR8Xiiu6hhYA3+cI+eJSANmXynO6hNagk+X PsCdOG46OFzs4b6BiZDMZmsQq2WBmlBcTLm2zeMSzGeWSsiUNSVYe8vzONKfJp3U3FZM b5P+jP/tiAN+/CZkQbUBIlshH4OQzIoWIa9LTfHBBeOTC7XD3aQW/gMTBYmwZQrugEgI PE7A== 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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=oUF6qq6ML7BZMDLirzNYo1fj00bq1HJLGtYGBSbDTogYOl10J4onS1nwQePfpu/fp/ /hWK7E+MEB8wn1B3bHDuH7u5NiejBQUMRGvMPfczU0fTBavtBG7vRWT/t7xzDsbnQ//L Cv7QYtZL6FoCLE19bxNacfHDaN+DisHG8afsdlx/z6SnAuYXtUEK51gctjvPRV6tj6si c3gSV9IL82x5lELjOu0m8aAj2kYEMOyQf7wpYkLc1JIt/Km58LTJ8OzwYsq/9NNvEoW5 RGhC0MiMEAR06oX3LXyTtDloKU7TJjAV2uPRi+ac6mpSBuEJBKRX0Of5BRMRPdlL/ob3 dzbA== X-Gm-Message-State: AOAM5333C3Q/d7QfviV9DqgJa/W9BtCz6J6OxJaCzg8jCvxG3oTl8Zvs 144vZPwd0uwxjmfirjMCZJY3R+sd54ksUe9/J+mRMw== X-Google-Smtp-Source: ABdhPJz/LZkSzORmcnKM+1dzW2Nc3IsEy0R3q6edufSEOKTPTDCILwf40IG7uWOovBicDJ7eg3FHc1anE6oqqnkjUB0= X-Received: by 2002:a17:90b:4acd:: with SMTP id mh13mr20584196pjb.230.1638604781530; Fri, 03 Dec 2021 23:59:41 -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: Fri, 3 Dec 2021 23:59:25 -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, 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 ? 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:) 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 _______________________________________________ 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 A70F7C433EF for ; Sat, 4 Dec 2021 08:01:38 +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=xylxSGhd4pCifghFWU6DI2ByRlsj9IpcdywEN5/AbpE=; b=GnrwLFm06UH4+7 nVwMs6wJYDKaNnj98xm99y1S/Vd78t36zBnBu5pw8WcIjSXr/VMq2xD61XExMzlxJzq04agbdiqx0 BM4Lcstc0qJ7rilCS4vq2LxXvfdaxvo2qXxUXfBAm/nxEI+hlgMYjpNzx9XCXClbvuzHoOEOrwlLH 7DTo28ju2rUWAiffQy/UXac5eIha9oTTKbmLCmabAs9i9QVuPASLPlWyuk6LITdzWKx5fBE4J7dhN HwsF8tuizptJlpeYIV6iaKBAE8BM2LbFcCLLIiM+lc62eJQgjaLI0t73NiseE9XnubCKxjW4x2mw5 dpd4+XkOFsQNPuGL6gww==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mtPxS-0005Gx-Cv; Sat, 04 Dec 2021 07:59:46 +0000 Received: from mail-pl1-x634.google.com ([2607:f8b0:4864:20::634]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mtPxO-0005GX-Mx for linux-arm-kernel@lists.infradead.org; Sat, 04 Dec 2021 07:59:44 +0000 Received: by mail-pl1-x634.google.com with SMTP id m24so3637376pls.10 for ; Fri, 03 Dec 2021 23:59:42 -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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=TeXbYPX+C4uPkxaaFLcgtGGJEujSGWZEHwDDrNsBd0VadDAJvtlEj0Iag/3oEQobME mSCi0i0sZxUT82ObDMEopmUc5fT1bocwKCmhlzBsTpDBbCM/IqCMysRkVQSU+hIZ7W7T LTv4Eu4l4MZEJbIFAIRPLCZ4Oft79rV92CR8Xiiu6hhYA3+cI+eJSANmXynO6hNagk+X PsCdOG46OFzs4b6BiZDMZmsQq2WBmlBcTLm2zeMSzGeWSsiUNSVYe8vzONKfJp3U3FZM b5P+jP/tiAN+/CZkQbUBIlshH4OQzIoWIa9LTfHBBeOTC7XD3aQW/gMTBYmwZQrugEgI PE7A== 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=/HNVa0CxFfdOsv36W3pOGvXrawzXJwdYz3KBqa4NE8A=; b=hCGJoRP48iPe3xoZpKNxMD/QvnSXeLovFO+cZMrz0RPdmxVGsH8023jLGRsoRjIjk4 sV230DV8l8A6SwkgsDcLUCk3596EXjOxDfEnCnFOg7hpG7cmYt/INA1qb7zS1rVETaIk 0QgkNsRrzLN2b+5kFttvw+urh4+TAnjFWD1WRaLRc9hCcCAlHG+fKjJkC18NqJrkRL8l q8DMSMqOnXMsWDgtLTan/d1rrzNFuMw4vnqg+KDEue/JMkKlH91rWVr4+HBffKvJkbsp i6m3+fi+PjvU4tW6OCfWzq8oPyiwZlRSRBvcLXuWZgQXEqczjivCqOblbo2FMA3ioIxA uR3Q== X-Gm-Message-State: AOAM532q1744jyqUGNEUlGkrdqEno80/BIDaXaNXyH2MNM42liC7duvu PxGubWB9T/KGZEe/RWk4Nhz09n8qmiL25jOCRBSFFw== X-Google-Smtp-Source: ABdhPJz/LZkSzORmcnKM+1dzW2Nc3IsEy0R3q6edufSEOKTPTDCILwf40IG7uWOovBicDJ7eg3FHc1anE6oqqnkjUB0= X-Received: by 2002:a17:90b:4acd:: with SMTP id mh13mr20584196pjb.230.1638604781530; Fri, 03 Dec 2021 23:59:41 -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: Fri, 3 Dec 2021 23:59:25 -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-20211203_235942_798659_1F790E0C X-CRM114-Status: GOOD ( 36.05 ) 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, 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 ? 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:) 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