linux-kernel - Re: [EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

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] [day] [month] [year] [list]

Message-ID: <3xpfs5m5q6o74z5lx3aujdqub6ref2yypwcbz55ec5iefyqoy7@42g5nbgom637>
Date: Fri, 18 Jul 2025 13:49:45 +0530
From: Naveen N Rao <naveen@...nel.org>
To: Sean Christopherson <seanjc@...gle.com>
Cc: mlevitsk@...hat.com, Paolo Bonzini <pbonzini@...hat.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Vasant Hegde <vasant.hegde@....com>, Suravee Suthikulpanit <suravee.suthikulpanit@....com>
Subject: Re: [EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

On Mon, Jul 07, 2025 at 04:21:53PM -0700, Sean Christopherson wrote:
> On Fri, Jun 27, 2025, Naveen N Rao wrote:
> > > Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
> > > this uses an undocumented feature.
> > > 
> > > It doesn't matter much though.
> > > 
> > > > 
> > > > I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:
> > 
> > I am wary of breaking existing users/deployments on Zen4/Zen5 enabling 
> > AVIC by specifying avic=on, or avic=true today. That's primarily the 
> > reason I chose not to change 'avic' into an integer. Also, post module 
> > load, sysfs reports the value for 'avic' as a 'Y' or 'N' today. So if 
> > there are scripts relying on that, those will break if we change 'avic' 
> > into an integer.
> 
> That's easy enough to handle, e.g. see nx_huge_pages_ops for a very similar case
> where KVM has "auto" behavior (and a "never" case too), but otherwise treats the
> param like a bool.

Nice! Looks like I can re-use existing callbacks for this too:
    static const struct kernel_param_ops avic_ops = {
	    .flags = KERNEL_PARAM_OPS_FL_NOARG,
	    .set = param_set_bint,
	    .get = param_get_bool,
    };

    /* enable/disable AVIC (-1 = auto) */
    int avic = -1;
    module_param_cb(avic, &avic_ops, &avic, 0444);
    __MODULE_PARM_TYPE(avic, "bool");

> 
> > For Zen1/Zen2, as I mentioned, it is unlikely that anyone today is 
> > enabling AVIC and expecting it to work since the workaround is only just 
> > hitting upstream. So, I'm hoping requiring force_avic=1 should be ok 
> > with the taint removed.
> 
> But if that's the motivation, changing the semantics of force_avic doesn't make
> any sense.  Once the workaround lands, the only reason for force_avic to exist
> is to allow forcing KVM to enable AVIC even when it's not supported.

Indeed.

> 
> > Longer term, once we get wider testing with the workaround on Zen1/Zen2, 
> > we can consider relaxing the need for force_avic, at which point AVIC 
> > can be default enabled
> 
> I don't see why the default value for "avic" needs to be tied to force_avic.
> If we're not confident that AVIC is 100% functional and a net positive for the
> vast majority of setups/workloads on Zen1/Zen2, then simply leave "avic" off by
> default for those CPUs.  If we ever want to enable AVIC by default across the
> board, we can simply change the default value of "avic".
> 
> But to be honest, I don't see any reason to bother trying to enable AVIC by default
> for Zen1/Zen2.  There's a very real risk that doing so would regress existing users
> that have been running setups for ~6 years, and we can't fudge around AVIC being
> hidden on Zen3 (and the IOMMU not supporting it at all), i.e. enabling AVIC by
> default only for Zen4+ provides a cleaner story for end users.

Works for me. I completely agree with that.


Thanks,
Naveen