linux-kernel - Re: [PATCH] KVM: SVM: Update dump_ghcb() to use the GHCB snapshot fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <926cde15-b1e2-1324-99e8-f5f07fc71ffa@amd.com>
Date: Tue, 29 Apr 2025 12:33:33 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Liam Merwick <liam.merwick@...cle.com>,
 Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
 Paolo Bonzini <pbonzini@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, Michael Roth <michael.roth@....com>
Subject: Re: [PATCH] KVM: SVM: Update dump_ghcb() to use the GHCB snapshot
 fields

On 4/29/25 11:31, Liam Merwick wrote:
> On 29/04/2025 16:22, Sean Christopherson wrote:
>> On Mon, Apr 28, 2025, Tom Lendacky wrote:
>>> @@ -3184,18 +3189,18 @@ static void dump_ghcb(struct vcpu_svm *svm)
>>>           return;
>>>       }
>>>   -    nbits = sizeof(ghcb->save.valid_bitmap) * 8;
>>> +    nbits = sizeof(svm->sev_es.valid_bitmap) * 8;
>>
>> I'm planning on adding this comment to explain the use of KVM's
>> snapshot.  Please
>> holler if it's wrong/misleading in any way.
>>
>>     /*
>>      * Print KVM's snapshot of the GHCB that was (unsuccessfully) used to
>>      * handle the exit.  If the guest has since modified the GHCB itself,
>>      * dumping the raw GHCB won't help debug why KVM was unable to handle
>>      * the VMGEXIT that KVM observed.
>>      */
>>
>>>       pr_err("GHCB (GPA=%016llx):\n", svm->vmcb->control.ghcb_gpa);
> 
> Would printing "GHCB snapshot (GPA= ...." here instead of just "GHCB (GPA=
> ..."
> help gently remind people just looking at the debug output of this too?

Except the GPA is that of the actual GHCB. And the values being printed
are the actual values sent by the guest and being used by KVM at the time
the GHCB was read. So I'm not sure if that would clear things up at all.

Thanks,
Tom

> 
> Either way, for patch:
> Reviewed-by: Liam Merwick <liam.merwick@...cle.com>
> 
> 
>>>       pr_err("%-20s%016llx is_valid: %u\n", "sw_exit_code",
>>> -           ghcb->save.sw_exit_code, ghcb_sw_exit_code_is_valid(ghcb));
>>> +           kvm_ghcb_get_sw_exit_code(control),
>>> kvm_ghcb_sw_exit_code_is_valid(svm));
>>>       pr_err("%-20s%016llx is_valid: %u\n", "sw_exit_info_1",
>>> -           ghcb->save.sw_exit_info_1,
>>> ghcb_sw_exit_info_1_is_valid(ghcb));
>>> +           control->exit_info_1, kvm_ghcb_sw_exit_info_1_is_valid(svm));
>>>       pr_err("%-20s%016llx is_valid: %u\n", "sw_exit_info_2",
>>> -           ghcb->save.sw_exit_info_2,
>>> ghcb_sw_exit_info_2_is_valid(ghcb));
>>> +           control->exit_info_2, kvm_ghcb_sw_exit_info_2_is_valid(svm));
>>>       pr_err("%-20s%016llx is_valid: %u\n", "sw_scratch",
>>> -           ghcb->save.sw_scratch, ghcb_sw_scratch_is_valid(ghcb));
>>> -    pr_err("%-20s%*pb\n", "valid_bitmap", nbits,
>>> ghcb->save.valid_bitmap);
>>> +           svm->sev_es.sw_scratch, kvm_ghcb_sw_scratch_is_valid(svm));
>>> +    pr_err("%-20s%*pb\n", "valid_bitmap", nbits,
>>> svm->sev_es.valid_bitmap);
>>>   }
>>
>