From patchwork Wed Mar 27 15:21:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandru Stefan ISAILA X-Patchwork-Id: 10873689 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 45D5B925 for ; Wed, 27 Mar 2019 15:23:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 30FAB28DB6 for ; Wed, 27 Mar 2019 15:23:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2F00E28DD3; Wed, 27 Mar 2019 15:23:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.0 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E845B28DB6 for ; Wed, 27 Mar 2019 15:23:18 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1h9AMp-0006tW-PP; Wed, 27 Mar 2019 15:21:27 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1h9AMo-0006tO-GK for xen-devel@lists.xenproject.org; Wed, 27 Mar 2019 15:21:26 +0000 X-Inumbo-ID: fb4356ba-50a3-11e9-bc90-bc764e045a96 Received: from EUR04-HE1-obe.outbound.protection.outlook.com (unknown [2a01:111:f400:fe0d::724]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTPS id fb4356ba-50a3-11e9-bc90-bc764e045a96; Wed, 27 Mar 2019 15:21:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bitdefender.onmicrosoft.com; s=selector1-bitdefender-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=BjE5heKMebVp77O6MTqFfTfQl3qr/UAgxK6o5294iPU=; b=zZZ2zT8rv6iGsBFtJhBnX1pyMNhZYgpj5vz5dh4uxygB5FvM5eObcZVjLBPKZLtJ90jU13RTOhC9U4k5KFydy00eAWYa9SCEgpOy/UF+saQ8mCBOhSIzRMihe5zzwP5Bf7SN2Sx+89yaiEz65dyv6SbesoanQO1OX3ZAELPmbvE= Received: from DB6PR0202MB2917.eurprd02.prod.outlook.com (10.171.76.8) by DB6PR0202MB2904.eurprd02.prod.outlook.com (10.171.72.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1750.16; Wed, 27 Mar 2019 15:21:22 +0000 Received: from DB6PR0202MB2917.eurprd02.prod.outlook.com ([fe80::f966:3d29:9673:6f28]) by DB6PR0202MB2917.eurprd02.prod.outlook.com ([fe80::f966:3d29:9673:6f28%5]) with mapi id 15.20.1730.019; Wed, 27 Mar 2019 15:21:22 +0000 From: Alexandru Stefan ISAILA To: "xen-devel@lists.xenproject.org" Thread-Topic: [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value Thread-Index: AQHU5LC86WyzpqhoKE2C9o362jczng== Date: Wed, 27 Mar 2019 15:21:22 +0000 Message-ID: <20190327152107.29288-1-aisaila@bitdefender.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: AM6PR08CA0016.eurprd08.prod.outlook.com (2603:10a6:20b:b2::28) To DB6PR0202MB2917.eurprd02.prod.outlook.com (2603:10a6:4:b1::8) authentication-results: spf=none (sender IP is ) smtp.mailfrom=aisaila@bitdefender.com; x-ms-exchange-messagesentrepresentingtype: 1 x-mailer: git-send-email 2.17.1 x-originating-ip: [91.199.104.6] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b42cce36-7d2a-4ee9-4ce1-08d6b2c7de53 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(2017052603328)(7153060)(7193020); SRVR:DB6PR0202MB2904; x-ms-traffictypediagnostic: DB6PR0202MB2904:|DB6PR0202MB2904: x-microsoft-antispam-prvs: x-forefront-prvs: 0989A7979C x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(376002)(396003)(346002)(39860400002)(136003)(366004)(189003)(199004)(2351001)(186003)(25786009)(26005)(71190400001)(71200400001)(256004)(316002)(1076003)(6436002)(6916009)(5660300002)(4326008)(106356001)(66066001)(105586002)(36756003)(14444005)(86362001)(102836004)(305945005)(7736002)(478600001)(107886003)(14454004)(2501003)(99286004)(2616005)(476003)(486006)(6512007)(52116002)(3846002)(6116002)(54906003)(8676002)(81166006)(6506007)(68736007)(81156014)(386003)(97736004)(50226002)(5640700003)(2906002)(6486002)(8936002)(53936002); DIR:OUT; SFP:1102; SCL:1; SRVR:DB6PR0202MB2904; H:DB6PR0202MB2917.eurprd02.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: bitdefender.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: blsfIct1nDdqawXOvH9hmwsrvKdqC8ndwwMXaMC3NTpBaU9EP0yNdqxLQnOBJueZI4o7DEU+R6Ieix3wdmMN7R/LAH2sd5aMrhHavzPMHGV54KGVEn3FynNlTweK3dNu0GA1UBj7x7/mBbRx2VjL/BIsoE/cqxPCSpb0IfGS95lTKxiZwh/xnLLlaqiizyxPtUHaVh+vsMhtIyNgEJa/rUKQy3DPfPfdSmemF6sOPb2VgjIJqPezurqV9L6IOfO/oGAz0TLqltcQwh8J5E5x+B12wCZzX/AwlPCd5ZAGqdpvRbtLrFaGixEXd3QYiZ5mjKQgFZ5j8pb6UWwtAMKGEMS1nExeQk7U74pkTmM6BSYwwoVFNSAGrSeuVqAbwFzHjavY/Q5RpCpIP6wNXjyoPtxE2l4UexyZEFKpfhPy25Y= MIME-Version: 1.0 X-OriginatorOrg: bitdefender.com X-MS-Exchange-CrossTenant-Network-Message-Id: b42cce36-7d2a-4ee9-4ce1-08d6b2c7de53 X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Mar 2019 15:21:22.2224 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 487baf29-f1da-469a-9221-243f830c36f3 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0202MB2904 Subject: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: "kevin.tian@intel.com" , "wei.liu2@citrix.com" , "jun.nakajima@intel.com" , "george.dunlap@eu.citrix.com" , "andrew.cooper3@citrix.com" , "Paul.Durrant@citrix.com" , "jbeulich@suse.com" , Alexandru Stefan ISAILA , "roger.pau@citrix.com" Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP In the case of any errors, finish_type_change() passes values returned from p2m->recalc() up the stack (with some exceptions in the case where an error is expected); this eventually ends up being returned to the XEN_DOMOP_map_mem_type_to_ioreq_server hypercall. However, on Intel processors (but not on AMD processor), p2m->recalc() can also return '1' as well as '0'. This case is handled very inconsistently: finish_type_change() will return the value of the final entry it attempts, discarding results for other entries; p2m_finish_type_change() will attempt to accumulate '1's, so that it returns '1' if any of the calls to finish_type_change() returns '1'; and dm_op() will again return '1' only if the very last call to p2m_finish_type_change() returns '1'. The result is that the XEN_DMOP_map_mem_type_to_ioreq_server() hypercall will sometimes return 0 and sometimes return 1 on success, in an unpredictable manner. The hypercall documentation doesn't mention return values; but it's not clear what the caller could do with the information about whether entries had been changed or not. At the moment it's always 0 on AMD boxes, and *usually* 1 on Intel boxes; so nothing can be relying on a '1' return value for correctness (or if it is, it's broken). Make the return value on success consistently '0' by only returning 0/-ERROR from finish_type_change(). Also remove the accumulation code from p2m_finish_type_change(). Suggested-by: George Dunlap Signed-off-by: Alexandru Isaila --- Changes since V3: - Remove positive returned values from p2m->recalc. --- xen/arch/x86/mm/p2m-ept.c | 10 ++++++---- xen/arch/x86/mm/p2m.c | 15 +++++---------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index e3044bee2e..d336c138b0 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -459,8 +459,7 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m, * for entries not involved in the translation of the given GFN. * Returns: * - negative errno values in error, - * - zero if no adjustment was done, - * - a positive value if at least one adjustment was done. + * - zero for ok */ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) { @@ -600,6 +599,9 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) v->arch.hvm.vmx.ept_spurious_misconfig = 1; } + if ( rc > 0 ) + rc = 0; + return rc; } @@ -621,7 +623,7 @@ bool_t ept_handle_misconfig(uint64_t gpa) p2m_unlock(p2m); - return spurious ? (rc >= 0) : (rc > 0); + return spurious && !rc; } /* @@ -668,7 +670,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn, /* Carry out any eventually pending earlier changes first. */ ret = resolve_misconfig(p2m, gfn); - if ( ret < 0 ) + if ( ret ) return ret; ASSERT((target == 2 && hap_has_1gb) || diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index b9bbb8f485..d5690b96bf 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1172,7 +1172,7 @@ static int finish_type_change(struct p2m_domain *p2m, { rc = p2m->recalc(p2m, gfn); /* - * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return + * ept->recalc could return 0/-ENOMEM. pt->recalc could return * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping * gfn here. */ @@ -1201,7 +1201,7 @@ int p2m_finish_type_change(struct domain *d, rc = finish_type_change(hostp2m, first_gfn, max_nr); - if ( rc < 0 ) + if ( rc ) goto out; #ifdef CONFIG_HVM @@ -1213,22 +1213,17 @@ int p2m_finish_type_change(struct domain *d, if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) ) { struct p2m_domain *altp2m = d->arch.altp2m_p2m[i]; - int rc1; p2m_lock(altp2m); - rc1 = finish_type_change(altp2m, first_gfn, max_nr); + rc = finish_type_change(altp2m, first_gfn, max_nr); p2m_unlock(altp2m); - if ( rc1 < 0 ) - { - rc = rc1; + if ( rc < 0 ) goto out; - } - - rc |= rc1; } } #endif + rc = 0; out: p2m_unlock(hostp2m);