[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip th
From: |
Shannon Zhao |
Subject: |
Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR |
Date: |
Thu, 24 May 2018 17:20:59 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 2018/5/24 17:04, Auger Eric wrote:
> Hi Shannon,
>
> On 05/23/2018 05:53 AM, Shannon Zhao wrote:
>> While we skip the GIC_INTERNAL irqs, we don't change the register offset
>> accordingly. This will overlap the GICR registers value and leave the
>> last GIC_INTERNAL irq's registers out of update.
>>
>> Fix this by skipping the registers banked by GICR.
>>
>> Also for migration compatibility if the migration source (old version
>> qemu) doesn't send gicd_no_shift_bug = 1 to destination, then we shift
>> the data of PPI to get the right data for SPI.
>>
>> Fixes: 367b9f527becdd20ddf116e17a3c0c2bbc486920
>> Cc: address@hidden
>> Signed-off-by: Shannon Zhao <address@hidden>
>> ---
>> Changes in V3: add migration compatibility and fix code style
>> ---
>> hw/intc/arm_gicv3_common.c | 36 ++++++++++++++++++++++++
>> hw/intc/arm_gicv3_kvm.c | 56
>> +++++++++++++++++++++++++++++++++++++-
>> include/hw/intc/arm_gicv3_common.h | 1 +
>> 3 files changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 7b54d52..f93e5d2 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -141,6 +141,38 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>> }
>> };
>>
>> +static int gicv3_gicd_no_shift_bug_pre_load(void *opaque)
>> +{
>> + GICv3State *cs = opaque;
>> +
>> + /*
>> + * If the gicd_no_shift_bug subsection is not transferred this
>> + * means gicd_no_shift_bug is 0x0 (which might not be the same as
>> + * our reset value).
>> + */
>> + cs->gicd_no_shift_bug = 0x0;
>> + return 0;
>> +}
>> +
>> +static bool gicv3_gicd_no_shift_bug_needed(void *opaque)
>> +{
>> + GICv3State *cs = opaque;
>> +
>> + return cs->gicd_no_shift_bug;
>> +}
>> +
>> +const VMStateDescription vmstate_gicv3_gicd_no_shift_bug = {
>> + .name = "arm_gicv3/gicd_no_shift_bug",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .pre_load = gicv3_gicd_no_shift_bug_pre_load,
>> + .needed = gicv3_gicd_no_shift_bug_needed,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_BOOL(gicd_no_shift_bug, GICv3State),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> static const VMStateDescription vmstate_gicv3 = {
>> .name = "arm_gicv3",
>> .version_id = 1,
>> @@ -165,6 +197,10 @@ static const VMStateDescription vmstate_gicv3 = {
>> VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, GICv3State, num_cpu,
>> vmstate_gicv3_cpu,
>> GICv3CPUState),
>> VMSTATE_END_OF_LIST()
>> + },
>> + .subsections = (const VMStateDescription * []) {
>> + &vmstate_gicv3_gicd_no_shift_bug,
>> + NULL
>> }
>> };
>>
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index 3536795..bd961f1 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -136,6 +136,12 @@ static void kvm_dist_get_priority(GICv3State *s,
>> uint32_t offset, uint8_t *bmp)
>> int irq;
>>
>> field = (uint32_t *)bmp;
>> + /* For the KVM GICv3, affinity routing is always enabled, and the first
>> 8
>> + * GICD_IPRIORITYR<n> registers are always RAZ/WI. The corresponding
>> + * functionality is replaced by GICR_IPRIORITYR<n>. So it doesn't need
>> to
>> + * sync them.
>> + */
>> + offset += (8 * sizeof(uint32_t));
> Now I am unclear about the semantics of the s->gicd_ipriority & friends.
> With that change, is it supposed to contain only the states of SPIs or
> contain the RAZ states of PPI/SGIs + states of SPIs. The array is
> dimensionned to contain states for PPI/SGI+SPIs, right? In other words,
> shouldn't we also shift field?
>
Yes, it should also shift the field. But not sure if this will affect
the new to old migration scenario. Let me think about this.
> Thanks
>
> Eric
>> for_each_dist_irq_reg(irq, s->num_irq, 8) {
>> kvm_gicd_access(s, offset, ®, false);
>> *field = reg;
>> @@ -149,7 +155,18 @@ static void kvm_dist_put_priority(GICv3State *s,
>> uint32_t offset, uint8_t *bmp)
>> uint32_t reg, *field;
>> int irq;
>>
>> - field = (uint32_t *)bmp;
>> + if (!s->gicd_no_shift_bug) {
>> + field = (uint32_t *)(bmp + 8 * sizeof(uint32_t));
>> + } else {
>> + field = (uint32_t *)bmp;
>> + }
>> +
>> + /* For the KVM GICv3, affinity routing is always enabled, and the first
>> 8
>> + * GICD_IPRIORITYR<n> registers are always RAZ/WI. The corresponding
>> + * functionality is replaced by GICR_IPRIORITYR<n>. So it doesn't need
>> to
>> + * sync them.
>> + */
>> + offset += (8 * sizeof(uint32_t));
> in the old to new migration scenario, IIUC,
> bmp contains state of 32 internal PPI/SGI(RAZ) + s->num_irq-32 SPIs
> Here aren't you "putting" the RAZ state of the 32 PPI/SGIs into the
> first 32 SPIs here?
>
Please see above code, this new version patch I add a flag for migration
compat suggested as Peter. If it migrates from old qemu, the
gicd_no_shift_bug will be 0, then it will shift the bmp, while that will
drop the data of PPI/SGI and also the last 32 SPIs info will be empty.
None of current machines use last 32 SPIs, so it's fine for the moment.
Thanks,
--
Shannon
- [Qemu-devel] [PATCH V3 1/2] arm_gicv3_kvm: increase clroffset accordingly, Shannon Zhao, 2018/05/22
- [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Shannon Zhao, 2018/05/22
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Auger Eric, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Peter Maydell, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Auger Eric, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Peter Maydell, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Auger Eric, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Peter Maydell, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Peter Maydell, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Auger Eric, 2018/05/24
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Shannon Zhao, 2018/05/25
- Re: [Qemu-devel] [PATCH V3 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR, Peter Maydell, 2018/05/25