[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn
> -----Original Message----- > From: julien@xxxxxxx <julien@xxxxxxx> > Sent: 22 March 2020 16:14 > To: xen-devel@xxxxxxxxxxxxxxxxxxxx > Cc: julien@xxxxxxx; Julien Grall <julien.grall@xxxxxxx>; Stefano Stabellini > <sstabellini@xxxxxxxxxx>; > Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper > <andrew.cooper3@xxxxxxxxxx>; George > Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; > Jan Beulich > <jbeulich@xxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné > <roger.pau@xxxxxxxxxx>; Paul Durrant > <paul@xxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian > <kevin.tian@xxxxxxxxx>; Tim Deegan > <tim@xxxxxxx> > Subject: [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use > typesafe gfn > > From: Julien Grall <julien.grall@xxxxxxx> > > No functional change intended. > > Only reasonable clean-ups are done in this patch. The rest will use _gfn > for the time being. > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx> Definitely an improvement so... Reviewed-by: Paul Durrant <paul@xxxxxxx> But a couple of things I noticed... [snip] > diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c > index 5d5a746a25..3c29ff86be 100644 > --- a/xen/arch/x86/hvm/domain.c > +++ b/xen/arch/x86/hvm/domain.c > @@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const > vcpu_hvm_context_t *ctx) > if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) ) > { > /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ > - struct page_info *page = get_page_from_gfn(v->domain, > - v->arch.hvm.guest_cr[3] >> PAGE_SHIFT, > + struct page_info *page; > + > + page = get_page_from_gfn(v->domain, > + gaddr_to_gfn(v->arch.hvm.guest_cr[3]), Should this be cr3_to_gfn? > NULL, P2M_ALLOC); > if ( !page ) > { > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index a3d115b650..9f720e7aa1 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2216,7 +2216,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer) > { > struct vcpu *v = current; > struct domain *d = v->domain; > - unsigned long gfn, old_value = v->arch.hvm.guest_cr[0]; > + unsigned long old_value = v->arch.hvm.guest_cr[0]; > struct page_info *page; > > HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value); > @@ -2271,7 +2271,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer) > if ( !paging_mode_hap(d) ) > { > /* The guest CR3 must be pointing to the guest physical. */ > - gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT; > + gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]); > + Same here. > page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > if ( !page ) > { > @@ -2363,7 +2364,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer) > { > /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ > HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value); > - page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, cr3_to_gfn(value), > NULL, P2M_ALLOC); > if ( !page ) > goto bad_cr3; > @@ -3191,7 +3192,7 @@ enum hvm_translation_result hvm_translate_get_page( > && hvm_mmio_internal(gfn_to_gaddr(gfn)) ) > return HVMTRANS_bad_gfn_to_mfn; > > - page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE); > > if ( !page ) > return HVMTRANS_bad_gfn_to_mfn; > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 32d8d847f2..a9abd6d3f1 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -299,7 +299,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct > hvm_hw_cpu *c) > { > if ( c->cr0 & X86_CR0_PG ) > { > - page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, cr3_to_gfn(c->cr3), > NULL, P2M_ALLOC); > if ( !page ) > { > @@ -2230,9 +2230,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr) > return NULL; > > /* Need to translate L1-GPA to MPA */ > - page = get_page_from_gfn(v->domain, > - nv->nv_vvmcxaddr >> PAGE_SHIFT, > - &p2mt, P2M_ALLOC | P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, > + gaddr_to_gfn(nv->nv_vvmcxaddr), > + &p2mt, P2M_ALLOC | P2M_UNSHARE); > if ( !page ) > return NULL; > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > b/xen/arch/x86/hvm/viridian/viridian.c > index 977c1bc54f..3d75a0f133 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -242,16 +242,16 @@ static void dump_hypercall(const struct domain *d) > > static void enable_hypercall_page(struct domain *d) > { > - unsigned long gmfn = d->arch.hvm.viridian->hypercall_gpa.pfn; > - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + gfn_t gfn = _gfn(d->arch.hvm.viridian->hypercall_gpa.pfn); > + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > uint8_t *p; > > if ( !page || !get_page_type(page, PGT_writable_page) ) > { > if ( page ) > put_page(page); > - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > return; > } > > @@ -719,13 +719,13 @@ void viridian_dump_guest_page(const struct vcpu *v, > const char *name, > > void viridian_map_guest_page(struct domain *d, struct viridian_page *vp) > { > - unsigned long gmfn = vp->msr.pfn; > + gfn_t gfn = _gfn(vp->msr.pfn); > struct page_info *page; > > if ( vp->ptr ) > return; > > - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > if ( !page ) > goto fail; > > @@ -746,8 +746,8 @@ void viridian_map_guest_page(struct domain *d, struct > viridian_page *vp) > return; > > fail: > - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > } > > void viridian_unmap_guest_page(struct viridian_page *vp) > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index a1e3a19c0a..f1898c63c5 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -681,7 +681,7 @@ static int vmx_restore_cr0_cr3( > { > if ( cr0 & X86_CR0_PG ) > { > - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), And here. > NULL, P2M_ALLOC); > if ( !page ) > { > @@ -1321,7 +1321,7 @@ static void vmx_load_pdptrs(struct vcpu *v) > if ( (cr3 & 0x1fUL) && !hvm_pcid_enabled(v) ) > goto crash; > > - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt, > P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), &p2mt, > P2M_UNSHARE); And here. Paul _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |