From patchwork Fri Jan 17 16:35:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 11339537 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8169A92A for ; Fri, 17 Jan 2020 16:36:47 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 67D902064C for ; Fri, 17 Jan 2020 16:36:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 67D902064C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1isUaz-0007gC-VE; Fri, 17 Jan 2020 16:35:41 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1isUay-0007g6-92 for xen-devel@lists.xenproject.org; Fri, 17 Jan 2020 16:35:40 +0000 X-Inumbo-ID: 627d8c22-3947-11ea-b590-12813bfff9fa Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 627d8c22-3947-11ea-b590-12813bfff9fa; Fri, 17 Jan 2020 16:35:35 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E342EAC6B; Fri, 17 Jan 2020 16:35:33 +0000 (UTC) To: "xen-devel@lists.xenproject.org" , Andrew Cooper , Wei Liu , =?utf-8?q?Roger_Pau_Monn=C3=A9?= From: Jan Beulich Message-ID: <54929bde-aa1c-598c-6d74-894f387ebd6c@suse.com> Date: Fri, 17 Jan 2020 17:35:36 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 Content-Language: en-US Subject: [Xen-devel] [PATCH] EPT: do away with hidden GUEST_TABLE_MAP_FAILED == 0 assumptions 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: George Dunlap , Kevin Tian Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" The code is quite a bit easier to read and to reason about this way, I think. In ept_set_entry() additionally change the function's return value in the MAP_FAILED case to -ENOMEM; -ENOENT would be applicable only when ept_next_entry() was invoked with "read_only" set to true. In two cases, where ept_next_level() follows an ept_split_superpage() invocation, actually tighten the loop exit condition from "== MAP_FAILED" to "!= NORMAL_PAGE". Continuing these loops for other than NORMAL_PAGE is invalid, and there are ASSERT()s in place after these loops. Also reduce the scope of "ret" variables where possible, in particular to better distinguish them from "rc" often used in the same function. Finally drop pointless "else" in a few areas touched anyway. Signed-off-by: Jan Beulich Reviewed-by: Kevin Tian --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -292,8 +292,8 @@ static bool_t ept_split_super_page(struc * and map the next table, if available. If the entry is empty * and read_only is set, * Return values: - * 0: Failed to map. Either read_only was set and the entry was - * empty, or allocating a new page failed. + * GUEST_TABLE_MAP_FAILED: Failed to map. Either read_only was set and the + * entry was empty, or allocating a new page failed. * GUEST_TABLE_NORMAL_PAGE: next level mapped normally * GUEST_TABLE_SUPER_PAGE: * The next entry points to a superpage, and caller indicates @@ -404,12 +404,13 @@ static int ept_invalidate_emt_range(stru ept_entry_t *table; unsigned long gfn_remainder = first_gfn; unsigned int i, index; - int wrc, rc = 0, ret = GUEST_TABLE_MAP_FAILED; + int wrc, rc = 0; table = map_domain_page(pagetable_get_mfn(p2m_get_pagetable(p2m))); for ( i = p2m->ept.wl; i > target; --i ) { - ret = ept_next_level(p2m, 1, &table, &gfn_remainder, i); + int ret = ept_next_level(p2m, 1, &table, &gfn_remainder, i); + if ( ret == GUEST_TABLE_MAP_FAILED ) goto out; if ( ret != GUEST_TABLE_NORMAL_PAGE ) @@ -434,8 +435,10 @@ static int ept_invalidate_emt_range(stru ASSERT(wrc == 0); for ( ; i > target; --i ) - if ( !ept_next_level(p2m, 1, &table, &gfn_remainder, i) ) + if ( ept_next_level(p2m, 1, &table, &gfn_remainder, i) != + GUEST_TABLE_NORMAL_PAGE ) break; + /* We just installed the pages we need. */ ASSERT(i == target); } @@ -694,12 +697,12 @@ ept_set_entry(struct p2m_domain *p2m, gf for ( i = ept->wl; i > target; i-- ) { ret = ept_next_level(p2m, 0, &table, &gfn_remainder, i); - if ( !ret ) + if ( ret == GUEST_TABLE_MAP_FAILED ) { - rc = -ENOENT; + rc = -ENOMEM; goto out; } - else if ( ret != GUEST_TABLE_NORMAL_PAGE ) + if ( ret != GUEST_TABLE_NORMAL_PAGE ) break; } @@ -756,7 +759,8 @@ ept_set_entry(struct p2m_domain *p2m, gf /* then move to the level we want to make real changes */ for ( ; i > target; i-- ) - if ( !ept_next_level(p2m, 0, &table, &gfn_remainder, i) ) + if ( ept_next_level(p2m, 0, &table, &gfn_remainder, i) != + GUEST_TABLE_NORMAL_PAGE ) break; /* We just installed the pages we need. */ ASSERT(i == target); @@ -859,7 +863,6 @@ static mfn_t ept_get_entry(struct p2m_do ept_entry_t *ept_entry; u32 index; int i; - int ret = 0; bool_t recalc = 0; mfn_t mfn = INVALID_MFN; struct ept_data *ept = &p2m->ept; @@ -883,13 +886,15 @@ static mfn_t ept_get_entry(struct p2m_do for ( i = ept->wl; i > 0; i-- ) { + int ret; + retry: if ( table[gfn_remainder >> (i * EPT_TABLE_ORDER)].recalc ) recalc = 1; ret = ept_next_level(p2m, 1, &table, &gfn_remainder, i); - if ( !ret ) + if ( ret == GUEST_TABLE_MAP_FAILED ) goto out; - else if ( ret == GUEST_TABLE_POD_PAGE ) + if ( ret == GUEST_TABLE_POD_PAGE ) { if ( !(q & P2M_ALLOC) ) { @@ -905,10 +910,9 @@ static mfn_t ept_get_entry(struct p2m_do if ( p2m_pod_demand_populate(p2m, gfn_, i * EPT_TABLE_ORDER) ) goto retry; - else - goto out; + goto out; } - else if ( ret == GUEST_TABLE_SUPER_PAGE ) + if ( ret == GUEST_TABLE_SUPER_PAGE ) break; } @@ -1289,7 +1293,6 @@ static void ept_dump_p2m_table(unsigned ept_entry_t *table, *ept_entry; int order; int i; - int ret = 0; unsigned long gfn, gfn_remainder; unsigned long record_counter = 0; struct p2m_domain *p2m; @@ -1307,6 +1310,7 @@ static void ept_dump_p2m_table(unsigned for ( gfn = 0; gfn <= p2m->max_mapped_pfn; gfn += 1UL << order ) { char c = 0; + int ret = GUEST_TABLE_MAP_FAILED; gfn_remainder = gfn; table = map_domain_page(pagetable_get_mfn(p2m_get_pagetable(p2m)));