Re: [PATCH 1/2] KVM: arm/arm64: vgic-its: Take the srcu lock when writing to guest memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux