Hi Eric,
On Tue, 19 Mar 2019 15:16:38 +0000,
Auger Eric <eric.auger@xxxxxxxxxx> wrote:
>
> Hi Marc,
>
> On 3/19/19 2:30 PM, Marc Zyngier wrote:
> > When halting a guest, QEMU flushes the virtual ITS caches, which
> > amounts to writing to the various tables that the guest has allocated.
> >
> > When doing this, we fail to take the srcu lock, and the kernel
> > shouts loudly if running a lockdep kernel:
> >
> > [ 69.680416] =============================
> > [ 69.680819] WARNING: suspicious RCU usage
> > [ 69.681526] 5.1.0-rc1-00008-g600025238f51-dirty #18 Not tainted
> > [ 69.682096] -----------------------------
> > [ 69.682501] ./include/linux/kvm_host.h:605 suspicious rcu_dereference_check() usage!
> > [ 69.683225]
> > [ 69.683225] other info that might help us debug this:
> > [ 69.683225]
> > [ 69.683975]
> > [ 69.683975] rcu_scheduler_active = 2, debug_locks = 1
> > [ 69.684598] 6 locks held by qemu-system-aar/4097:
> > [ 69.685059] #0: 0000000034196013 (&kvm->lock){+.+.}, at: vgic_its_set_attr+0x244/0x3a0
> > [ 69.686087] #1: 00000000f2ed935e (&its->its_lock){+.+.}, at: vgic_its_set_attr+0x250/0x3a0
> > [ 69.686919] #2: 000000005e71ea54 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xd0
> > [ 69.687698] #3: 00000000c17e548d (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xd0
> > [ 69.688475] #4: 00000000ba386017 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xd0
> > [ 69.689978] #5: 00000000c2c3c335 (&vcpu->mutex){+.+.}, at: lock_all_vcpus+0x64/0xd0
> > [ 69.690729]
> > [ 69.690729] stack backtrace:
> > [ 69.691151] CPU: 2 PID: 4097 Comm: qemu-system-aar Not tainted 5.1.0-rc1-00008-g600025238f51-dirty #18
> > [ 69.691984] Hardware name: rockchip evb_rk3399/evb_rk3399, BIOS 2019.04-rc3-00124-g2feec69fb1 03/15/2019
> > [ 69.692831] Call trace:
> > [ 69.694072] lockdep_rcu_suspicious+0xcc/0x110
> > [ 69.694490] gfn_to_memslot+0x174/0x190
> > [ 69.694853] kvm_write_guest+0x50/0xb0
> > [ 69.695209] vgic_its_save_tables_v0+0x248/0x330
> > [ 69.695639] vgic_its_set_attr+0x298/0x3a0
> > [ 69.696024] kvm_device_ioctl_attr+0x9c/0xd8
> > [ 69.696424] kvm_device_ioctl+0x8c/0xf8
> > [ 69.696788] do_vfs_ioctl+0xc8/0x960
> > [ 69.697128] ksys_ioctl+0x8c/0xa0
> > [ 69.697445] __arm64_sys_ioctl+0x28/0x38
> > [ 69.697817] el0_svc_common+0xd8/0x138
> > [ 69.698173] el0_svc_handler+0x38/0x78
> > [ 69.698528] el0_svc+0x8/0xc
> >
> > The fix is to obviously take the srcu lock, just like we do on the
> > read side of things since bf308242ab98. One wonders why this wasn't
> > fixed at the same time, but hey...
> >
> > Fixes: bf308242ab98 ("KVM: arm/arm64: VGIC/ITS: protect kvm_read_guest() calls with SRCU lock")
> > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > ---
> > arch/arm/include/asm/kvm_mmu.h | 11 +++++++++++
> > arch/arm64/include/asm/kvm_mmu.h | 11 +++++++++++
> > virt/kvm/arm/vgic/vgic-its.c | 8 ++++----
> > 3 files changed, 26 insertions(+), 4 deletions(-)
[...]
> Don't we need to use kvm_write_guest_lock() as well also in
> vgic_v3_lpi_sync_pending_status and vgic_v3_save_pending_tables?
Ah, well spotted. Yes, that's needed too. I'll fix that.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
