| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250730154310.00003143@huawei.com>
Date: Wed, 30 Jul 2025 15:43:10 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org>
CC: <linux-coco@...ts.linux.dev>, <kvmarm@...ts.linux.dev>,
<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <aik@....com>,
<lukas@...ner.de>, Samuel Ortiz <sameo@...osinc.com>, Xu Yilun
<yilun.xu@...ux.intel.com>, Jason Gunthorpe <jgg@...pe.ca>, "Suzuki K
Poulose" <Suzuki.Poulose@....com>, Steven Price <steven.price@....com>,
Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
Will Deacon <will@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [RFC PATCH v1 30/38] coco: host: arm64: Add support for realm
host interface (RHI)
On Mon, 28 Jul 2025 19:22:07 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org> wrote:
> Device assignment-related RHI calls result in a REC exit, which is
> handled by the tsm guest_request callback.
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>
Comments below.
> ---
> arch/arm64/include/asm/rhi.h | 32 ++++++++++
> drivers/virt/coco/arm-cca-host/arm-cca.c | 68 ++++++++++++++++++++
> drivers/virt/coco/arm-cca-host/rmm-da.c | 81 ++++++++++++++++++++++++
> drivers/virt/coco/arm-cca-host/rmm-da.h | 4 ++
> include/linux/tsm.h | 3 +
> 5 files changed, 188 insertions(+)
> create mode 100644 arch/arm64/include/asm/rhi.h
> diff --git a/drivers/virt/coco/arm-cca-host/arm-cca.c b/drivers/virt/coco/arm-cca-host/arm-cca.c
> index be1296fb1bf2..0807fcf8d222 100644
> --- a/drivers/virt/coco/arm-cca-host/arm-cca.c
> +++ b/drivers/virt/coco/arm-cca-host/arm-cca.c
> +static int cca_tsm_guest_req(struct pci_dev *pdev, struct tsm_guest_req_info *info)
> +{
> + int ret;
> +
> + switch (info->type) {
> + case ARM_CCA_DA_OBJECT_SIZE:
> + {
> + int object_size;
> + struct da_object_size_req req;
> +
> + if (sizeof(req) != info->req_len)
> + return -EINVAL;
> +
> + if (copy_from_user(&req, info->req, info->req_len))
> + return -EFAULT;
> +
> + object_size = rme_get_da_object_size(pdev, req.object_type);
> + if (object_size > 0) {
> + if (info->resp_len < sizeof(object_size))
> + return -EINVAL;
> + if (copy_to_user(info->resp, &object_size, sizeof(object_size)))
> + return -EFAULT;
> + info->resp_len = sizeof(object_size);
> + ret = 0;
return 0;
then else isn't needed.
> + } else
#Style is {} on all legs if any need it.
> + /* error */
> + ret = object_size;
return object_size;
> + break;
> + }
> + case ARM_CCA_DA_OBJECT_READ:
> + {
> + int resp_len;
> + struct da_object_read_req req;
> +
> + if (sizeof(req) != info->req_len)
> + return -EINVAL;
> +
> + if (copy_from_user(&req, info->req, info->req_len))
> + return -EFAULT;
> +
> + resp_len = rme_da_object_read(pdev, req.object_type, req.offset,
> + info->resp_len,
> + info->resp);
> + if (resp_len > 0) {
> + info->resp_len = resp_len;
> + ret = 0;
Similar to above.
> + } else
> + /* error */
> + ret = resp_len;
> + break;
> + }
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + return ret;
As you've probably figured out, I love an early return. This sort
of function shows why as it reduced indent etc in lots of places.
Here you mix and match. Maybe it will make sense later in the series though!
> +}
> diff --git a/drivers/virt/coco/arm-cca-host/rmm-da.c b/drivers/virt/coco/arm-cca-host/rmm-da.c
> index bef33e618fd3..c7da9d12f258 100644
> --- a/drivers/virt/coco/arm-cca-host/rmm-da.c
> +++ b/drivers/virt/coco/arm-cca-host/rmm-da.c
> @@ -10,7 +10,9 @@
> #include <keys/asymmetric-type.h>
> #include <keys/x509-parser.h>
> #include <linux/kvm_types.h>
> +#include <linux/kvm_host.h>
> #include <asm/kvm_rme.h>
> +#include <asm/kvm_emulate.h>
>
> #include "rmm-da.h"
>
> @@ -769,6 +771,85 @@ static int rme_exit_vdev_req_handler(struct realm_rec *rec)
> return 1;
> }
>
> +int rme_get_da_object_size(struct pci_dev *pdev, int type)
> +{
> + int ret = 0;
> + unsigned long len;
> + struct pci_tsm *tsm = pdev->tsm;
> + struct cca_host_dsc_pf0 *dsc_pf0;
> +
> + if (!tsm)
> + return -EINVAL;
> +
> + dsc_pf0 = to_cca_dsc_pf0(tsm->dsm_dev);
> +
> + /* Determine the buffer that should be used */
> + if (type == RHI_DA_OBJECT_INTERFACE_REPORT) {
> + len = dsc_pf0->interface_report.size;
> + } else if (type == RHI_DA_OBJECT_MEASUREMENT) {
> + len = dsc_pf0->measurements.size;
> + } else if (type == RHI_DA_OBJECT_CERTIFICATE) {
> + len = dsc_pf0->cert_chain.cache.size;
> + } else if (type == RHI_DA_OBJECT_VCA) {
> + len = dsc_pf0->vca.size;
> + } else {
> + ret = -EINVAL;
> + goto err_out;
> + }
> +
> + return len;
> +err_out:
Similar to below. This pattern just makes things more complex.
If we need to introduce a label, do it in the patch where you add
code to do something on error.
> + return ret;
> +}
> +
> +int rme_da_object_read(struct pci_dev *pdev, int type, unsigned long offset,
> + unsigned long max_len, void __user *user_buf)
> +{
> + void *buf;
> + int ret = 0;
> + unsigned long len;
> + struct cca_host_dsc_pf0 *dsc_pf0;
> + struct pci_tsm *tsm = pdev->tsm;
> +
> + if (!tsm)
> + return -EINVAL;
> +
> + dsc_pf0 = to_cca_dsc_pf0(tsm->dsm_dev);
> +
> + /* Determine the buffer that should be used */
> + if (type == RHI_DA_OBJECT_INTERFACE_REPORT) {
> + len = dsc_pf0->interface_report.size;
> + buf = dsc_pf0->interface_report.buf;
> + } else if (type == RHI_DA_OBJECT_MEASUREMENT) {
> + len = dsc_pf0->measurements.size;
> + buf = dsc_pf0->measurements.buf;
> + } else if (type == RHI_DA_OBJECT_CERTIFICATE) {
> + len = dsc_pf0->cert_chain.cache.size;
> + buf = dsc_pf0->cert_chain.cache.buf;
> + } else if (type == RHI_DA_OBJECT_VCA) {
> + len = dsc_pf0->vca.size;
> + buf = dsc_pf0->vca.buf;
> + } else {
> + ret = -EINVAL;
> + goto err_out;
> + }
> +
> + /* Assume that the buffer is large enough for the whole report */
> + if ((max_len - offset) < len) {
> + /* FIXME!! the error code */
> + ret = -ENOMEM;
> + goto err_out;
> + }
> +
> + if (copy_to_user(user_buf + offset, buf, len)) {
> + ret = -EIO;
> + goto err_out;
> + }
> + ret = len;
> +err_out:
Definitely makes sense to just return directly in the error paths above and
just return len here
> + return ret;
> +}
> +
> void rme_register_exit_handlers(void)
> {
> realm_exit_vdev_comm_handler = rme_exit_vdev_comm_handler;
> diff --git a/drivers/virt/coco/arm-cca-host/rmm-da.h b/drivers/virt/coco/arm-cca-host/rmm-da.h
> index cebddab8464d..457660ff3b69 100644
> --- a/drivers/virt/coco/arm-cca-host/rmm-da.h
> +++ b/drivers/virt/coco/arm-cca-host/rmm-da.h
> @@ -10,6 +10,7 @@
> #include <linux/pci-ide.h>
> #include <linux/pci-tsm.h>
> #include <asm/rmi_smc.h>
> +#include <asm/rhi.h>
>
> #define MAX_CACHE_OBJ_SIZE 4096
> struct cache_object {
> @@ -101,4 +102,7 @@ void *rme_create_vdev(struct realm *realm, struct pci_dev *pdev,
> void rme_unbind_vdev(struct realm *realm, struct pci_dev *pdev,
> struct pci_dev *pf0_dev);
> void rme_register_exit_handlers(void);
> +int rme_get_da_object_size(struct pci_dev *pdev, int type);
> +int rme_da_object_read(struct pci_dev *pdev, int type, unsigned long offset,
> + unsigned long max_len, void __user *user_buf);
> #endif
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 497a3b4df5a0..e82046b0c7fa 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -145,5 +145,8 @@ struct tsm_guest_req_info {
> size_t resp_len;
> };
>
> +#define ARM_CCA_DA_OBJECT_SIZE 0x1
> +#define ARM_CCA_DA_OBJECT_READ 0x2
> +
> int tsm_guest_req(struct device *dev, struct tsm_guest_req_info *info);
> #endif /* __TSM_H */
Powered by blists - more mailing lists