Re: [RFC 5/8] powerpc/slb: Add documentation to runtime patching of SLB encoding

On Wed, 2015-07-22 at 07:57 +0200, Gabriel Paubert wrote:

On Wed, Jul 22, 2015 at 03:51:03PM +1000, Michael Ellerman wrote:

quoted

On Tue, 2015-07-21 at 12:28 +0530, Anshuman Khandual wrote:

quoted

From: "khandual@linux.vnet.ibm.com" <redacted>

This patch adds some documentation to 'patch_slb_encoding' function
explaining about how it clears the existing immediate value in the
given instruction and inserts a new one there.

diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index dcba4c2..8083a9e 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c

@@ -278,7 +278,13 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
 static inline void patch_slb_encoding(unsigned int *insn_addr,
 				      unsigned int immed)
 {
-	int insn = (*insn_addr & 0xffff0000) | immed;
+	/*
+	 * Currently this patches only "li" and "cmpldi"
+	 * instructions with an immediate value. Here it
+	 * just clears the existing immediate value from
+	 * the instruction and inserts a new one there.
+	 */
+	unsigned int insn = (*insn_addr & 0xffff0000) | immed;
 	patch_instruction(insn_addr, insn);
 }

How about:

	/*
	 * This function patches either an li or a cmpldi instruction with
	 * a new immediate value. This relies on the fact that both li
	 * (which is actually ori) and cmpldi both take a 16-bit immediate

Hmm, li is actually encoded as addi with r0 as source register...

Correct.

quoted

	 * value, and it is situated in the same location in the instruction,
	 * ie. bits 0-15.

In PPC documentation, it's rather bits 16-31 (big endian bit order).
Or say lower half which is endian agnostic.

Yeah, but who reads the PPC documentation ;)

In the kernel we almost always use the sane bit numbering, so I'd use that, but
maybe "low 16-bits" will avoid confusion.

cheers