Re: [Qemu-devel] [PATCH v12 4/4] arm: SoC model for Calxeda Highbank

[Peter Maydell]

On 20 January 2012 19:25, Mark Langsdorf <address@hidden> wrote:
> It looks like there's an issue with commit 2be276242135eac6,
> in that target-arm/helper.c:cpu_reset() is called after
> hw/highbank.c:highbank_cpu_reset() and keeps clobbering
> our c15_config_base_address.

You may recall that when you first sent the patch with
highbank_cpu_reset I said it looked like the resets would be
in the wrong order, and you assured me they weren't :-)
(Commit 2be276242 is the SCU register change, incidentally,
and doesn't seem relevant; did you mean some other commit?)

> Is there a way to prioritize the callbacks or something
> so that arm's cpu_reset() doesn't clobber highbank's
> cpu_reset()? Alternately, is there some other way to
> set c15 values outside of cpu_reset()?

Reset callbacks aren't supposed to do things such that they
care about what order they're called in. This one does because
we're uncleanly reaching into the CPUState from the highbank
reset callback rather than doing CPU reset in the CPU proper.
I was willing to let that pass since it happened to work
OK and we didn't have a clean mechanism to hand, but if it
doesn't work anyhow I guess we need to rethink.

I can't think of anything nicer (since we don't have a proper
qdev object to set properties on) than adding code to cpu_reset()
which saves the value of env->cp15_config_base_address across
the memset(). That's quite ugly but will work. [And in fact
we'd need that even with a hypothetical qdev property because
the property would only set the state field once at init, and
we'd need to avoid the memset() on reset trashing it. There's
an argument that it's the block memset() that's ugly...]

OTOH it's 2.30am here so I may be missing a nicer approach.
Suggestions welcome.

-- PMM