Re: [PATCH 7/7] KVM: SVM: Enable save/restore of FRED MSRs
Shivansh Dhiman <[email protected]>
| Newsgroups | org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 21-08-26 18:58, Sean Christopherson wrote: > On Wed, Aug 19, 2026, Shivansh Dhiman wrote: >> On 13-08-26 04:57, Sean Christopherson wrote: >>>> Would you prefer it handled in svm_recalc_fred_msr_intercepts() instead >>>> while setting intercepts? Or is there a better way to deal with this? >>> >>> Take a hard dependency on an upcoming APM update that states the control area >>> is valid after shutdown, and rework KVM to not clobber control fields on shutdown >>> interception. >> >> Sure, Sean. I'm thinking something like the diff below. It contains three >> parts: >> >> 1. Clear the save area and offsets 60h, 61h, and 68h of the control area, >> as per the APM. >> >> 2. Zero the intercepts as well. The APM doesn't require it, but it seemed >> worthwhile so that the new intercepts are computed on a clean slate >> rather than on top of whatever survived the triple fault. Is that worth >> doing? > > My gut reaction is I don't see how this can possibly be safe or correct. How > does the guest taking a series of faults have anything to do with what accesses > KVM needs to intercept? Fair point, there's no connection. init_vmcb() sets the intercepts it wants and the recalc on reset covers the rest, so zeroing them buys nothing. I'll drop it. > >> 3. For nested, same fields are copied to vmcb12 when a shutdown is intercepted >> in L2. Clear them while copying so L1 gets a deterministic value >> rather than whatever the hardware left behind. Does that seem like right >> thing to do? > > No, because that violates the rule of "don't make stuff up". It could also be > actively detrimental to L1's ability to debug issues (e.g. if hardware leaves > breadcrumbs in the save area), and risks creating guest<=>host ABI that we don't > want to support. Understood, I'll leave vmcb12 alone. That leaves just the save area and the control fields the APM says are undefined after a shutdown. I'll post that as a separate patch. Thanks, Shivansh