[PATCH v6 04/26] arm64: KVM: Dynamically patch the kernel/hyp VA mask

[Marc Zyngier]

On 15/03/18 19:15, James Morse wrote:
> Hi Marc,
> 
> On 14/03/18 16:50, Marc Zyngier wrote:
>> So far, we're using a complicated sequence of alternatives to
>> patch the kernel/hyp VA mask on non-VHE, and NOP out the
>> masking altogether when on VHE.
>>
>> The newly introduced dynamic patching gives us the opportunity
>> to simplify that code by patching a single instruction with
>> the correct mask (instead of the mind bending cummulative masking
> 
> (Nit: cumulative)
> 
> (so this series removes mind bending code?)

Absolutely. And replaces it with... erm... (/me shuts up).

> 
>> we have at the moment) or even a single NOP on VHE. This also
>> adds some initial code that will allow the patching callback
>> to switch to a more complex patching.
> 
>> diff --git a/arch/arm64/kvm/va_layout.c b/arch/arm64/kvm/va_layout.c
>> new file mode 100644
>> index 000000000000..45e7802328d4
>> --- /dev/null
>> +++ b/arch/arm64/kvm/va_layout.c
> 
>> +void __init kvm_update_va_mask(struct alt_instr *alt,
>> +			       __le32 *origptr, __le32 *updptr, int nr_inst)
>> +{
>> +	int i;
>> +
>> +	/* We only expect a single instruction in the alternative sequence */
>> +	BUG_ON(nr_inst != 1);
>> +
>> +	if (!has_vhe() && !va_mask)
>> +		compute_layout();
>> +
>> +	for (i = 0; i < nr_inst; i++) {
>> +		u32 rd, rn, insn, oinsn;
>> +
>> +		/*
>> +		 * VHE doesn't need any address translation, let's NOP
>> +		 * everything.
>> +		 */
>> +		if (has_vhe()) {
>> +			updptr[i] = aarch64_insn_gen_nop();
> 
> cpu_to_le32()? (I'm not going to try an boot a BE VHE model...)

You're missing out. Let's make BE big again.

> 
> aarch64_insn_gen_nop() returns:
> | aarch64_insn_get_hint_value() | AARCH64_INSN_HINT_NOP;
> 
> It doesn't look like these aarch64_insn_get_XXX_value() helpers are forcing a
> particular endianness. ftrace uses this, via ftrace_modify_code() ->
> aarch64_insn_patch_text_nosync() -> aarch64_insn_write(), which does:
> | return  __aarch64_insn_write(addr, cpu_to_le32(insn));
> 
> So it looks like the conversion is required. Patch 16 looks fine for this.

Absolutely correct, I'm missing the byte swap. Now fixed.

> (and, I ran the teardown code on Juno big-endian...)

Wow. You *the* user!!!! ;-)

> 
> 
>> +			continue;
>> +		}
>> +
>> +		oinsn = le32_to_cpu(origptr[i]);
>> +		rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
>> +		rn = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, oinsn);
>> +
>> +		insn = compute_instruction(i, rd, rn);
>> +		BUG_ON(insn == AARCH64_BREAK_FAULT);
>> +
>> +		updptr[i] = cpu_to_le32(insn);
>> +	}
>> +}
> 
> With that,
> 
> Reviewed-by: James Morse <james.morse at arm.com>

Thanks a lot,

	M.
-- 
Jazz is not dead. It just smells funny...