|
[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 |