| Message ID | 20231004174628.2073263-1-paul@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: x86/xen: ignore the VCPU_SSHOTTMR_future flag | expand |
On Wed, Oct 04, 2023, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > Upstream Xen now ignores this flag [1], since the only guest kernel ever to > use it was buggy. By ignoring the flag the guest will always get a callback > if it sets a negative timeout which upstream Xen has determined not to > cause problems for any guest setting the flag. > > [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=19c6cbd909 > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > --- > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: x86@kernel.org If you're going to manually Cc folks, put the Cc's in the changelog proper so that there's a record of who was Cc'd on the patch. Or even better, just use scripts/get_maintainers.pl and only manually Cc people when necessary.
On Wed, 04 Oct 2023 17:46:28 +0000, Paul Durrant wrote: > Upstream Xen now ignores this flag [1], since the only guest kernel ever to > use it was buggy. By ignoring the flag the guest will always get a callback > if it sets a negative timeout which upstream Xen has determined not to > cause problems for any guest setting the flag. > > [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=19c6cbd909 > > [...] Applied to kvm-x86 xen, thanks! [1/1] KVM: x86/xen: ignore the VCPU_SSHOTTMR_future flag https://github.com/kvm-x86/linux/commit/409f2e92a27a -- https://github.com/kvm-x86/linux/tree/next
On 04/10/2023 19:30, Sean Christopherson wrote: > On Wed, Oct 04, 2023, Paul Durrant wrote: >> From: Paul Durrant <pdurrant@amazon.com> >> >> Upstream Xen now ignores this flag [1], since the only guest kernel ever to >> use it was buggy. By ignoring the flag the guest will always get a callback >> if it sets a negative timeout which upstream Xen has determined not to >> cause problems for any guest setting the flag. >> >> [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=19c6cbd909 >> >> Signed-off-by: Paul Durrant <pdurrant@amazon.com> >> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> >> --- >> Cc: David Woodhouse <dwmw2@infradead.org> >> Cc: Sean Christopherson <seanjc@google.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: x86@kernel.org > > If you're going to manually Cc folks, put the Cc's in the changelog proper so that > there's a record of who was Cc'd on the patch. > FTR, the basic list was generated: ./scripts/get_maintainer.pl --no-rolestats 0001-KVM-xen-ignore-the-VCPU_SSHOTTMR_future-flag.patch | while read line; do echo Cc: $line; done and then lightly hacked put x86 at the end and remove my own name... so not really manual. Also not entirely sure why you'd want the Cc list making it into the actual commit. > Or even better, just use scripts/get_maintainers.pl and only manually Cc people > when necessary. I guess this must be some other way of using get_maintainers.pl that you are expecting?
On Thu, Oct 05, 2023, Paul Durrant wrote: > On 04/10/2023 19:30, Sean Christopherson wrote: > > On Wed, Oct 04, 2023, Paul Durrant wrote: > > > --- > > > Cc: David Woodhouse <dwmw2@infradead.org> > > > Cc: Sean Christopherson <seanjc@google.com> > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > > Cc: Ingo Molnar <mingo@redhat.com> > > > Cc: Borislav Petkov <bp@alien8.de> > > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > > Cc: x86@kernel.org > > > > If you're going to manually Cc folks, put the Cc's in the changelog proper so that > > there's a record of who was Cc'd on the patch. > > > > FTR, the basic list was generated: > > ./scripts/get_maintainer.pl --no-rolestats > 0001-KVM-xen-ignore-the-VCPU_SSHOTTMR_future-flag.patch | while read line; > do echo Cc: $line; done > > and then lightly hacked put x86 at the end and remove my own name... so not > really manual. > Also not entirely sure why you'd want the Cc list making it into the actual > commit. It's useful for Cc's that *don't* come from get_maintainers, as it provides a record in the commit of who was Cc'd on a patch. E.g. if someone encounters an issue with a commit, the Cc records provide additional contacts that might be able to help sort things out. Or if a maintainer further up the stream has questions or concerns about a pull request, they can use the Cc list to grab the right audience for a discussion, or be more confident in merging the request because the maintainer knows that the "right" people at least saw the patch. Lore links provide much of that functionality, but following a link is almost always slower, and some maintainers are allergic to web browsers :-) > > Or even better, just use scripts/get_maintainers.pl and only manually Cc people > > when necessary. > > I guess this must be some other way of using get_maintainers.pl that you are > expecting? Ah, I was just assuming that you were handcoding the Cc "list", but it sounds like you're piping the results into each patch. That's fine, just a bit noisy and uncommon. FWIW, my scripts gather the To/Cc for all patches in a series, and then use the results for the entire series, e.g. git send-email --confirm=always --suppress-cc=all $to $bcc $cc ... That way everyone that gets sent mail gets all patches in a series. Most contributors, myself included, don't like to receive bits and pieces of a series, e.g. it makes doing quick triage/reviews annoying, especially if the patches I didn't receive weren't sent to any of the mailing list to which I'm subscribed.
On 06/10/2023 02:48, Sean Christopherson wrote: > On Thu, Oct 05, 2023, Paul Durrant wrote: >> On 04/10/2023 19:30, Sean Christopherson wrote: >>> On Wed, Oct 04, 2023, Paul Durrant wrote: >>>> --- >>>> Cc: David Woodhouse <dwmw2@infradead.org> >>>> Cc: Sean Christopherson <seanjc@google.com> >>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>> Cc: Thomas Gleixner <tglx@linutronix.de> >>>> Cc: Ingo Molnar <mingo@redhat.com> >>>> Cc: Borislav Petkov <bp@alien8.de> >>>> Cc: Dave Hansen <dave.hansen@linux.intel.com> >>>> Cc: "H. Peter Anvin" <hpa@zytor.com> >>>> Cc: x86@kernel.org >>> >>> If you're going to manually Cc folks, put the Cc's in the changelog proper so that >>> there's a record of who was Cc'd on the patch. >>> >> >> FTR, the basic list was generated: >> >> ./scripts/get_maintainer.pl --no-rolestats >> 0001-KVM-xen-ignore-the-VCPU_SSHOTTMR_future-flag.patch | while read line; >> do echo Cc: $line; done >> >> and then lightly hacked put x86 at the end and remove my own name... so not >> really manual. >> Also not entirely sure why you'd want the Cc list making it into the actual >> commit. > > It's useful for Cc's that *don't* come from get_maintainers, as it provides a > record in the commit of who was Cc'd on a patch. > > E.g. if someone encounters an issue with a commit, the Cc records provide additional > contacts that might be able to help sort things out. > > Or if a maintainer further up the stream has questions or concerns about a pull > request, they can use the Cc list to grab the right audience for a discussion, > or be more confident in merging the request because the maintainer knows that the > "right" people at least saw the patch. > > Lore links provide much of that functionality, but following a link is almost > always slower, and some maintainers are allergic to web browsers :-) > Ok... makes sense. >>> Or even better, just use scripts/get_maintainers.pl and only manually Cc people >>> when necessary. >> >> I guess this must be some other way of using get_maintainers.pl that you are >> expecting? > > Ah, I was just assuming that you were handcoding the Cc "list", but it sounds > like you're piping the results into each patch. That's fine, just a bit noisy > and uncommon. > > FWIW, my scripts gather the To/Cc for all patches in a series, and then use the > results for the entire series, e.g. > > git send-email --confirm=always --suppress-cc=all $to $bcc $cc ... > > That way everyone that gets sent mail gets all patches in a series. Most > contributors, myself included, don't like to receive bits and pieces of a series, > e.g. it makes doing quick triage/reviews annoying, especially if the patches I > didn't receive weren't sent to any of the mailing list to which I'm subscribed. Ok, I'll send stuff that way in future. Thanks, Paul
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index 40edf4d1974c..8f1d46df0f3b 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -1374,12 +1374,8 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd, return true; } + /* A delta <= 0 results in an immediate callback, which is what we want */ delta = oneshot.timeout_abs_ns - get_kvmclock_ns(vcpu->kvm); - if ((oneshot.flags & VCPU_SSHOTTMR_future) && delta < 0) { - *r = -ETIME; - return true; - } - kvm_xen_start_timer(vcpu, oneshot.timeout_abs_ns, delta); *r = 0; return true;