| Message ID | 20170429002552.GA693@fergus.ozlabs.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
On 29/04/2017 02:25, Paul Mackerras wrote: > > I would like this to go into 4.12. I know this pull request is > somewhat late, but that is mostly because I had 3 weeks vacation this > month. I have tested this on POWER9 and also tested that it doesn't > break other platforms such as POWER8. No problem, I'm a bit late too and this is linux-next anyway. I'll pull it next Tuesday because I have a public holiday. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: > > > On 29/04/2017 02:25, Paul Mackerras wrote: > > > > I would like this to go into 4.12. I know this pull request is > > somewhat late, but that is mostly because I had 3 weeks vacation this > > month. I have tested this on POWER9 and also tested that it doesn't > > break other platforms such as POWER8. > > No problem, I'm a bit late too and this is linux-next anyway. I'll pull > it next Tuesday because I have a public holiday. I haven't seen it go into kvm/next yet. Is there a problem with it, or have you just not got up to it yet, or did you do it and I missed it? Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/05/2017 12:02, Paul Mackerras wrote: > On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: >> >> >> On 29/04/2017 02:25, Paul Mackerras wrote: >>> >>> I would like this to go into 4.12. I know this pull request is >>> somewhat late, but that is mostly because I had 3 weeks vacation this >>> month. I have tested this on POWER9 and also tested that it doesn't >>> break other platforms such as POWER8. >> >> No problem, I'm a bit late too and this is linux-next anyway. I'll pull >> it next Tuesday because I have a public holiday. > > I haven't seen it go into kvm/next yet. Is there a problem with it, > or have you just not got up to it yet, or did you do it and I missed > it? It was in kvm/queue only, now it's there. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, May 06, 2017 at 05:22:13PM +0200, Paolo Bonzini wrote: > > > On 06/05/2017 12:02, Paul Mackerras wrote: > > On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: > >> > >> > >> On 29/04/2017 02:25, Paul Mackerras wrote: > >>> > >>> I would like this to go into 4.12. I know this pull request is > >>> somewhat late, but that is mostly because I had 3 weeks vacation this > >>> month. I have tested this on POWER9 and also tested that it doesn't > >>> break other platforms such as POWER8. > >> > >> No problem, I'm a bit late too and this is linux-next anyway. I'll pull > >> it next Tuesday because I have a public holiday. > > > > I haven't seen it go into kvm/next yet. Is there a problem with it, > > or have you just not got up to it yet, or did you do it and I missed > > it? > > It was in kvm/queue only, now it's there. I'm still not seeing it. Did you forget to push it out or something? I don't know if you saw the email exchange between Michael Ellerman and Linus. Michael put into his tree a commit that moves the declaration of powerpc_debugfs_root from asm/debug.h to asm/debugfs.h, and that commit is now in Linus' tree. Since my tree adds a use of powerpc_debugfs_root, it is necessary when merging my tree into Linus' to add #include <asm/debugfs.h> to arch/powerpc/kvm/book3s_xive.c. I can't do that in my tree because asm/debugfs.h doesn't exist there. Thanks, Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/05/2017 05:31, Paul Mackerras wrote: > On Sat, May 06, 2017 at 05:22:13PM +0200, Paolo Bonzini wrote: >> >> >> On 06/05/2017 12:02, Paul Mackerras wrote: >>> On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: >>>> >>>> >>>> On 29/04/2017 02:25, Paul Mackerras wrote: >>>>> >>>>> I would like this to go into 4.12. I know this pull request is >>>>> somewhat late, but that is mostly because I had 3 weeks vacation this >>>>> month. I have tested this on POWER9 and also tested that it doesn't >>>>> break other platforms such as POWER8. >>>> >>>> No problem, I'm a bit late too and this is linux-next anyway. I'll pull >>>> it next Tuesday because I have a public holiday. >>> >>> I haven't seen it go into kvm/next yet. Is there a problem with it, >>> or have you just not got up to it yet, or did you do it and I missed >>> it? >> >> It was in kvm/queue only, now it's there. > > I'm still not seeing it. Did you forget to push it out or something? Ok, connected my brain and pulled now. Sorry for the inconvenience. > I don't know if you saw the email exchange between Michael Ellerman > and Linus. Michael put into his tree a commit that moves the > declaration of powerpc_debugfs_root from asm/debug.h to asm/debugfs.h, > and that commit is now in Linus' tree. Since my tree adds a use of > powerpc_debugfs_root, it is necessary when merging my tree into Linus' > to add #include <asm/debugfs.h> to arch/powerpc/kvm/book3s_xive.c. I > can't do that in my tree because asm/debugfs.h doesn't exist there. Yes, I noted that and will remind Linus today when sending the pull request. Paolo > Thanks, > Paul. > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/05/2017 09:28, Paolo Bonzini wrote: > > > On 08/05/2017 05:31, Paul Mackerras wrote: >> On Sat, May 06, 2017 at 05:22:13PM +0200, Paolo Bonzini wrote: >>> >>> >>> On 06/05/2017 12:02, Paul Mackerras wrote: >>>> On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: >>>>> >>>>> >>>>> On 29/04/2017 02:25, Paul Mackerras wrote: >>>>>> >>>>>> I would like this to go into 4.12. I know this pull request is >>>>>> somewhat late, but that is mostly because I had 3 weeks vacation this >>>>>> month. I have tested this on POWER9 and also tested that it doesn't >>>>>> break other platforms such as POWER8. >>>>> >>>>> No problem, I'm a bit late too and this is linux-next anyway. I'll pull >>>>> it next Tuesday because I have a public holiday. >>>> >>>> I haven't seen it go into kvm/next yet. Is there a problem with it, >>>> or have you just not got up to it yet, or did you do it and I missed >>>> it? >>> >>> It was in kvm/queue only, now it's there. >> >> I'm still not seeing it. Did you forget to push it out or something? > > Ok, connected my brain and pulled now. Sorry for the inconvenience. I'm un-pulling this, because it creates merge conflicts with Linus's tree due to an unwanted rebase (?) by either you or Michael. In particular this commit commit 8bf8f2e8c786f37991bd27332c75edcc524d2232 Author: Nicholas Piggin <npiggin@gmail.com> Date: Wed Apr 19 05:12:16 2017 +1000 powerpc/64s: Revert setting of LPCR[LPES] on POWER9 is already present in Linus's tree with a different hash: commit 8d1b48ef580097e111c2644e6fc6041b9784d218 Author: Nicholas Piggin <npiggin@gmail.com> Date: Wed Apr 19 05:12:16 2017 +1000 powerpc/64s: Revert setting of LPCR[LPES] on POWER9 and causes conflicts with this subsequent commit: commit 700b7eadd5625d22b8235fb21259b3d7d564c000 Author: Nicholas Piggin <npiggin@gmail.com> Date: Wed Apr 19 05:12:17 2017 +1000 powerpc/64s: Power9 has no LPCR[VRMASD] field so don't set it I'll take care of sending a separate pull request to Linus, but please avoid doing this kind of cherry-pick in the future. Paolo >> I don't know if you saw the email exchange between Michael Ellerman >> and Linus. Michael put into his tree a commit that moves the >> declaration of powerpc_debugfs_root from asm/debug.h to asm/debugfs.h, >> and that commit is now in Linus' tree. Since my tree adds a use of >> powerpc_debugfs_root, it is necessary when merging my tree into Linus' >> to add #include <asm/debugfs.h> to arch/powerpc/kvm/book3s_xive.c. I >> can't do that in my tree because asm/debugfs.h doesn't exist there. > > Yes, I noted that and will remind Linus today when sending the pull request. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 08, 2017 at 03:39:26PM +0200, Paolo Bonzini wrote: > > Ok, connected my brain and pulled now. Sorry for the inconvenience. > > I'm un-pulling this, because it creates merge conflicts with Linus's > tree due to an unwanted rebase (?) by either you or Michael. > > In particular this commit > > commit 8bf8f2e8c786f37991bd27332c75edcc524d2232 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:16 2017 +1000 > > powerpc/64s: Revert setting of LPCR[LPES] on POWER9 > > is already present in Linus's tree with a different hash: > > commit 8d1b48ef580097e111c2644e6fc6041b9784d218 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:16 2017 +1000 > > powerpc/64s: Revert setting of LPCR[LPES] on POWER9 > > and causes conflicts with this subsequent commit: > > commit 700b7eadd5625d22b8235fb21259b3d7d564c000 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:17 2017 +1000 > > powerpc/64s: Power9 has no LPCR[VRMASD] field so don't set it > > > I'll take care of sending a separate pull request to Linus, but please > avoid doing this kind of cherry-pick in the future. OK, sorry about that. The resolution is just to take the version of cpu_setup_power.S that is in Linus' tree. I think if you just send it to Linus that Linus would be able to resolve it easily. Let me know if you need me to do anything. Thanks, Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: > On 08/05/2017 09:28, Paolo Bonzini wrote: >> On 08/05/2017 05:31, Paul Mackerras wrote: >>> On Sat, May 06, 2017 at 05:22:13PM +0200, Paolo Bonzini wrote: >>>> On 06/05/2017 12:02, Paul Mackerras wrote: >>>>> On Sat, Apr 29, 2017 at 02:39:11PM +0200, Paolo Bonzini wrote: >>>>>> On 29/04/2017 02:25, Paul Mackerras wrote: >>>>>>> >>>>>>> I would like this to go into 4.12. I know this pull request is >>>>>>> somewhat late, but that is mostly because I had 3 weeks vacation this >>>>>>> month. I have tested this on POWER9 and also tested that it doesn't >>>>>>> break other platforms such as POWER8. >>>>>> >>>>>> No problem, I'm a bit late too and this is linux-next anyway. I'll pull >>>>>> it next Tuesday because I have a public holiday. >>>>> >>>>> I haven't seen it go into kvm/next yet. Is there a problem with it, >>>>> or have you just not got up to it yet, or did you do it and I missed >>>>> it? >>>> >>>> It was in kvm/queue only, now it's there. >>> >>> I'm still not seeing it. Did you forget to push it out or something? >> >> Ok, connected my brain and pulled now. Sorry for the inconvenience. > > I'm un-pulling this, because it creates merge conflicts with Linus's > tree due to an unwanted rebase (?) by either you or Michael. That was a deliberate cherry-pick, I applied it to my branch and then later realised it should also be on the topic branch. > In particular this commit > > commit 8bf8f2e8c786f37991bd27332c75edcc524d2232 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:16 2017 +1000 > > powerpc/64s: Revert setting of LPCR[LPES] on POWER9 > > is already present in Linus's tree with a different hash: > > commit 8d1b48ef580097e111c2644e6fc6041b9784d218 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:16 2017 +1000 > > powerpc/64s: Revert setting of LPCR[LPES] on POWER9 > > and causes conflicts with this subsequent commit: > > commit 700b7eadd5625d22b8235fb21259b3d7d564c000 > Author: Nicholas Piggin <npiggin@gmail.com> > Date: Wed Apr 19 05:12:17 2017 +1000 > > powerpc/64s: Power9 has no LPCR[VRMASD] field so don't set it You mean this conflict, or was there more? Sure it's annoying, but it's hardly the worst conflict ever. I really think Linus could have coped with it. diff --cc arch/powerpc/kernel/cpu_setup_power.S index 10cb2896b2ae,1fce4ddd2e6c..000000000000 --- a/arch/powerpc/kernel/cpu_setup_power.S +++ b/arch/powerpc/kernel/cpu_setup_power.S @@@ -108,7 -108,7 +108,11 @@@ _GLOBAL(__setup_cpu_power9 LOAD_REG_IMMEDIATE(r4, LPCR_UPRT | LPCR_HR) andc r3, r3, r4 li r4,0 /* LPES = 0 */ ++<<<<<<< HEAD + bl __init_LPCR_ISA300 ++======= + bl __init_LPCR ++>>>>>>> fb7dcf723dd2cb1d5d8f2f49c3023130938848e3 bl __init_HFSCR bl __init_tlb_power9 bl __init_PMU_HV cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 8, 2017 at 7:03 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > You mean this conflict, or was there more? Sure it's annoying, but it's > hardly the worst conflict ever. I really think Linus could have coped > with it. Yeah, I do tens of conflicts a day during the merge window, no worries about conflicts. I do tend to want to be *notified* about the conflicts, though. That's not because conflicts are bad per se, or because I'd find them hard to resolve (most are trivial). It's mostly because I want to know that both parties knew about the conflict and it was seen in -next, which just makes me feel a lot better about the it. Why? Exactly the same way I don't want submaintainers to silently fix up conflicts for me, because I want to be aware of where conflicts happened, I also want to know in turn that the submaintainers knew about it too. So to me, it's largely about *awareness*, not about "conflicts are hard to handle". I want to be aware of the conflicts, but I also want the people who caused them to be aware of them, and realize that "oops, we're stepping on each others toes". So conflicts aren't bad per se. Conflicts are really only bad if they cause subtle problems because people weren't even aware of them, or if they *keep*on* happening. The occasional random conflict that people knew about just isn't a problem. But if the same subsystem just keeps on generating conflicts with others time and again, _that_ is indicative of a real problem. And then the problem isn't the conflict per se, but the fact that two submaintainers keep on stepping on each others toes, and we need to figure out why, and how to solve it. So even then, the problem isn't about resolving the conflict itself, but about resolving whatever issue in our code flow that keeps causing that kind of annoying friction. The same is true of cherry-picking and even the occasional rebase. If a cherry-pick happens once in a blue moon and we have a duplicate commit, that's *fine*. It can cause conflicts too (perhaps the cherry-pick was followed by new development of the same area on one of the branches), but there are perfectly natural and valid reasons why the same commit could show up on multiple branches. It's only if it ends up being a "pattern" - some developer keeps on duplicating the commits in one branch that I see in other branches too - that it is a problem. That, again, indicates that there's something wrong in the process. Linus -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/05/2017 04:17, Linus Torvalds wrote: > I do tend to want to be *notified* about the conflicts, though. > > That's not because conflicts are bad per se, or because I'd find them > hard to resolve (most are trivial). It's mostly because I want to know > that both parties knew about the conflict and it was seen in -next, > which just makes me feel a lot better about the it. > > Why? Exactly the same way I don't want submaintainers to silently fix > up conflicts for me, because I want to be aware of where conflicts > happened, I also want to know in turn that the submaintainers knew > about it too. > > So to me, it's largely about *awareness*, not about "conflicts are > hard to handle". I want to be aware of the conflicts, but I also want > the people who caused them to be aware of them, and realize that > "oops, we're stepping on each others toes". Good, that was my understanding - and I wanted to single out this conflict because 1) I wasn't even sure if Paul was aware of it, since the conflict surprised me when doing my own trial merge. I'd like to be aware of arch vs. KVM conflicts, just like Linus, so that I can relay the information to him precisely; 2) I'm not sure about why PPC/KVM conflicts keep on happening. KVM on PPC seems to be more tightly integrated with non-KVM code than other architectures. _But it's also the other way round_ which seems less healthy to me. I'm not sure if that is really necessary, but if not, please refactor things so that we don't step on each other's toes unnecessarily. Perhaps real mode handlers should be in arch/powerpc/kernel/ and only arch/powerpc/kvm/, so that the latter only cares about virtualization? Maybe the whole napping and CPU thread management should be done outside arch/powerpc/kvm/? Not sure if that makes any sense, but to my eyes there are definitely too many KVM parts in hardware enablement patches (and BTW it's just crazy that PPC has about 4k lines of assembly). Maybe PPC *is* the special snowflake where interaction should be primarily with arch maintainers rather than myself. Then let's just have topic branches for the occasional API patch (new KVM_CAP_* constants would be the gist of it) and otherwise let Michael merge KVM/PPC patches. But in any case, to my eyes there _is_ a pattern, and a problem. It doesn't matter that the canary is a cherry-pick conflict that Linus could solve blinded and with one hand tied behind his back. Paolo > But if the same subsystem just keeps on generating conflicts with > others time and again, _that_ is indicative of a real problem. And > then the problem isn't the conflict per se, but the fact that two > submaintainers keep on stepping on each others toes, and we need to > figure out why, and how to solve it. So even then, the problem isn't > about resolving the conflict itself, but about resolving whatever > issue in our code flow that keeps causing that kind of annoying > friction. > > The same is true of cherry-picking and even the occasional rebase. If > a cherry-pick happens once in a blue moon and we have a duplicate > commit, that's *fine*. It can cause conflicts too (perhaps the > cherry-pick was followed by new development of the same area on one of > the branches), but there are perfectly natural and valid reasons why > the same commit could show up on multiple branches. > > It's only if it ends up being a "pattern" - some developer keeps on > duplicating the commits in one branch that I see in other branches too > - that it is a problem. That, again, indicates that there's something > wrong in the process. > > Linus > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2017-05-09 at 11:24 +0200, Paolo Bonzini wrote: > 2) I'm not sure about why PPC/KVM conflicts keep on happening. KVM on > PPC seems to be more tightly integrated with non-KVM code than other > architectures. _But it's also the other way round_ which seems less > healthy to me. I'm not sure if that is really necessary, but if not, > please refactor things so that we don't step on each other's toes > unnecessarily. Perhaps real mode handlers should be in > arch/powerpc/kernel/ and only arch/powerpc/kvm/, so that the latter only > cares about virtualization? Maybe the whole napping and CPU thread > management should be done outside arch/powerpc/kvm/? Not sure if that > makes any sense, but to my eyes there are definitely too many KVM parts > in hardware enablement patches (and BTW it's just crazy that PPC has > about 4k lines of assembly). Heh, we've been improving that lately :-) > Maybe PPC *is* the special snowflake where interaction should be > primarily with arch maintainers rather than myself. Then let's just > have topic branches for the occasional API patch (new KVM_CAP_* > constants would be the gist of it) and otherwise let Michael merge > KVM/PPC patches. Sadly it *is* a bit of a special snowflake due to mostly two things, which are the way the old hash MMU worked (the CPU wasn't designed for a hypervisor operating with MMU on among other things, so we have this whole business with doing things in "real mode"), and the way P8 forces threads to be in the same partition. We hope to be able to make a much simpler and much more classic "backend" at some point with the new radix MMU for P9, though we'll probably continue dragging some of that existing cruft as long as hash still has to be supported among other things. > But in any case, to my eyes there _is_ a pattern, and a problem. It > doesn't matter that the canary is a cherry-pick conflict that Linus > could solve blinded and with one hand tied behind his back. There is but it's more or less inherent to the CPU architecture. All I can say is we are trying to move away from a lot of that crap with the new MMU, but that will take time. On thing that will help too is that we have been working hard to get full hypervisor level backward compatibility for P9 onward. So hopefully, there will be less overlap between pure enablement and KVM in the future for that reason too. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/05/2017 15:31, Benjamin Herrenschmidt wrote: >> Maybe PPC *is* the special snowflake where interaction should be >> primarily with arch maintainers rather than myself. Then let's just >> have topic branches for the occasional API patch (new KVM_CAP_* >> constants would be the gist of it) and otherwise let Michael merge >> KVM/PPC patches. > > Sadly it *is* a bit of a special snowflake Thanks for confirming! ;) Anyhow, let's keep doing things like this for a few release cycles. Michael and Paul can keep doing topic branches like they have done for other recent releases; I'll communicate more with Paul, who however shouldn't be worried if I don't pull timely from him or if I un-pull after seeing unexpected conflicts. When this happens, it just means that I prefer Michael to go first. Patches are in linux-next anyway, and it's not rare at all that I send two KVM pull request per merge window. And I'll keep reporting conflicts if they happen so that Linus can keep an eye on what's going on. If things don't improve, and I'd hope they do based on Ben's message, we can have more work go through Michael, but that would be a last resort. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 09, 2017 at 11:24:49AM +0200, Paolo Bonzini wrote: > > Good, that was my understanding - and I wanted to single out this > conflict because > > 1) I wasn't even sure if Paul was aware of it, since the conflict > surprised me when doing my own trial merge. I'd like to be aware of > arch vs. KVM conflicts, just like Linus, so that I can relay the > information to him precisely; Stephen did mention it to me, but I didn't have it in mind when I sent the pull request. Sorry about that. > 2) I'm not sure about why PPC/KVM conflicts keep on happening. KVM on > PPC seems to be more tightly integrated with non-KVM code than other > architectures. _But it's also the other way round_ which seems less > healthy to me. I'm not sure if that is really necessary, but if not, > please refactor things so that we don't step on each other's toes > unnecessarily. Perhaps real mode handlers should be in > arch/powerpc/kernel/ and only arch/powerpc/kvm/, so that the latter only > cares about virtualization? Maybe the whole napping and CPU thread > management should be done outside arch/powerpc/kvm/? Not sure if that > makes any sense, but to my eyes there are definitely too many KVM parts > in hardware enablement patches (and BTW it's just crazy that PPC has > about 4k lines of assembly). The last two merge windows have seen a lot of changes in the PPC code, both generally and for KVM, because of the POWER9 support coming in. Compared to POWER8, POWER9 has a lot of very significant architecture changes specifically designed to benefit Linux and KVM, which is wonderful, but has also meant a lot of code changes. We're working on moving the assembly code to C where possible, and as Ben said, things are actually getting a lot simpler on POWER9 (though of course we'll have to keep supporting POWER8 for a while yet). > Maybe PPC *is* the special snowflake where interaction should be > primarily with arch maintainers rather than myself. Then let's just > have topic branches for the occasional API patch (new KVM_CAP_* > constants would be the gist of it) and otherwise let Michael merge > KVM/PPC patches. There are certainly parts of the PPC KVM code that would naturally be considered as part of low-level machine enablement, and I think it does make sense for them to go via Michael. > But in any case, to my eyes there _is_ a pattern, and a problem. It > doesn't matter that the canary is a cherry-pick conflict that Linus > could solve blinded and with one hand tied behind his back. Point taken. Cheers, Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: > On 09/05/2017 04:17, Linus Torvalds wrote: >> I do tend to want to be *notified* about the conflicts, though. >> >> That's not because conflicts are bad per se, or because I'd find them >> hard to resolve (most are trivial). It's mostly because I want to know >> that both parties knew about the conflict and it was seen in -next, >> which just makes me feel a lot better about the it. >> >> Why? Exactly the same way I don't want submaintainers to silently fix >> up conflicts for me, because I want to be aware of where conflicts >> happened, I also want to know in turn that the submaintainers knew >> about it too. >> >> So to me, it's largely about *awareness*, not about "conflicts are >> hard to handle". I want to be aware of the conflicts, but I also want >> the people who caused them to be aware of them, and realize that >> "oops, we're stepping on each others toes". > > Good, that was my understanding - and I wanted to single out this > conflict because > > 1) I wasn't even sure if Paul was aware of it, since the conflict > surprised me when doing my own trial merge. I'd like to be aware of > arch vs. KVM conflicts, just like Linus, so that I can relay the > information to him precisely; Sure. In this case I think the conflict might have appeared after Paul sent the original pull request to you, because it took a while before you merged it. There was also the issue with needing to add the include of debugfs.h which was more subtle and I think distracted us from the simple conflict in the asm. > 2) I'm not sure about why PPC/KVM conflicts keep on happening. KVM on > PPC seems to be more tightly integrated with non-KVM code than other > architectures. _But it's also the other way round_ which seems less > healthy to me. I'm not sure if that is really necessary, but if not, > please refactor things so that we don't step on each other's toes > unnecessarily. Perhaps real mode handlers should be in > arch/powerpc/kernel/ and only arch/powerpc/kvm/, so that the latter only > cares about virtualization? Maybe the whole napping and CPU thread > management should be done outside arch/powerpc/kvm/? Not sure if that > makes any sense, but to my eyes there are definitely too many KVM parts > in hardware enablement patches (and BTW it's just crazy that PPC has > about 4k lines of assembly). I can't really say if it's more tightly integrated, I haven't looked that closely at other arches. What has happened is we've added support to the arch code and KVM for a new CPU version, which is not entirely backward compatible at the hypervisor level, support for an entirely new MMU which has implications for KVM, and for a new interrupt controller architecture. There've been some conflicts in the process, but I don't think we've done too badly. I suspect if x86 added support for a hash table MMU there might be some conflicts with KVM in the process ;) > Maybe PPC *is* the special snowflake where interaction should be > primarily with arch maintainers rather than myself. Then let's just > have topic branches for the occasional API patch (new KVM_CAP_* > constants would be the gist of it) and otherwise let Michael merge > KVM/PPC patches. In hindsight for the last couple of merge windows that probably would have worked better. But it's a bit late now :) I do wonder if at some point the relationship between the arch trees and KVM should flip. Something similar happened with perf, the hardware specific parts used to be merged through the perf tree, but now they go via arch maintainers. I'm not saying KVM is ready for that, but it might be one day. > But in any case, to my eyes there _is_ a pattern, and a problem. It > doesn't matter that the canary is a cherry-pick conflict that Linus > could solve blinded and with one hand tied behind his back. I don't think that there's really a "problem". It's not that Paul and I are oblivious to each other and "stepping on each other's toes", it's just we're working on areas of the code that are interdependent and sometimes there are conflicts. We could avoid all conflicts by forcing Paul to merge all his arch code one release before he merges the KVM code that needs it, but that would be silly IMHO. cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: > > Anyhow, let's keep doing things like this for a few release cycles. Sure. I think a lot of the churn for Power9 has already happened, so things should settle down hopefully. > Michael and Paul can keep doing topic branches like they have done for > other recent releases; I'll communicate more with Paul, who however > shouldn't be worried if I don't pull timely from him or if I un-pull > after seeing unexpected conflicts. When this happens, it just means > that I prefer Michael to go first. OK. In the past I've actually been waiting for you to merge Paul's tree, because I don't want to merge the shared code in case you Nak something in his tree and we have to back things out. But I'll stop doing that unless there's something in particular we are worried about. cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Linus Torvalds <torvalds@linux-foundation.org> writes: > On Mon, May 8, 2017 at 7:03 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: >> You mean this conflict, or was there more? Sure it's annoying, but it's >> hardly the worst conflict ever. I really think Linus could have coped >> with it. ... > The occasional random conflict that people knew about just isn't a problem. > > But if the same subsystem just keeps on generating conflicts with > others time and again, _that_ is indicative of a real problem. And > then the problem isn't the conflict per se, but the fact that two > submaintainers keep on stepping on each others toes, and we need to > figure out why, and how to solve it. So even then, the problem isn't > about resolving the conflict itself, but about resolving whatever > issue in our code flow that keeps causing that kind of annoying > friction. Thanks for the explanation. We have had some small conflicts over the last few releases, but I don't think it's really indicative of a problem. It's certainly not that Paul and I are oblivious of each other - he pesters me endlessly - but just that the code is intertwined. Perhaps we can organise the code better to avoid problems in future, and now that a lot of the changes for Power9 are in, things should hopefully get better anyway. cheers -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo, Please do a second pull from my kvm-ppc-next branch. The main thing here is a new implementation of the in-kernel XICS interrupt controller emulation for POWER9 machines, from Ben Herrenschmidt. POWER9 has a new interrupt controller called XIVE (eXternal Interrupt Virtualization Engine) which is able to deliver interrupts directly to guest virtual CPUs in hardware without hypervisor intervention. With this new code, the guest still sees the old XICS interface but performance is better because the XICS emulation in the host uses the XIVE directly rather than going through a XICS emulation in firmware. The series that implements this was committed to the topic/xive branch in the powerpc tree by Michael Ellerman, and I have merged that into my kvm-ppc-next branch. I would like this to go into 4.12. I know this pull request is somewhat late, but that is mostly because I had 3 weeks vacation this month. I have tested this on POWER9 and also tested that it doesn't break other platforms such as POWER8. The only other thing apart from that is a trivial warning fix. Thanks, Paul. The following changes since commit bd17117bb2af60d4bc44e0f9c859e800a3d99722: Merge tag 'kvm-s390-next-4.12-2' of git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux into HEAD (2017-04-21 12:47:47 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc kvm-ppc-next for you to fetch changes up to fb7dcf723dd2cb1d5d8f2f49c3023130938848e3: Merge remote-tracking branch 'remotes/powerpc/topic/xive' into kvm-ppc-next (2017-04-28 08:23:16 +1000) ---------------------------------------------------------------- Benjamin Herrenschmidt (11): powerpc/powernv: Add XIVE related definitions to opal-api.h powerpc: Add more PPC bit conversion macros powerpc: Add optional smp_ops->prepare_cpu SMP callback powerpc/smp: Remove migrate_irq() custom implementation powerpc/xive: Native exploitation of the XIVE interrupt controller powerpc/kvm: Massage order of #include powerpc/kvm: Make kvmppc_xics_create_icp static powerpc/kvm: Remove obsolete kvm_vm_ioctl_xics_irq declaration powerpc: Consolidate variants of real-mode MMIOs powerpc: Fixup LPCR:PECE and HEIC setting on POWER9 KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller Denis Kirjanov (1): KVM: PPC: Book3S HV: Avoid preemptibility warning in module initialization Nicholas Piggin (1): powerpc/64s: Revert setting of LPCR[LPES] on POWER9 Paul Mackerras (1): Merge remote-tracking branch 'remotes/powerpc/topic/xive' into kvm-ppc-next arch/powerpc/include/asm/bitops.h | 8 + arch/powerpc/include/asm/io.h | 98 +- arch/powerpc/include/asm/kvm_book3s_asm.h | 4 +- arch/powerpc/include/asm/kvm_host.h | 28 +- arch/powerpc/include/asm/kvm_ppc.h | 84 +- arch/powerpc/include/asm/opal-api.h | 74 +- arch/powerpc/include/asm/opal.h | 36 + arch/powerpc/include/asm/reg.h | 1 + arch/powerpc/include/asm/smp.h | 2 +- arch/powerpc/include/asm/xive-regs.h | 97 ++ arch/powerpc/include/asm/xive.h | 162 ++ arch/powerpc/include/asm/xmon.h | 2 + arch/powerpc/kernel/asm-offsets.c | 10 + arch/powerpc/kernel/cpu_setup_power.S | 15 +- arch/powerpc/kernel/irq.c | 40 - arch/powerpc/kernel/smp.c | 19 +- arch/powerpc/kvm/Kconfig | 5 + arch/powerpc/kvm/Makefile | 4 +- arch/powerpc/kvm/book3s.c | 83 +- arch/powerpc/kvm/book3s_hv.c | 69 +- arch/powerpc/kvm/book3s_hv_builtin.c | 132 +- arch/powerpc/kvm/book3s_hv_rm_xics.c | 14 +- arch/powerpc/kvm/book3s_hv_rm_xive.c | 47 + arch/powerpc/kvm/book3s_hv_rmhandlers.S | 62 +- arch/powerpc/kvm/book3s_rtas.c | 21 +- arch/powerpc/kvm/book3s_xics.c | 37 +- arch/powerpc/kvm/book3s_xics.h | 7 + arch/powerpc/kvm/book3s_xive.c | 1893 ++++++++++++++++++++++++ arch/powerpc/kvm/book3s_xive.h | 256 ++++ arch/powerpc/kvm/book3s_xive_template.c | 503 +++++++ arch/powerpc/kvm/irq.h | 1 + arch/powerpc/kvm/powerpc.c | 17 +- arch/powerpc/platforms/Kconfig.cputype | 1 + arch/powerpc/platforms/powernv/Kconfig | 1 + arch/powerpc/platforms/powernv/opal-wrappers.S | 15 + arch/powerpc/platforms/powernv/opal.c | 1 + arch/powerpc/platforms/powernv/rng.c | 2 +- arch/powerpc/platforms/powernv/setup.c | 15 +- arch/powerpc/platforms/powernv/smp.c | 39 +- arch/powerpc/sysdev/Kconfig | 1 + arch/powerpc/sysdev/Makefile | 1 + arch/powerpc/sysdev/xics/icp-native.c | 8 +- arch/powerpc/sysdev/xive/Kconfig | 11 + arch/powerpc/sysdev/xive/Makefile | 4 + arch/powerpc/sysdev/xive/common.c | 1432 ++++++++++++++++++ arch/powerpc/sysdev/xive/native.c | 715 +++++++++ arch/powerpc/sysdev/xive/xive-internal.h | 62 + arch/powerpc/xmon/xmon.c | 94 +- include/linux/kvm_host.h | 1 - virt/kvm/kvm_main.c | 4 - 50 files changed, 6014 insertions(+), 224 deletions(-) create mode 100644 arch/powerpc/include/asm/xive-regs.h create mode 100644 arch/powerpc/include/asm/xive.h create mode 100644 arch/powerpc/kvm/book3s_hv_rm_xive.c create mode 100644 arch/powerpc/kvm/book3s_xive.c create mode 100644 arch/powerpc/kvm/book3s_xive.h create mode 100644 arch/powerpc/kvm/book3s_xive_template.c create mode 100644 arch/powerpc/sysdev/xive/Kconfig create mode 100644 arch/powerpc/sysdev/xive/Makefile create mode 100644 arch/powerpc/sysdev/xive/common.c create mode 100644 arch/powerpc/sysdev/xive/native.c create mode 100644 arch/powerpc/sysdev/xive/xive-internal.h -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html