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 9C969C64EC4 for ; Fri, 10 Mar 2023 02:14:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229819AbjCJCOh (ORCPT ); Thu, 9 Mar 2023 21:14:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47174 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229751AbjCJCOf (ORCPT ); Thu, 9 Mar 2023 21:14:35 -0500 Received: from mail-oa1-x32.google.com (mail-oa1-x32.google.com [IPv6:2001:4860:4864:20::32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A5E815B5CC for ; Thu, 9 Mar 2023 18:14:34 -0800 (PST) Received: by mail-oa1-x32.google.com with SMTP id 586e51a60fabf-17671fb717cso4400809fac.8 for ; Thu, 09 Mar 2023 18:14:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678414474; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UM2gsUWwVSpEQWLnAnDEt7Ju3bE40/PbNcNR4oeIX2E=; b=czYP0S2zC0Bfx0whBmhspkdXqQsiv9IBS1vTRpN6404GElyuNGqPG2WKbkpw4mry8p BRejBew2K3QtczJW2rgu/S44/8c++rZBaCifneLBH6SofnpAIvNclAZxwKRn9mzixySW uqUOBf03mC1k4g0ITTiUA/t6sHCJ51v80QPmSoWcgDtdNKC2S6sB0sA6Rt+THLB8xl6U EQU+6FT6vmYaQSkyn4mSUvff9Pz/RpKHYa/IB1R2rkkxAC383TfOSlK2Krg7UvnmUNCR bUr+2s9RdD6PQsJroiMxQywfRwTfj1FGGGJRZEajW4IIpE0TT4Vs6/CfyDRKWjIfFFbb tBQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678414474; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UM2gsUWwVSpEQWLnAnDEt7Ju3bE40/PbNcNR4oeIX2E=; b=oCheCNEJDWLKKZTJyXTeBKnOPeCOpQEfZVH4ILo9WyaYFzlJmL77JaCgSHIzPnv9nn 6q6ZQDdlwvOZcO7dBBKdPImSzVV93apHjsVUr5tpsQQnyhdDxz1TbrpcyM2TVQMJfK69 9Rm7MePZ8eAIwPW3jdmUgaUFWv69cUUtbf/sGrR2r69NZ0W0Mvq/Lr5EGkWvK6sG/vvM P+PS/p60bdJiMqz47cRAczZBn/Swgg9x12L93i5XdrbdqNcUOqEYc8VxePqH35ElMh00 5Ffbw8HfsmdgrS0YMT2nFftXjs2iFGWhKTPdaMrxJ0oUvs6ddQ+j9fldeQ46fYHJCy21 Hqig== X-Gm-Message-State: AO0yUKURbAeN8IV3210M0azepzmURTaHI2TbG3a2sQVcB5keSbhW+/po ElwKYJanaaxQaT57KqD9RlYVkzU5g4Cc1xSNzelEwA== X-Google-Smtp-Source: AK7set+UZWwWlbF+ncnAtRwRtYIOCwi2htXVbIVui7/Bd76qnjiQJE8YdNXuy7rPQbZmfxWyQgtgs+24KwVDHVVibCU= X-Received: by 2002:a05:6870:5aa5:b0:175:2698:9a85 with SMTP id dt37-20020a0568705aa500b0017526989a85mr8465693oab.11.1678414473836; Thu, 09 Mar 2023 18:14:33 -0800 (PST) MIME-Version: 1.0 References: <20230228062246.1222387-1-jingzhangos@google.com> <20230228062246.1222387-3-jingzhangos@google.com> In-Reply-To: From: Jing Zhang Date: Thu, 9 Mar 2023 18:14:21 -0800 Message-ID: Subject: Re: [PATCH v3 2/6] KVM: arm64: Save ID registers' sanitized value per guest To: Oliver Upton Cc: KVM , KVMARM , ARMLinux , Marc Zyngier , Oliver Upton , Will Deacon , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Fuad Tabba , Reiji Watanabe , Ricardo Koller , Raghavendra Rao Ananta Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Oliver, On Tue, Mar 7, 2023 at 2:44 PM Oliver Upton wrote: > > Hi Jing, > > On Tue, Feb 28, 2023 at 06:22:42AM +0000, Jing Zhang wrote: > > From: Reiji Watanabe > > > > Introduce id_regs[] in kvm_arch as a storage of guest's ID registers, > > and save ID registers' sanitized value in the array at KVM_CREATE_VM. > > Use the saved ones when ID registers are read by the guest or > > userspace (via KVM_GET_ONE_REG). > > > > No functional change intended. > > > > Signed-off-by: Reiji Watanabe > > Co-developed-by: Jing Zhang > > Signed-off-by: Jing Zhang > > --- > > arch/arm64/include/asm/kvm_host.h | 12 +++++++++ > > arch/arm64/kvm/arm.c | 1 + > > arch/arm64/kvm/id_regs.c | 44 ++++++++++++++++++++++++------- > > arch/arm64/kvm/sys_regs.c | 2 +- > > arch/arm64/kvm/sys_regs.h | 1 + > > 5 files changed, 50 insertions(+), 10 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index a1892a8f6032..5c1cec4efa37 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -245,6 +245,16 @@ struct kvm_arch { > > * the associated pKVM instance in the hypervisor. > > */ > > struct kvm_protected_vm pkvm; > > + > > + /* > > + * Save ID registers for the guest in id_regs[]. > > + * (Op0, Op1, CRn, CRm, Op2) of the ID registers to be saved in it > > + * is (3, 0, 0, crm, op2), where 1<=crm<8, 0<=op2<8. > > + */ > > +#define KVM_ARM_ID_REG_NUM 56 > > +#define IDREG_IDX(id) (((sys_reg_CRm(id) - 1) << 3) | sys_reg_Op2(id)) > > +#define IDREG(kvm, id) kvm->arch.id_regs[IDREG_IDX(id)] > > I feel like the IDREG(...) macro just obfuscates what is otherwise a > simple array access. > Sure, will use array access. > > +static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r) > > +{ > > + if (sysreg_visible_as_raz(vcpu, r)) > > + return 0; > > + > > + return kvm_arm_read_id_reg_with_encoding(vcpu, reg_to_encoding(r)); > > nit: you could probably drop the '_with_encoding' suffix, as I don't > believe there are any other flavors of accessors. > Yes, will do. > > +} > > + > > /* cpufeature ID register access trap handlers */ > > > > static bool access_id_reg(struct kvm_vcpu *vcpu, > > @@ -504,3 +505,28 @@ int kvm_arm_walk_id_regs(struct kvm_vcpu *vcpu, u64 __user *uind) > > } > > return total; > > } > > + > > +/* > > + * Set the guest's ID registers that are defined in id_reg_descs[] > > + * with ID_SANITISED() to the host's sanitized value. > > + */ > > +void kvm_arm_set_default_id_regs(struct kvm *kvm) > > +{ > > + int i; > > + u32 id; > > + u64 val; > > + > > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++) { > > + id = reg_to_encoding(&id_reg_descs[i]); > > + if (WARN_ON_ONCE(!is_id_reg(id))) > > + /* Shouldn't happen */ > > + continue; > > Could you instead wire in a check to kvm_sys_reg_table_init() or do > something similar to it? Benefit of going that route is we outright > refuse to run KVM with such an egregious bug. > The check is already added in the first commit, which is kvm_arm_check_idreg_table() and improved progressively in later commits. > > + > > + if (id_reg_descs[i].visibility == raz_visibility) > > + /* Hidden or reserved ID register */ > > + continue; > > This sort of check works only for registers known to be RAZ at compile > time, but not for others that depend on runtime configuration. Is it > possible to reset the ID register values on the first call to > KVM_ARM_VCPU_INIT? > > The set of configured features is available at that point so you can > actually handle things like SVE and 32-bit ID registers correctly. > This function actually will be replaced with another init function as you suggested in a later commit (6/6), which would be called in kvm_arch_init_vm. > -- > Thanks, > Oliver 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 6E0E5C61DA4 for ; Fri, 10 Mar 2023 02:15:46 +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=5s5BBJlqixZJ403ThKteXB9pwxnoUq73+qMp3jvIO0o=; b=locYJFXY608MRd OKQCDOdOsD9ey9JHtTELrAzMaWalNdC2ctkxl21/F1o57J6GNyf+b1NbUpZ90q7qwOydsGH1E0N91 Fi9gmLr+DpJwjolZghoXTTssJdBLA2IG2yIE6+Otlw0sPAso+YsRCok0QVCP2eIq1v7Gd7ljMqm8d dytfgIRVci4zlhanqtiODhIh4PLNtE1hP8+yGM1cH3Okl57QlVdrZ7nxpIe+Ucz7/LkjIqQ8KaBxX v7m0ZU6WqnReFD6zf9i0DTrXtEQl2pMu7LcUA/4jd2knX6bg4Q51gVlGdZu8lWa3oVR+7LdryizQK Ueileac9nP9xs3Tq/+Pg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1paSHM-00Cgb5-KS; Fri, 10 Mar 2023 02:14:44 +0000 Received: from mail-oa1-x30.google.com ([2001:4860:4864:20::30]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1paSHI-00CgZv-JZ for linux-arm-kernel@lists.infradead.org; Fri, 10 Mar 2023 02:14:42 +0000 Received: by mail-oa1-x30.google.com with SMTP id 586e51a60fabf-176b90e14a9so4396739fac.9 for ; Thu, 09 Mar 2023 18:14:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678414474; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UM2gsUWwVSpEQWLnAnDEt7Ju3bE40/PbNcNR4oeIX2E=; b=czYP0S2zC0Bfx0whBmhspkdXqQsiv9IBS1vTRpN6404GElyuNGqPG2WKbkpw4mry8p BRejBew2K3QtczJW2rgu/S44/8c++rZBaCifneLBH6SofnpAIvNclAZxwKRn9mzixySW uqUOBf03mC1k4g0ITTiUA/t6sHCJ51v80QPmSoWcgDtdNKC2S6sB0sA6Rt+THLB8xl6U EQU+6FT6vmYaQSkyn4mSUvff9Pz/RpKHYa/IB1R2rkkxAC383TfOSlK2Krg7UvnmUNCR bUr+2s9RdD6PQsJroiMxQywfRwTfj1FGGGJRZEajW4IIpE0TT4Vs6/CfyDRKWjIfFFbb tBQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678414474; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UM2gsUWwVSpEQWLnAnDEt7Ju3bE40/PbNcNR4oeIX2E=; b=ZqYamZyciT7F6BcAhMIO2ddveJDEe/LVjWC/w4dPjZ5FS/UXb7roFn2ixwyT70C2dp g1LcaZKoL9fdWMRV+QzsyeQQwA4D44Mydpjyrxyu9ZPlEx/BLqfhav3h1vC4YrnbGDbx WtAFIj/BrRwY0TNVgeWUPekRFflF2BwSbICAc1nC0dZvU67snwVmb3Vq7Bou8J/R/vPx wiFXz932olhF3VJvfxIoayAUuAfZLsVPOz0vLDDlBYARSSSnIgGmQXnOv/XxbDVWocsP YBEAtTNJK2iiNWuSNV05KWW8zWRC0rztdBVvKepgw0tgAT0hSQw487ovsTvMENME1cZE jtAA== X-Gm-Message-State: AO0yUKVg1gYBP0HtZdgTO1uDI8Jgt+Ls8Dn5q8BO0cei6OQy4/sA/Kt/ Q3f0wlPMQnNKDPivICtl1mycai6PVQ5bj2Gk8QILiQ== X-Google-Smtp-Source: AK7set+UZWwWlbF+ncnAtRwRtYIOCwi2htXVbIVui7/Bd76qnjiQJE8YdNXuy7rPQbZmfxWyQgtgs+24KwVDHVVibCU= X-Received: by 2002:a05:6870:5aa5:b0:175:2698:9a85 with SMTP id dt37-20020a0568705aa500b0017526989a85mr8465693oab.11.1678414473836; Thu, 09 Mar 2023 18:14:33 -0800 (PST) MIME-Version: 1.0 References: <20230228062246.1222387-1-jingzhangos@google.com> <20230228062246.1222387-3-jingzhangos@google.com> In-Reply-To: From: Jing Zhang Date: Thu, 9 Mar 2023 18:14:21 -0800 Message-ID: Subject: Re: [PATCH v3 2/6] KVM: arm64: Save ID registers' sanitized value per guest To: Oliver Upton Cc: KVM , KVMARM , ARMLinux , Marc Zyngier , Oliver Upton , Will Deacon , Paolo Bonzini , James Morse , Alexandru Elisei , Suzuki K Poulose , Fuad Tabba , Reiji Watanabe , Ricardo Koller , Raghavendra Rao Ananta X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230309_181440_665458_0CC6820C X-CRM114-Status: GOOD ( 37.48 ) 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 Oliver, On Tue, Mar 7, 2023 at 2:44 PM Oliver Upton wrote: > > Hi Jing, > > On Tue, Feb 28, 2023 at 06:22:42AM +0000, Jing Zhang wrote: > > From: Reiji Watanabe > > > > Introduce id_regs[] in kvm_arch as a storage of guest's ID registers, > > and save ID registers' sanitized value in the array at KVM_CREATE_VM. > > Use the saved ones when ID registers are read by the guest or > > userspace (via KVM_GET_ONE_REG). > > > > No functional change intended. > > > > Signed-off-by: Reiji Watanabe > > Co-developed-by: Jing Zhang > > Signed-off-by: Jing Zhang > > --- > > arch/arm64/include/asm/kvm_host.h | 12 +++++++++ > > arch/arm64/kvm/arm.c | 1 + > > arch/arm64/kvm/id_regs.c | 44 ++++++++++++++++++++++++------- > > arch/arm64/kvm/sys_regs.c | 2 +- > > arch/arm64/kvm/sys_regs.h | 1 + > > 5 files changed, 50 insertions(+), 10 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index a1892a8f6032..5c1cec4efa37 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -245,6 +245,16 @@ struct kvm_arch { > > * the associated pKVM instance in the hypervisor. > > */ > > struct kvm_protected_vm pkvm; > > + > > + /* > > + * Save ID registers for the guest in id_regs[]. > > + * (Op0, Op1, CRn, CRm, Op2) of the ID registers to be saved in it > > + * is (3, 0, 0, crm, op2), where 1<=crm<8, 0<=op2<8. > > + */ > > +#define KVM_ARM_ID_REG_NUM 56 > > +#define IDREG_IDX(id) (((sys_reg_CRm(id) - 1) << 3) | sys_reg_Op2(id)) > > +#define IDREG(kvm, id) kvm->arch.id_regs[IDREG_IDX(id)] > > I feel like the IDREG(...) macro just obfuscates what is otherwise a > simple array access. > Sure, will use array access. > > +static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r) > > +{ > > + if (sysreg_visible_as_raz(vcpu, r)) > > + return 0; > > + > > + return kvm_arm_read_id_reg_with_encoding(vcpu, reg_to_encoding(r)); > > nit: you could probably drop the '_with_encoding' suffix, as I don't > believe there are any other flavors of accessors. > Yes, will do. > > +} > > + > > /* cpufeature ID register access trap handlers */ > > > > static bool access_id_reg(struct kvm_vcpu *vcpu, > > @@ -504,3 +505,28 @@ int kvm_arm_walk_id_regs(struct kvm_vcpu *vcpu, u64 __user *uind) > > } > > return total; > > } > > + > > +/* > > + * Set the guest's ID registers that are defined in id_reg_descs[] > > + * with ID_SANITISED() to the host's sanitized value. > > + */ > > +void kvm_arm_set_default_id_regs(struct kvm *kvm) > > +{ > > + int i; > > + u32 id; > > + u64 val; > > + > > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++) { > > + id = reg_to_encoding(&id_reg_descs[i]); > > + if (WARN_ON_ONCE(!is_id_reg(id))) > > + /* Shouldn't happen */ > > + continue; > > Could you instead wire in a check to kvm_sys_reg_table_init() or do > something similar to it? Benefit of going that route is we outright > refuse to run KVM with such an egregious bug. > The check is already added in the first commit, which is kvm_arm_check_idreg_table() and improved progressively in later commits. > > + > > + if (id_reg_descs[i].visibility == raz_visibility) > > + /* Hidden or reserved ID register */ > > + continue; > > This sort of check works only for registers known to be RAZ at compile > time, but not for others that depend on runtime configuration. Is it > possible to reset the ID register values on the first call to > KVM_ARM_VCPU_INIT? > > The set of configured features is available at that point so you can > actually handle things like SVE and 32-bit ID registers correctly. > This function actually will be replaced with another init function as you suggested in a later commit (6/6), which would be called in kvm_arch_init_vm. > -- > Thanks, > Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel