On Mon, Jan 13, 2025 at 10:47:57AM -0800, Kevin Loughlin wrote:
> On Fri, Jan 10, 2025 at 12:23 AM Kirill A. Shutemov
> <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, Jan 09, 2025 at 10:55:33PM +0000, Kevin Loughlin wrote:
> > > @@ -710,6 +711,14 @@ static void sev_clflush_pages(struct page *pages[], unsigned long npages)
> > > }
> > > }
> > >
> > > +static void sev_wb_on_all_cpus(void)
> > > +{
> > > + if (boot_cpu_has(X86_FEATURE_WBNOINVD))
> > > + wbnoinvd_on_all_cpus();
> > > + else
> > > + wbinvd_on_all_cpus();
> >
> > I think the X86_FEATURE_WBNOINVD check should be inside wbnoinvd().
> > wbnoinvd() should fallback to WBINVD if the instruction is not supported
> > rather than trigger #UD.
>
> I debated this as well and am open to doing it that way. One argument
> against silently falling back to WBINVD within wbnoinvd() (in general,
> not referring to this specific case) is that frequent WBINVD can cause
> soft lockups, whereas WBNOINVD is much less likely to do so. As such,
> there are potentially use cases where falling back to WBINVD would be
> undesirable (and would potentially be non-obvious behavior to the
> programmer calling wbnoinvd()), hence why I left the decision outside
> wbnoinvd().
>
> That said, open to either way, especially since that "potential" use
> case doesn't apply here; just lemme know if you still have a strong
> preference for doing the check within wbnoinvd().
An alternative would be to fail wbnoinvd() with an error code and
possibly a WARN(). Crash on #UD is not helpful.
--
Kiryl Shutsemau / Kirill A. Shutemov
