From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F322C433E0 for ; Wed, 17 Feb 2021 10:02:21 +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 22F7864DF0 for ; Wed, 17 Feb 2021 10:02:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 22F7864DF0 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine 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.86194.161570 (Exim 4.92) (envelope-from ) id 1lCJen-0004kO-Cn; Wed, 17 Feb 2021 10:02:05 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 86194.161570; Wed, 17 Feb 2021 10:02:05 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lCJen-0004kH-9l; Wed, 17 Feb 2021 10:02:05 +0000 Received: by outflank-mailman (input) for mailman id 86194; Wed, 17 Feb 2021 10:02:03 +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 1lCJel-0004kC-KS for xen-devel@lists.xenproject.org; Wed, 17 Feb 2021 10:02:03 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 06ba39fc-1995-4513-bee8-63d2c3b41f9f; Wed, 17 Feb 2021 10:02:02 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id BBDFDB7A7; Wed, 17 Feb 2021 10:02:01 +0000 (UTC) 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" X-Inumbo-ID: 06ba39fc-1995-4513-bee8-63d2c3b41f9f 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=1613556121; h=from:from:reply-to: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=Iv15mlQqaiiuwkeJAdzOnIHYlDJnW7R5IWksA1/NNVM=; b=bFJYfDfqZ5LQNgoULYOYGUM08RGu95+AmZAIp7kYUKA2APeAMTusaOB92L6Vx7OKI3PzlZ lwWxgpROGq1p0//HiIvXCp6m2BZUOTb/buG/tF8EnIJ0pMcXRlKy7Mcmy1Lfr+dqMqSNrI KfwhvXe0FxFb8fPhAYrdeJKFDzVgwJ0= Subject: Re: [PATCH v2 2/2] IOREQ: refine when to send mapcache invalidation request From: Jan Beulich To: Stefano Stabellini , Julien Grall Cc: Andrew Cooper , Wei Liu , =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , Paul Durrant , George Dunlap , "xen-devel@lists.xenproject.org" References: <0e7265fe-8d89-facb-790d-9232c742c3fa@suse.com> Message-ID: <454ffaeb-f0d5-a2fe-420b-f28e51d9aabf@suse.com> Date: Wed, 17 Feb 2021 11:02:00 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 02.02.2021 16:14, Jan Beulich wrote: > XENMEM_decrease_reservation isn't the only means by which pages can get > removed from a guest, yet all removals ought to be signaled to qemu. Put > setting of the flag into the central p2m_remove_page() underlying all > respective hypercalls as well as a few similar places, mainly in PoD > code. > > Additionally there's no point sending the request for the local domain > when the domain acted upon is a different one. The latter domain's ioreq > server mapcaches need invalidating. We assume that domain to be paused > at the point the operation takes place, so sending the request in this > case happens from the hvm_do_resume() path, which as one of its first > steps calls handle_hvm_io_completion(). > > Even without the remote operation aspect a single domain-wide flag > doesn't do: Guests may e.g. decrease-reservation on multiple vCPU-s in > parallel. Each of them needs to issue an invalidation request in due > course, in particular because exiting to guest context should not happen > before the request was actually seen by (all) the emulator(s). > > Signed-off-by: Jan Beulich > --- > v2: Preemption related adjustment split off. Make flag per-vCPU. More > places to set the flag. Also handle acting on a remote domain. > Re-base. Can I get an Arm side ack (or otherwise) please? Thanks, Jan > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -759,10 +759,9 @@ static void p2m_free_entry(struct p2m_do > * has failed (error case). > * So, at worst, the spurious mapcache invalidation might be sent. > */ > - if ( (p2m->domain == current->domain) && > - domain_has_ioreq_server(p2m->domain) && > - p2m_is_ram(entry.p2m.type) ) > - p2m->domain->mapcache_invalidate = true; > + if ( p2m_is_ram(entry.p2m.type) && > + domain_has_ioreq_server(p2m->domain) ) > + ioreq_request_mapcache_invalidate(p2m->domain); > #endif > > p2m->stats.mappings[level]--; > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1509,8 +1509,8 @@ static void do_trap_hypercall(struct cpu > * Note that sending the invalidation request causes the vCPU to block > * until all the IOREQ servers have acknowledged the invalidation. > */ > - if ( unlikely(curr->domain->mapcache_invalidate) && > - test_and_clear_bool(curr->domain->mapcache_invalidate) ) > + if ( unlikely(curr->mapcache_invalidate) && > + test_and_clear_bool(curr->mapcache_invalidate) ) > ioreq_signal_mapcache_invalidate(); > #endif > } > --- a/xen/arch/x86/hvm/hypercall.c > +++ b/xen/arch/x86/hvm/hypercall.c > @@ -32,7 +32,6 @@ > > static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > { > - const struct vcpu *curr = current; > long rc; > > switch ( cmd & MEMOP_CMD_MASK ) > @@ -42,14 +41,11 @@ static long hvm_memory_op(int cmd, XEN_G > return -ENOSYS; > } > > - if ( !curr->hcall_compat ) > + if ( !current->hcall_compat ) > rc = do_memory_op(cmd, arg); > else > rc = compat_memory_op(cmd, arg); > > - if ( (cmd & MEMOP_CMD_MASK) == XENMEM_decrease_reservation ) > - curr->domain->mapcache_invalidate = true; > - > return rc; > } > > @@ -327,9 +323,11 @@ int hvm_hypercall(struct cpu_user_regs * > > HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu -> %lx", eax, regs->rax); > > - if ( unlikely(currd->mapcache_invalidate) && > - test_and_clear_bool(currd->mapcache_invalidate) ) > + if ( unlikely(curr->mapcache_invalidate) ) > + { > + curr->mapcache_invalidate = false; > ioreq_signal_mapcache_invalidate(); > + } > > return curr->hcall_preempted ? HVM_HCALL_preempted : HVM_HCALL_completed; > } > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -815,6 +816,8 @@ p2m_remove_page(struct p2m_domain *p2m, > } > } > > + ioreq_request_mapcache_invalidate(p2m->domain); > + > return p2m_set_entry(p2m, gfn, INVALID_MFN, page_order, p2m_invalid, > p2m->default_access); > } > @@ -1301,6 +1304,8 @@ static int set_typed_p2m_entry(struct do > ASSERT(mfn_valid(mfn_add(omfn, i))); > set_gpfn_from_mfn(mfn_x(omfn) + i, INVALID_M2P_ENTRY); > } > + > + ioreq_request_mapcache_invalidate(d); > } > > P2M_DEBUG("set %d %lx %lx\n", gfn_p2mt, gfn_l, mfn_x(mfn)); > --- a/xen/arch/x86/mm/p2m-pod.c > +++ b/xen/arch/x86/mm/p2m-pod.c > @@ -20,6 +20,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -647,6 +648,8 @@ p2m_pod_decrease_reservation(struct doma > set_gpfn_from_mfn(mfn_x(mfn), INVALID_M2P_ENTRY); > p2m_pod_cache_add(p2m, page, cur_order); > > + ioreq_request_mapcache_invalidate(d); > + > steal_for_cache = ( p2m->pod.entry_count > p2m->pod.count ); > > ram -= n; > @@ -835,6 +838,8 @@ p2m_pod_zero_check_superpage(struct p2m_ > p2m_pod_cache_add(p2m, mfn_to_page(mfn0), PAGE_ORDER_2M); > p2m->pod.entry_count += SUPERPAGE_PAGES; > > + ioreq_request_mapcache_invalidate(d); > + > ret = SUPERPAGE_PAGES; > > out_reset: > @@ -997,6 +1002,8 @@ p2m_pod_zero_check(struct p2m_domain *p2 > /* Add to cache, and account for the new p2m PoD entry */ > p2m_pod_cache_add(p2m, mfn_to_page(mfns[i]), PAGE_ORDER_4K); > p2m->pod.entry_count++; > + > + ioreq_request_mapcache_invalidate(d); > } > } > > @@ -1315,6 +1322,8 @@ guest_physmap_mark_populate_on_demand(st > p2m->pod.entry_count -= pod_count; > BUG_ON(p2m->pod.entry_count < 0); > pod_unlock(p2m); > + > + ioreq_request_mapcache_invalidate(d); > } > > out: > --- a/xen/common/ioreq.c > +++ b/xen/common/ioreq.c > @@ -35,6 +35,17 @@ > #include > #include > > +void ioreq_request_mapcache_invalidate(const struct domain *d) > +{ > + struct vcpu *v = current; > + > + if ( d == v->domain ) > + v->mapcache_invalidate = true; > + else if ( d->creation_finished ) > + for_each_vcpu ( d, v ) > + v->mapcache_invalidate = true; > +} > + > /* Ask ioemu mapcache to invalidate mappings. */ > void ioreq_signal_mapcache_invalidate(void) > { > @@ -206,6 +217,7 @@ bool vcpu_ioreq_handle_completion(struct > struct ioreq_server *s; > struct ioreq_vcpu *sv; > enum vio_completion completion; > + bool res = true; > > if ( has_vpci(d) && vpci_process_pending(v) ) > { > @@ -232,17 +244,27 @@ bool vcpu_ioreq_handle_completion(struct > break; > > case VIO_mmio_completion: > - return arch_ioreq_complete_mmio(); > + res = arch_ioreq_complete_mmio(); > + break; > > case VIO_pio_completion: > - return handle_pio(vio->req.addr, vio->req.size, > - vio->req.dir); > + res = handle_pio(vio->req.addr, vio->req.size, > + vio->req.dir); > + break; > > default: > - return arch_vcpu_ioreq_completion(completion); > + res = arch_vcpu_ioreq_completion(completion); > + break; > } > > - return true; > + if ( res && unlikely(v->mapcache_invalidate) ) > + { > + v->mapcache_invalidate = false; > + ioreq_signal_mapcache_invalidate(); > + res = false; > + } > + > + return res; > } > > static int ioreq_server_alloc_mfn(struct ioreq_server *s, bool buf) > --- a/xen/include/xen/ioreq.h > +++ b/xen/include/xen/ioreq.h > @@ -103,6 +103,7 @@ struct ioreq_server *ioreq_server_select > int ioreq_send(struct ioreq_server *s, ioreq_t *proto_p, > bool buffered); > unsigned int ioreq_broadcast(ioreq_t *p, bool buffered); > +void ioreq_request_mapcache_invalidate(const struct domain *d); > void ioreq_signal_mapcache_invalidate(void); > > void ioreq_domain_init(struct domain *d); > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -225,6 +225,14 @@ struct vcpu > bool hcall_compat; > #endif > > +#ifdef CONFIG_IOREQ_SERVER > + /* > + * Indicates that mapcache invalidation request should be sent to > + * the device emulator. > + */ > + bool mapcache_invalidate; > +#endif > + > /* The CPU, if any, which is holding onto this VCPU's state. */ > #define VCPU_CPU_CLEAN (~0u) > unsigned int dirty_cpu; > @@ -444,11 +452,6 @@ struct domain > * unpaused for the first time by the systemcontroller. > */ > bool creation_finished; > - /* > - * Indicates that mapcache invalidation request should be sent to > - * the device emulator. > - */ > - bool mapcache_invalidate; > > /* Which guest this guest has privileges on */ > struct domain *target; >