On 26 October 2015 at 07:59, Pavel Fedin <p.fe...@samsung.com> 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, ®, 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
