Re: [PATCH 7/7] KVM: SVM: Enable save/restore of FRED MSRs

[Shivansh Dhiman <shivansh.dhiman@amd.com>]

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