The ITS is not fully properly reset at the moment. Caches are
not emptied.

After a reset, in case we attempt to save the state before
the bound devices have registered their MSIs and after the
1st level table has been allocated by the ITS driver
(device BASER is valid), the first level entries are still
invalid. If the device cache is not empty (devices registered
before the reset), vgic_its_save_device_tables fails with -EINVAL.
This causes a QEMU abort().

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: wanghaibin <wanghaibin.wang@huawei.com>

---

this patch would deserve being cc'ed stable (2.10)
This goes along with patches 1-5 of
[PATCH v5 00/10] vITS Migration fixes and reset, candidate
for being cc'ed stable
---
 hw/intc/arm_gicv3_its_kvm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index 39903d5..1ae205f 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -64,20 +64,16 @@ static void vm_change_state_handler(void *opaque, int running,
 {
     GICv3ITSState *s = (GICv3ITSState *)opaque;
     Error *err = NULL;
-    int ret;
 
     if (running) {
         return;
     }
 
-    ret = kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
-                            KVM_DEV_ARM_ITS_SAVE_TABLES, NULL, true, &err);
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+                      KVM_DEV_ARM_ITS_SAVE_TABLES, NULL, true, &err);
     if (err) {
         error_report_err(err);
     }
-    if (ret < 0 && ret != -EFAULT) {
-        abort();
-    }
 }
 
 static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
-- 
2.5.5

On 23 October 2017 at 16:35, Eric Auger <eric.auger@redhat.com> wrote:
> The ITS is not fully properly reset at the moment. Caches are
> not emptied.
>
> After a reset, in case we attempt to save the state before
> the bound devices have registered their MSIs and after the
> 1st level table has been allocated by the ITS driver
> (device BASER is valid), the first level entries are still
> invalid. If the device cache is not empty (devices registered
> before the reset), vgic_its_save_device_tables fails with -EINVAL.
> This causes a QEMU abort().
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: wanghaibin <wanghaibin.wang@huawei.com>
>
> ---
>
> this patch would deserve being cc'ed stable (2.10)
> This goes along with patches 1-5 of
> [PATCH v5 00/10] vITS Migration fixes and reset, candidate
> for being cc'ed stable
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

We could/should put this patch into qemu now, right (it's
the rest of the series that's RFC) ?

thanks
-- PMM

Hi Peter,

On 02/11/2017 13:53, Peter Maydell wrote:
> On 23 October 2017 at 16:35, Eric Auger <eric.auger@redhat.com> wrote:
>> The ITS is not fully properly reset at the moment. Caches are
>> not emptied.
>>
>> After a reset, in case we attempt to save the state before
>> the bound devices have registered their MSIs and after the
>> 1st level table has been allocated by the ITS driver
>> (device BASER is valid), the first level entries are still
>> invalid. If the device cache is not empty (devices registered
>> before the reset), vgic_its_save_device_tables fails with -EINVAL.
>> This causes a QEMU abort().
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reported-by: wanghaibin <wanghaibin.wang@huawei.com>
>>
>> ---
>>
>> this patch would deserve being cc'ed stable (2.10)
>> This goes along with patches 1-5 of
>> [PATCH v5 00/10] vITS Migration fixes and reset, candidate
>> for being cc'ed stable
>> ---
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> We could/should put this patch into qemu now, right (it's
> the rest of the series that's RFC) ?

Yes that's correct.

Thanks

Eric
> 
> thanks
> -- PMM
> 

On 6 November 2017 at 10:09, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Peter,
>
> On 02/11/2017 13:53, Peter Maydell wrote:
>> On 23 October 2017 at 16:35, Eric Auger <eric.auger@redhat.com> wrote:
>>> The ITS is not fully properly reset at the moment. Caches are
>>> not emptied.
>>>
>>> After a reset, in case we attempt to save the state before
>>> the bound devices have registered their MSIs and after the
>>> 1st level table has been allocated by the ITS driver
>>> (device BASER is valid), the first level entries are still
>>> invalid. If the device cache is not empty (devices registered
>>> before the reset), vgic_its_save_device_tables fails with -EINVAL.
>>> This causes a QEMU abort().
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Reported-by: wanghaibin <wanghaibin.wang@huawei.com>
>>>
>>> ---
>>>
>>> this patch would deserve being cc'ed stable (2.10)
>>> This goes along with patches 1-5 of
>>> [PATCH v5 00/10] vITS Migration fixes and reset, candidate
>>> for being cc'ed stable
>>> ---
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> We could/should put this patch into qemu now, right (it's
>> the rest of the series that's RFC) ?
>
> Yes that's correct.

OK. I have applied 1/4 (and none of the rest) to target-arm.next,
with a cc-stable annotation.

thanks
-- PMM