From patchwork Wed Oct 28 09:24:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 11862637 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 5FDEF6A2 for ; Wed, 28 Oct 2020 09:25:18 +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 0E21424680 for ; Wed, 28 Oct 2020 09:25:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=suse.com header.i=@suse.com header.b="IaRkl9wN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0E21424680 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.13435.34082 (Exim 4.92) (envelope-from ) id 1kXhhQ-0005gz-Mb; Wed, 28 Oct 2020 09:24:56 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 13435.34082; Wed, 28 Oct 2020 09:24:56 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kXhhQ-0005gs-JF; Wed, 28 Oct 2020 09:24:56 +0000 Received: by outflank-mailman (input) for mailman id 13435; Wed, 28 Oct 2020 09:24:56 +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.92) (envelope-from ) id 1kXhhP-0005gl-UQ for xen-devel@lists.xenproject.org; Wed, 28 Oct 2020 09:24:55 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 7387b699-f4a1-4898-bf58-bea01d086430; Wed, 28 Oct 2020 09:24:53 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id D76F5AD1E; Wed, 28 Oct 2020 09:24:52 +0000 (UTC) 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.92) (envelope-from ) id 1kXhhP-0005gl-UQ for xen-devel@lists.xenproject.org; Wed, 28 Oct 2020 09:24:55 +0000 X-Inumbo-ID: 7387b699-f4a1-4898-bf58-bea01d086430 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 7387b699-f4a1-4898-bf58-bea01d086430; Wed, 28 Oct 2020 09:24:53 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1603877092; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BDnAoknm+d841ow9/Slk5qnPpHUKxVRIWzU0TPE9W4Q=; b=IaRkl9wNnRkAYR4tHYkztpaya1S+G4WqI1ofPVXEweLaJHLizWLBP6alUy/hXF9q8YaIW4 1bVoFRXzw9WjWgnDGPmRESha8NKmfKLb/wfvunYDHj5k26YTos9zGLIif3KmGJPbbsBvrr FDqdTRFs5ZRo3AYDeZRPdQbC713JjNQ= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id D76F5AD1E; Wed, 28 Oct 2020 09:24:52 +0000 (UTC) Subject: [PATCH 5/5] x86/p2m: split write_p2m_entry() hook From: Jan Beulich To: "xen-devel@lists.xenproject.org" Cc: Andrew Cooper , Wei Liu , =?utf-8?q?Roger_Pau_Monn=C3=A9?= , George Dunlap , Tim Deegan References: <29d30de1-2a8d-aee2-d3c3-331758766fc9@suse.com> Message-ID: <7b2b7cc9-8828-41bd-7949-764161bbe7ff@suse.com> Date: Wed, 28 Oct 2020 10:24:53 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <29d30de1-2a8d-aee2-d3c3-331758766fc9@suse.com> Content-Language: en-US Fair parts of the present handlers are identical; in fact nestedp2m_write_p2m_entry() lacks a call to p2m_entry_modify(). Move common parts right into write_p2m_entry(), splitting the hooks into a "pre" one (needed just by shadow code) and a "post" one. For the common parts moved I think that the p2m_flush_nestedp2m() is, at least from an abstract perspective, also applicable in the shadow case. Hence it doesn't get a 3rd hook put in place. The initial comment that was in hap_write_p2m_entry() gets dropped: Its placement was bogus, and looking back the the commit introducing it (dd6de3ab9985 "Implement Nested-on-Nested") I can't see either what use of a p2m it was meant to be associated with. Signed-off-by: Jan Beulich Acked-by: Tim Deegan Acked-by: Roger Pau Monné --- RFC: This is effectively the alternative to the suggestion in an earlier patch that we might do away with the hook altogether. Of course a hybrid approach would also be possible, by using direct calls here instead of splitting the hook into two. --- I'm unsure whether p2m_init_nestedp2m() zapping the "pre" hook is actually correct, or whether previously the sh_unshadow_for_p2m_change() invocation was wrongly skipped. --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -774,55 +774,18 @@ static void hap_update_paging_modes(stru put_gfn(d, cr3_gfn); } -static int -hap_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, l1_pgentry_t *p, - l1_pgentry_t new, unsigned int level) +static void +hap_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags) { struct domain *d = p2m->domain; - uint32_t old_flags; - mfn_t omfn; - int rc; - /* We know always use the host p2m here, regardless if the vcpu - * is in host or guest mode. The vcpu can be in guest mode by - * a hypercall which passes a domain and chooses mostly the first - * vcpu. */ - - paging_lock(d); - old_flags = l1e_get_flags(*p); - omfn = l1e_get_mfn(*p); - - rc = p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)), - p2m_flags_to_type(old_flags), l1e_get_mfn(new), - omfn, level); - if ( rc ) - { - paging_unlock(d); - return rc; - } - - safe_write_pte(p, new); - if ( old_flags & _PAGE_PRESENT ) + if ( oflags & _PAGE_PRESENT ) guest_flush_tlb_mask(d, d->dirty_cpumask); - - paging_unlock(d); - - if ( nestedhvm_enabled(d) && (old_flags & _PAGE_PRESENT) && - !p2m_get_hostp2m(d)->defer_nested_flush && - /* - * We are replacing a valid entry so we need to flush nested p2ms, - * unless the only change is an increase in access rights. - */ - (!mfn_eq(omfn, l1e_get_mfn(new)) || - !perms_strictly_increased(old_flags, l1e_get_flags(new))) ) - p2m_flush_nestedp2m(d); - - return 0; } void hap_p2m_init(struct p2m_domain *p2m) { - p2m->write_p2m_entry = hap_write_p2m_entry; + p2m->write_p2m_entry_post = hap_write_p2m_entry_post; } static unsigned long hap_gva_to_gfn_real_mode( --- a/xen/arch/x86/mm/hap/nested_hap.c +++ b/xen/arch/x86/mm/hap/nested_hap.c @@ -71,24 +71,11 @@ /* NESTED VIRT P2M FUNCTIONS */ /********************************************/ -int -nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, - l1_pgentry_t *p, l1_pgentry_t new, unsigned int level) +void +nestedp2m_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags) { - struct domain *d = p2m->domain; - uint32_t old_flags; - - paging_lock(d); - - old_flags = l1e_get_flags(*p); - safe_write_pte(p, new); - - if (old_flags & _PAGE_PRESENT) - guest_flush_tlb_mask(d, p2m->dirty_cpumask); - - paging_unlock(d); - - return 0; + if ( oflags & _PAGE_PRESENT ) + guest_flush_tlb_mask(p2m->domain, p2m->dirty_cpumask); } /********************************************/ --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -122,17 +122,55 @@ static int write_p2m_entry(struct p2m_do { struct domain *d = p2m->domain; struct vcpu *v = current; - int rc = 0; if ( v->domain != d ) v = d->vcpu ? d->vcpu[0] : NULL; if ( likely(v && paging_mode_enabled(d) && paging_get_hostmode(v)) || p2m_is_nestedp2m(p2m) ) - rc = p2m->write_p2m_entry(p2m, gfn, p, new, level); + { + unsigned int oflags; + mfn_t omfn; + int rc; + + paging_lock(d); + + if ( p2m->write_p2m_entry_pre ) + p2m->write_p2m_entry_pre(d, gfn, p, new, level); + + oflags = l1e_get_flags(*p); + omfn = l1e_get_mfn(*p); + + rc = p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)), + p2m_flags_to_type(oflags), l1e_get_mfn(new), + omfn, level); + if ( rc ) + { + paging_unlock(d); + return rc; + } + + safe_write_pte(p, new); + + if ( p2m->write_p2m_entry_post ) + p2m->write_p2m_entry_post(p2m, oflags); + + paging_unlock(d); + + if ( nestedhvm_enabled(d) && !p2m_is_nestedp2m(p2m) && + (oflags & _PAGE_PRESENT) && + !p2m_get_hostp2m(d)->defer_nested_flush && + /* + * We are replacing a valid entry so we need to flush nested p2ms, + * unless the only change is an increase in access rights. + */ + (!mfn_eq(omfn, l1e_get_mfn(new)) || + !perms_strictly_increased(oflags, l1e_get_flags(new))) ) + p2m_flush_nestedp2m(d); + } else safe_write_pte(p, new); - return rc; + return 0; } // Find the next level's P2M entry, checking for out-of-range gfn's... --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -198,7 +198,8 @@ static int p2m_init_nestedp2m(struct dom return -ENOMEM; } p2m->p2m_class = p2m_nested; - p2m->write_p2m_entry = nestedp2m_write_p2m_entry; + p2m->write_p2m_entry_pre = NULL; + p2m->write_p2m_entry_post = nestedp2m_write_p2m_entry_post; list_add(&p2m->np2m_list, &p2m_get_hostp2m(d)->np2m_list); } --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -3137,34 +3137,22 @@ static void sh_unshadow_for_p2m_change(s } } -static int -shadow_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, - l1_pgentry_t *p, l1_pgentry_t new, - unsigned int level) +static void +sh_write_p2m_entry_pre(struct domain *d, unsigned long gfn, l1_pgentry_t *p, + l1_pgentry_t new, unsigned int level) { - struct domain *d = p2m->domain; - int rc; - - paging_lock(d); - /* If there are any shadows, update them. But if shadow_teardown() * has already been called then it's not safe to try. */ if ( likely(d->arch.paging.shadow.total_pages != 0) ) sh_unshadow_for_p2m_change(d, gfn, p, new, level); - - rc = p2m_entry_modify(p2m, p2m_flags_to_type(l1e_get_flags(new)), - p2m_flags_to_type(l1e_get_flags(*p)), - l1e_get_mfn(new), l1e_get_mfn(*p), level); - if ( rc ) - { - paging_unlock(d); - return rc; - } - - /* Update the entry with new content */ - safe_write_pte(p, new); +} #if (SHADOW_OPTIMIZATIONS & SHOPT_FAST_FAULT_PATH) +static void +sh_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags) +{ + struct domain *d = p2m->domain; + /* If we're doing FAST_FAULT_PATH, then shadow mode may have cached the fact that this is an mmio region in the shadow page tables. Blow the tables away to remove the cache. @@ -3176,16 +3164,15 @@ shadow_write_p2m_entry(struct p2m_domain shadow_blow_tables(d); d->arch.paging.shadow.has_fast_mmio_entries = false; } -#endif - - paging_unlock(d); - - return 0; } +#else +# define sh_write_p2m_entry_post NULL +#endif void shadow_p2m_init(struct p2m_domain *p2m) { - p2m->write_p2m_entry = shadow_write_p2m_entry; + p2m->write_p2m_entry_pre = sh_write_p2m_entry_pre; + p2m->write_p2m_entry_post = sh_write_p2m_entry_post; } /**************************************************************************/ --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -272,10 +272,13 @@ struct p2m_domain { unsigned long first_gfn, unsigned long last_gfn); void (*memory_type_changed)(struct p2m_domain *p2m); - - int (*write_p2m_entry)(struct p2m_domain *p2m, - unsigned long gfn, l1_pgentry_t *p, - l1_pgentry_t new, unsigned int level); + void (*write_p2m_entry_pre)(struct domain *d, + unsigned long gfn, + l1_pgentry_t *p, + l1_pgentry_t new, + unsigned int level); + void (*write_p2m_entry_post)(struct p2m_domain *p2m, + unsigned int oflags); #if P2M_AUDIT long (*audit_p2m)(struct p2m_domain *p2m); #endif @@ -472,7 +475,7 @@ void __put_gfn(struct p2m_domain *p2m, u * * This is also used in the shadow code whenever the paging lock is * held -- in those cases, the caller is protected against concurrent - * p2m updates by the fact that shadow_write_p2m_entry() also takes + * p2m updates by the fact that write_p2m_entry() also takes * the paging lock. * * Note that an unlocked accessor only makes sense for a "query" lookup. @@ -841,8 +844,8 @@ void np2m_flush_base(struct vcpu *v, uns void hap_p2m_init(struct p2m_domain *p2m); void shadow_p2m_init(struct p2m_domain *p2m); -int nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, - l1_pgentry_t *p, l1_pgentry_t new, unsigned int level); +void nestedp2m_write_p2m_entry_post(struct p2m_domain *p2m, + unsigned int oflags); /* * Alternate p2m: shadow p2m tables used for alternate memory views