linux-kernel - Re: [PATCH v2 2/5] KVM: add KVM request variants without barrier

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20170302011533.GI13926@pxdev.xzpeter.org>
Date:   Thu, 2 Mar 2017 09:15:33 +0800
From:   Peter Xu <peterx@...hat.com>
To:     Radim Krčmář <rkrcmar@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Andrew Jones <drjones@...hat.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Cornelia Huck <cornelia.huck@...ibm.com>,
        James Hogan <james.hogan@...tec.com>,
        Paul Mackerras <paulus@...abs.org>,
        Christoffer Dall <christoffer.dall@...aro.org>,
        David Hildenbrand <david@...hat.com>
Subject: Re: [PATCH v2 2/5] KVM: add KVM request variants without barrier

On Wed, Mar 01, 2017 at 06:02:49PM +0100, Radim Krčmář wrote:
> 2017-02-28 15:40+0800, Peter Xu:
> > On Tue, Feb 28, 2017 at 03:34:24PM +0800, Peter Xu wrote:
> > 
> > [...]
> > 
> >> > diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> >> > index ee4af898bcf6..552ae2b5e911 100644
> >> > --- a/arch/mips/kvm/emulate.c
> >> > +++ b/arch/mips/kvm/emulate.c
> >> > @@ -865,7 +865,7 @@ enum emulation_result kvm_mips_emul_wait(struct kvm_vcpu *vcpu)
> >> >  		 * check if any I/O interrupts are pending.
> >> >  		 */
> >> >  		if (kvm_request_test_and_clear(KVM_REQ_UNHALT, vcpu)) {
> >> > -			clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
> >> > +			__kvm_request_clear(KVM_REQ_UNHALT, vcpu);
> >> 
> >> Shall we just remove above line since we cleared it already?
> > 
> > Please ignore this since I see patch 4. :-)
> > 
> > It'll be nice if patch 4 will be before this one, but it's trivial.
> 
> I put [4/5] there to demonstrate that this error would have been less
> likely with the new naming.  I didn't expect that reviewers would go
> through the coccinelle transformation. :)

Yeah, I noticed it mostly because it's the first one touched.

Meanwhile, I think it's still worthwhile to go through the patch even
it's from cocinelle since sometimes coccinelle might do something that
we (or only me?) didn't expect. E.g., afaik it cannot handle well with
over-80-chars lines, so we need to wrap them on our own (I got a patch
from the author though to fix this, but not yet tested). And also
since patches are going to be merged changes, it just feel unsafe if
we merge something without reading it. :)

Thanks,

-- peterx