linux-kernel - Re: [PATCH v3 1/2] KVM: SVM: Increase X2AVIC limit to 4096 vcpus

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: <6dl4vsf3k7qhx2aunc5vdhvtxpnwqp45lilpdsp4jksxtgdu6t@kubfenz4bdey>
Date: Fri, 18 Jul 2025 15:51:35 +0530
From: Naveen N Rao <naveen@...nel.org>
To: Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Paolo Bonzini <pbonzini@...hat.com>, Suravee Suthikulpanit <suravee.suthikulpanit@....com>, 
	Vasant Hegde <vasant.hegde@....com>
Subject: Re: [PATCH v3 1/2] KVM: SVM: Increase X2AVIC limit to 4096 vcpus

On Mon, Jun 23, 2025 at 04:17:13PM -0700, Sean Christopherson wrote:
> On Thu, Feb 20, 2025, Naveen N Rao (AMD) wrote:
> > From: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
> > 
> > Newer AMD platforms enhance x2AVIC feature to support up to 4096 vcpus.
> > This capatility is detected via CPUID_Fn8000000A_ECX[x2AVIC_EXT].
> > 
> > Modify the SVM driver to check the capability. If detected, extend bitmask
> > for guest max physical APIC ID to 0xFFF, increase maximum vcpu index to
> > 4095, and increase the size of the Phyical APIC ID table from 4K to 32K in
> > order to accommodate up to 4096 entries.
> 
> Kinda silly, but please split this into (at least) two patches.  One to introduce
> the variables to replace the macros, and then one to actually implement support
> for 4096 entries.  That makes it a _lot_ easier to review each change (I'm having
> trouble teasing out what's actually changing for 4k support).

Sure, let me re-work the patches.

> 
> The changelog also needs more info.  Unless I'm misreading the diff, only the
> physical table is being expanded?  How does that work?  (I might be able to
> figure it out if I think hard, but I shouldn't have to think that hard).

Right - it is primarily just the physical ID table being expanded to 
allow AVIC hardware to lookup APIC IDs for vCPUs beyond 511.

As an aside, updated APM covering 4k vCPU support (and other updates) 
has now been published:
https://docs.amd.com/v/u/en-US/24593_3.43

> > @@ -1218,8 +1224,19 @@ bool avic_hardware_setup(void)
> >  
> >  	/* AVIC is a prerequisite for x2AVIC. */
> >  	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
> > -	if (x2avic_enabled)
> > -		pr_info("x2AVIC enabled\n");
> > +	if (x2avic_enabled) {
> > +		x2avic_4k_vcpu_supported = !!(cpuid_ecx(0x8000000a) & 0x40);
> 
> No, add an X86_FEATURE_xxx for this, don't open code the CPUID lookup.  I think
> I'd even be tempted to use helpers instead of  

Ack.

> 
> > +		if (x2avic_4k_vcpu_supported) {
> > +			x2avic_max_physical_id = X2AVIC_MAX_PHYSICAL_ID_4K;
> > +			avic_physical_max_index_mask = AVIC_PHYSICAL_MAX_INDEX_4K_MASK;
> > +		} else {
> > +			x2avic_max_physical_id = X2AVIC_MAX_PHYSICAL_ID;
> > +			avic_physical_max_index_mask = AVIC_PHYSICAL_MAX_INDEX_MASK;
> > +		}
> > +
> > +		pr_info("x2AVIC enabled%s\n",
> > +			x2avic_4k_vcpu_supported ? " (w/ 4K-vcpu)" : "");
> 
> Maybe print the max number of vCPUs that are supported?  That way there is clear
> signal when 4k *isn't* supported (and communicating the max number of vCPUs in
> the !4k case would be helpful too).

I'm tempted to go the opposite way and not print that 4k vCPUs are 
supported by x2AVIC. As it is, there are many reasons AVIC may be 
inhibited and lack of 4k vCPU support is just one other reason, but only
for large VMs.

Most users shouldn't have to care: where possible, AVIC will be enabled 
by default (once that patch series lands). Users who truly care about 
AVIC will anyway need to confirm AVIC isn't inhibited since looking at 
the kernel log won't be sufficient. Those users can very well use cpuid 
to figure out if 4k vCPU support is present.


Thanks,
Naveen