On Thu, 11 May 2023 18:15:15 +0100, Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > > Hi James, > > On Thu, Mar 30, 2023 at 05:51:22PM +0100, James Morse wrote: > > When stage1 translation is disabled, the SCTRL_E1.I bit controls the > > attributes used for instruction fetch, one of the options results in a > > non-cacheable access. A whole host of CPUs missed the FWB override > > in this case, meaning a KVM guest could fetch stale/junk data instead of > > instructions. > > > > The workaround is to disable FWB, and do the required cache maintenance > > instead. > > I think the workaround can be to only do the required cache maintenance > without disabling FWB. Having FWB on doesn't bring any performance > benefits if we do the cache maintenance anyway but keeping it around may > be useful for other reasons (e.g. KVM device pass-through using > cacheable mappings, though not something KVM supports currently). But you'd also rely on the guest doing its own cache maintenance for instructions it writes, right? Which probably means exposing a different CLIDR_EL1 so that LoC/LoUU/LoUIS are consistent with *not* having FWB... I also wonder if keeping FWB set has the potential to change the semantics of the CMOs (the spec seems silent on that front). > > Unfortunately, no-one has firmware that supports this new interface yet, > > and the least surprising thing to do is to enable the workaround by default, > > meaning FWB is disabled on all these cores, even for unaffected platforms. > > ACPI Platforms that are not-affected can either take a firmware-update to > > support the interface, or if the kernel they run will only run on hardware > > that is unaffected, disable the workaround at build time. > > Given that we know of more platforms that are _not_ affected and vendors > are pretty vague on whether they need this, I'd rather swap the default > and keep the workaround off with a firmware interface, DT or command > line opt-in. That'd be my preferred way. I really dislike putting the onus on working systems to declare themselves safe (although there are some Spectre-shaped precedents to that). > That said, maybe we can reduce the risk further by doing the > vcpu_has_run_once() trick with !FWB and clean the D side to PoC on a > stage 2 exec fault (together with the I-cache invalidation). We can then > ignore any other cache maintenance on S2 faults until someone shouts (we > can maybe recommend forcing FWB off on the command line through the > cpuid override). You lost me here with your vcpu_has_run_once(). Keeping the CMOs on exec fault is definitely manageable. But is that enough? Thanks, M. -- Without deviation from the norm, progress is not possible.
