Re: [Qemu-devel] [RFC PATCH v3 3/4] hw/intc/arm_gicv3_kvm: Implement get/put functions

[Peter Maydell]

On 26 October 2015 at 07:59, Pavel Fedin <address@hidden> wrote:
>  Hello!
>
>> > +            reg = c->pendbaser & (GICR_PENDBASER_OUTER_CACHEABILITY_MASK |
>> > +                                  GICR_PENDBASER_ADDR_MASK |
>> > +                                  GICR_PENDBASER_SHAREABILITY_MASK |
>> > +                                  GICR_PENDBASER_CACHEABILITY_MASK);
>> > +            if (!c->redist_ctlr & GICR_CTLR_ENABLE_LPIS) {
>> > +                reg |= GICR_PENDBASER_PTZ;
>> > +            }
>>
>> Why does the state of the pendbaser register depend on state in the
>> redist_ctlr ?
>
>  PTZ bit is write-only, we cannot read it back. And spec says that setting 
> PTZ is adviced while LPIs are not enabled, because it shortens down the time 
> of GIC initialization. So, i had to implement this small heuristics here. Is 
> this approach OK?

OK, with a comment to say that's what we're doing. (I assume that
when we support LPIs we'll then set PTZ appropriately, so this
code will change later.)

>> Worth a comment, whatever the answer is.
>
>  I will.
>
>> > +            kvm_gicr_access(s, GICR_PENDBASER, ncpu, &reg, false);
>> > +            c->pendbaser = reg & (GICR_PENDBASER_OUTER_CACHEABILITY_MASK |
>> > +                                  GICR_PENDBASER_ADDR_MASK |
>> > +                                  GICR_PENDBASER_SHAREABILITY_MASK |
>> > +                                  GICR_PENDBASER_CACHEABILITY_MASK);
>>
>> Why do we need to mask these values?
>
>  I decided to do this at least for the case of KVM->TCG migration (as far as 
> i understand, such things are possible). In this case i think we should not 
> pollute our state with read-only bits, which get added by the emulation code 
> itself.

We don't do this for other system registers which might contain
RO bits, so I think for consistency we shouldn't mask bits out
here either.

(Transferring RO bits in migration state gives the destination
an opportunity in theory to reject a migration which is for
a config it can't handle. And reserved bits may end up having a
use in a future GIC version, so it's nice not to have to do a
QEMU update just to remove them from the mask.)

thanks
-- PMM