linux-kernel - Re: [PATCH v3 4/5] KVM: X86: Drop KVM_APIC_SHORT_MASK and KVM_APIC_DEST

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87tv6hbl7v.fsf@vitty.brq.redhat.com>
Date:   Tue, 03 Dec 2019 14:19:16 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Peter Xu <peterx@...hat.com>, kvm@...r.kernel.org
Cc:     Nitesh Narayan Lal <nitesh@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] KVM: X86: Drop KVM_APIC_SHORT_MASK and KVM_APIC_DEST_MASK

Peter Xu <peterx@...hat.com> writes:

> We have both APIC_SHORT_MASK and KVM_APIC_SHORT_MASK defined for the
> shorthand mask.  Similarly, we have both APIC_DEST_MASK and
> KVM_APIC_DEST_MASK defined for the destination mode mask.
>
> Drop the KVM_APIC_* macros and replace the only user of them to use
> the APIC_DEST_* macros instead.  At the meantime, move APIC_SHORT_MASK
> and APIC_DEST_MASK from lapic.c to lapic.h.
>
> Signed-off-by: Peter Xu <peterx@...hat.com>
> ---
>  arch/x86/kvm/lapic.c | 3 ---
>  arch/x86/kvm/lapic.h | 5 +++--
>  arch/x86/kvm/svm.c   | 4 ++--
>  3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 1eabe58bb6d5..805c18178bbf 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -56,9 +56,6 @@
>  #define APIC_VERSION			(0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
>  #define LAPIC_MMIO_LENGTH		(1 << 12)
>  /* followed define is not in apicdef.h */
> -#define APIC_SHORT_MASK			0xc0000
> -#define APIC_DEST_NOSHORT		0x0
> -#define APIC_DEST_MASK			0x800
>  #define MAX_APIC_VECTOR			256
>  #define APIC_VECTORS_PER_REG		32
>  
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 0b9bbadd1f3c..5a9f29ed9a4b 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -10,8 +10,9 @@
>  #define KVM_APIC_SIPI		1
>  #define KVM_APIC_LVT_NUM	6
>  
> -#define KVM_APIC_SHORT_MASK	0xc0000
> -#define KVM_APIC_DEST_MASK	0x800
> +#define APIC_SHORT_MASK			0xc0000
> +#define APIC_DEST_NOSHORT		0x0
> +#define APIC_DEST_MASK			0x800
>  
>  #define APIC_BUS_CYCLE_NS       1
>  #define APIC_BUS_FREQUENCY      (1000000000ULL / APIC_BUS_CYCLE_NS)
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 362e874297e4..65a27a7e9cb1 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4519,9 +4519,9 @@ static int avic_incomplete_ipi_interception(struct vcpu_svm *svm)
>  		 */
>  		kvm_for_each_vcpu(i, vcpu, kvm) {
>  			bool m = kvm_apic_match_dest(vcpu, apic,
> -						     icrl & KVM_APIC_SHORT_MASK,
> +						     icrl & APIC_SHORT_MASK,
>  						     GET_APIC_DEST_FIELD(icrh),
> -						     icrl & KVM_APIC_DEST_MASK);
> +						     icrl & APIC_DEST_MASK);
>  
>  			if (m && !avic_vcpu_is_running(vcpu))
>  				kvm_vcpu_wake_up(vcpu);

Personal taste but I would've preserved KVM_ prefix. The patch itself
looks correct, so

Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com>

-- 
Vitaly