Re: [Qemu-devel] [PATCH v3 3/4] target-arm: Add 32/64-bit register sync

[Peter Maydell]

On 4 February 2015 at 23:37, Greg Bellows <address@hidden> wrote:
> Peter Maydell wrote:
>> This doesn't look correct. If this CPU doesn't have the AArch64
>> feature then pretty much all of what this function does is wrong,
>> because it is the "take exception to an EL that is using AArch64"
>> function.

> I think this is deeper than the one routine.  Essentially, we have to
> convert the AArch64 CPU object into an ARM CPU object on the fly. This would
> have to happen after class instantiation because we don't have the
> properties until after.  From looking at the code, I'm not convinced it is
> safe to override the fields, but I need to look a bit more.

I think we probably want the other approach -- leave the object being
the class it is, but the functions have to cope with the CPU possibly
being in 32 bit mode. We need that anyway for interworking between
64 bit EL3 and 32 bit EL1.

> I wanted to understand more, so I ran without my changes to see if aarch64
> ever gets cleared and sure enough it does.  Is this expected?

I'm not sure what you mean here.

> I wasn't convinced this should occur, so I investigated further. The only
> place I see us explicitly set it to 0 is in exception return when spsr has
> nRW set. So I set a watch point on the spsr, and sure enough I see an update
> occur setting nRW which means aarch64 gets set to 0.

This sounds like "exception return to a 32-bit EL0", which obviously
clears the aarch64 flag.

> I realize it is possible for the PSR to get set but are we prepared to
> handle it?

To handle what? At the moment the code assumes that if we're a 64
bit CPU then we can only be in 32 bit mode for an AArch32 EL0.
This obviously needs to be fixed to support both 32-bit EL1
under 64-bit EL3 and also in this case to support a 64-bit CPU
that only has 32-bit EL1. My original point is that the change
to aarch64_cpu_do_interrupt() as you have it in this patch can't
possibly work, because it is not actually doing what you need
to do to handle taking exceptions to a 32 bit EL.

-- PMM