linux-kernel - Re: [PATCH v3 3/3] KVM: SVM: Extend host physical APIC ID field to support more than 8-bit

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]

Date:   Wed, 2 Feb 2022 15:53:58 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org,
        pbonzini@...hat.com, joro@...tes.org, mlevitsk@...hat.com,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        peterz@...radead.org, hpa@...or.com, thomas.lendacky@....com,
        jon.grimm@....com
Subject: Re: [PATCH v3 3/3] KVM: SVM: Extend host physical APIC ID field to
 support more than 8-bit

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.