linux-kernel - Re: [PATCH 4/4] KVM: x86: remove notifiers from PIT discard policy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160204131558.GA9295@potion.brq.redhat.com>
Date:	Thu, 4 Feb 2016 14:15:59 +0100
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	Yuki Shibuya <shibuya.yk@...s.nec.co.jp>
Subject: Re: [PATCH 4/4] KVM: x86: remove notifiers from PIT discard policy

2016-02-03 17:51+0100, Paolo Bonzini:
> On 03/02/2016 17:23, Radim Krčmář wrote:
>> Discard policy doesn't rely on information from notifiers, so we don't
>> need to register notifiers unconditionally.
>> 
>> Use of ps->lock doesn't make sense, but isn't any worse than before.

Oops, it is worse than before ... toggling KVM_REINJECT_CONTROL when the
guest is running and reading reinject without locking is now far more
complex.  This patch should have also ignored KVM_REINJECT_CONTROL when
PIT has been started.

> Oh, it's perfectly okay.  Too fine-grained locks are bad, and lock
> contention on ps->lock is a non-issue.
> 
> Can you however add a patch that says what fields of kvm_kpit_state are
> protected by which locks?

Ok.  (I'll be careful to not rewrite the whole PIT while at it. :])

>                            Then this patch will just add
> 
> 	/* Protected by kvm_kpit_state lock.  */
> 
> above the reinject field.

There was no need to lock reinject in the past and v2 will hopefully
achieve it again.

> Otherwise
> 
> Reviewed-by: Paolo Bonzini <pbonzini@...hat.com>

Thanks.  (Might not be applicable to v2, though; sorry.)