[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v3 3/4] hw/intc/arm_gicv3_kvm: Implement get
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [RFC PATCH v3 3/4] hw/intc/arm_gicv3_kvm: Implement get/put functions |
Date: |
Mon, 26 Oct 2015 11:09:06 +0000 |
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, ®, 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
- [Qemu-devel] [RFC PATCH v3 1/4] hw/intc/arm_gicv3_common: Add state information, (continued)
[Qemu-devel] [RFC PATCH v3 2/4] kernel: Add definitions for GICv3 attributes, Pavel Fedin, 2015/10/22
[Qemu-devel] [RFC PATCH v3 3/4] hw/intc/arm_gicv3_kvm: Implement get/put functions, Pavel Fedin, 2015/10/22
[Qemu-devel] [RFC PATCH v3 4/4] hw/intc/arm_gicv3_common: Add vmstate descriptors, Pavel Fedin, 2015/10/22