Re: [PATCH v22 06/10] firmware: arm_rmm: Ensure the RMM has GPT entries for memory

[Kohei Enju <enju.kohei@fujitsu.com>]

Hi Soby, Suzuki,

On 10/22 13:07, Suzuki K Poulose wrote:
> On 21/10/2026 15:00, Kohei Enju wrote:
> > On 10/21 14:35, Suzuki K Poulose wrote:
> > > Hi
> > > 
> > > On 21/10/2026 14:07, Kohei Enju wrote:
> > > > Hi Suzuki,
> > > > 
> > > > I've tested this series and found a possible TF-RMM issue.
> > > > Please  below.
> > > > 
> > > > On 10/02 07:15, Suzuki K Poulose wrote:
> > > > > From: Steven Price <steven.price@arm.com>
> > > > > 
> > > > > The RMM maintains the state of all the granules in the system to make
> > > > > sure that the host is abiding by the rules. This state can be maintained
> > > > > at different granularity, per page (TRACKING_FINE) or per region
> > > > > (TRACKING_COARSE or TRACKING_INTERMEDIATE). The region size depends on the
> > > > > underlying "RMI_GRANULE_SIZE". For a "coarse"/"intermediate" region,
> > > > > all pages in the region must be of the same state, this implies we need to
> > > > > have "fine" tracking for DRAM, so that we can delegate individual pages.
> > > > > 
> > > > > For now we only support a statically carved out memory for tracking
> > > > > granules for the "fine" regions. This can be extended in the future to
> > > > > allow modifying the tracking granularity and remove the need for a
> > > > > static allocation by the firmware.
> > > > > 
> > > > > Similarly, the firmware may create L0 GPT entries describing the total
> > > > > address space. But if we change the "PAS" (Physical Address Space) of a
> > > > > granule, then the firmware may need to create L1 tables to track the PAS
> > > > > at a finer granularity. Linux therefore checks if the platform firmware
> > > > > manages the PAR region. i.e., the firmware is in charge of managing the
> > > > > L1 GPTs (creation and the required memory for the GPT tables - via static
> > > > > carveouts) without host intervention. Support for dynamic GPT creation by
> > > > > the host will be added later.
> > > > > 
> > > > > If the firmware requires us to manage the tracking or GPT memory,
> > > > > deactivate the RMM and reclaim any memory donated at RMM activation.
> > > > > 
> > > > > Apply the same checks when hotplugged memory is brought online.
> > > > > 
> > > > > Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> > > > > Reviewed-by: Gavin Shan <gshan@redhat.com>
> > > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > Tested-by: Gavin Shan <gshan@redhat.com>
> > > > > Signed-off-by: Steven Price <steven.price@arm.com>
> > > > > Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > > ---
> > > > [...]
> > > > > +/*
> > > > > + * We do not support creating L1 GPTs yet. So, make sure that all the regions
> > > > > + * are managed by the firmware.
> > > > > + */
> > > > > +static int rmi_verify_gpt_firmware_managed(phys_addr_t start, phys_addr_t end)
> > > > > +{
> > > > > +	unsigned long l0gpt_sz;
> > > > > +	unsigned long next, par_state;
> > > > > +
> > > > > +	l0gpt_sz = 1UL << (30 + FIELD_GET(RMI_FEATURE_REGISTER_1_L0GPTSZ,
> > > > > +					  rmi_feat_reg(1)));
> > > > > +	start = ALIGN_DOWN(start, l0gpt_sz);
> > > > > +	end = ALIGN(end, l0gpt_sz);
> > > > > +
> > > > > +	while (start < end) {
> > > > > +		long ret = rmi_gpt_info(start, end, &next, &par_state);
> > > > > +
> > > > > +		if (ret != RMI_SUCCESS) {
> > > > > +			pr_err("RMI_GPT_INFO failed for %llx-%llx: (%ld)\n",
> > > > > +				start, end, ret);
> > > > > +			return -ENXIO;
> > > > > +		}
> > > > > +
> > > > > +		if (WARN_ON(next <= start))
> > > > > +			return -ENXIO;
> > > > > +
> > > > > +		if (par_state != RMI_GPT_PAR_PLAT) {
> > > > > +			pr_err("GPT for the region is not managed by firmware %llx-%lx\n",
> > > > > +				start, next);
> > > > > +			return -ENODEV;
> > > > 
> > > > When system RAM starts at a non-L0GPTSZ-aligned address, the alignment
> > > > in rmi_verify_gpt_firmware_managed() may cause the queried L0GPT range
> > > > to include a memory hole.
> > > 
> > > > 
> > > > On out platform, the relevant memory layout is:
> > > > 
> > > >       800c0000000-800dc3fffff : memory hole
> > > >       800dc400000-82fffffffff : System RAM
> > > > 
> > > 
> > > Thanks for the report.
> > > 
> > > > Consequently, Linux queries the following L0GPT range:
> > > > 
> > > >       800c0000000-80100000000
> > > > 
> > > > The query fails with RMI_GPT_PAR_RESERVED:
> > > > 
> > > >       [    8.551855] GPT for the region is not managed by firmware 800c0000000-80100000000
> > > > 
> > > > and arm64_init_rmi() subsequently fails.
> > > > 
> > > > As I understand it, smc_gpt_info() in TF-RMM checks only the beginning
> > > > of the aligned L0GPT range. Since that address is in the hole, the whole
> > > > range is reported as RMI_GPT_PAR_RESERVED, even though it contains
> > > > platform-managed System RAM.
> > > > 
> > > > I believe this should be addressed in TF-RMM. Do you have any thoughts
> > > > on the expected RMI_GPT_INFO behavior in this case?
> > > 
> > > I believe the RMM should report a value that represents the situation
> > > better. In this case, there is a hole, and some conventional memory
> > > areas. For the conventional memory, the RMM has to manage the GPTs.
> > > (This applies to the tf-RMM implementation of TRACKING Granule state
> > > too. It only looks at the first granule and reports the type of memory.)
> > > 
> > > RMI_GPT_PAR_RESERVED is not accurate as the memory region is not
> > > reserved, unless it is reserved for firmware (in which case the
> > > memory description is wrong).
> > > 
> > > I would say RMI_GPT_PAR_PLAT  is accurate if the L1GPT exists, as the
> > >   region covered by the L0GPT has some memory that are delegable.
> > > 
> > > Otherwise it must be reported as RMI_GPT_HOST_NOT_CREATED.
> > > 
> > > The information that a given memory region is not "memory" (i.e. hole)
> > > is basically TRACKING_STATE returned as RMI_MEM_CATEGORY_NONE
> > 
> > Thanks for the clarification. Could you please raise this with the TF-RMM
> > team? Please let me know if I can help with testing or provide any
> > additional information.
> 
> [Duplicating Soby's response, which may have been filtered due to the HTML
> email]

Thanks for relaying Soby's response :)

> 
> --->8---
> 
> Thanks Kohei for reporting this issue. I have pushed a fix for this in
> TF-RMM which we hope to get merged soon.
> 
> https://review.trustedfirmware.org/c/TF-RMM/tf-rmm/+/55689/1

Thank you for the quick fix! I'll check the patch and give it a try on our
platform.

Thanks,
Kohei

> 
> Best Regards
> Soby Mathew