| Message ID | 20210422155355.471c7751@canb.auug.org.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-next: manual merge of the cgroup tree with the kvm tree | expand |
On 22/04/21 07:53, Stephen Rothwell wrote: > Hi all, > > Today's linux-next merge of the cgroup tree got conflicts in: > > arch/x86/kvm/svm/sev.c > > between commit: > > 9fa1521daafb ("KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes") > > from the kvm tree and commit: > > 7aef27f0b2a8 ("svm/sev: Register SEV and SEV-ES ASIDs to the misc controller") > > from the cgroup tree. > > I fixed it up (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. > Tejun, please don't commit patches to other tree without an Acked-by from the maintainer (which I wouldn't have provided, as the right way to go would have been a topic branch). Fortunately these patches are at the bottom of your tree. If it's okay, I'll just pull from there "as if" you had provided a topic branch all the time. Thanks, Paolo
On Wed, Apr 21, 2021 at 11:34 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 22/04/21 07:53, Stephen Rothwell wrote: > > Hi all, > > > > Today's linux-next merge of the cgroup tree got conflicts in: > > > > arch/x86/kvm/svm/sev.c > > > > between commit: > > > > 9fa1521daafb ("KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes") > > > > from the kvm tree and commit: > > > > 7aef27f0b2a8 ("svm/sev: Register SEV and SEV-ES ASIDs to the misc controller") > > > > from the cgroup tree. > > > > I fixed it up (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. > > > > Tejun, please don't commit patches to other tree without an Acked-by > from the maintainer (which I wouldn't have provided, as the right way to > go would have been a topic branch). > > Fortunately these patches are at the bottom of your tree. If it's okay, > I'll just pull from there "as if" you had provided a topic branch all > the time. > > Thanks, > > Paolo > First of all, I am sorry that my patch series has caused this trouble to all of you. I am not aware of the correct way to submit a patch series which changes files in more than one maintainer's territory. Any guidance for the future will be helpful. Paolo, Stephen, We need a little more fix in the sev_asid_free() function for Stephen's changes to work correctly as es_active is used in that function also. Is there a repo and branch where I can see the final state of merges and then I can send my patch against that? Thanks Vipin
On 22/04/21 09:33, Vipin Sharma wrote: > First of all, I am sorry that my patch series has caused this trouble to > all of you. I am not aware of the correct way to submit a patch series > which changes files in more than one maintainer's territory. Any > guidance for the future will be helpful. Vipin, don't worry as you don't have to do anything (and didn't do anything wrong, for that matter). It's all in the maintainers' hands; Stephen takes care of warning maintainers of what will happen in the next merge window, but as far as you're concerned you're all set. You can check the current state of the merge in the queue branch of the KVM tree. This is what I plan to merge if Tejun agrees. That would be helpful indeed! Paolo
Hello, Paolo. On Thu, Apr 22, 2021 at 08:34:15AM +0200, Paolo Bonzini wrote: > Tejun, please don't commit patches to other tree without an Acked-by from > the maintainer (which I wouldn't have provided, as the right way to go would > have been a topic branch). My apologies, for some reason, I was incorrectly assuming it was all dandy on the kvm side. > Fortunately these patches are at the bottom of your tree. If it's okay, > I'll just pull from there "as if" you had provided a topic branch all the > time. I'd be happy with however you wanna resolve it. Please let me know if there's anything I can do to help. Thanks.
On 22/04/21 13:48, Tejun Heo wrote: > Hello, Paolo. > > On Thu, Apr 22, 2021 at 08:34:15AM +0200, Paolo Bonzini wrote: >> Tejun, please don't commit patches to other tree without an Acked-by from >> the maintainer (which I wouldn't have provided, as the right way to go would >> have been a topic branch). > > My apologies, for some reason, I was incorrectly assuming it was all dandy > on the kvm side. It was, just full of conflicts. :) It's only a process thing and not a problem with the code. >> Fortunately these patches are at the bottom of your tree. If it's okay, >> I'll just pull from there "as if" you had provided a topic branch all the >> time. > > I'd be happy with however you wanna resolve it. Please let me know if > there's anything I can do to help. Great, I've pulled from your tree then. Stephen, that will also fix the conflicts with Linus's tree, since Tejun's merge base was newer than mine. Paolo
On Thu, Apr 22, 2021 at 12:47 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > You can check the current state of the merge in the queue branch of the > KVM tree. This is what I plan to merge if Tejun agrees. That would be > helpful indeed! Merge looks fine from my patch perspective. However, one thing is missing: In sev_guest_init() after sev_asid_free() call we should also write set sev->es_false = false. Without this the main intent of Sean's patch will be missing in the merge. I can send you the patch but just wanted to know if that will be right because originally it is Sean's fix and I am not sure how to give him credit in my patch. May be Reported-By? Thanks Vipin
On 22/04/21 19:09, Vipin Sharma wrote: > On Thu, Apr 22, 2021 at 12:47 AM Paolo Bonzini <pbonzini@redhat.com> wrote: >> You can check the current state of the merge in the queue branch of the >> KVM tree. This is what I plan to merge if Tejun agrees. That would be >> helpful indeed! > > Merge looks fine from my patch perspective. However, one thing is missing: > > In sev_guest_init() after sev_asid_free() call we should also write > set sev->es_false = false. > > Without this the main intent of Sean's patch will be missing in the merge. So this: diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 3539201278bd..2632852be856 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -224,7 +224,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) sev->es_active = es_active; asid = sev_asid_new(sev); if (asid < 0) - return ret; + goto e_no_asid; sev->asid = asid; ret = sev_platform_init(&argp->error); @@ -240,6 +240,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) e_free: sev_asid_free(sev); sev->asid = 0; +e_no_asid: + sev->es_active = false; return ret; } Sounds good, I'll squash it and push to kvm.git. Paolo
On Thu, Apr 22, 2021 at 10:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 22/04/21 19:09, Vipin Sharma wrote: > > On Thu, Apr 22, 2021 at 12:47 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > >> You can check the current state of the merge in the queue branch of the > >> KVM tree. This is what I plan to merge if Tejun agrees. That would be > >> helpful indeed! > > > > Merge looks fine from my patch perspective. However, one thing is missing: > > > > In sev_guest_init() after sev_asid_free() call we should also write > > set sev->es_false = false. > > > > Without this the main intent of Sean's patch will be missing in the merge. > > So this: > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 3539201278bd..2632852be856 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -224,7 +224,7 @@ static int sev_guest_init(struct kvm *kvm, struct > kvm_sev_cmd *argp) > sev->es_active = es_active; > asid = sev_asid_new(sev); > if (asid < 0) > - return ret; > + goto e_no_asid; > sev->asid = asid; > > ret = sev_platform_init(&argp->error); > @@ -240,6 +240,8 @@ static int sev_guest_init(struct kvm *kvm, struct > kvm_sev_cmd *argp) > e_free: > sev_asid_free(sev); > sev->asid = 0; > +e_no_asid: > + sev->es_active = false; > return ret; > } > > > Sounds good, I'll squash it and push to kvm.git. > > Paolo Thanks. This looks good.
diff --cc arch/x86/kvm/svm/sev.c index 63923fa0b172,214eefb20414..000000000000 --- a/arch/x86/kvm/svm/sev.c
Hi all, Today's linux-next merge of the cgroup tree got conflicts in: arch/x86/kvm/svm/sev.c between commit: 9fa1521daafb ("KVM: SVM: Do not set sev->es_active until KVM_SEV_ES_INIT completes") from the kvm tree and commit: 7aef27f0b2a8 ("svm/sev: Register SEV and SEV-ES ASIDs to the misc controller") from the cgroup tree. I fixed it up (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.