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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]

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.

-- 
Cheers,
Stephen Rothwell

diff --cc arch/x86/kvm/svm/sev.c
index 63923fa0b172,214eefb20414..000000000000
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@@ -93,10 -103,21 +109,21 @@@ static bool __sev_recycle_asids(int min
  	return true;
  }
  
 -static int sev_asid_new(struct kvm_sev_info *sev)
 +static int sev_asid_new(bool es_active)
  {
- 	int pos, min_asid, max_asid;
+ 	int pos, min_asid, max_asid, ret;
  	bool retry = true;
+ 	enum misc_res_type type;
+ 
+ 	type = sev->es_active ? MISC_CG_RES_SEV_ES : MISC_CG_RES_SEV;
+ 	WARN_ON(sev->misc_cg);
+ 	sev->misc_cg = get_current_misc_cg();
+ 	ret = misc_cg_try_charge(type, sev->misc_cg, 1);
+ 	if (ret) {
+ 		put_misc_cg(sev->misc_cg);
+ 		sev->misc_cg = NULL;
+ 		return ret;
+ 	}
  
  	mutex_lock(&sev_bitmap_lock);
  
@@@ -182,17 -224,16 +221,17 @@@ static int sev_guest_init(struct kvm *k
  	if (unlikely(sev->active))
  		return ret;
  
 -	asid = sev_asid_new(sev);
 +	asid = sev_asid_new(es_active);
  	if (asid < 0)
  		return ret;
+ 	sev->asid = asid;
  
  	ret = sev_platform_init(&argp->error);
  	if (ret)
  		goto e_free;
  
  	sev->active = true;
 +	sev->es_active = es_active;
- 	sev->asid = asid;
  	INIT_LIST_HEAD(&sev->regions_list);
  
  	return 0;

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Paolo Bonzini]

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

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Vipin Sharma]

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

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Paolo Bonzini]

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

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Vipin Sharma]

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

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Paolo Bonzini]

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

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Vipin Sharma]

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.

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Tejun Heo]

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.

-- 
tejun

Re: linux-next: manual merge of the cgroup tree with the kvm tree

[Paolo Bonzini]

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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]

Hi all,

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

  arch/x86/kvm/svm/svm.h

between commit:

  54526d1fd593 ("KVM: x86: Support KVM VMs sharing SEV context")

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.

-- 
Cheers,
Stephen Rothwell

diff --cc arch/x86/kvm/svm/svm.h
index 454da1c1d9b7,9806aaebc37f..000000000000
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@@ -68,7 -65,7 +68,8 @@@ struct kvm_sev_info 
  	unsigned long pages_locked; /* Number of pages locked */
  	struct list_head regions_list;  /* List of registered regions */
  	u64 ap_jump_table;	/* SEV-ES AP Jump Table address */
 +	struct kvm *enc_context_owner; /* Owner of copied encryption context */
+ 	struct misc_cg *misc_cg; /* For misc cgroup accounting */
  };
  
  struct kvm_svm {

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]