| Message ID | 0e688a18a97e495352e2b08cb7634abbc238da1b.1710149462.git.vaishali.thakkar@vates.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/svm : Misc changes for few vmcb bits | expand |
On 11.03.2024 13:40, Vaishali Thakkar wrote: > 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. Yet there are more bits there. I'm okay with leaving off printing of them here, but it wants clarifying why some are printed and some are not. > --- a/xen/arch/x86/hvm/svm/svmdebug.c > +++ b/xen/arch/x86/hvm/svm/svmdebug.c > @@ -51,8 +51,11 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb) > vmcb->exitcode, vmcb->exit_int_info.raw); > printk("exitinfo1 = %#"PRIx64" exitinfo2 = %#"PRIx64"\n", > vmcb->exitinfo1, vmcb->exitinfo2); > - printk("np_ctrl = %#"PRIx64" asid = %#x\n", > - vmcb_get_np_ctrl(vmcb), vmcb_get_asid(vmcb)); > + printk("asid = %#x np_ctrl = %#"PRIx64" - %s%s%s\n", > + vmcb_get_asid(vmcb), vmcb_get_np_ctrl(vmcb), > + vmcb_get_np(vmcb) ? "NP" : "", > + vmcb_get_sev(vmcb) ? "SEV" : "", > + vmcb_get_sev_es(vmcb) ? "SEV_ES" : ""); Each of these three string literals needs a leading blank as separator. In exchange the one in the format string immediately after '-' then will want dropping. That'll still lead to slightly odd output if none of the bits is set; imo it would be slightly less odd if you used printk("asid = %#x np_ctrl = %#"PRIx64":%s%s%s\n", instead. Jan
On 3/12/24 09:05, Jan Beulich wrote: > On 11.03.2024 13:40, Vaishali Thakkar wrote: >> 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. > > Yet there are more bits there. I'm okay with leaving off printing of > them here, but it wants clarifying why some are printed and some are > not. Well, the idea is to print the bits that are either enabled or has WIP to enable them. (e.g. sev and sev_es) I didn't include other bits as I'm not sure if there is any WIP to enable them. Particularly including sev and sev_es is useful for us while working on the enablement of it. Does a commit log like the following makes it clear for you? " Currently only raw _np_ctrl is being printed. It can be informational to know about which particular bits are enabled. So, this commit adds the bit-by-bit decode for np, sev and sev_es bits. Note that while only np is enabled in certain scenarios at the moment, work for enabling sev and sev_es is in progress. And it's useful to have this information as part of svmdebug. " I'm also fine with including other bits here if that's preferred. >> --- a/xen/arch/x86/hvm/svm/svmdebug.c >> +++ b/xen/arch/x86/hvm/svm/svmdebug.c >> @@ -51,8 +51,11 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb) >> vmcb->exitcode, vmcb->exit_int_info.raw); >> printk("exitinfo1 = %#"PRIx64" exitinfo2 = %#"PRIx64"\n", >> vmcb->exitinfo1, vmcb->exitinfo2); >> - printk("np_ctrl = %#"PRIx64" asid = %#x\n", >> - vmcb_get_np_ctrl(vmcb), vmcb_get_asid(vmcb)); >> + printk("asid = %#x np_ctrl = %#"PRIx64" - %s%s%s\n", >> + vmcb_get_asid(vmcb), vmcb_get_np_ctrl(vmcb), >> + vmcb_get_np(vmcb) ? "NP" : "", >> + vmcb_get_sev(vmcb) ? "SEV" : "", >> + vmcb_get_sev_es(vmcb) ? "SEV_ES" : ""); > > Each of these three string literals needs a leading blank as separator. > In exchange the one in the format string immediately after '-' then > will want dropping. That'll still lead to slightly odd output if none > of the bits is set; imo it would be slightly less odd if you used > > printk("asid = %#x np_ctrl = %#"PRIx64":%s%s%s\n", > > instead. > > Jan
On 13.03.2024 14:54, Vaishali Thakkar wrote: > On 3/12/24 09:05, Jan Beulich wrote: >> On 11.03.2024 13:40, Vaishali Thakkar wrote: >>> 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. >> >> Yet there are more bits there. I'm okay with leaving off printing of >> them here, but it wants clarifying why some are printed and some are >> not. > > Well, the idea is to print the bits that are either enabled or has WIP > to enable them. (e.g. sev and sev_es) I didn't include other bits as > I'm not sure if there is any WIP to enable them. Particularly including > sev and sev_es is useful for us while working on the enablement of it. > > Does a commit log like the following makes it clear for you? > > " Currently only raw _np_ctrl is being printed. It can > be informational to know about which particular bits > are enabled. So, this commit adds the bit-by-bit decode > for np, sev and sev_es bits. > > Note that while only np is enabled in certain scenarios > at the moment, work for enabling sev and sev_es is in > progress. And it's useful to have this information as > part of svmdebug. " I think that's sufficient, yes. Jan
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c index 0d714c728c..ba5fa40071 100644 --- a/xen/arch/x86/hvm/svm/svmdebug.c +++ b/xen/arch/x86/hvm/svm/svmdebug.c @@ -51,8 +51,11 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb) vmcb->exitcode, vmcb->exit_int_info.raw); printk("exitinfo1 = %#"PRIx64" exitinfo2 = %#"PRIx64"\n", vmcb->exitinfo1, vmcb->exitinfo2); - printk("np_ctrl = %#"PRIx64" asid = %#x\n", - vmcb_get_np_ctrl(vmcb), vmcb_get_asid(vmcb)); + printk("asid = %#x np_ctrl = %#"PRIx64" - %s%s%s\n", + vmcb_get_asid(vmcb), vmcb_get_np_ctrl(vmcb), + vmcb_get_np(vmcb) ? "NP" : "", + vmcb_get_sev(vmcb) ? "SEV" : "", + vmcb_get_sev_es(vmcb) ? "SEV_ES" : ""); 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> --- Changes since v1: - Pretty printing --- xen/arch/x86/hvm/svm/svmdebug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)