Re: [PATCH v19 3/7] firmware: arm_rmm: Configure the RMM with the host's page size

[Venkata Rao Kakani <venkata.kakani@oss.qualcomm.com>]

On 25-09-2026 08:26 pm, Suzuki K Poulose wrote:
> On 25/09/2026 14:23, Venkata Rao Kakani wrote:
>> Hi Suzuki,
>>
>
> Hi Venkat,
>
>> Now, with this patch Host can set RMM Granule size equal to 
>> PAGE_SIZE. Does RMM send the request to root to set GPT physical 
>> granule size = PAGE_SIZE?
>
> I don't think FIRME lets you do that and it would be too complex. Please
> remember that the GPT is not just for the Non-secure/Realm world, but
> also for the Secure world.
> So, the Physical Granule size would be statically configured to 4KB on a
> platform unless it supports the 4KB granule size, to support all
> possible Sofware components.
>
>
>>
>> In a scenario where, GPT granule size (|GPCCR_EL3|) set to 64K and 
>> Host PAGE_SIZE set to 4K, how does root handle GPT granule mappings, 
>> when Host send RMI_GRANULE_DELEGATE(PAGE_SIZE)?
>
> If GPCCR_EL3 is set to 64K, RMM must not report it supports 4K GRANULE
> size.
>
> nit: Please avoid top posting your comments and use plain text for
> discussions on the list.
>
> Suzuki
>
>
>
>
>>
>>
>> -- Venkat
>>
>> On 24-09-2026 10:33 pm, Jonathan Cameron wrote:
>>> On Thu, 24 Sep 2026 14:51:57 +0100
>>> Suzuki K Poulose<suzuki.poulose@arm.com> wrote:
>>>
>>>> RMM v2.0 brings the ability to set the RMM's granule size. Check the
>>>> feature registers and configure the RMM so that it matches the host's
>>>> page size. This means that operations can be done with a granularity
>>>> equal to PAGE_SIZE.
>>>>
>>>> Signed-off-by: Steven Price<steven.price@arm.com>
>>>> Signed-off-by: Suzuki K Poulose<suzuki.poulose@arm.com>
>>> Hi Suzuki,
>>>
>>> Some trivial stuff inline.  Assuming that is addressed.
>>>
>>> Reviewed-by: Jonathan Cameron<jonathan.cameron@oss.qualcomm.com>
>>>
>>>> ---
>>>>   drivers/firmware/arm_rmm/rmi.c | 70 
>>>> ++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 70 insertions(+)
>>>>
>>>> diff --git a/drivers/firmware/arm_rmm/rmi.c 
>>>> b/drivers/firmware/arm_rmm/rmi.c
>>>> index 3baba931f92e4..c9ea964fd9081 100644
>>>> --- a/drivers/firmware/arm_rmm/rmi.c
>>>> +++ b/drivers/firmware/arm_rmm/rmi.c
>>>> +
>>>> +static int rmi_configure(void)
>>>> +{
>>>> +    unsigned long granule_feature;
>>>> +    unsigned long granule_size;
>>>> +    int ret = 0;
>>> Value not used.  Fine if it is future churn reduction, but I didn't 
>>> spot
>>> where if so. I'm guessing left over from refactoring
>>>
>>>> +
>>>> +    switch (PAGE_SIZE) {
>>>> +    case SZ_4K:
>>>> +        granule_size = RMI_GRANULE_SIZE_4KB;
>>>> +        granule_feature = RMI_FEATURE_REGISTER_1_RMI_GRAN_SZ_4KB;
>>>> +        break;
>>>> +    case SZ_16K:
>>>> +        granule_size = RMI_GRANULE_SIZE_16KB;
>>>> +        granule_feature = RMI_FEATURE_REGISTER_1_RMI_GRAN_SZ_16KB;
>>>> +        break;
>>>> +    case SZ_64K:
>>>> +        granule_size = RMI_GRANULE_SIZE_64KB;
>>>> +        granule_feature = RMI_FEATURE_REGISTER_1_RMI_GRAN_SZ_64KB;
>>>> +        break;
>>>> +    default:
>>>> +        BUILD_BUG();
>>>> +    }
>>>> +
>>>> +    if (!(rmi_feat_reg(1) & granule_feature)) {
>>>> +        pr_err("RMM does not support %luKB granules\n",
>>>> +               PAGE_SIZE >> 10);
>>>> +        return -ENXIO;
>>>> +    }
>>>> +
>>>> +    struct rmm_config *config __free(free_page) =
>>>> +        (struct rmm_config *)get_zeroed_page(GFP_KERNEL);
>>>> +
>>>> +    if (!config) {
>>>> +        pr_err("Unable to allocate memory for RMM config\n");
>>>> +        return -ENOMEM;
>>>> +    }
>>>> +
>>>> +    config->rmi_granule_size = granule_size;
>>>> +
>>>> +    /*
>>>> +     * For now we set the tracking_region_size to 0 which is the 
>>>> only option
>>>> +     * for 4KB PAGE_SIZE (1GB for 4KB PAGE_SIZE, 32MB/512MB for 
>>>> 16KB/64KB).
>>>> +     * TODO: Support other tracking sizes via Kconfig option for 
>>>> other
>>>> +     * PAGE_SIZES
>>>> +     */
>>>> +    config->tracking_region_size = 0;
>>>> +
>>>> +    ret = rmi_rmm_config_set(virt_to_phys(config));
>>>> +    if (ret) {
>>>> +        pr_err("RMM config set failed (%d)\n", ret);
>>>> +        ret = -EINVAL;
>>>         return -EINVAL;
>>>
>>>> +    }
>>>> +
>>>> +    return ret;
>>>     return 0;
>>>
>>> So obvious this is the good path without anyone having to think 
>>> about it!
>>>
>>> I did quick check on whether this was to reduce churn due to later 
>>> changes
>>> but couldn't immediately spot anything
>>>
>>>
>>>> +}
>
Understood. Thanks Suzuki.