linux-kernel - Re: [GIT PULL] KVM changes for 4.8 merge window

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: <87d1lql58t.fsf@concordia.ellerman.id.au>
Date:	Wed, 03 Aug 2016 16:46:10 +1000
From:	Michael Ellerman <mpe@...erman.id.au>
To:	Paolo Bonzini <pbonzini@...hat.com>, torvalds@...ux-foundation.org
Cc:	linux-kernel@...r.kernel.org, rkrcmar@...hat.com,
	kvm@...r.kernel.org,
	Christian Borntraeger <borntraeger@...ibm.com>,
	Christoffer Dall <christoffer.dall@...aro.org>,
	Dan Williams <dan.j.williams@...el.com>,
	Marc Zyngier <marc.zyngier@....com>,
	Paul Mackerras <paulus@...abs.org>
Subject: Re: [GIT PULL] KVM changes for 4.8 merge window

Paolo Bonzini <pbonzini@...hat.com> writes:

> On 03/08/2016 05:21, Michael Ellerman wrote:
>> Paolo Bonzini <pbonzini@...hat.com> writes:
>> ...
>>> - arch/powerpc: what a mess.  For the idle_book3s.S conflict, the KVM
>>> tree is the right one; everything else is trivial.  In this case I am
>>> not quite sure what went wrong.  The commit that is causing the mess
>>> (fd7bacbca47a, "KVM: PPC: Book3S HV: Fix TB corruption in guest exit
>>> path on HMI interrupt", 2016-05-15) touches both arch/powerpc/kernel/
>>> and arch/powerpc/kvm/.  It's large, but at 396 insertions/5 deletions
>>> I guessed that it wasn't really possible to split it and that the 5
>>> deletions wouldn't conflict.  That wasn't the case.
>> 
>> In fact I think the problem is that this patch shouldn't have gone via the KVM
>> tree at all.
>> 
>> If you look at the diffstat, it doesn't touch anything in generic KVM, but lots
>> of arch code:
>
> The KVM tree merges all arch/*/kvm code from submaintainers.  Only Radim
> and I send patches directly to Linus.

Sure.

But that's really my point. Just because a patch touches arch/*/kvm,
doesn't mean it must go via the KVM tree.

If the only arch code it touches is arch/*/kvm, then it should trivially
go via the KVM tree.

But if it touches other arch code then it's quite likely it will end up
conflicting with the arch tree, in which case it it's less likely to
cause problems if it goes into the arch tree, possibly in a topic
branch.

> Considering the h in "hmi" is for hypervisor,

Well hypervisor != KVM.

Though in this case hmi.c was pretty safe because it was new code. But
if I'd received a powerpc patch to hmi.c I wouldn't have thought to
check if it conflicted with the KVM tree.

> actual non-virt code in that patch was this:
>
>   arch/powerpc/include/asm/paca.h         |   6 +++
>   arch/powerpc/kernel/Makefile            |   2 +-
>   arch/powerpc/kernel/exceptions-64s.S    |   4 +-
>   arch/powerpc/kernel/idle_power7.S       |   5 ++-
>   arch/powerpc/kernel/traps.c             |   5 +++
>
> So the changes are pretty small, yet apart from paca.h every file ended
> up having a conflict with the PPC tree.  So I think it's just very bad
> luck in this case.

I guess. Makefile changes often conflict, though they're usually
trivial, and I'd guess exceptions-64s.S is patched in some way in most
releases.

But if there are changes outside arch/*/kvm in the KVM tree then it's
just luck if they don't conflict.

> Having this patch in a topic branch merged by both PPC and KVM
> maintainers would have still been a good idea, because I guess Paul
> knew of Ben's idle_power7.S cleanup.

I'm not sure who knew what when. Paul was travelling for some of the
merge window, and I was sick for some of it, so mistakes were made :)

cheers