| Message ID | 20230126235451.469087-5-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve dirty ring warning report | expand |
On 2023/1/27 07:54, Gavin Shan wrote: > We don't have a running VCPU context to save vgic3 pending table due > to KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} command on KVM > device "kvm-arm-vgic-v3". The unknown case is caught by kvm-unit-tests. > > # ./kvm-unit-tests/tests/its-pending-migration > WARNING: CPU: 120 PID: 7973 at arch/arm64/kvm/../../../virt/kvm/kvm_main.c:3325 \ > mark_page_dirty_in_slot+0x60/0xe0 > : > mark_page_dirty_in_slot+0x60/0xe0 > __kvm_write_guest_page+0xcc/0x100 > kvm_write_guest+0x7c/0xb0 > vgic_v3_save_pending_tables+0x148/0x2a0 > vgic_set_common_attr+0x158/0x240 > vgic_v3_set_attr+0x4c/0x5c > kvm_device_ioctl+0x100/0x160 > __arm64_sys_ioctl+0xa8/0xf0 > invoke_syscall.constprop.0+0x7c/0xd0 > el0_svc_common.constprop.0+0x144/0x160 > do_el0_svc+0x34/0x60 > el0_svc+0x3c/0x1a0 > el0t_64_sync_handler+0xb4/0x130 > el0t_64_sync+0x178/0x17c > > Use vgic_write_guest_lock() to save vgic3 pending table. > > Reported-by: Zenghui Yu <yuzenghui@huawei.com> > Signed-off-by: Gavin Shan <gshan@redhat.com> > Reviewed-by: Oliver Upton <oliver.upton@linux.dev> > --- > Documentation/virt/kvm/api.rst | 4 +++- > arch/arm64/kvm/vgic/vgic-v3.c | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 40ada313faa3..07f07668995e 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -8074,7 +8074,9 @@ NOTE: Multiple examples of using the backup bitmap: (1) save vgic/its > tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} on > KVM device "kvm-arm-vgic-its". (2) restore vgic/its tables through > command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM device > -"kvm-arm-vgic-its". vgic3 LPI pending status is restored. > +"kvm-arm-vgic-its". vgic3 LPI pending status is restored. (3) save > +vgic3 pending table through KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} > +command on KVM device "kvm-arm-vgic-v3". Can we summarize these 3 examples with something like: "when the guest memory (pending tables, ITS tables, etc) is dirtied by the virtual GIC or ITS, which is typically triggered by a userspace request (e.g., KVM_DEV_ARM_ITS_SAVE_TABLES) and doesn't require a running VCPU context"? In case there will be more no-running-vcpu kvm_write_guest_lock() cases in the VGIC emulation code in future and we have to extend the documentation.. But I don't have objection to your writing and the whole series looks good. Thanks, Zenghui
Hi Zenghui, On 1/28/23 2:57 AM, Zenghui Yu wrote: > On 2023/1/27 07:54, Gavin Shan wrote: >> We don't have a running VCPU context to save vgic3 pending table due >> to KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} command on KVM >> device "kvm-arm-vgic-v3". The unknown case is caught by kvm-unit-tests. >> >> # ./kvm-unit-tests/tests/its-pending-migration >> WARNING: CPU: 120 PID: 7973 at arch/arm64/kvm/../../../virt/kvm/kvm_main.c:3325 \ >> mark_page_dirty_in_slot+0x60/0xe0 >> : >> mark_page_dirty_in_slot+0x60/0xe0 >> __kvm_write_guest_page+0xcc/0x100 >> kvm_write_guest+0x7c/0xb0 >> vgic_v3_save_pending_tables+0x148/0x2a0 >> vgic_set_common_attr+0x158/0x240 >> vgic_v3_set_attr+0x4c/0x5c >> kvm_device_ioctl+0x100/0x160 >> __arm64_sys_ioctl+0xa8/0xf0 >> invoke_syscall.constprop.0+0x7c/0xd0 >> el0_svc_common.constprop.0+0x144/0x160 >> do_el0_svc+0x34/0x60 >> el0_svc+0x3c/0x1a0 >> el0t_64_sync_handler+0xb4/0x130 >> el0t_64_sync+0x178/0x17c >> >> Use vgic_write_guest_lock() to save vgic3 pending table. >> >> Reported-by: Zenghui Yu <yuzenghui@huawei.com> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> Reviewed-by: Oliver Upton <oliver.upton@linux.dev> >> --- >> Documentation/virt/kvm/api.rst | 4 +++- >> arch/arm64/kvm/vgic/vgic-v3.c | 2 +- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst >> index 40ada313faa3..07f07668995e 100644 >> --- a/Documentation/virt/kvm/api.rst >> +++ b/Documentation/virt/kvm/api.rst >> @@ -8074,7 +8074,9 @@ NOTE: Multiple examples of using the backup bitmap: (1) save vgic/its >> tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} on >> KVM device "kvm-arm-vgic-its". (2) restore vgic/its tables through >> command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM device >> -"kvm-arm-vgic-its". vgic3 LPI pending status is restored. >> +"kvm-arm-vgic-its". vgic3 LPI pending status is restored. (3) save >> +vgic3 pending table through KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} >> +command on KVM device "kvm-arm-vgic-v3". > > Can we summarize these 3 examples with something like: "when the guest > memory (pending tables, ITS tables, etc) is dirtied by the virtual GIC > or ITS, which is typically triggered by a userspace request (e.g., > KVM_DEV_ARM_ITS_SAVE_TABLES) and doesn't require a running VCPU > context"? In case there will be more no-running-vcpu > kvm_write_guest_lock() cases in the VGIC emulation code in future and we > have to extend the documentation.. > > But I don't have objection to your writing and the whole series looks > good. > There are discussions about the documentation when dirty ring is enabled on ARM64. We prefer to keep the layout where the KVM devices and commands are explicitly documented. The application developer can identify them easily and to enable the backup bitmap when those KVM devices have been used. By the way, 'vgic3' will be replaced with 'VGICv3' as you suggested in another reply. Thanks, Gavin
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 40ada313faa3..07f07668995e 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -8074,7 +8074,9 @@ NOTE: Multiple examples of using the backup bitmap: (1) save vgic/its tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} on KVM device "kvm-arm-vgic-its". (2) restore vgic/its tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM device -"kvm-arm-vgic-its". vgic3 LPI pending status is restored. +"kvm-arm-vgic-its". vgic3 LPI pending status is restored. (3) save +vgic3 pending table through KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} +command on KVM device "kvm-arm-vgic-v3". 8.30 KVM_CAP_XEN_HVM -------------------- diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c index c94e4d7520fc..558ccc805fff 100644 --- a/arch/arm64/kvm/vgic/vgic-v3.c +++ b/arch/arm64/kvm/vgic/vgic-v3.c @@ -436,7 +436,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm) else val &= ~(1 << bit_nr); - ret = kvm_write_guest_lock(kvm, ptr, &val, 1); + ret = vgic_write_guest_lock(kvm, ptr, &val, 1); if (ret) goto out; }
We don't have a running VCPU context to save vgic3 pending table due to KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} command on KVM device "kvm-arm-vgic-v3". The unknown case is caught by kvm-unit-tests. # ./kvm-unit-tests/tests/its-pending-migration WARNING: CPU: 120 PID: 7973 at arch/arm64/kvm/../../../virt/kvm/kvm_main.c:3325 \ mark_page_dirty_in_slot+0x60/0xe0 : mark_page_dirty_in_slot+0x60/0xe0 __kvm_write_guest_page+0xcc/0x100 kvm_write_guest+0x7c/0xb0 vgic_v3_save_pending_tables+0x148/0x2a0 vgic_set_common_attr+0x158/0x240 vgic_v3_set_attr+0x4c/0x5c kvm_device_ioctl+0x100/0x160 __arm64_sys_ioctl+0xa8/0xf0 invoke_syscall.constprop.0+0x7c/0xd0 el0_svc_common.constprop.0+0x144/0x160 do_el0_svc+0x34/0x60 el0_svc+0x3c/0x1a0 el0t_64_sync_handler+0xb4/0x130 el0t_64_sync+0x178/0x17c Use vgic_write_guest_lock() to save vgic3 pending table. Reported-by: Zenghui Yu <yuzenghui@huawei.com> Signed-off-by: Gavin Shan <gshan@redhat.com> Reviewed-by: Oliver Upton <oliver.upton@linux.dev> --- Documentation/virt/kvm/api.rst | 4 +++- arch/arm64/kvm/vgic/vgic-v3.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)