Thread (59 messages) 59 messages, 8 authors, 8d ago

Re: [PATCH v19 4/7] firmware: arm_rmm: Add support for SRO

flat view

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2026-09-25 11:50:44
Also in: kvm, kvmarm, linux-coco, lkml

On Thu, Sep 24, 2026 at 02:51:58PM +0100, Suzuki K Poulose wrote:
quoted hunk ↗ jump to hunk
+static int rmi_sro_donate_contig(struct rmi_sro_state *sro,
+				 unsigned long sro_handle,
+				 unsigned long donatereq,
+				 struct arm_smccc_1_2_regs *out_regs,
+				 gfp_t gfp)
+{
+	unsigned long block_size_fld = RMI_DONATE_BLOCK_SIZE(donatereq);
+	unsigned long block_size = rmi_addr_block_size_to_bytes(block_size_fld);
+	unsigned long count = RMI_DONATE_COUNT(donatereq);
+	unsigned long state = RMI_DONATE_STATE(donatereq);
+	unsigned long size = block_size * count;
+	unsigned long addr_range;
+	unsigned long donated_granules;
+	unsigned long donated_size;
+	int ret;
+	void *virt;
+	phys_addr_t phys;
+
+	/*
+	 * The RMM specification requires contiguous allocations are always a
+	 * power of 2
+	 */
+	if (WARN_ON_ONCE(!is_power_of_2(size)))
+		return -EINVAL;
+
+	/* Reuse the cached address range if we have one */
+	for (int i = 0; i < sro->addr_count; i++) {
+		unsigned long entry = sro->addr_list[i];
+
+		if (RMI_ADDR_RANGE_BLOCK_SIZE(entry) == block_size_fld &&
+		    RMI_ADDR_RANGE_COUNT(entry) == count &&
+		    RMI_ADDR_RANGE_STATE(entry) == state &&
+		    IS_ALIGNED(RMI_ADDR_RANGE_ADDR(entry), size)) {
+			sro->addr_count--;
+			swap(sro->addr_list[sro->addr_count],
+			     sro->addr_list[i]);
+
+			goto mem_donate;
+		}
+	}
+
+	ret = rmi_sro_ensure_capacity(sro, 1);
+	if (ret)
+		return ret;
+
+	virt = alloc_pages_exact(size, gfp);
+	if (!virt)
+		return -ENOMEM;
+	phys = virt_to_phys(virt);
+
+	if (state == RMI_OP_MEM_DELEGATED) {
+		phys_addr_t delegated_phys;
+
+		if (rmi_delegate_range(phys, size, &delegated_phys)) {
+			if (!rmi_undelegate_range(phys, delegated_phys - phys))
+				free_pages_exact(virt, size);
+			return -ENXIO;
+		}
+	}
+
+	addr_range = phys & RMI_ADDR_RANGE_ADDR_MASK;
+	FIELD_MODIFY(RMI_ADDR_RANGE_BLOCK_SIZE_MASK, &addr_range, block_size_fld);
+	FIELD_MODIFY(RMI_ADDR_RANGE_COUNT_MASK, &addr_range, count);
Courtesy of an LLM - if we get a contiguous/4K pages request for 4MB
(1024 pages), the above 10-bit field becomes 0. Should we reject the
request or the updated spec will guarantee this won't happen?

Somewhat related, with RMI_BLOCK_L2 or higher and appropriate count we
can easily go over the MAX_PAGE_ORDER allocation and fail alloc_pages()
(the contig case). IIUC, the kernel can reject the donation but current
TF-RMM does not report RMI_OP_CAN_CANCEL (R_SZVNK says it can be
cancelled). It gets complicated if we want to support large contiguous
allocations here (e.g. alloc_contig_pages() can sleep). I'd rather just
cancel the request, not support such large sizes.

[...]
quoted hunk ↗ jump to hunk
+static int rmi_sro_donate_noncontig(struct rmi_sro_state *sro,
+				    unsigned long sro_handle,
+				    unsigned long donatereq,
+				    struct arm_smccc_1_2_regs *out_regs,
+				    gfp_t gfp)
+{
+	unsigned long block_size_fld = RMI_DONATE_BLOCK_SIZE(donatereq);
+	unsigned long block_size = rmi_addr_block_size_to_bytes(block_size_fld);
+	unsigned long count = RMI_DONATE_COUNT(donatereq);
+	unsigned long state = RMI_DONATE_STATE(donatereq);
+	unsigned long found = 0;
+	unsigned long donated_granules;
+	unsigned long granules_per_block = block_size >> PAGE_SHIFT;
+	unsigned long consumed_blocks;
+	int addr_list_start = sro->addr_count;
+	int ret, i;
[...]
+	for (int i = 0, src = addr_list_start + consumed_blocks;
Nit: we have 'int ret, i' earlier already, so you can drop the 'int'
here.

[...]
quoted hunk ↗ jump to hunk
+/*
+ * rmi_sro_execute: Execute an RMI command that is Stateful but not memory
+ * tranfserring. Takes regs, filled with the FIDs and the arguments in place.
+ *
+ * Returns :
+ *  -ECANCELLED - If the operation had to be aborted and SRO was cancellable.
Nit: -ECANCELED.
quoted hunk ↗ jump to hunk
+ *  Otherwise, returns the result of the RMI command.
+ */
+long rmi_sro_execute(struct arm_smccc_1_2_regs *regs)
+{
+	bool cancelled = false;
+	unsigned long sro_handle = regs->a1;
+
+	rmi_smccc_invoke(regs);
+
+	sro_handle = regs->a1;
Nit: drop the first initialisation maybe?

-- 
Catalin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help