From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [Patch] hvm/vidirian: Avoid printing page_to_mfn(NULL) on error paths. Date: Tue, 8 Oct 2013 21:17:34 +0100 Message-ID: <1381263454-28115-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Keir Fraser , Jan Beulich , Tim Deegan List-Id: xen-devel@lists.xenproject.org While working in the viridian code, I noticed that 4cb6c4f4941 "x86/hvm: Use get_page_from_gfn() instead of get_gfn()/put_gfn." introduced two error paths where page_to_mfn(NULL) would be formatted and presented as a bad MFN. This provides junk in the warning rather than something useful. These two codepaths are fixed up to match their counterpart in wrmsr_hypervisor_regs() While auditing the other changes from 4cb6c4f4941, I noticed a small optimisation which could be made by changing the order of the validity checks to remove 6 NULL pointer checks. Signed-off-by: Andrew Cooper CC: Keir Fraser CC: Jan Beulich CC: Tim Deegan -- To help identify similar errors in the future, I wonder whether __page_to_mfn (and perhaps some of the other transformation functions) should turn into a static inline which asserts that its struct page_info pointer is within the bounds of the frame table. --- xen/arch/x86/hvm/hvm.c | 28 ++++++++++++---------------- xen/arch/x86/hvm/viridian.c | 4 ++-- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index de81e45..6fcd95a 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2499,27 +2499,25 @@ static enum hvm_copy_result __hvm_copy( page = get_page_from_gfn(curr->domain, gfn, &p2mt, P2M_UNSHARE); + if ( !page ) + return HVMCOPY_bad_gfn_to_mfn; + if ( p2m_is_paging(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); p2m_mem_paging_populate(curr->domain, gfn); return HVMCOPY_gfn_paged_out; } if ( p2m_is_shared(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); return HVMCOPY_gfn_shared; } if ( p2m_is_grant(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); return HVMCOPY_unhandleable; } - if ( !page ) - return HVMCOPY_bad_gfn_to_mfn; p = (char *)__map_domain_page(page) + (addr & ~PAGE_MASK); @@ -2597,27 +2595,25 @@ static enum hvm_copy_result __hvm_clear(paddr_t addr, int size) page = get_page_from_gfn(curr->domain, gfn, &p2mt, P2M_UNSHARE); + if ( !page ) + return HVMCOPY_bad_gfn_to_mfn; + if ( p2m_is_paging(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); p2m_mem_paging_populate(curr->domain, gfn); return HVMCOPY_gfn_paged_out; } if ( p2m_is_shared(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); return HVMCOPY_gfn_shared; } if ( p2m_is_grant(p2mt) ) { - if ( page ) - put_page(page); + put_page(page); return HVMCOPY_unhandleable; } - if ( !page ) - return HVMCOPY_bad_gfn_to_mfn; p = (char *)__map_domain_page(page) + (addr & ~PAGE_MASK); diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index d5462f2..0a0a4da 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -157,7 +157,7 @@ static void enable_hypercall_page(struct domain *d) if ( page ) put_page(page); gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n", gmfn, - page_to_mfn(page)); + page ? page_to_mfn(page) : ~0UL); return; } @@ -202,7 +202,7 @@ static void initialize_apic_assist(struct vcpu *v) if ( page ) put_page(page); gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n", gmfn, - page_to_mfn(page)); + page ? page_to_mfn(page) : ~0UL); return; } -- 1.7.10.4