From patchwork Mon Dec 21 17:16:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 7897021 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CF744BEEE5 for ; Mon, 21 Dec 2015 17:19:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CF5EA202DD for ; Mon, 21 Dec 2015 17:19:54 +0000 (UTC) Received: from lists.xen.org (lists.xenproject.org [50.57.142.19]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 61A6A20259 for ; Mon, 21 Dec 2015 17:19:53 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aB44h-0000ez-5s; Mon, 21 Dec 2015 17:16:43 +0000 Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aB44f-0000eu-SE for xen-devel@lists.xen.org; Mon, 21 Dec 2015 17:16:42 +0000 Received: from [85.158.143.35] by server-3.bemta-4.messagelabs.com id 46/81-31122-9F338765; Mon, 21 Dec 2015 17:16:41 +0000 X-Env-Sender: prvs=790384ce8=Andrew.Cooper3@citrix.com X-Msg-Ref: server-6.tower-21.messagelabs.com!1450718196!6537108!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 7.35.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 30668 invoked from network); 21 Dec 2015 17:16:38 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-6.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 21 Dec 2015 17:16:38 -0000 X-IronPort-AV: E=Sophos;i="5.20,460,1444694400"; d="scan'208";a="326738333" From: Andrew Cooper To: Xen-devel Date: Mon, 21 Dec 2015 17:16:00 +0000 Message-ID: <1450718160-744-1-git-send-email-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 X-DLP: MIA2 Cc: Andrew Cooper , Boris Ostrovsky , Jan Beulich Subject: [Xen-devel] [PATCH] x86/mmuext: Unify okay/rc error handling in do_mmuext_op() X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP c/s 506db90 "x86/HVM: merge HVM and PVH hypercall tables" introduced a path whereby 'okay' was used uninitialised, with broke compilation on CentOS 7. Splitting the error handling like this is fragile and unnecessary. Drop the okay variable entirely and just use rc directly, substituting rc = -EINVAL/0 for okay = 0/1. In addition, two error messages are updated to print rc, and some stray whitespace is dropped. Signed-off-by: Andrew Cooper Reviewed-by: Boris Ostrovsky Reviewed-by: Jan Beulich --- CC: Jan Beulich CC: Boris Ostrovsky --- xen/arch/x86/mm.c | 65 +++++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 717546a..977b2f4 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2974,7 +2974,7 @@ long do_mmuext_op( struct vcpu *curr = current; struct domain *d = curr->domain; struct domain *pg_owner; - int okay, rc = put_old_guest_table(curr); + int rc = put_old_guest_table(curr); if ( unlikely(rc) ) { @@ -3053,7 +3053,7 @@ long do_mmuext_op( } } - okay = 1; + rc = 0; switch ( op.cmd ) { @@ -3087,33 +3087,32 @@ long do_mmuext_op( page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC); if ( unlikely(!page) ) { - okay = 0; + rc = -EINVAL; break; } rc = get_page_type_preemptible(page, type); - okay = !rc; - if ( unlikely(!okay) ) + if ( unlikely(rc) ) { if ( rc == -EINTR ) rc = -ERESTART; else if ( rc != -ERESTART ) - MEM_LOG("Error while pinning mfn %lx", page_to_mfn(page)); + MEM_LOG("Error %d while pinning mfn %lx", + rc, page_to_mfn(page)); if ( page != curr->arch.old_guest_table ) put_page(page); break; } - if ( (rc = xsm_memory_pin_page(XSM_HOOK, d, pg_owner, page)) != 0 ) - okay = 0; - else if ( unlikely(test_and_set_bit(_PGT_pinned, - &page->u.inuse.type_info)) ) + rc = xsm_memory_pin_page(XSM_HOOK, d, pg_owner, page); + if ( !rc && unlikely(test_and_set_bit(_PGT_pinned, + &page->u.inuse.type_info)) ) { MEM_LOG("Mfn %lx already pinned", page_to_mfn(page)); - okay = 0; + rc = -EINVAL; } - if ( unlikely(!okay) ) + if ( unlikely(rc) ) goto pin_drop; /* A page is dirtied when its pin status is set. */ @@ -3150,14 +3149,14 @@ long do_mmuext_op( page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC); if ( unlikely(!page) ) { - okay = 0; + rc = -EINVAL; MEM_LOG("Mfn %lx bad domain", op.arg1.mfn); break; } if ( !test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) ) { - okay = 0; + rc = -EINVAL; put_page(page); MEM_LOG("Mfn %lx not pinned", op.arg1.mfn); break; @@ -3212,20 +3211,18 @@ long do_mmuext_op( if ( op.arg1.mfn != 0 ) { if ( paging_mode_refcounts(d) ) - okay = get_page_from_pagenr(op.arg1.mfn, d); + rc = get_page_from_pagenr(op.arg1.mfn, d) ? 0 : -EINVAL; else - { rc = get_page_and_type_from_pagenr( op.arg1.mfn, PGT_root_page_table, d, 0, 1); - okay = !rc; - } - if ( unlikely(!okay) ) + + if ( unlikely(rc) ) { if ( rc == -EINTR ) rc = -ERESTART; else if ( rc != -ERESTART ) - MEM_LOG("Error while installing new mfn %lx", - op.arg1.mfn); + MEM_LOG("Error %d while installing new mfn %lx", + rc, op.arg1.mfn); break; } if ( VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) ) @@ -3248,7 +3245,6 @@ long do_mmuext_op( /* fallthrough */ case -ERESTART: curr->arch.old_guest_table = page; - okay = 0; break; default: BUG_ON(rc); @@ -3258,14 +3254,14 @@ long do_mmuext_op( break; } - + case MMUEXT_TLB_FLUSH_LOCAL: if ( likely(d == pg_owner) ) flush_tlb_local(); else rc = -EPERM; break; - + case MMUEXT_INVLPG_LOCAL: if ( unlikely(d != pg_owner) ) rc = -EPERM; @@ -3342,7 +3338,7 @@ long do_mmuext_op( else { MEM_LOG("Non-physdev domain tried to FLUSH_CACHE_GLOBAL"); - okay = 0; + rc = -EINVAL; } break; @@ -3356,12 +3352,12 @@ long do_mmuext_op( else if ( paging_mode_external(d) ) { MEM_LOG("ignoring SET_LDT hypercall from external domain"); - okay = 0; + rc = -EINVAL; } else if ( ((ptr & (PAGE_SIZE - 1)) != 0) || !__addr_ok(ptr) || (ents > 8192) ) { - okay = 0; + rc = -EINVAL; MEM_LOG("Bad args to SET_LDT: ptr=%lx, ents=%lx", ptr, ents); } else if ( (curr->arch.pv_vcpu.ldt_ents != ents) || @@ -3385,7 +3381,7 @@ long do_mmuext_op( if ( page ) put_page(page); MEM_LOG("Error while clearing mfn %lx", op.arg1.mfn); - okay = 0; + rc = -EINVAL; break; } @@ -3406,15 +3402,16 @@ long do_mmuext_op( P2M_ALLOC); if ( unlikely(!src_page) ) { - okay = 0; + rc = -EINVAL; MEM_LOG("Error while copying from mfn %lx", op.arg2.src_mfn); break; } dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC); - okay = (dst_page && get_page_type(dst_page, PGT_writable_page)); - if ( unlikely(!okay) ) + rc = (dst_page && + get_page_type(dst_page, PGT_writable_page)) ? 0 : -EINVAL; + if ( unlikely(rc) ) { put_page(src_page); if ( dst_page ) @@ -3443,7 +3440,7 @@ long do_mmuext_op( else if ( mfn & (L1_PAGETABLE_ENTRIES-1) ) { MEM_LOG("Unaligned superpage reference mfn %lx", mfn); - okay = 0; + rc = -EINVAL; } else if ( !opt_allow_superpage ) { @@ -3464,7 +3461,7 @@ long do_mmuext_op( else if ( mfn & (L1_PAGETABLE_ENTRIES-1) ) { MEM_LOG("Unaligned superpage reference mfn %lx", mfn); - okay = 0; + rc = -EINVAL; } else if ( !opt_allow_superpage ) { @@ -3483,8 +3480,6 @@ long do_mmuext_op( } done: - if ( unlikely(!okay) && !rc ) - rc = -EINVAL; if ( unlikely(rc) ) break;