Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1578167 > unrolled thread

linux-next: manual merge of the kvm tree with the powerpc tree

Started byStephen Rothwell <sfr@canb.auug.org.au>
First post2017-02-10 05:10 +0100
Last post2017-02-15 12:30 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  linux-next: manual merge of the kvm tree with the powerpc tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-02-10 05:10 +0100
    Re: linux-next: manual merge of the kvm tree with the powerpc tree Paolo Bonzini <pbonzini@redhat.com> - 2017-02-10 11:10 +0100
      Re: linux-next: manual merge of the kvm tree with the powerpc tree Michael Ellerman <mpe@ellerman.id.au> - 2017-02-14 09:50 +0100
        Re: linux-next: manual merge of the kvm tree with the powerpc tree Paolo Bonzini <pbonzini@redhat.com> - 2017-02-14 14:40 +0100
          Re: linux-next: manual merge of the kvm tree with the powerpc tree Michael Ellerman <mpe@ellerman.id.au> - 2017-02-15 12:20 +0100
            Re: linux-next: manual merge of the kvm tree with the powerpc tree Paolo Bonzini <pbonzini@redhat.com> - 2017-02-15 12:30 +0100

#1578167 — linux-next: manual merge of the kvm tree with the powerpc tree

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2017-02-10 05:10 +0100
Subjectlinux-next: manual merge of the kvm tree with the powerpc tree
Message-ID<t9aGZ-6Ul-7@gated-at.bofh.it>
Hi all,

Today's linux-next merge of the kvm tree got a conflict in:

  arch/powerpc/kernel/exceptions-64s.S

between commit:

  1a6822d194c3 ("powerpc/64s: Use (start, size) rather than (start, end) for exception handlers")

from the powerpc tree and commit:

  bc3551257af8 ("powerpc/64: Allow for relocation-on interrupts from guest to host")

from the kvm tree.

I fixed it up (I think - see below) and can carry the fix as necessary.
This is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc arch/powerpc/kernel/exceptions-64s.S
index a6205a4a3574,34a04a5fa468..000000000000
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@@ -720,14 -720,10 +720,10 @@@ hardware_interrupt_hv
  	FTR_SECTION_ELSE
  		_MASKABLE_EXCEPTION_PSERIES(0x500, hardware_interrupt_common,
  					    EXC_STD, SOFTEN_TEST_PR)
- do_kvm_0x500:
- 		KVM_HANDLER(PACA_EXGEN, EXC_STD, 0x500)
  	ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
 -EXC_REAL_END(hardware_interrupt, 0x500, 0x600)
 +EXC_REAL_END(hardware_interrupt, 0x500, 0x100)
  
 -EXC_VIRT_BEGIN(hardware_interrupt, 0x4500, 0x4600)
 +EXC_VIRT_BEGIN(hardware_interrupt, 0x4500, 0x100)
  	.globl hardware_interrupt_relon_hv;
  hardware_interrupt_relon_hv:
  	BEGIN_FTR_SECTION
@@@ -735,8 -731,10 +731,10 @@@
  	FTR_SECTION_ELSE
  		_MASKABLE_RELON_EXCEPTION_PSERIES(0x500, hardware_interrupt_common, EXC_STD, SOFTEN_TEST_PR)
  	ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
 -EXC_VIRT_END(hardware_interrupt, 0x4500, 0x4600)
 +EXC_VIRT_END(hardware_interrupt, 0x4500, 0x100)
  
+ TRAMP_KVM(PACA_EXGEN, 0x500)
+ TRAMP_KVM_HV(PACA_EXGEN, 0x500)
  EXC_COMMON_ASYNC(hardware_interrupt_common, 0x500, do_IRQ)
  
  
@@@ -884,35 -907,15 +907,15 @@@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE
  	b	system_call_common ;
  #endif
  
 -EXC_REAL_BEGIN(system_call, 0xc00, 0xd00)
 +EXC_REAL_BEGIN(system_call, 0xc00, 0x100)
- 	 /*
- 	  * If CONFIG_KVM_BOOK3S_64_HANDLER is set, save the PPR (on systems
- 	  * that support it) before changing to HMT_MEDIUM. That allows the KVM
- 	  * code to save that value into the guest state (it is the guest's PPR
- 	  * value). Otherwise just change to HMT_MEDIUM as userspace has
- 	  * already saved the PPR.
- 	  */
- #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
- 	SET_SCRATCH0(r13)
- 	GET_PACA(r13)
- 	std	r9,PACA_EXGEN+EX_R9(r13)
- 	OPT_GET_SPR(r9, SPRN_PPR, CPU_FTR_HAS_PPR);
- 	HMT_MEDIUM;
- 	std	r10,PACA_EXGEN+EX_R10(r13)
- 	OPT_SAVE_REG_TO_PACA(PACA_EXGEN+EX_PPR, r9, CPU_FTR_HAS_PPR);
- 	mfcr	r9
- 	KVMTEST_PR(0xc00)
- 	GET_SCRATCH0(r13)
- #else
- 	HMT_MEDIUM;
- #endif
+ 	SYSCALL_KVMTEST
  	SYSCALL_PSERIES_1
  	SYSCALL_PSERIES_2_RFID
  	SYSCALL_PSERIES_3
 -EXC_REAL_END(system_call, 0xc00, 0xd00)
 +EXC_REAL_END(system_call, 0xc00, 0x100)
  
 -EXC_VIRT_BEGIN(system_call, 0x4c00, 0x4d00)
 +EXC_VIRT_BEGIN(system_call, 0x4c00, 0x100)
- 	HMT_MEDIUM
+ 	SYSCALL_KVMTEST
  	SYSCALL_PSERIES_1
  	SYSCALL_PSERIES_2_DIRECT
  	SYSCALL_PSERIES_3
@@@ -926,8 -929,8 +929,8 @@@ EXC_VIRT(single_step, 0x4d00, 0x100, 0x
  TRAMP_KVM(PACA_EXGEN, 0xd00)
  EXC_COMMON(single_step_common, 0xd00, single_step_exception)
  
 -EXC_REAL_OOL_HV(h_data_storage, 0xe00, 0xe20)
 -EXC_VIRT_OOL_HV(h_data_storage, 0x4e00, 0x4e20, 0xe00)
 +EXC_REAL_OOL_HV(h_data_storage, 0xe00, 0x20)
- EXC_VIRT_NONE(0x4e00, 0x20)
++EXC_VIRT_OOL_HV(h_data_storage, 0x4e00, 0x20, 0xe00)
  TRAMP_KVM_HV_SKIP(PACA_EXGEN, 0xe00)
  EXC_COMMON_BEGIN(h_data_storage_common)
  	mfspr   r10,SPRN_HDAR
@@@ -942,8 -945,8 +945,8 @@@
  	b       ret_from_except
  
  
 -EXC_REAL_OOL_HV(h_instr_storage, 0xe20, 0xe40)
 -EXC_VIRT_OOL_HV(h_instr_storage, 0x4e20, 0x4e40, 0xe20)
 +EXC_REAL_OOL_HV(h_instr_storage, 0xe20, 0x20)
- EXC_VIRT_NONE(0x4e20, 0x20)
++EXC_VIRT_OOL_HV(h_instr_storage, 0x4e20, 0x20, 0xe20)
  TRAMP_KVM_HV(PACA_EXGEN, 0xe20)
  EXC_COMMON(h_instr_storage_common, 0xe20, unknown_exception)
  

[toc] | [next] | [standalone]


#1578348

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-02-10 11:10 +0100
Message-ID<t9gjo-23D-13@gated-at.bofh.it>
In reply to#1578167

On 10/02/2017 04:59, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the kvm tree got a conflict in:
> 
>   arch/powerpc/include/asm/head-64.h
> 
> between commit:
> 
>   852e5da99d15 ("powerpc/64s: Tidy up after exception handler rework")
> 
> from the powerpc tree and commit:
> 
>   7ede531773ea ("KVM: PPC: Book3S: Move 64-bit KVM interrupt handler out from alt section")
> 
> from the kvm tree.
>
> Today's linux-next merge of the kvm tree got a conflict in:
> 
>   arch/powerpc/kernel/exceptions-64s.S
> 
> between commit:
> 
>   1a6822d194c3 ("powerpc/64s: Use (start, size) rather than (start, end) for exception handlers")
> 
> from the powerpc tree and commit:
> 
>   bc3551257af8 ("powerpc/64: Allow for relocation-on interrupts from guest to host")
> 
> from the kvm tree.

Michael, please pull the topic branch as soon as possible, so that the
conflicts don't hit Linus.

That said, the topic branch is a mess.  It starts with generic arch
patches (until "powerpc/64: Allow for relocation-on interrupts from
guest to host") then it's only KVM, then on the top there's two more
generic patches that were added _after_ Paul merged it.

If possible, please pull only up to "powerpc/64: Allow for relocation-on
interrupts from guest to host" and cherry-pick the top two patches
("powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts" and
"powerpc/powernv: Remove separate entry for OPAL real mode calls") into
your next branch, but leave the rest for my tree only.

Paolo

[toc] | [prev] | [next] | [standalone]


#1580405

FromMichael Ellerman <mpe@ellerman.id.au>
Date2017-02-14 09:50 +0100
Message-ID<taGYb-7UK-31@gated-at.bofh.it>
In reply to#1578348
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/02/2017 04:59, Stephen Rothwell wrote:
>> Hi all,
>> 
>> Today's linux-next merge of the kvm tree got a conflict in:
>> 
>>   arch/powerpc/include/asm/head-64.h
>> 
>> between commit:
>> 
>>   852e5da99d15 ("powerpc/64s: Tidy up after exception handler rework")
>> 
>> from the powerpc tree and commit:
>> 
>>   7ede531773ea ("KVM: PPC: Book3S: Move 64-bit KVM interrupt handler out from alt section")
>> 
>> from the kvm tree.
>
> Michael, please pull the topic branch as soon as possible, so that the
> conflicts don't hit Linus.

They won't hit Linus until I send my pull request.

> That said, the topic branch is a mess.  It starts with generic arch
> patches (until "powerpc/64: Allow for relocation-on interrupts from
> guest to host") then it's only KVM, then on the top there's two more
> generic patches that were added _after_ Paul merged it.

It's not a mess, it's a collection of patches which touch either arch/powerpc
or arch/powerpc/kvm, or are otherwise related.

Yeah I could have merged just the start of Paul's series, but that
seemed pointless, it doesn't prevent or add any conflicts, and it means
I'm unable to test his series as a whole.

Paul has also now merged the remaining two commits, and sent you a pull
request including them.

> If possible, please pull only up to "powerpc/64: Allow for relocation-on
> interrupts from guest to host" and cherry-pick the top two patches
> ("powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts" and
> "powerpc/powernv: Remove separate entry for OPAL real mode calls") into
> your next branch, but leave the rest for my tree only.

I don't see how that helps anything.

In fact it guarantees a mess because those two commits would now go to
Linus via my tree (cherry picked) and via Paul's as part of his second
merge of the topic branch.

So unless you can give me a good reason I'll merge the tip of the topic
branch into my next, as planned.

cheers

[toc] | [prev] | [next] | [standalone]


#1580540

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-02-14 14:40 +0100
Message-ID<taLuO-2sF-21@gated-at.bofh.it>
In reply to#1580405

On 14/02/2017 09:45, Michael Ellerman wrote:
>> If possible, please pull only up to "powerpc/64: Allow for relocation-on
>> interrupts from guest to host" and cherry-pick the top two patches
>> ("powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts" and
>> "powerpc/powernv: Remove separate entry for OPAL real mode calls") into
>> your next branch, but leave the rest for my tree only.
>
> I don't see how that helps anything.
> 
> In fact it guarantees a mess because those two commits would now go to
> Linus via my tree (cherry picked) and via Paul's as part of his second
> merge of the topic branch.
>
> So unless you can give me a good reason I'll merge the tip of the topic
> branch into my next, as planned.

Yes, Paul's second merge did guarantee a mess, so go ahead.

However, the reason was that this is simply not how topic branches
should work: topic branches should be the base for other work, they
shouldn't contain _all_ the work.  So the right workflow would have been:

- Paul submits topic branch A to you

- you merge A

- Paul merges topic branch A into his "next" branch

- Paul applies KVM-specific patches B1 on top of his "next" branch.

- Paul sends pull request to me (with A + kvmppc work).

As far as I understand, there was no reason for you to get B1.

The last two patches (let's call them B2) also didn't need to go through
the kvm-ppc branch at all.  You could have applied them directly on top
of A.  Linus then would get A and B2 from you, and A and B1 from me:

                  base -→ A -----→ B1
                          ↓        ↓
                   ppc -→ ▪        ▪ ←- kvm
                          ↓        |
                          B2       |
                          ↓        ↓
                      torvalds/linux.git


If necessary, things could have been arranged so that Linus got A and B2
from you, and all three of A/B1/B2 from me:

- Paul submits topic branch B2 to you, based on topic branch A

- you merge B2

- Paul merges B2 and I get it from him

The result would have been:

                  base -→ A -----→ B1
                          ↓ ↘      ↓
                   ppc -→ ▪   B2 → ▪
                          ↓ ↙      ↓
                          ▪        ▪ ←- kvm
                          ↓        ↓
                      torvalds/linux.git

Paolo

[toc] | [prev] | [next] | [standalone]


#1581223

FromMichael Ellerman <mpe@ellerman.id.au>
Date2017-02-15 12:20 +0100
Message-ID<tb5MR-7Et-5@gated-at.bofh.it>
In reply to#1580540
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 14/02/2017 09:45, Michael Ellerman wrote:
>>> If possible, please pull only up to "powerpc/64: Allow for relocation-on
>>> interrupts from guest to host" and cherry-pick the top two patches
>>> ("powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts" and
>>> "powerpc/powernv: Remove separate entry for OPAL real mode calls") into
>>> your next branch, but leave the rest for my tree only.
>>
>> I don't see how that helps anything.
>> 
>> In fact it guarantees a mess because those two commits would now go to
>> Linus via my tree (cherry picked) and via Paul's as part of his second
>> merge of the topic branch.
>>
>> So unless you can give me a good reason I'll merge the tip of the topic
>> branch into my next, as planned.
>
> Yes, Paul's second merge did guarantee a mess, so go ahead.

OK, glad we agree on that.

I've now merged it and most of the conflicts are gone. The one remaining
one will be fixed if you take Paul's second pull, or otherwise it's
trivial for Linus to fix.

> However, the reason was that this is simply not how topic branches
> should work: topic branches should be the base for other work, they
> shouldn't contain _all_ the work.

I think that's an overly specific definition of what a topic branch is.

It's just a branch related to some "topic", in this case powerpc kvm,
where commits can go so they can be shared between two trees.


> So the right workflow would have been:
>
> - Paul submits topic branch A to you

That never existed though. Paul had a series of 20 patches to enable KVM
with the radix MMU.

So to create 'A' he would have to split his series in two. Which he can
obviously do, but it's unnecessary IMHO.

> - you merge A
>
> - Paul merges topic branch A into his "next" branch
>
> - Paul applies KVM-specific patches B1 on top of his "next" branch.
>
> - Paul sends pull request to me (with A + kvmppc work).

Yeah we could have done it that way. But it unnecessarily splits the
series across the trees, and means I have no way of testing the whole in
my tree.

> As far as I understand, there was no reason for you to get B1.

Well no reason other than it's ~1300 lines of code in my arch, which I
would like to go through my normal testing procedures.

I also don't see how it hurts in any way for B1 to go to Linus via both
trees.

> The last two patches (let's call them B2) also didn't need to go through
> the kvm-ppc branch at all.  You could have applied them directly on top
> of A.

I'm pretty sure Paul did need the last patch to fix a bug, but maybe
he reworked that code, I forget.

You're right the second last patch didn't need to go via kvm-ppc. I put
it in the topic branch because it was based on earlier patches in there,
but I could have put it in my tree and fixed up the conflict when I
merged the topic branch.

cheers

[toc] | [prev] | [next] | [standalone]


#1581226

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-02-15 12:30 +0100
Message-ID<tb5Wy-7HU-1@gated-at.bofh.it>
In reply to#1581223
On 15/02/2017 12:16, Michael Ellerman wrote:
>> However, the reason was that this is simply not how topic branches
>> should work: topic branches should be the base for other work, they
>> shouldn't contain _all_ the work.
> 
> I think that's an overly specific definition of what a topic branch is.
> 
> It's just a branch related to some "topic", in this case powerpc kvm,
> where commits can go so they can be shared between two trees.

Right.  However, in the specific case of working across maintainers, I
think there is an interest in minimizing the number of files that are
updated in two trees.  That limits conflicts.

Typically in x86 land people send a series with generic+KVM patches,
Thomas Gleixner picks the generic ones and places them in a topic branch
that we both pull from.  I then apply the KVM patches independently.
It's worth noting that x86 arch maintainers don't care that much about
what's going on in arch/x86/kvm/, and especially they delegate all
testing to me.  So I guess that may be the source of the disagreement.

If you would like to unify testing of non-KVM and KVM code for
arch/powerpc, it doesn't make much sense for Paul to send his patches to
me at all.  Instead, _I_ should prepare topic branches for Paul whenever
I make sweeping all-arch changes to KVM, that he can include in his pull
requests to you.  It'd feel weird though.

Paolo

>> As far as I understand, there was no reason for you to get B1.
>
> Well no reason other than it's ~1300 lines of code in my arch, which I
> would like to go through my normal testing procedures.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web