| Message ID | 1508772937-21054-2-git-send-email-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,v2,1/4] hw/intc/arm_gicv3_its: Don't abort on table save failure | expand |
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
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)
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(-)