On 11/17/2015 12:58 PM, Peter Maydell wrote:
> On 9 November 2015 at 01:11, Michael Davidsaver <mdavidsa...@gmail.com> wrote:
>> Signed-off-by: Michael Davidsaver <mdavidsa...@gmail.com>
>> ---
>> hw/intc/armv7m_nvic.c | 64
>> +++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
>> index 3b10dee..c860b36 100644
>> --- a/hw/intc/armv7m_nvic.c
>> +++ b/hw/intc/armv7m_nvic.c
>> @@ -810,15 +810,75 @@ static const MemoryRegionOps nvic_sysreg_ops = {
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> +static
>> +int nvic_post_load(void *opaque, int version_id)
>> +{
>> + nvic_state *s = opaque;
>> + unsigned i;
>> +
>> + /* evil hack to get ARMCPU* ahead of time */
>> + assert(cpus.tqh_first);
>> + assert(!CPU_NEXT(cpus.tqh_first));
>> + s->cpu = ARM_CPU(cpus.tqh_first);
>> + assert(s->cpu);
>
> Why do we need to do this? By the time we get to loading
> state into the system, s->cpu should already have been set.
Just so, removed.
>> +
>> + /* recalculate priorities */
>> + for (i = 4; i < s->num_irq; i++) {
>> + set_prio(s, i, s->vectors[i].raw_prio);
>> + }
>> +
>> + nvic_irq_update(s, highest_runnable_prio(s->cpu));
>> +
>> + return 0;
>> +}
>> +
>> +static
>> +int vec_info_get(QEMUFile *f, void *pv, size_t size)
>> +{
>> + vec_info *I = pv;
>> + I->prio_sub = qemu_get_be16(f);
>> + I->prio_group = qemu_get_byte(f);
>> + I->raw_prio = qemu_get_ubyte(f);
>> + I->enabled = qemu_get_ubyte(f);
>> + I->pending = qemu_get_ubyte(f);
>> + I->active = qemu_get_ubyte(f);
>> + I->level = qemu_get_ubyte(f);
>> + return 0;
>> +}
>> +
>> +static
>> +void vec_info_put(QEMUFile *f, void *pv, size_t size)
>> +{
>> + vec_info *I = pv;
>> + qemu_put_be16(f, I->prio_sub);
>> + qemu_put_byte(f, I->prio_group);
>> + qemu_put_ubyte(f, I->raw_prio);
>> + qemu_put_ubyte(f, I->enabled);
>> + qemu_put_ubyte(f, I->pending);
>> + qemu_put_ubyte(f, I->active);
>> + qemu_put_ubyte(f, I->level);
>> +}
>> +
>> +static const VMStateInfo vmstate_info = {
>> + .name = "armv7m_nvic_info",
>> + .get = vec_info_get,
>> + .put = vec_info_put,
>> +};
>
> I don't think there's any need to use get and put functions here:
> better to define a VMStateDescripton for the struct vec_info,
> and then you can just refer to that from the main vmstate_nvic
> description. (hw/audio/pl041.c has some examples of this.)
Fixed
>> +
>> static const VMStateDescription vmstate_nvic = {
>> .name = "armv7m_nvic",
>> - .version_id = 1,
>> - .minimum_version_id = 1,
>> + .version_id = 2,
>> + .minimum_version_id = 2,
>> + .post_load = &nvic_post_load,
>> .fields = (VMStateField[]) {
>> + VMSTATE_ARRAY(vectors, nvic_state, NVIC_MAX_VECTORS, 0,
>> + vmstate_info, vec_info),
>> + VMSTATE_UINT8(prigroup, nvic_state),
>> VMSTATE_UINT32(systick.control, nvic_state),
>> VMSTATE_UINT32(systick.reload, nvic_state),
>> VMSTATE_INT64(systick.tick, nvic_state),
>> VMSTATE_TIMER_PTR(systick.timer, nvic_state),
>> + VMSTATE_UINT32(num_irq, nvic_state),
>> VMSTATE_END_OF_LIST()
>> }
>> };
>
> Ideally the VMState should be added in the same patches that
> add new fields or structures, rather than in its own patch
> at the end.
I'll try to do this.
