From: Peter Xu <peterx@redhat.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Jay Zhou <jianjay.zhou@huawei.com>,
kvm@vger.kernel.org, pbonzini@redhat.com,
wangxinxin.wang@huawei.com, weidong.huang@huawei.com,
liu.jinsong@huawei.com
Subject: Re: [PATCH v2] KVM: x86: enable dirty log gradually in small chunks
Date: Thu, 20 Feb 2020 14:42:29 -0500 [thread overview]
Message-ID: <20200220194229.GB15253@xz-x1> (raw)
In-Reply-To: <20200220192350.GG3972@linux.intel.com>
On Thu, Feb 20, 2020 at 11:23:50AM -0800, Sean Christopherson wrote:
> On Thu, Feb 20, 2020 at 02:17:06PM -0500, Peter Xu wrote:
> > On Thu, Feb 20, 2020 at 12:28:28PM +0800, Jay Zhou wrote:
> > > @@ -3348,7 +3352,14 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm *kvm,
> > > switch (cap->cap) {
> > > #ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT
> > > case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2:
> > > - if (cap->flags || (cap->args[0] & ~1))
> > > + if (cap->flags ||
> > > + (cap->args[0] & ~KVM_DIRTY_LOG_MANUAL_CAPS) ||
> > > + /* The capability of KVM_DIRTY_LOG_INITIALLY_SET depends
> > > + * on KVM_DIRTY_LOG_MANUAL_PROTECT, it should not be
> > > + * set individually
> > > + */
> > > + ((cap->args[0] & KVM_DIRTY_LOG_MANUAL_CAPS) ==
> > > + KVM_DIRTY_LOG_INITIALLY_SET))
> >
> > How about something easier to read? :)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 70f03ce0e5c1..9dfbab2a9929 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -3348,7 +3348,10 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm *kvm,
> > switch (cap->cap) {
> > #ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT
> > case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2:
> > - if (cap->flags || (cap->args[0] & ~1))
> > + if (cap->flags || (cap->args[0] & ~3))
> > + return -EINVAL;
> > + /* Allow 00, 01, and 11. */
> > + if (cap->args[0] == KVM_DIRTY_LOG_INITIALLY_SET)
> > return -EINVAL;
>
> Oof, "easier" is subjective :-) How about this?
>
> case KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2: {
> u64 allowed_options = KVM_DIRTY_LOG_MANUAL_PROTECT;
>
> if (cap->args[0] & KVM_DIRTY_LOG_MANUAL_PROTECT)
> allowed_options = KVM_DIRTY_LOG_INITIALLY_SET;
>
> if (cap->flags || (cap->args[0] & ~allowed_options))
> return -EINVAL;
> kvm->manual_dirty_log_protect = cap->args[0];
> return 0;
> }
Fine by me! (But I won't tell I still prefer mine ;)
--
Peter Xu
next prev parent reply other threads:[~2020-02-20 19:42 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 4:28 [PATCH v2] KVM: x86: enable dirty log gradually in small chunks Jay Zhou
2020-02-20 19:17 ` Peter Xu
2020-02-20 19:23 ` Sean Christopherson
2020-02-20 19:42 ` Peter Xu [this message]
2020-02-21 9:43 ` Zhoujian (jay)
2020-02-21 9:31 ` Zhoujian (jay)
2020-02-21 15:19 ` Peter Xu
2020-02-22 8:11 ` Zhoujian (jay)
2020-02-20 19:28 ` Peter Xu
2020-02-21 9:53 ` Zhoujian (jay)
2020-02-21 15:41 ` Peter Xu
2020-02-22 8:18 ` Zhoujian (jay)
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=20200220194229.GB15253@xz-x1 \
--to=peterx@redhat.com \
--cc=jianjay.zhou@huawei.com \
--cc=kvm@vger.kernel.org \
--cc=liu.jinsong@huawei.com \
--cc=pbonzini@redhat.com \
--cc=sean.j.christopherson@intel.com \
--cc=wangxinxin.wang@huawei.com \
--cc=weidong.huang@huawei.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;
as well as URLs for NNTP newsgroup(s).