| Message ID | 3c419824febca229cedf2a3bd42cb68d3a3d56a8.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: > The suffix is redundant for np/sev/sev-es bits. Drop it > to avoid adding extra code volume. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>i Typo on the end of your email address? > diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c > index e4e01add8c..7e285cf85a 100644 > --- a/xen/arch/x86/hvm/svm/nestedsvm.c > +++ b/xen/arch/x86/hvm/svm/nestedsvm.c > @@ -706,7 +706,7 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs, > } > > /* nested paging for the guest */ > - svm->ns_hap_enabled = !!ns_vmcb->_np_enable; > + svm->ns_hap_enabled = !!ns_vmcb->_np; Because the type of is bool, the !! can be dropped too while changing this line. It seems that this was missing cleanup from f57ae00635 "SVM: split _np_enable VMCB field". Anyway, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> and I'm happy to fix on commit.
On 3/8/24 00:22, Andrew Cooper wrote: > On 07/03/2024 9:40 pm, Vaishali Thakkar wrote: >> The suffix is redundant for np/sev/sev-es bits. Drop it >> to avoid adding extra code volume. >> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>i > > Typo on the end of your email address? Oops, thanks for catching it. >> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c >> index e4e01add8c..7e285cf85a 100644 >> --- a/xen/arch/x86/hvm/svm/nestedsvm.c >> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c >> @@ -706,7 +706,7 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs, >> } >> >> /* nested paging for the guest */ >> - svm->ns_hap_enabled = !!ns_vmcb->_np_enable; >> + svm->ns_hap_enabled = !!ns_vmcb->_np; > > Because the type of is bool, the !! can be dropped too while changing > this line. Thanks for the review. As I'm sending the revised patchset anyway, will fix both things in this patch too. > It seems that this was missing cleanup from f57ae00635 "SVM: split > _np_enable VMCB field". > > Anyway, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> and I'm > happy to fix on commit.
On 07.03.2024 22:40, Vaishali Thakkar wrote: > --- a/xen/arch/x86/hvm/svm/nestedsvm.c > +++ b/xen/arch/x86/hvm/svm/nestedsvm.c > @@ -571,7 +571,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) > if ( nestedhvm_paging_mode_hap(v) ) > { > /* host nested paging + guest nested paging. */ > - n2vmcb->_np_enable = 1; > + n2vmcb->_np = 1; Given the field is bool, "true" here and elsewhere as well as ... > @@ -601,7 +601,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) > else > { > /* host shadow paging + guest shadow paging. */ > - n2vmcb->_np_enable = 0; > + n2vmcb->_np = 0; ... "false" here (and once again further down)? Jan
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c index e4e01add8c..7e285cf85a 100644 --- a/xen/arch/x86/hvm/svm/nestedsvm.c +++ b/xen/arch/x86/hvm/svm/nestedsvm.c @@ -571,7 +571,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) if ( nestedhvm_paging_mode_hap(v) ) { /* host nested paging + guest nested paging. */ - n2vmcb->_np_enable = 1; + n2vmcb->_np = 1; nestedsvm_vmcb_set_nestedp2m(v, ns_vmcb, n2vmcb); @@ -585,7 +585,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) else if ( paging_mode_hap(v->domain) ) { /* host nested paging + guest shadow paging. */ - n2vmcb->_np_enable = 1; + n2vmcb->_np = 1; /* Keep h_cr3 as it is. */ n2vmcb->_h_cr3 = n1vmcb->_h_cr3; /* When l1 guest does shadow paging @@ -601,7 +601,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) else { /* host shadow paging + guest shadow paging. */ - n2vmcb->_np_enable = 0; + n2vmcb->_np = 0; n2vmcb->_h_cr3 = 0x0; /* TODO: Once shadow-shadow paging is in place come back to here @@ -706,7 +706,7 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs, } /* nested paging for the guest */ - svm->ns_hap_enabled = !!ns_vmcb->_np_enable; + svm->ns_hap_enabled = !!ns_vmcb->_np; /* Remember the V_INTR_MASK in hostflags */ svm->ns_hostflags.fields.vintrmask = !!ns_vmcb->_vintr.fields.intr_masking; @@ -1084,7 +1084,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs) if ( nestedhvm_paging_mode_hap(v) ) { /* host nested paging + guest nested paging. */ - ns_vmcb->_np_enable = n2vmcb->_np_enable; + ns_vmcb->_np = n2vmcb->_np; ns_vmcb->_cr3 = n2vmcb->_cr3; /* The vmcb->h_cr3 is the shadowed h_cr3. The original * unshadowed guest h_cr3 is kept in ns_vmcb->h_cr3, @@ -1093,7 +1093,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs) else if ( paging_mode_hap(v->domain) ) { /* host nested paging + guest shadow paging. */ - ns_vmcb->_np_enable = 0; + ns_vmcb->_np = 0; /* Throw h_cr3 away. Guest is not allowed to set it or * it can break out, otherwise (security hole!) */ ns_vmcb->_h_cr3 = 0x0; @@ -1104,7 +1104,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs) else { /* host shadow paging + guest shadow paging. */ - ns_vmcb->_np_enable = 0; + ns_vmcb->_np = 0; ns_vmcb->_h_cr3 = 0x0; /* The vmcb->_cr3 is the shadowed cr3. The original * unshadowed guest cr3 is kept in ns_vmcb->_cr3, diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index b551eac807..1b38f6a494 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -473,7 +473,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c) if ( paging_mode_hap(v->domain) ) { - vmcb_set_np_enable(vmcb, 1); + vmcb_set_np(vmcb, 1); vmcb_set_g_pat(vmcb, MSR_IA32_CR_PAT_RESET /* guest PAT */); vmcb_set_h_cr3(vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m))); } diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c index 282fe7cdbe..a8dd87ca36 100644 --- a/xen/arch/x86/hvm/svm/vmcb.c +++ b/xen/arch/x86/hvm/svm/vmcb.c @@ -133,7 +133,7 @@ static int construct_vmcb(struct vcpu *v) if ( paging_mode_hap(v->domain) ) { - vmcb->_np_enable = 1; /* enable nested paging */ + vmcb->_np = 1; /* enable nested paging */ vmcb->_g_pat = MSR_IA32_CR_PAT_RESET; /* guest PAT */ vmcb->_h_cr3 = pagetable_get_paddr( p2m_get_pagetable(p2m_get_hostp2m(v->domain))); diff --git a/xen/arch/x86/include/asm/hvm/svm/vmcb.h b/xen/arch/x86/include/asm/hvm/svm/vmcb.h index 91221ff4e2..76507576ba 100644 --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h @@ -473,12 +473,12 @@ struct vmcb_struct { intinfo_t exit_int_info; /* offset 0x88 */ union { /* offset 0x90 - cleanbit 4 */ struct { - bool _np_enable :1; - bool _sev_enable :1; - bool _sev_es_enable :1; - bool _gmet :1; - bool _np_sss :1; - bool _vte :1; + bool _np :1; + bool _sev :1; + bool _sev_es :1; + bool _gmet :1; + bool _np_sss :1; + bool _vte :1; }; uint64_t _np_ctrl; }; @@ -645,9 +645,9 @@ VMCB_ACCESSORS(msrpm_base_pa, iopm) VMCB_ACCESSORS(guest_asid, asid) VMCB_ACCESSORS(vintr, tpr) VMCB_ACCESSORS(np_ctrl, np) -VMCB_ACCESSORS_(np_enable, bool, np) -VMCB_ACCESSORS_(sev_enable, bool, np) -VMCB_ACCESSORS_(sev_es_enable, bool, np) +VMCB_ACCESSORS_(np, bool, np) +VMCB_ACCESSORS_(sev, bool, np) +VMCB_ACCESSORS_(sev_es, bool, np) VMCB_ACCESSORS_(gmet, bool, np) VMCB_ACCESSORS_(vte, bool, np) VMCB_ACCESSORS(h_cr3, np)
The suffix is redundant for np/sev/sev-es bits. Drop it to avoid adding extra code volume. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>i --- xen/arch/x86/hvm/svm/nestedsvm.c | 14 +++++++------- xen/arch/x86/hvm/svm/svm.c | 2 +- xen/arch/x86/hvm/svm/vmcb.c | 2 +- xen/arch/x86/include/asm/hvm/svm/vmcb.h | 18 +++++++++--------- 4 files changed, 18 insertions(+), 18 deletions(-)