public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, pbonzini@redhat.com
Subject: Re: [PATCH] KVM: do not prepare new memslot for KVM_MR_DELETE
Date: Thu, 10 Nov 2022 01:53:47 +0000	[thread overview]
Message-ID: <Y2xZq8Xn0Bk4de+R@google.com> (raw)
In-Reply-To: <20221110012204.3919-1-yan.y.zhao@intel.com>

On Thu, Nov 10, 2022, Yan Zhao wrote:
> kvm_prepare_memory_region() is not useful for KVM_MR_DELETE,
> and each kvm_arch_prepare_memory_region() does nothing more than returning
> 0 for KVM_MR_DELETE.

This is not true, s390 has an error path that fires in the DELETE case.

	/* When we are protected, we should not change the memory slots */
	if (kvm_s390_pv_get_handle(kvm))
		return -EINVAL;

	if (change == KVM_MR_DELETE || change == KVM_MR_FLAGS_ONLY)
		return 0;


> So, just don't call into kvm_prepare_memory_region() to avoid unnecessary
> error handling for KVM_MR_DELETE.

Even if the s390 case didn't exit, I would still prefer keeping the code as is.
I agree that a "dummy" call is somewhat confusing, but skipping the arch call
for one operation is equally confusing and could easily lead to future bugs,
e.g. if the order of changes were reversed and an s390 developer wasn't aware
that s390's arch hook would be bypassed on memslot deletion.

  reply	other threads:[~2022-11-10  1:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10  1:22 [PATCH] KVM: do not prepare new memslot for KVM_MR_DELETE Yan Zhao
2022-11-10  1:53 ` Sean Christopherson [this message]
2022-11-10  2:14   ` Yan Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2xZq8Xn0Bk4de+R@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=yan.y.zhao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox