Re: [PATCH v6 7/7] KVM: SVM: Add Page modification logging support

["Nikunj A. Dadhania" <nikunj@amd.com>]

On 4/28/2026 1:46 AM, Sean Christopherson wrote:
> On Sat, Apr 25, 2026, Tom Lendacky wrote:
>> On 4/24/26 11:25, Tom Lendacky wrote:
>>>> Now, maybe the APM is trying to say only the save area is undefined, in which
>>>> case PML Index is fine and can and should be left alone.  But if that's the case,
>>>> the APM needs to be updated to make explicitly clear what fields in the VMCS are
>>>> and are not valid after SHUTDOWN.
>>>
>>> I'm checking to see if I can get clarification on this.
>>
>> PML_INDEX will be valid even when a SHUTDOWN is intercepted. I'm working
>> to see if we can update the APM to identify what would be valid and what
>> wouldn't be in this situation.
>>
>> For a triple-fault SHUTDOWN, the saved x86 state could be inconsistent
>> because the CPU stopped in the middle of a sequence and redirected to
>> shutdown. Everything will be saved, but it might not be valid x86 state.
> 
> Yeah, I assumed that the APM wording was an overly-broad description of why guest
> state would be garbage.
> 
>> The question then becomes are there fields within the VMCB control area
>> that are based on x86 state that might not be valid (e.g. V_IRQ or nRIP,
>> etc.).
> 
> FWIW, even a conservative iset of "these fields/areas are guaranteed to be sane
> and consistent" would be super helpful.  The big problem right now is that the
> wording reads like the entire GHCB is suspect.

I believe you meant VMCB above, not GHCB. :-)

>>> The APM states that the VMCB is undefined after SHUTDOWN. PML index
>>> could be anything:
>>>
>>>   15.14.3 Shutdown Intercept
>>>   When this intercept occurs, any condition that normally causes a shutdown causes a #VMEXIT to the
>>>   VMM instead. After an intercepted shutdown, the state saved in the VMCB is undefined.

The anticipated APM update will clarify the wording as follows:

  "After an intercepted shutdown, the VMCB control area is valid (with the
   exception of offsets 60h, 61h, and 68h) and the VMCB state save area is
   undefined."

Based on this clarification, I will initialize the PML index only during
reset and leave it unchanged in the case of an INIT event.

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d9dc725786e6..f5a17274868e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1216,7 +1216,9 @@ static void init_vmcb(struct kvm_vcpu *vcpu, bool init_event)
 		 * svm_update_cpu_dirty_logging()
 		 */
 		control->pml_addr = (u64)__sme_set(page_to_phys(vcpu->arch.pml_page));
-		control->pml_index = PML_HEAD_INDEX;
+
+		if (!init_event)
+			control->pml_index = PML_HEAD_INDEX;
 	}
 
 	if (sev_guest(vcpu->kvm))

Regards,
Nikunj