On Fri, Aug 23, 2024 at 1:45 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Fri, Aug 23, 2024, Vipin Sharma wrote: > > and if that didn't suceed then return the error and let caller handle however > > they want to. NX huge page recovery should be tolerant of this zapping > > failure and move on to the next shadow page. > > Again, I agree, but that is orthogonal to the WARN I am suggesting. The intent > of the WARN isn't to detect that zapping failed, it's to flag that the impossible > has happened. > > > May be we can put WARN if NX huge page recovery couldn't zap any pages during > > its run time. For example, if it was supposed to zap 10 pages and it couldn't > > zap any of them then print using WARN_ON_ONCE. > > Why? It's improbable, but absolutely possible that zapping 10 SPTEs could fail. > Would it make sense to publish a stat so that userspace can alert on NX huge page > recovery not being as effective as it should be? Maybe. But the kernel should > never WARN because an unlikely scenario occurred. I misunderstood your WARN requirement in the response to this patch and that's why suggested that if you are preferring WARN this much :) maybe we can move it out outside and print only if nothing got zapped. Glad it's not. We don't need stat, userspace can use page stats and NX parameters to observe if it is working effectively or not. > If you look at all the KVM_MMU_WARN_ON() and (hopefully) WARN_ON_ONCE() calls in > KVM x86, they're either for things that should never happen absent software or > hardware bugs, or to ensure future changes update all relevant code paths. > > The WARN I am suggesting is the same. It can only fire if there's a hardware > bug, a KVM bug, or if KVM changes how it behaves, e.g. starts doing A/D tracking > on non-leaf SPTEs. > I responded to this email before moving on to your next email :) I agree with WARN there which is for checking if new SPTE is pointing to the old page table. I have a few other comments (not related to WARN), I will respond there. Thanks!
