This patch came from a discussion on the KVM call the other day.
It may well be the case there is a better/different implementation - so the
patch is more by way of asking the question.
Re-phrasing your question - I think it boils down to “should HVF (and KVM)
support executing instructions from IO space?”.
In that case, this is a ‘partial’ answer to providing such support for HVF -
partial in that it relies upon a memory region being created “dynamically” for
the IO space that has been accessed as a side-effect of a normal access. In
principle I think this is at least a reasonable approach to dynamically create
memory regions in this way, potentially a cache could be constructed etc… of
course the MR could be subsequently removed too…. I’m less happy about it being
a side-effect of a memory operation, but I don’t see a better path?
Perhaps there is a better way of handling this.
Cheers
Mark.
> On 1 Jun 2023, at 17:34, Antonio Caggiano <quic_acagg...@quicinc.com> wrote:
>
> Hi Peter,
>
> On 01/06/2023 16:58, Peter Maydell wrote:
>> On Thu, 1 Jun 2023 at 15:33, Antonio Caggiano <quic_acagg...@quicinc.com>
>> wrote:
>>>
>>> Instead of aborting immediately, try reading the physical address where
>>> the instruction should be fetched by calling address_space_read. This
>>> would give any memory regions ops callback a chance to allocate and/or
>>> register an RAM/Alias memory region needed for resolving that physical
>>> address. Then, if the memory transaction is OK, retry HVF execution at
>>> the same PC.
>> What are the circumstances where this happens?
>> Do we try to support this on KVM ?
>
> We use qemu as a library and manage memory regions externally, allocating
> them on demand when there is a read or a write (through memory region ops
> callbacks).
>
> When enabling HVF, we hit an instruction abort on the very first instruction
> as there is no memory region alias for it yet in system memory.
>
>>> Signed-off-by: Antonio Caggiano <quic_acagg...@quicinc.com>
>>> Co-authored-by: Mark Burton <quic_mbur...@quicinc.com>
>>> ---
>>> target/arm/hvf/hvf.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
>>> index ad65603445..6e527254b1 100644
>>> --- a/target/arm/hvf/hvf.c
>>> +++ b/target/arm/hvf/hvf.c
>>> @@ -1446,6 +1446,18 @@ int hvf_vcpu_exec(CPUState *cpu)
>>> hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
>>> }
>>> break;
>>> + case EC_INSNABORT: {
>>> + uint32_t sas = (syndrome >> 22) & 3;
>>> + uint32_t len = 1 << sas;
>>> + uint64_t val = 0;
>>> +
>>> + MemTxResult res = address_space_read(
>>> + &address_space_memory, hvf_exit->exception.physical_address,
>>> + MEMTXATTRS_UNSPECIFIED, &val, len);
>>> + assert(res == MEMTX_OK);
>> You can't assert() this, it might not be true, especially if
>> we're here because hvf couldn't read from this address.
>
> The idea is to try reading so that memory region ops can create the Alias MR
> required, in which case it would return MEMTX_OK. In case of error, maybe we
> can just exit and report the error like the default case.
>
>>> + flush_cpu_state(cpu);
>>> + break;
>>> + }
>>> default:
>>> cpu_synchronize_state(cpu);
>>> trace_hvf_exit(syndrome, ec, env->pc);
>> thanks
>> -- PMM
>
> Cheers,
> Antonio
