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?)

> 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...)

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.

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


> +                     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 <[email protected]>


Thanks,

James
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to