| Message ID | 9cc6b407ec3b45f034b7deb6f5f44a561eecca47.1709846387.git.vaishali.thakkar@vates.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/svm : Misc changes for few vmcb bits | expand |
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
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",
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(+)