2023-04-03 09:56:02

by Xinghui Li

[permalink] [raw]
Subject: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

From: Xinghui Li <[email protected]>

VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
which is meaningless. Delete the duplicate one.

Fixes: 391503528257 ("KVM: x86: SVM: move avic definitions from AMD's spec to svm.h")
Signed-off-by: Xinghui Li <[email protected]>
---
arch/x86/include/asm/svm.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 770dcf75eaa9..e236b896f8b4 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -278,7 +278,6 @@ static_assert((AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == AVIC_MAX_
static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_MAX_PHYSICAL_ID);

#define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF)
-#define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL


struct vmcb_seg {
--
2.31.1


2023-04-04 08:42:43

by Like Xu

[permalink] [raw]
Subject: Re: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

On 3/4/2023 5:52 pm, [email protected] wrote:
> From: Xinghui Li <[email protected]>
>
> VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
> which is meaningless. Delete the duplicate one.
>
> Fixes: 391503528257 ("KVM: x86: SVM: move avic definitions from AMD's spec to svm.h")
> Signed-off-by: Xinghui Li <[email protected]>

Reviewed-by: Like Xu <[email protected]>

Do we have any tool to find out more similar issues across numerous subsystems ?

> ---
> arch/x86/include/asm/svm.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 770dcf75eaa9..e236b896f8b4 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -278,7 +278,6 @@ static_assert((AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == AVIC_MAX_
> static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_MAX_PHYSICAL_ID);
>
> #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF)
> -#define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL
>
>
> struct vmcb_seg {

2023-04-04 10:40:16

by Xinghui Li

[permalink] [raw]
Subject: Re: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

On Tue, Apr 4, 2023 at 4:40 PM Like Xu <[email protected]> wrote:
>
> On 3/4/2023 5:52 pm, [email protected] wrote:
> > From: Xinghui Li <[email protected]>
> >
> > VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
> > which is meaningless. Delete the duplicate one.
> >
> > Fixes: 391503528257 ("KVM: x86: SVM: move avic definitions from AMD's spec to svm.h")
> > Signed-off-by: Xinghui Li <[email protected]>
>
> Reviewed-by: Like Xu <[email protected]>
>
> Do we have any tool to find out more similar issues across numerous subsystems ?
>
As far as I know, there is no such tool.
But It seems possible to develop one, I will research it.

Thanks~

2023-04-04 23:45:34

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

On Mon, 03 Apr 2023 17:52:00 +0800, [email protected] wrote:
> VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
> which is meaningless. Delete the duplicate one.

Applied to kvm-x86 svm, thanks!

In the future, please don't use "PATCH REBASED". If you're sending a new
version of a patch that's been rebased, then the revision number needs to be
bumped. The fact that the only change is that the patch was rebased isn't
relevant as far as versioning is concerned, it's still a new version. The
cover letter and/or ignored part of the patch is where the delta between
versions should be captured.

And in this case, there really was no need to send a new version, the original
patch still applies cleanly. I suspect that the REBASED version was sent as a
form of a ping, which again is not the right way to ping a patch/series. If you
want to ping, please reply to the original patch. Unnecessarily sending new
versions means more patches to sort through, i.e. makes maintainers lives harder,
not easier.

[1/1] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK
https://github.com/kvm-x86/linux/commit/c0d0ce9b5a85

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes

2023-04-06 02:34:00

by Xinghui Li

[permalink] [raw]
Subject: Re: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

On Wed, Apr 5, 2023 at 7:44 AM Sean Christopherson <[email protected]> wrote:
>
> On Mon, 03 Apr 2023 17:52:00 +0800, [email protected] wrote:
> > VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
> > which is meaningless. Delete the duplicate one.
>
> Applied to kvm-x86 svm, thanks!
>
> In the future, please don't use "PATCH REBASED". If you're sending a new
> version of a patch that's been rebased, then the revision number needs to be
> bumped. The fact that the only change is that the patch was rebased isn't
> relevant as far as versioning is concerned, it's still a new version. The
> cover letter and/or ignored part of the patch is where the delta between
> versions should be captured.
>
> And in this case, there really was no need to send a new version, the original
> patch still applies cleanly. I suspect that the REBASED version was sent as a
> form of a ping, which again is not the right way to ping a patch/series. If you
> want to ping, please reply to the original patch. Unnecessarily sending new
> versions means more patches to sort through, i.e. makes maintainers lives harder,
> not easier.
>
Firstly, I'm so so SORRY to burden you in this way.
I found the last patch can't be am directly, so I send a new patch
with the last rebased code.
I used to believe that this would alleviate your burden, but
unfortunately, it had the opposite effect.
Again, sorry for my wrong operation.

Thanks~

2023-04-06 02:37:02

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH REBASED] KVM: x86: SVM: Fix one redefine issue about VMCB_AVIC_APIC_BAR_MASK

On Thu, Apr 06, 2023, Xinghui Li wrote:
> On Wed, Apr 5, 2023 at 7:44 AM Sean Christopherson <[email protected]> wrote:
> >
> > On Mon, 03 Apr 2023 17:52:00 +0800, [email protected] wrote:
> > > VMCB_AVIC_APIC_BAR_MASK is defined twice with the same value in svm.h,
> > > which is meaningless. Delete the duplicate one.
> >
> > Applied to kvm-x86 svm, thanks!
> >
> > In the future, please don't use "PATCH REBASED". If you're sending a new
> > version of a patch that's been rebased, then the revision number needs to be
> > bumped. The fact that the only change is that the patch was rebased isn't
> > relevant as far as versioning is concerned, it's still a new version. The
> > cover letter and/or ignored part of the patch is where the delta between
> > versions should be captured.
> >
> > And in this case, there really was no need to send a new version, the original
> > patch still applies cleanly. I suspect that the REBASED version was sent as a
> > form of a ping, which again is not the right way to ping a patch/series. If you
> > want to ping, please reply to the original patch. Unnecessarily sending new
> > versions means more patches to sort through, i.e. makes maintainers lives harder,
> > not easier.
> >
> Firstly, I'm so so SORRY to burden you in this way.
> I found the last patch can't be am directly, so I send a new patch
> with the last rebased code.

Ah, try `git am -3`, i.e. tell git to try a 3-way merge between the patch, its
base, and what you're applying on. I'm sure there are situations where a 3-way
merge is unwanted, e.g. maybe if someone needs to be super paranoid? But for me
personally at least, I pretty much always run am with -3.

> I used to believe that this would alleviate your burden, but
> unfortunately, it had the opposite effect.
> Again, sorry for my wrong operation.

No worries, it's not a big deal. My lengthy response was purely to help avoid
similar mistakes in the future.

Thanks!