While sev and sev_es bits are not yet enabled in xen,
including their status in the VMCB dump could be
informational.Therefore, print it via svmdebug.

Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
JFYI, we'll send the follow-up patches with the enablement
of sev and ASP driver.
---
 xen/arch/x86/hvm/svm/svmdebug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index 24358c6eea..f54b426fb3 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -53,6 +53,8 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
            vmcb->exitinfo1, vmcb->exitinfo2);
     printk("np_ctrl = %#"PRIx64" guest_asid = %#x\n",
            vmcb_get_np_ctrl(vmcb), vmcb_get_guest_asid(vmcb));
+    printk("sev = %d sev_es = %d\n",
+           vmcb_get_sev(vmcb), vmcb_get_sev_es(vmcb));
     printk("virtual vmload/vmsave = %d, virt_ext = %#"PRIx64"\n",
            vmcb->virt_ext.fields.vloadsave_enable, vmcb->virt_ext.bytes);
     printk("cpl = %d efer = %#"PRIx64" star = %#"PRIx64" lstar = %#"PRIx64"\n",
-- 
2.44.0

On 07/03/2024 9:40 pm, Vaishali Thakkar wrote:
> diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
> index 24358c6eea..f54b426fb3 100644
> --- a/xen/arch/x86/hvm/svm/svmdebug.c
> +++ b/xen/arch/x86/hvm/svm/svmdebug.c
> @@ -53,6 +53,8 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
>             vmcb->exitinfo1, vmcb->exitinfo2);
>      printk("np_ctrl = %#"PRIx64" guest_asid = %#x\n",
>             vmcb_get_np_ctrl(vmcb), vmcb_get_guest_asid(vmcb));
> +    printk("sev = %d sev_es = %d\n",
> +           vmcb_get_sev(vmcb), vmcb_get_sev_es(vmcb));

Hmm.  These are covered by the previous line printing all of np_ctrl. 
What about rearranging the previous line to be something like:

printk("asid: %#x, np_ctrl: %#"PRIx64" -%s%s%s\n",
    vmcb->_asid, vmcb->_np_ctrl,
    vmcb->_np ? " NP" : "",
    vmcb->_sev ? " SEV" : "",
    ...);

This is more compact (things like "guest" in "guest asid" is entirely
redundant), and provides both the raw _np_ctrl field and a bit-by-bit
decode on the same line, rather than having different parts of the info
on different lines and bools written out in longhand?

See xen/arch/x86/spec_ctrl.c: print_details() for a rather more complete
example of this style of printk() rendering for bits, including how to
tabulate it for better readability.

~Andrew

On 3/8/24 00:34, Andrew Cooper wrote:
> On 07/03/2024 9:40 pm, Vaishali Thakkar wrote:
>> diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
>> index 24358c6eea..f54b426fb3 100644
>> --- a/xen/arch/x86/hvm/svm/svmdebug.c
>> +++ b/xen/arch/x86/hvm/svm/svmdebug.c
>> @@ -53,6 +53,8 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
>>              vmcb->exitinfo1, vmcb->exitinfo2);
>>       printk("np_ctrl = %#"PRIx64" guest_asid = %#x\n",
>>              vmcb_get_np_ctrl(vmcb), vmcb_get_guest_asid(vmcb));
>> +    printk("sev = %d sev_es = %d\n",
>> +           vmcb_get_sev(vmcb), vmcb_get_sev_es(vmcb));
> 
> Hmm.  These are covered by the previous line printing all of np_ctrl.
> What about rearranging the previous line to be something like:
> 
> printk("asid: %#x, np_ctrl: %#"PRIx64" -%s%s%s\n",
>      vmcb->_asid, vmcb->_np_ctrl,
>      vmcb->_np ? " NP" : "",
>      vmcb->_sev ? " SEV" : "",
>      ...);
> 
> This is more compact (things like "guest" in "guest asid" is entirely
> redundant), and provides both the raw _np_ctrl field and a bit-by-bit
> decode on the same line, rather than having different parts of the info
> on different lines and bools written out in longhand?

Good point. Will change it in the revised v2.

> See xen/arch/x86/spec_ctrl.c: print_details() for a rather more complete
> example of this style of printk() rendering for bits, including how to
> tabulate it for better readability.

Thanks for pointing to the reference.

> ~Andrew