Re: [PATCH] hvf: arm: Ignore cache operations on MMIO

[Richard Henderson]

On 10/25/21 12:13 PM, Alexander Graf wrote:
> +    /*
> +     * We ran into an instruction that traps for data, but is not
> +     * hardware predecoded. This should not ever happen for well
> +     * behaved guests. Let's try to see if we can somehow rescue
> +     * the situation.
> +     */
> +
> +    cpu_synchronize_state(cpu);
> +    if (cpu_memory_rw_debug(cpu, env->pc, &insn, 4, 0)) {

This isn't correct, since this would be a physical address access, and env->pc 
is virtual.

Phil's idea of cpu_ldl_data may be correct, and cpu_ldl_code may be slightly more so, 
because we got EC_DATAABORT not EC_INSNABORT, which means that the virtual address at 
env->pc is mapped and executable.

However, in the event that there's some sort of race condition in between this data abort 
and hvf stopping all threads for the vm exit, by which the page tables could have been 
modified between here and there, then cpu_ldl_code *could* produce another exception.

In which case the interface that gdbstub uses, cc->memory_rw_debug, will be 
most correct.


> @@ -1156,6 +1183,11 @@ int hvf_vcpu_exec(CPUState *cpu)
>                                hvf_exit->exception.physical_address, isv,
>                                iswrite, s1ptw, len, srt);
>   
> +        if (!isv) {
> +            g_assert(hvf_emulate_insn(cpu));
> +            advance_pc = true;
> +            break;
> +        }
>           assert(isv);

Ouch.  HVF really passes along an invalid syndrome?  I was expecting that you'd be able to 
avoid all of the instruction parsing and check syndrome.cm (bit 8) for a cache management 
instruction.


r~