Message-ID: <0ed33e2d-87b4-71fc-3de7-5e193e386331@amd.com>
Date:   Wed, 15 Feb 2023 22:50:11 +0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2 2/3] KVM: SVM: Modify AVIC GATag to support max number
 of 512 vCPUs
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
References: <20230207002156.521736-1-seanjc@google.com>
 <20230207002156.521736-3-seanjc@google.com>
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>
In-Reply-To: <20230207002156.521736-3-seanjc@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Zk9qbEpjUnI4VGJpMTZtcXlDbGpSWnpHZzJlV2NoUER0a2NUVitsTmluZ0hQ?=
 =?utf-8?B?OVVkdzR2SGdmVkFFZzdVZTJyT2hPbnZ1WUE5RjNXTmZFL2ZXRmNtdXdPR3k4?=
 =?utf-8?B?Q2RyUU9qVTZwbXBPWmNuWUljUTNzSkYvV2FFSFN4cUxwb1JTdVM5bElqaUVk?=
 =?utf-8?B?VWJFWkIyZ2MvUXBhWlU1b0RHS0NBZU5sMlVpNmZscXFDZElRWjFFZkt0Mm1j?=
 =?utf-8?B?UWJHdE14NmxteFk3Um0vMFY0WUpEeXNWOVJxa0o5ZjBVUjI0M0VtSjRoRi9Q?=
 =?utf-8?B?TFR3YStXZDRtWWFyTmtjS01abVN3U250aTBMTGhlV0xFRlJxYlpjalByaXZO?=
 =?utf-8?B?ZFpYYUIzbTFzUUJ2NFRReFR5QnlNUmtRMGN6WHFKMzNrUDRwbXJ5OUxPa1hy?=
 =?utf-8?B?R3JCVWxyZ1VsbHZRZlo4UlE3bGEyMTA3WU1sNEVPUGtEZGRmcTJDbmlEOXJ0?=
 =?utf-8?B?YnhHSytpRHdQODFpRW1rZzlpZXpkTG8vVk9tRHNmTHV0aytmQUpxR256Q1dH?=
 =?utf-8?B?aWd1aWlidEdialhaWERDdEg5S1d5V2VNRjVPbi9Hd2xtcW93ODRJU3E1bG9w?=
 =?utf-8?B?MXllWit4ZWt5MWZMdzQ5aVQ0bkphRTFHek9FcVlyQ252ZVFhbTZSWnhHL3dL?=
 =?utf-8?B?TVcrMFkwUm1EalZnQ3hSY3NGYmhVUWE4czZGOHd0TkNJOHFKdWlXREk4VkZZ?=
 =?utf-8?B?dmozRGtpa2RTMTdJRkNrQzg2RStiTlduQzNDcGFzaHVIdGtCd3JyT3V6U0FI?=
 =?utf-8?B?RzViM1gxNUZKRnB6VHB6dXJ0c3BROG50SXFYTzR6bVBCL21mcHVrUG40cUc5?=
 =?utf-8?B?Nk9XZ3dRdVIrcjlUSWJpQVA5dzRweEgwQnlSMDN2K3NZRFQ5UG1NdmRHZW1P?=
 =?utf-8?B?ZnpvcUlsN1VVdTIxVEQ3SU9kUjhVaE5MSW1iaVpIbWkyVmt6cjUwQ1VFZDA0?=
 =?utf-8?B?YXdZVURrZXB3d1pBZDIyODlMSGlVaTg1ckgwMHNpMWpWZW5rR1NvQm1IdUJ1?=
 =?utf-8?B?ckhNTDVhQTU1N2dxVVlrRzJCL1ZVTW5CeUFoLzZQOTQrYU9ZdENmemQvRGpH?=
 =?utf-8?B?TmdlNzd5WXpuVUs1YVg0YUt1aWt3dlJhYlVaSTNXS294eGZzQ0FBSi9XZG5I?=
 =?utf-8?B?SWdMZ1ROVDlKekdIRW1XTzFyMUhGcnlMUys4TW01VjNrSXVVdTIvTjFWSDAx?=
 =?utf-8?B?RFp2ZnA4SkE0NWk4ZEt5TythS2dLNXJiUWxhV3hOMGRUK3lVeFd0TE5ta2NL?=
 =?utf-8?B?czJ2ZTVnMEJJVnMrTzlST1FFZzJtR3lMWWlDUGk2eldmZVU1d0cxUDhxdlRw?=
 =?utf-8?B?aGcwNUVrd0x3aHpRQTY1YUxWZEdoQklqVi93Q2dpV0hiRWIzZ2p0MERKd2ti?=
 =?utf-8?B?YmtuSzVQckhVNGtOVEdyWXF4N1JRNkhKdnA1NnRqMm0zNWxpVEVWbTFSYmZ2?=
 =?utf-8?B?SVdXR040SzV5VGUyWmdUZnMzSFhNNVVpNk9HNzFKQ2NnNmVhb3hId3dtTmJL?=
 =?utf-8?B?bjJWNGV5MU1xc0ZqUHN4YnBzVnZEMk1MQkF3a29xRnVIczVINitsbWt0a0R0?=
 =?utf-8?B?UkZZa05yYW4vQ2NzU3NQQmtHMHczODZjSlBkcDRNZzRVcGc3SXk2elJSSC9h?=
 =?utf-8?B?alZKUVZ3Wm1nMVprS3VrT1phdStXd2FSNFVJNk9idjZWSUUyVEVjb0Zib3Uv?=
 =?utf-8?B?L3dZb2s0K2pKNlFRQU1IVTM0S0Z5cHVNZnQ0dXBHZDNRaVY5R1JwZHA2RENx?=
 =?utf-8?B?dUlNdXVUb0MrZThsL3ErZ2UrZ2RpZXFQZDRONzg4L3NwN2MyUTVxa1NJSlVC?=
 =?utf-8?B?aUM5dmloR2dWZkthYXZPVUNEOUFMamxuNnJFZ3IvNFoxckJ3Y1F5c040S3dk?=
 =?utf-8?B?UHRnZjFBbVZNYVowKzdCSDQ4RE5oUjVWSDExRFdKS0JBSFVCNVNib2JKVG9j?=
 =?utf-8?B?cVNoRU55NGIrcER4N2NRMngwNzdKSmorNXVXV29DQTczMWFrd1pRZGtSMWRr?=
 =?utf-8?B?M3huaWgwb29Kb3dzUi9EYnY4QTN5YjFtWkNJcm5Ca2twVWtIUHgvOUtERXZM?=
 =?utf-8?B?aEFzOFpFeUxZYmFHamVXVVoxOVFGMXJxdEdFejhici85OENLOGJTeTZjNlk2?=
 =?utf-8?Q?3HUuEGhEIL7tjTdLrjJ/WP30d?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: da1b2018-804e-4cf4-1e2a-08db0f6c576f
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5445.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 15:50:20.5059
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fA44DF9adWwc9BBm3ia6H9rKqP6fB93iX0/5W4xJWb0EZuMh6kQU7PBG7wFe3uS0VTOBEQWItuPBQ7Y7XYa+Xw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7280
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2/7/2023 7:21 AM, Sean Christopherson wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask
> that effectively controls the largest guest physical APIC ID supported by
> x2AVIC, instead of hardcoding the number of bits to 8 (and the number of
> VM bits to 24).
> 
> The AVIC GATag is programmed into the AMD IOMMU IRTE to provide a
> reference back to KVM in case the IOMMU cannot inject an interrupt into a
> non-running vCPU.  In such a case, the IOMMU notifies software by creating
> a GALog entry with the corresponded GATag, and KVM then uses the GATag to
> find the correct VM+vCPU to kick.  Dropping bit 8 from the GATag results
> in kicking the wrong vCPU when targeting vCPUs with x2APIC ID > 255.
> 
> Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
> Cc: stable@vger.kernel.org
> Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/svm/avic.c | 26 ++++++++++++++++++--------
>   1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index ca684979e90d..326341a22153 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -27,19 +27,29 @@
>   #include "irq.h"
>   #include "svm.h"
>   
> -/* AVIC GATAG is encoded using VM and VCPU IDs */
> -#define AVIC_VCPU_ID_BITS		8
> -#define AVIC_VCPU_ID_MASK		((1 << AVIC_VCPU_ID_BITS) - 1)
> +/*
> + * Encode the arbitrary VM ID and the vCPU's default APIC ID, i.e the vCPU ID,
> + * into the GATag so that KVM can retrieve the correct vCPU from a GALog entry
> + * if an interrupt can't be delivered, e.g. because the vCPU isn't running.
> + *
> + * For the vCPU ID, use however many bits are currently allowed for the max
> + * guest physical APIC ID (limited by the size of the physical ID table), and
> + * use whatever bits remain to assign arbitrary AVIC IDs to VMs.  Note, the
> + * size of the GATag is defined by hardware (32 bits), but is an opaque value
> + * as far as hardware is concerned.
> + */
> +#define AVIC_VCPU_ID_MASK		AVIC_PHYSICAL_MAX_INDEX_MASK
>   
> -#define AVIC_VM_ID_BITS			24
> -#define AVIC_VM_ID_NR			(1 << AVIC_VM_ID_BITS)
> -#define AVIC_VM_ID_MASK			((1 << AVIC_VM_ID_BITS) - 1)
> +#define AVIC_VM_ID_SHIFT		HWEIGHT32(AVIC_PHYSICAL_MAX_INDEX_MASK)
> +#define AVIC_VM_ID_MASK			(GENMASK(31, AVIC_VM_ID_SHIFT) >> AVIC_VM_ID_SHIFT)
>   
> -#define AVIC_GATAG(x, y)		(((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \
> +#define AVIC_GATAG(x, y)		(((x & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \
>   						(y & AVIC_VCPU_ID_MASK))
> -#define AVIC_GATAG_TO_VMID(x)		((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK)
> +#define AVIC_GATAG_TO_VMID(x)		((x >> AVIC_VM_ID_SHIFT) & AVIC_VM_ID_MASK)
>   #define AVIC_GATAG_TO_VCPUID(x)		(x & AVIC_VCPU_ID_MASK)
>   
> +static_assert(AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u);
> +
>   static bool force_avic;
>   module_param_unsafe(force_avic, bool, 0444);
>   

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Thanks,
Suravee