Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Feb 2022 15:53:58 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     "Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org,
        pbonzini@redhat.com, joro@8bytes.org, mlevitsk@redhat.com,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        peterz@infradead.org, hpa@zytor.com, thomas.lendacky@amd.com,
        jon.grimm@amd.com
Subject: Re: [PATCH v3 3/3] KVM: SVM: Extend host physical APIC ID field to
 support more than 8-bit
Message-ID: <YfqpFt5raCd/LZzr@google.com>
References: <20211213113110.12143-1-suravee.suthikulpanit@amd.com>
 <20211213113110.12143-4-suravee.suthikulpanit@amd.com>
 <Yc3qt/x1YPYKe4G0@google.com>
 <34a47847-d80d-e93d-a3fe-c22382977c1c@amd.com>
 <Yfms5evHbN8JVbVX@google.com>
 <9d2ca4ab-b945-6356-5e4b-265b1a474657@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9d2ca4ab-b945-6356-5e4b-265b1a474657@amd.com>
Precedence: bulk

On Wed, Feb 02, 2022, Suthikulpanit, Suravee wrote:
> As I mentioned, the APM will be corrected to remove the word "x2APIC".

Ah, I misunderstood what part was being amended.

> Essentially, it will be changed to:
> 
>  * 7:0  - For systems w/ max APIC ID upto 255 (a.k.a old system)
>  * 11:8 - For systems w/ max APIC ID 256 and above (a.k.a new system). Otherwise, reserved and should be zero.
> 
> As for the required number of bits, there is no good way to tell what's the max
> APIC ID would be on a particular system. Hence, we utilize the apic_get_max_phys_apicid()
> to figure out how to properly program the table (which is leaving the reserved field
> alone when making change to the table).
> 
> The avic_host_physical_id_mask is not just for protecting APIC ID larger than
> the allowed fields. It is also currently used for clearing the old physical APIC ID table entry
> before programing it with the new APIC ID.

Just clear 11:0 unconditionally, the reserved bits are Should Be Zero.

> So, What if we use the following logic:
> 
> +	u32 count = get_count_order(apic_get_max_phys_apicid());
> +
> +	/*
> +	 * Depending on the maximum host physical APIC ID available
> +	 * on the system, AVIC can support upto 8-bit or 12-bit host
> +	 * physical APIC ID.
> +	 */
> +	if (count <= 8)
> +		avic_host_physical_id_mask = GENMASK(7, 0);
> +	else if (count <= 12)
> +		avic_host_physical_id_mask = GENMASK(11, 0);
> +	else
> +		/* Warn and Disable AVIC here due to unable to satisfy APIC ID requirement */

I still don't see the point.  It's using the max APIC ID to verify that the max
APIC ID is valid.  Either we trust hardware to not screw up assigning APIC IDs,
or we don't use AVIC.