qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] KVM: arm/arm64: Add VGICv3 save/restore API doc


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] KVM: arm/arm64: Add VGICv3 save/restore API documentation
Date: Thu, 8 Oct 2015 14:37:29 +0100

On 8 October 2015 at 14:28, Pavel Fedin <address@hidden> wrote:
>  Hello!
>
>  Since we are discussing qemu here, i removed kernel mailing lists and added 
> qemu-devel.
>
>> A quick look at your patch suggests you still have data
>> structures like
>>
>> +struct gicv3_irq_state {
>> +    /* The enable bits are only banked for per-cpu interrupts.  */
>> +    unsigned long *enabled;
>> +    unsigned long *pending;
>> +    unsigned long *active;
>> +    unsigned long *level;
>> +    unsigned long *group;
>> +    bool edge_trigger; /* true: edge-triggered, false: level-triggered  */
>> +    uint32_t mask_size; /* Size of bitfields in long's, for migration */
>> +};
>>
>> I think it's probably going to be better to have an array
>> of redistributor structures (one per CPU)
>
>  Already done. See struct GICv3CPUState - this is per-CPU data. During 
> realize, i allocate an array of these structures and put the pointer into 
> s->cpu.
>  GICv3CPUState holds both redistributor and CPU interface data, for 
> simplicity, because they come in pairs.
>
>> and then keep
>> the state that in hardware is in each redistributor inside
>> those sub-structures.
>
>  I can do it easily, but here i left the original layout by Shlomo
> Pongratz, because i thought that it will be easier for his SW
> emulation code to handle it.

The general idea is that we should get the data structure layout
right (ie generally in line with how hardware is organized).
Then the emulation code should be straightforward. Yes, it will
need some rework, but that's expected and not something to be avoided.

>  I had even better idea of splitting up gicv3_irq_state, and
> storing only per-CPU IRQs in the GICv3CPUState, and SPIs in
> GICv3State.

Yes, I think this sounds right. The per-CPU IRQ data is in
the redistributor/cpu interface. The not-per-CPU IRQ data
is in the distributor.

> In this case bitfields would be of a fixed size,
> and one bit would represent one interrupt. This would decrease
> memory usage, because we would not have to duplicate SPI bits
> for every CPU. But will this be good for SW emulation?
>  OTOH, i have rechecked current SW emulation code, and i see
> that it uses macros like GIC_TEST_xxx, and cpu mask is always
> (1 << ncpu). So, can we safely replace mask with just CPU number
> in these macros? It would solve the problem. Shlomo, your word?

One of the major problems with the emulation code is that it tried
to keep the old GICv2 emulation datastructures, macros, etc. Don't
try to use it as a guide for how to arrange the data structures.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]