On 2/13/20 4:39 PM, Richard Henderson wrote:
On 2/12/20 7:49 PM, Gavin Shan wrote:
On 2/12/20 10:34 PM, Peter Maydell wrote:
Yeah, this is on my list to look at; Richard Henderson also could
have a look at it. From a quick scan I suspect you may be missing
handling for AArch32.


[Thanks for copying Richard Henderson]

Yes, the functionality is only supported on aarch64 currently by intention
because the next patch enables it on "max" and "host" CPU models and both
of them are running in aarch64 mode.

We shouldn't leave the aarch32 exception entry paths unimplemented though.  C.f.

AArch32.TakePhysicalSErrorException()
AArch32.TakeVirtualSErrorException()

It really shouldn't be more than a couple of lines, just like
arm_cpu_do_interrupt_aarch64.  Remember both arm_cpu_do_interrupt_aarch32 and
arm_cpu_do_interrupt_aarch32_hyp.


Thanks for the details. The SError injection for aarch32 will be included in v3.

However, it seems there is a long list of aarch32 CPU models, defined
in target/arm/cpu.c::arm_cpus. so which CPU models you prefer to see with
this supported? I think we might choose one or two popular CPU models if
you agree.

Even qemu-system-aarch64 -cpu max can exercise this path when EL1 is running in
aarch32 mode.  Admittedly it would be easier if we had the rest of the plumbing
so that -cpu max,aarch64=off worked.

FWIW, the rest of the patch looks good.


I think "-cpu max,aarch64=off" is only valid when KVM is enabled? If that's 
true,
the ioctl(cpu, KVM_SET_VCPU_EVENTS, &events) already worked for aarch32 or 
aarch64
guest if I'm correct enough. But yes, I need to test it because I never tested 
this
series on aarch32 guest :)

Thanks,
Gavin



Reply via email to