[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8019da1-d361-445b-a224-0761640aa616@arm.com>
Date: Tue, 16 Apr 2024 14:14:37 +0100
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Steven Price <steven.price@....com>, kvm@...r.kernel.org,
kvmarm@...ts.linux.dev
Cc: Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
Oliver Upton <oliver.upton@...ux.dev>, Zenghui Yu <yuzenghui@...wei.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Joey Gouly <joey.gouly@....com>, Alexandru Elisei
<alexandru.elisei@....com>, Christoffer Dall <christoffer.dall@....com>,
Fuad Tabba <tabba@...gle.com>, linux-coco@...ts.linux.dev,
Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
Subject: Re: [PATCH v2 06/43] arm64: RME: Add wrappers for RMI calls
Hi Steven
On 12/04/2024 09:42, Steven Price wrote:
> The wrappers make the call sites easier to read and deal with the
> boiler plate of handling the error codes from the RMM.
>
I have compared the parameters and output values to that of the RMM spec
and they match. There are some minor nits below.
> Signed-off-by: Steven Price <steven.price@....com>
> ---
> arch/arm64/include/asm/rmi_cmds.h | 509 ++++++++++++++++++++++++++++++
> 1 file changed, 509 insertions(+)
> create mode 100644 arch/arm64/include/asm/rmi_cmds.h
>
> diff --git a/arch/arm64/include/asm/rmi_cmds.h b/arch/arm64/include/asm/rmi_cmds.h
> new file mode 100644
> index 000000000000..c21414127e8e
> --- /dev/null
> +++ b/arch/arm64/include/asm/rmi_cmds.h
> @@ -0,0 +1,509 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023 ARM Ltd.
> + */
> +
> +#ifndef __ASM_RMI_CMDS_H
> +#define __ASM_RMI_CMDS_H
> +
> +#include <linux/arm-smccc.h>
> +
> +#include <asm/rmi_smc.h>
> +
> +struct rtt_entry {
> + unsigned long walk_level;
> + unsigned long desc;
> + int state;
> + int ripas;
> +};
> +
..
> +/**
> + * rmi_data_destroy() - Destroy a Data Granule
> + * @rd: PA of the RD
> + * @ipa: IPA at which the granule is mapped in the guest
> + * @data_out: PA of the granule which was destroyed
> + * @top_out: Top IPA of non-live RTT entries
> + *
> + * Transitions the granule to DESTROYED state, the address cannot be used by
> + * the guest for the lifetime of the Realm.
> + *
> + * Return: RMI return code
> + */
> +static inline int rmi_data_destroy(unsigned long rd, unsigned long ipa,
> + unsigned long *data_out,
> + unsigned long *top_out)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(SMC_RMI_DATA_DESTROY, rd, ipa, &res);
> +
> + *data_out = res.a1;
> + *top_out = res.a2;
minor nit: Do we need to be safer by checking the parameters before
filling them in ? i.e.,
if (ptr)
*ptr = result_out;
This applies for others calls below.
> +
> + return res.a0;
> +}
> +
> +/**
> + * rmi_realm_destroy() - Destroy a Realm
> + * @rd: PA of the RD
> + *
> + * Destroys a Realm, all objects belonging to the Realm must be destroyed first.
> + *
> + * Return: RMI return code
> + */
> +static inline int rmi_realm_destroy(unsigned long rd)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(SMC_RMI_REALM_DESTROY, rd, &res);
> +
> + return res.a0;
> +}
> +
> +/**
> + * rmi_rec_aux_count() - Get number of auxiliary Granules required
> + * @rd: PA of the RD
> + * @aux_count: Number of pages written to this pointer
> + *
> + * A REC may require extra auxiliary pages to be delegateed for the RMM to
minor nit: "s/delegateed/delegated/"
..
> +/**
> + * rmi_rtt_read_entry() - Read an RTTE
> + * @rd: PA of the RD
> + * @ipa: IPA for which to read the RTTE
> + * @level: RTT level at which to read the RTTE
> + * @rtt: Output structure describing the RTTE
> + *
> + * Reads a RTTE (Realm Translation Table Entry).
> + *
> + * Return: RMI return code
> + */
> +static inline int rmi_rtt_read_entry(unsigned long rd, unsigned long ipa,
> + long level, struct rtt_entry *rtt)
> +{
> + struct arm_smccc_1_2_regs regs = {
> + SMC_RMI_RTT_READ_ENTRY,
> + rd, ipa, level
> + };
> +
> + arm_smccc_1_2_smc(®s, ®s);
> +
> + rtt->walk_level = regs.a1;
> + rtt->state = regs.a2 & 0xFF;
minor nit: We mask the state, but not the "ripas". Both of them are u8.
For consistency, we should mask both or neither.
> + rtt->desc = regs.a3;
> + rtt->ripas = regs.a4;
> +
> + return regs.a0;
> +}
> +
..
> +/**
> + * rmi_rtt_get_phys() - Get the PA from a RTTE
> + * @rtt: The RTTE
> + *
> + * Return: the physical address from a RTT entry.
> + */
> +static inline phys_addr_t rmi_rtt_get_phys(struct rtt_entry *rtt)
> +{
> + return rtt->desc & GENMASK(47, 12);
> +}
I guess this may need to change with the LPA2 support in RMM and must be
used in conjunction with the "realm" object to make the correct
conversion.
Suzuki
> +
> +#endif
Powered by blists - more mailing lists