On 26/06/26 06:05PM, Shivang Upadhyay wrote:
> `opal_check_token()` function is used to determine if a specific
> OPAL firmware call is supported on the current platform. This check
> is performed frequently during boot and runtime, resulting in
> unnecessary firmware calls for the same token values.
>
> Add a caching layer for the opal_check_token() OPAL call to avoid
> repeated firmware calls for token availability checks. This reduces
> firmware call overhead during boot.
>
> Testing with buildroot images shows OPAL calls reduced from
> 35578 to 28983, before console bring-up.
nice optimisation !
>
> <...snip...>
> int64_t opal_xscom_read(uint32_t gcid, uint64_t pcb_addr, __be64 *val);
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c
> b/arch/powerpc/platforms/powernv/opal-call.c
> index 021b0ec29e24..00325c189e69 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -207,7 +207,7 @@ OPAL_CALL(opal_validate_flash,
> OPAL_FLASH_VALIDATE);
> OPAL_CALL(opal_manage_flash, OPAL_FLASH_MANAGE);
> OPAL_CALL(opal_update_flash, OPAL_FLASH_UPDATE);
> OPAL_CALL(opal_resync_timebase, OPAL_RESYNC_TIMEBASE);
> -OPAL_CALL(opal_check_token, OPAL_CHECK_TOKEN);
> +OPAL_CALL(opal_check_token_call, OPAL_CHECK_TOKEN);
> OPAL_CALL(opal_dump_init, OPAL_DUMP_INIT);
> OPAL_CALL(opal_dump_info, OPAL_DUMP_INFO);
> OPAL_CALL(opal_dump_info2, OPAL_DUMP_INFO2);
> diff --git a/arch/powerpc/platforms/powernv/opal.c
> b/arch/powerpc/platforms/powernv/opal.c
> index 1946dbdc9fa1..1e9cb5271ee7 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -1125,6 +1125,36 @@ EXPORT_SYMBOL_GPL(opal_flash_read);
> EXPORT_SYMBOL_GPL(opal_flash_write);
> EXPORT_SYMBOL_GPL(opal_flash_erase);
> EXPORT_SYMBOL_GPL(opal_prd_msg);
> +
> +/**
> + * opal_check_token - Check if an OPAL call token is supported
> + * @token: OPAL token number to check
> + *
> + * Returns 1 if supported, 0 if not.
> + */
> +int64_t opal_check_token(uint64_t token)
> +{
> + static u8 token_cache[OPAL_LAST];
not a suggestion to change anything, just an observation:
based on this in skiboot:
```
void __opal_register(uint64_t token, void *func, unsigned int nargs)
{
assert(token <= OPAL_LAST);
...
static int64_t opal_check_token(uint64_t token)
{
if (token > OPAL_LAST)
return OPAL_TOKEN_ABSENT;
```
along with other usages of OPAL_LAST in skiboot, OPAL_LAST is being
treated as a valid token.
there are no handlers for OPAL_LAST and kernel doesn't do
opal_check_token for this
I feel the logic should better be fixed in opal to not consider
OPAL_LAST
at the same time it maybe safer to have length as OPAL_LAST+1 here,
though it is not necessary, so the patch looks good to me
Reviewed-by: Aditya Gupta <[email protected]>
Thanks,
- Aditya G
