Received: by 2002:a05:6359:6284:b0:131:369:b2a3 with SMTP id se4csp1342249rwb; Sat, 5 Aug 2023 12:38:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFtgl4LlK2L6OvgXMT+rW5nogmldpUcepuTEEFpWeMBLF3v80AxP0Tb9GH3Oq1gpJ1IeX06 X-Received: by 2002:a05:6a20:12cd:b0:13f:6e26:e198 with SMTP id v13-20020a056a2012cd00b0013f6e26e198mr7255088pzg.54.1691264296288; Sat, 05 Aug 2023 12:38:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691264296; cv=none; d=google.com; s=arc-20160816; b=ln2Tcd5OgqvvNmYlNLhbxDCRDkzlM5ph8s3llsI9aJZZRcouRzYPMbgW1owsj1GEld rYSZwDWmikRYiLL1/OdiDAxcK215sZeRRRIgRnTDjWyaPTrvVfKFuCj2pWtXpq05Er// 2+aa0iAAfgkkUEiMcGXTVOWwEwv5s8dgOAcmSscGFOC4xj8chnl2NeBikYEOd+mUpIOX CudpJgOkJJLUld3uBWqm1Le5HqmNu1zQVcMPxOE80KruLqL3wxuI1l/MT579fKJO2Lrx GJH0h2ztblC3+2KNbVVgjxlkc+tIQ0H3pCi3VXyH1kjs9n2UyAM9AEgySa+qpPsMrGjt VZEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=EHniaT40XZNXmA55mmWKANONgA7yI+Cq2emx1fAa3pQ=; fh=iMjFSzO3l9gXADssU1Q67/ONc9ruJ8Gn5fwooRpFPoU=; b=wBnspiy9/1pFpig/hRuOcv2g/Xkbk15K91TSsAG9gGxd9L25mj77qW44F/SE0cez+t RJ8PuxjOnW7OSeqC9ndYWvIqnvp4GI9g/SYrItV4Jnl+FvrScM713UG1W60t36ht4oBh jYSLoNdhicSXwICOEpICVLVcUkkIuayCyzNd4tXUz1kLdD5gOECA6ZbS7IcDYyibiE4O rT6y9EXyow+Mkc6xHpSJyOz0CyUBOgq7hdMtxfnaur+ZL1gwTN4wmLFy03XPybbElg2/ lhYjoNqIHC9rFuzqtR+dyUtoCSH/PB11wm8TihDxE/Vui4S62W9jM+mqzt1eSbTUmIvo UJFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=oBXocZdC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a23-20020a62d417000000b00666c9148d03si970140pfh.6.2023.08.05.12.38.05; Sat, 05 Aug 2023 12:38:16 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=oBXocZdC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229733AbjHESIl (ORCPT + 99 others); Sat, 5 Aug 2023 14:08:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229481AbjHESIj (ORCPT ); Sat, 5 Aug 2023 14:08:39 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C574619B; Sat, 5 Aug 2023 11:08:38 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 59B9360DB5; Sat, 5 Aug 2023 18:08:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CA552C433C7; Sat, 5 Aug 2023 18:08:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691258917; bh=o1T6WnCOl5nls/Z32FQUuI4vIWqJLvZNcazUro6lNio=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oBXocZdCA2osDvHvD8WVyNio/gV6OKzFo9MtHyK/iBf0DY56PO9xJPizOMgKFDOPc dOx6hqB60YygH69+Jtn7OBjJG7RAF0yWkzsx0SYt+cZr9nZRtiHT9yCBb2jWsXddsg pFhgSg4krC+k/HQOV/w7tMEeEnStdR0V+NuQHhr5Eg3EAp10JmgAsNpfj1dN+HCZ9F a6/S/ATA530tyoOV7sxtuD0e+tLBKDRPQIVfBlJO6bsENv4qSr60KE8zDamwCTm1Xr aYSvNj6fQ20QcLuMEiYsnPkwqtjgwCUNgLfzuxMtb6SWcxzrTT6jK+j3GeSvmjOEp8 quIqeBavQjmLg== Date: Sat, 5 Aug 2023 11:11:34 -0700 From: Bjorn Andersson To: Elliot Berman Cc: Alex Elder , Srinivas Kandagatla , Prakruthi Deepak Heragu , Murali Nalajala , Trilok Soni , Srivatsa Vaddagiri , Carl van Schaik , Dmitry Baryshkov , Konrad Dybcio , Arnd Bergmann , Greg Kroah-Hartman , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Jonathan Corbet , Bagas Sanjaya , Will Deacon , Andy Gross , Catalin Marinas , Jassi Brar , linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v14 15/25] virt: gunyah: Add Qualcomm Gunyah platform ops Message-ID: <7bpdzvlrdxwqpxor36i4nha5qs4opvnf7tr4sfb7ta3ebl7fdl@c5nylbvtnv5y> References: <20230613172054.3959700-1-quic_eberman@quicinc.com> <20230613172054.3959700-16-quic_eberman@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230613172054.3959700-16-quic_eberman@quicinc.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 13, 2023 at 10:20:43AM -0700, Elliot Berman wrote: > Qualcomm platforms have a firmware entity which performs access control > to physical pages. Dynamically started Gunyah virtual machines use the > QCOM_SCM_RM_MANAGED_VMID for access. Linux thus needs to assign access > to the memory used by guest VMs. Gunyah doesn't do this operation for us > since it is the current VM (typically VMID_HLOS) delegating the access > and not Gunyah itself. Use the Gunyah platform ops to achieve this so > that only Qualcomm platforms attempt to make the needed SCM calls. > > Reviewed-by: Alex Elder > Co-developed-by: Prakruthi Deepak Heragu > Signed-off-by: Prakruthi Deepak Heragu > Signed-off-by: Elliot Berman > --- > drivers/virt/gunyah/Kconfig | 13 +++ > drivers/virt/gunyah/Makefile | 1 + > drivers/virt/gunyah/gunyah_qcom.c | 153 ++++++++++++++++++++++++++++++ > 3 files changed, 167 insertions(+) > create mode 100644 drivers/virt/gunyah/gunyah_qcom.c > > diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig > index de815189dab6c..0421b751aad4f 100644 > --- a/drivers/virt/gunyah/Kconfig > +++ b/drivers/virt/gunyah/Kconfig > @@ -5,6 +5,7 @@ config GUNYAH > depends on ARM64 > depends on MAILBOX > select GUNYAH_PLATFORM_HOOKS > + imply GUNYAH_QCOM_PLATFORM if ARCH_QCOM > help > The Gunyah drivers are the helper interfaces that run in a guest VM > such as basic inter-VM IPC and signaling mechanisms, and higher level > @@ -15,3 +16,15 @@ config GUNYAH > > config GUNYAH_PLATFORM_HOOKS > tristate > + > +config GUNYAH_QCOM_PLATFORM > + tristate "Support for Gunyah on Qualcomm platforms" > + depends on GUNYAH > + select GUNYAH_PLATFORM_HOOKS > + select QCOM_SCM > + help > + Enable support for interacting with Gunyah on Qualcomm > + platforms. Interaction with Qualcomm firmware requires > + extra platform-specific support. > + > + Say Y/M here to use Gunyah on Qualcomm platforms. > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile > index 4fbeee521d60a..2aa9ff038ed02 100644 > --- a/drivers/virt/gunyah/Makefile > +++ b/drivers/virt/gunyah/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_GUNYAH_PLATFORM_HOOKS) += gunyah_platform_hooks.o > +obj-$(CONFIG_GUNYAH_QCOM_PLATFORM) += gunyah_qcom.o > > gunyah-y += rsc_mgr.o rsc_mgr_rpc.o vm_mgr.o vm_mgr_mm.o > obj-$(CONFIG_GUNYAH) += gunyah.o > diff --git a/drivers/virt/gunyah/gunyah_qcom.c b/drivers/virt/gunyah/gunyah_qcom.c > new file mode 100644 > index 0000000000000..f06a598f2e1b3 > --- /dev/null > +++ b/drivers/virt/gunyah/gunyah_qcom.c > @@ -0,0 +1,153 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define QCOM_SCM_RM_MANAGED_VMID 0x3A > +#define QCOM_SCM_MAX_MANAGED_VMID 0x3F > + > +static int qcom_scm_gh_rm_pre_mem_share(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel) > +{ > + struct qcom_scm_vmperm *new_perms; > + u64 src, src_cpy; > + int ret = 0, i, n; > + u16 vmid; > + > + new_perms = kcalloc(mem_parcel->n_acl_entries, sizeof(*new_perms), GFP_KERNEL); > + if (!new_perms) > + return -ENOMEM; > + > + for (n = 0; n < mem_parcel->n_acl_entries; n++) { > + vmid = le16_to_cpu(mem_parcel->acl_entries[n].vmid); > + if (vmid <= QCOM_SCM_MAX_MANAGED_VMID) > + new_perms[n].vmid = vmid; > + else > + new_perms[n].vmid = QCOM_SCM_RM_MANAGED_VMID; > + if (mem_parcel->acl_entries[n].perms & GH_RM_ACL_X) > + new_perms[n].perm |= QCOM_SCM_PERM_EXEC; > + if (mem_parcel->acl_entries[n].perms & GH_RM_ACL_W) > + new_perms[n].perm |= QCOM_SCM_PERM_WRITE; > + if (mem_parcel->acl_entries[n].perms & GH_RM_ACL_R) > + new_perms[n].perm |= QCOM_SCM_PERM_READ; > + } > + > + src = BIT_ULL(QCOM_SCM_VMID_HLOS); > + > + for (i = 0; i < mem_parcel->n_mem_entries; i++) { > + src_cpy = src; > + ret = qcom_scm_assign_mem(le64_to_cpu(mem_parcel->mem_entries[i].phys_addr), > + le64_to_cpu(mem_parcel->mem_entries[i].size), > + &src_cpy, new_perms, mem_parcel->n_acl_entries); > + if (ret) > + break; > + } > + > + if (!ret) > + goto out; I was going to complain that you don't unroll the assignments on failure, but now realized that this "error handling" is backwards and actually represents the success case. Please don't do that, goto error_handling || kfree(new_perms); return 0; here instead. That also means that you don't need to zero-initialize ret, because you would goto err_reclaim inside the loop. > + > + src = 0; > + for (n = 0; n < mem_parcel->n_acl_entries; n++) { > + vmid = le16_to_cpu(mem_parcel->acl_entries[n].vmid); > + if (vmid <= QCOM_SCM_MAX_MANAGED_VMID) > + src |= BIT_ULL(vmid); > + else > + src |= BIT_ULL(QCOM_SCM_RM_MANAGED_VMID); > + } > + > + new_perms[0].vmid = QCOM_SCM_VMID_HLOS; > + > + for (i--; i >= 0; i--) { > + src_cpy = src; > + WARN_ON_ONCE(qcom_scm_assign_mem( > + le64_to_cpu(mem_parcel->mem_entries[i].phys_addr), > + le64_to_cpu(mem_parcel->mem_entries[i].size), > + &src_cpy, new_perms, 1)); > + } > + > +out: > + kfree(new_perms); > + return ret; > +} Regards, Bjorn