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

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

[korantwork]

From: Xinghui Li <korantli@tencent.com>

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 <korantli@tencent.com>
---
 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

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

[Like Xu]

On 3/4/2023 5:52 pm, korantwork@gmail.com wrote:
> From: Xinghui Li <korantli@tencent.com>
> 
> 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 <korantli@tencent.com>

Reviewed-by: Like Xu <likexu@tencent.com>

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 {

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

[Xinghui Li]

On Tue, Apr 4, 2023 at 4:40 PM Like Xu <like.xu.linux@gmail.com> wrote:
>
> On 3/4/2023 5:52 pm, korantwork@gmail.com wrote:
> > From: Xinghui Li <korantli@tencent.com>
> >
> > 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 <korantli@tencent.com>
>
> Reviewed-by: Like Xu <likexu@tencent.com>
>
> 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~

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

[Sean Christopherson]

On Mon, 03 Apr 2023 17:52:00 +0800, korantwork@gmail.com 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

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

[Xinghui Li]

On Wed, Apr 5, 2023 at 7:44 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, 03 Apr 2023 17:52:00 +0800, korantwork@gmail.com 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~

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

[Sean Christopherson]

On Thu, Apr 06, 2023, Xinghui Li wrote:
> On Wed, Apr 5, 2023 at 7:44 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, 03 Apr 2023 17:52:00 +0800, korantwork@gmail.com 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!