Subject: Re: [PATCH 4/4] KVM: x86: allow hotplug of VCPU with APIC ID over
 0xff
To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
References: <20161202194401.10038-1-rkrcmar@redhat.com>
 <20161202194401.10038-5-rkrcmar@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <8ea7ddb3-84dd-e205-a728-cc0912462362@redhat.com>
Date: Wed, 7 Dec 2016 13:03:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <20161202194401.10038-5-rkrcmar@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3318
Lines: 89


On 02/12/2016 20:44, Radim Krčmář wrote:
> LAPIC after reset is in xAPIC mode, which poses a problem for hotplug of
> VCPUs with high APIC ID, because reset VCPU is waiting for INIT/SIPI,
> but there is no way to uniquely address it using xAPIC.
> 
> From many possible options, we chose the one that also works on real
> hardware: accepting interrupts addressed to LAPIC's x2APIC ID even in
> xAPIC mode.
> 
> KVM intentionally differs from real hardware, because real hardware
> (Knights Landing) does just "x2apic_id & 0xff" to decide whether to
> accept the interrupt in xAPIC mode and it can deliver one interrupt to
> more than one physical destination, e.g. 0x123 to 0x123 and 0x23.
> 
> Add a capability to let userspace know that we do something now.

I wouldn't even bother with the capability.  It's a bit borderline for
stable, but I think it's okay.  We can always Cc and let the maintainer
reject it.

Paolo

> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  arch/x86/kvm/lapic.c     | 13 +++++++++++--
>  arch/x86/kvm/x86.c       |  1 +
>  include/uapi/linux/kvm.h |  1 +
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 42edf1ea2909..b85985985ac8 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -199,10 +199,15 @@ static void recalculate_apic_map(struct kvm *kvm)
>  		xapic_id = kvm_xapic_id(apic);
>  		x2apic_id = kvm_x2apic_id(apic);
>  
> -		if (apic_x2apic_mode(apic) &&
> +		/* Hotplug hack: see kvm_apic_match_physical_addr(), ... */
> +		if ((apic_x2apic_mode(apic) || x2apic_id > 0xff) &&
>  				x2apic_id <= new->max_apic_id)
>  			new->phys_map[x2apic_id] = apic;
> -		else if (!apic_x2apic_mode(apic))
> +		/*
> +		 * ... xAPIC ID of VCPUs with APIC ID > 0xff will wrap-around,
> +		 * prevent them from masking VCPUs with APIC ID <= 0xff.
> +		 */
> +		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
>  			new->phys_map[xapic_id] = apic;
>  
>  		ldr = kvm_lapic_get_reg(apic, APIC_LDR);
> @@ -612,6 +617,10 @@ static bool kvm_apic_match_physical_addr(struct kvm_lapic *apic, u32 mda)
>  	if (apic_x2apic_mode(apic))
>  		return mda == kvm_x2apic_id(apic);
>  
> +	/* Hotplug hack: LAPIC in xAPIC mode also accepts x2APIC. */
> +	if (kvm_x2apic_id(apic) > 0xff && mda == kvm_x2apic_id(apic))
> +		return true;
> +
>  	return mda == kvm_xapic_id(apic);
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7770d77c828d..945e8eeb4eb1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2618,6 +2618,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_DISABLE_QUIRKS:
>  	case KVM_CAP_SET_BOOT_CPU_ID:
>   	case KVM_CAP_SPLIT_IRQCHIP:
> +	case KVM_CAP_X86_ALWAYS_ACCEPT_X2APIC_DEST:
>  #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
>  	case KVM_CAP_ASSIGN_DEV_IRQ:
>  	case KVM_CAP_PCI_2_3:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index e9f5ceffd741..f25efa375255 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -871,6 +871,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_S390_USER_INSTR0 130
>  #define KVM_CAP_MSI_DEVID 131
>  #define KVM_CAP_PPC_HTM 132
> +#define KVM_CAP_X86_ALWAYS_ACCEPT_X2APIC_DEST 133
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
>