| 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: <yq5aa50cm326.fsf@kernel.org>
Date: Mon, 24 Nov 2025 10:37:13 +0530
From: Aneesh Kumar K.V <aneesh.kumar@...nel.org>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: linux-coco@...ts.linux.dev, kvmarm@...ts.linux.dev,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
dan.j.williams@...el.com, 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>,
Bjorn Helgaas <helgaas@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH v2 02/11] coco: guest: arm64: Add Realm Host Interface
and guest DA helper
Jonathan Cameron <jonathan.cameron@...wei.com> writes:
> On Mon, 17 Nov 2025 19:29:58 +0530
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org> wrote:
>
>> - describe the Realm Host Interface SMC IDs and result codes in a new
>> `asm/rhi.h` header
>> - expose `struct rsi_host_call` plus an `rsi_host_call()` helper so we can
>> invoke `SMC_RSI_HOST_CALL` from C code
>> - build a guest-side `rhi-da` helper that drives the vdev TDI state machine
>> via RHI host calls and translates the firmware status codes
>>
>> This provides the basic RHI plumbing that later DA features rely on.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>
> Hi Aneesh, minor comments follow.
>
>> diff --git a/drivers/virt/coco/arm-cca-guest/rhi-da.c b/drivers/virt/coco/arm-cca-guest/rhi-da.c
>> new file mode 100644
>> index 000000000000..3430d8df4424
>> --- /dev/null
>> +++ b/drivers/virt/coco/arm-cca-guest/rhi-da.c
> ...
>
>> +
>> +bool rhi_has_da_support(void)
>> +{
>> + int ret;
>> + struct rsi_host_call *rhicall;
>> +
>> + rhicall = kmalloc(sizeof(struct rsi_host_call), GFP_KERNEL);
>
> Doesn't look to be passed out anywhere, so not obvious why the lifetime
> of this extends beyond this function. Maybe I'm missing something.
>
This should be similar to other calls
struct rsi_host_call *rhicall __free(kfree) =
kmalloc(sizeof(struct rsi_host_call), GFP_KERNEL);
I’ll update the code to reflect that. Thanks for pointing it out.
>
>> + if (!rhicall)
>> + return -ENOMEM;
>> +
>> + rhicall->imm = 0;
>> + rhicall->gprs[0] = RHI_DA_FEATURES;
>> +
>> + ret = rsi_host_call(virt_to_phys(rhicall));
>> + if (ret != RSI_SUCCESS || rhicall->gprs[0] == SMCCC_RET_NOT_SUPPORTED)
>> + return false;
>> +
>> + /* For base DA to work we need these to be supported */
>> + if ((rhicall->gprs[0] & RHI_DA_BASE_FEATURE) == RHI_DA_BASE_FEATURE)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static inline int rhi_vdev_continue(unsigned long vdev_id, unsigned long cookie)
>> +{
>> + unsigned long ret;
>> +
>> + struct rsi_host_call *rhi_call __free(kfree) =
>> + kmalloc(sizeof(struct rsi_host_call), GFP_KERNEL);
>
> sizeof(*rhi_call) Same for all other cases of this.
>
>> + if (!rhi_call)
>> + return -ENOMEM;
>> +
>> + rhi_call->imm = 0;
>> + rhi_call->gprs[0] = RHI_DA_VDEV_CONTINUE;
>> + rhi_call->gprs[1] = vdev_id;
>> + rhi_call->gprs[2] = cookie;
>> +
>> + ret = rsi_host_call(virt_to_phys(rhi_call));
>> + if (ret != RSI_SUCCESS)
>> + return -EIO;
>> +
>> + return map_rhi_da_error(rhi_call->gprs[0]);
>> +}
>> +
>> +static int __rhi_vdev_abort(unsigned long vdev_id, unsigned long *da_error)
>> +{
>> + unsigned long ret;
>> + struct rsi_host_call *rhi_call __free(kfree) =
>> + kmalloc(sizeof(struct rsi_host_call), GFP_KERNEL);
>
> sizeof(*rhi_call) probably preferred.
>
>> + if (!rhi_call)
>> + return -ENOMEM;
>> +
>> + rhi_call->imm = 0;
>> + rhi_call->gprs[0] = RHI_DA_VDEV_ABORT;
>> + rhi_call->gprs[1] = vdev_id;
>> +
>> + ret = rsi_host_call(virt_to_phys(rhi_call));
>> + if (ret != RSI_SUCCESS)
>> + return -EIO;
>> +
>> + return *da_error = rhi_call->gprs[0];
>> + return 0;
>
> ? Run builds after each patch and you may catch stuff like this.
>
>> +}
>> +
>> +static bool should_abort_rhi_call_loop(unsigned long vdev_id)
>> +{
>> + int ret;
>> +
>> + cond_resched();
>> + if (signal_pending(current)) {
>> + unsigned long da_error;
>> +
>> + ret = __rhi_vdev_abort(vdev_id, &da_error);
>> + /* consider all kind of error as not aborted */
>> + if (!ret && (da_error == RHI_DA_SUCCESS))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static int __rhi_vdev_set_tdi_state(unsigned long vdev_id,
>> + unsigned long target_state,
>
> Maybe use an enum for target state? Can name it to align with the
> RHIDAVDevTDIState used as the type for this in the RHI spec.
>
how about enum rhi_tdi_state ?
>
>
>> + unsigned long *cookie)
>> +{
>> + unsigned long ret;
>> +
>> + struct rsi_host_call *rhi_call __free(kfree) =
>> + kmalloc(sizeof(struct rsi_host_call), GFP_KERNEL);
>> + if (!rhi_call)
>> + return -ENOMEM;
>> +
>> + rhi_call->imm = 0;
>> + rhi_call->gprs[0] = RHI_DA_VDEV_SET_TDI_STATE;
>> + rhi_call->gprs[1] = vdev_id;
>> + rhi_call->gprs[2] = target_state;
>> +
>> + ret = rsi_host_call(virt_to_phys(rhi_call));
>> + if (ret != RSI_SUCCESS)
>> + return -EIO;
>> +
>> + *cookie = rhi_call->gprs[1];
>> + return map_rhi_da_error(rhi_call->gprs[0]);
>> +}
>
>> diff --git a/drivers/virt/coco/arm-cca-guest/rhi-da.h b/drivers/virt/coco/arm-cca-guest/rhi-da.h
>> new file mode 100644
>> index 000000000000..8dd77c7ed645
>> --- /dev/null
>> +++ b/drivers/virt/coco/arm-cca-guest/rhi-da.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2024 ARM Ltd.
>
> Possibly update if this has changed much this year.
>
>> + */
>> +
>> +#ifndef _VIRT_COCO_RHI_DA_H_
>> +#define _VIRT_COCO_RHI_DA_H_
>> +
>> +#include <asm/rhi.h>
>> +
>> +struct pci_dev;
>> +bool rhi_has_da_support(void);
>> +int rhi_vdev_set_tdi_state(struct pci_dev *pdev, unsigned long target_state);
>> +#endif
-aneesh
Powered by blists - more mailing lists