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

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

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.

Thanks,
Kohei

> 
> 
> > 
> > As a side note, we tested this series and the v23 integration branch
> > with a Linux-side workaround to avoid the problem above, and we have not
> > observed any other major issues. Sorry for the delay in testing this
> > series.
> 
> Thanks for the testing !
> 
> Suzuki
> > 
> > Thanks,
> > Kohei
> > 
> > > +		}
> > > +		start = next;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
>