linux-kernel - Re: [PATCH v6 01/11] arm64: rsi: Add RSI definitions

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <77030ef8-e180-46bb-872c-e41c8b25bbc2@arm.com>
Date: Fri, 11 Oct 2024 15:14:24 +0100
From: Steven Price <steven.price@....com>
To: Gavin Shan <gshan@...hat.com>, kvm@...r.kernel.org, kvmarm@...ts.linux.dev
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
 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>,
 Shanker Donthineni <sdonthineni@...dia.com>, Alper Gun
 <alpergun@...gle.com>, "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>,
 Gavin Shan <gshan@...ht.com>
Subject: Re: [PATCH v6 01/11] arm64: rsi: Add RSI definitions

On 08/10/2024 00:08, Gavin Shan wrote:
> On 10/5/24 12:42 AM, Steven Price wrote:
>> From: Suzuki K Poulose <suzuki.poulose@....com>
>>
>> The RMM (Realm Management Monitor) provides functionality that can be
>> accessed by a realm guest through SMC (Realm Services Interface) calls.
>>
>> The SMC definitions are based on DEN0137[1] version 1.0-rel0.
>>
>> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> Acked-by: Catalin Marinas <catalin.marinas@....com>
>> Reviewed-by: Gavin Shan <gshan@...ht.com>
>> Signed-off-by: Steven Price <steven.price@....com>
>> ---
> 
> [...]
> 
>> +
>> +static inline unsigned long rsi_set_addr_range_state(phys_addr_t start,
>> +                             phys_addr_t end,
>> +                             enum ripas state,
>> +                             unsigned long flags,
>> +                             phys_addr_t *top)
>> +{
>> +    struct arm_smccc_res res;
>> +
>> +    arm_smccc_smc(SMC_RSI_IPA_STATE_SET, start, end, state,
>> +              flags, 0, 0, 0, &res);
>> +
>> +    if (top)
>> +        *top = res.a1;
>> +
>> +    if (res.a2 != RSI_ACCEPT)
>> +        return -EPERM;
>> +
>> +    return res.a0;
>> +}
>> +
> 
> Similar to rsi_attestation_token_init(), the return value type needs to
> be 'long'
> since '-EPERM' can be returned from the function.

Good spot.

>> +/**
>> + * rsi_attestation_token_init - Initialise the operation to retrieve an
>> + * attestation token.
>> + *
>> + * @challenge:    The challenge data to be used in the attestation token
>> + *        generation.
>> + * @size:    Size of the challenge data in bytes.
>> + *
>> + * Initialises the attestation token generation and returns an upper
>> bound
>> + * on the attestation token size that can be used to allocate an
>> adequate
>> + * buffer. The caller is expected to subsequently call
>> + * rsi_attestation_token_continue() to retrieve the attestation token
>> data on
>> + * the same CPU.
>> + *
>> + * Returns:
>> + *  On success, returns the upper limit of the attestation report size.
>> + *  Otherwise, -EINVAL
>> + */
>> +static inline long
>> +rsi_attestation_token_init(const u8 *challenge, unsigned long size)
>> +{
>> +    struct arm_smccc_1_2_regs regs = { 0 };
>> +
>> +    /* The challenge must be at least 32bytes and at most 64bytes */
>> +    if (!challenge || size < 32 || size > 64)
>> +        return -EINVAL;
>> +
>> +    regs.a0 = SMC_RSI_ATTESTATION_TOKEN_INIT;
>> +    memcpy(&regs.a1, challenge, size);
>> +    arm_smccc_1_2_smc(&regs, &regs);
>> +
>> +    if (regs.a0 == RSI_SUCCESS)
>> +        return regs.a1;
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +/**
>> + * rsi_attestation_token_continue - Continue the operation to
>> retrieve an
>> + * attestation token.
>> + *
>> + * @granule: {I}PA of the Granule to which the token will be written.
>> + * @offset:  Offset within Granule to start of buffer in bytes.
>> + * @size:    The size of the buffer.
>> + * @len:     The number of bytes written to the buffer.
>> + *
>> + * Retrieves up to a RSI_GRANULE_SIZE worth of token data per call.
>> The caller
>> + * is expected to call rsi_attestation_token_init() before calling this
>> + * function to retrieve the attestation token.
>> + *
>> + * Return:
>> + * * %RSI_SUCCESS     - Attestation token retrieved successfully.
>> + * * %RSI_INCOMPLETE  - Token generation is not complete.
>> + * * %RSI_ERROR_INPUT - A parameter was not valid.
>> + * * %RSI_ERROR_STATE - Attestation not in progress.
>> + */
>> +static inline int rsi_attestation_token_continue(phys_addr_t granule,
>> +                         unsigned long offset,
>> +                         unsigned long size,
>> +                         unsigned long *len)
>> +{
>> +    struct arm_smccc_res res;
>> +
>> +    arm_smccc_1_1_invoke(SMC_RSI_ATTESTATION_TOKEN_CONTINUE,
>> +                 granule, offset, size, 0, &res);
>> +
>> +    if (len)
>> +        *len = res.a1;
>> +    return res.a0;
>> +}
>> +
> 
> The return value type of this function needs to be 'unsigned long' even
> it's
> converted to 'int' in arm_cca_attestation_continue(). In this way, the
> wrapper
> functions has consistent return value type, which is 'unsigned long' or
> 'long'.

Ack, seems reasonable.

Thanks,
Steve