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=-0.3 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no 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 CCCF4C433E0 for ; Wed, 17 Jun 2020 13:32:25 +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 9A7D221532 for ; Wed, 17 Jun 2020 13:32:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XQSBq9nP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9A7D221532 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass 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.92) (envelope-from ) id 1jlYAp-0005Zz-H6; Wed, 17 Jun 2020 13:32:15 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jlYAn-0005Zs-OO for xen-devel@lists.xenproject.org; Wed, 17 Jun 2020 13:32:13 +0000 X-Inumbo-ID: f3a57f4c-b09e-11ea-bca7-bc764e2007e4 Received: from mail-wr1-x444.google.com (unknown [2a00:1450:4864:20::444]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id f3a57f4c-b09e-11ea-bca7-bc764e2007e4; Wed, 17 Jun 2020 13:32:13 +0000 (UTC) Received: by mail-wr1-x444.google.com with SMTP id e1so2367109wrt.5 for ; Wed, 17 Jun 2020 06:32:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=41N96wGg45X8tNPvMWjE+2C4QZnFC4aqzvjOlMDH9V8=; b=XQSBq9nPiE5u/KTDybiNKVnh9I1mxrJ+TzhzGRtjdQB9e0AI0Rd0PMyYbG7D1kRJDD prLuyIxYm+i4WUb9GmD3hmfi7TmGa02gyrF5CsFPb/tRh5yQijh9u+m8bjKjkIGtWy9O hPiFkHPagM69xAhnt+g5lItIXxNGtuLGqek3f2uXqlIkk9a64OAE3OHMJroFbDJF9cKx l031fppaYaYktGbs7JFLRDLHtNUievXt7i9N6ELqxAaEGPcMRUbIzhpLNep+4bJMRdb2 UrVDe1x8Lth0s3PuPN85jx8tFQCJJa0tGU5y/SwPwBbmTuvDGomU/lytuWDh/lUrNMpf 4PQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=41N96wGg45X8tNPvMWjE+2C4QZnFC4aqzvjOlMDH9V8=; b=I97/4H7RVnarydaSA9qL7wN8GkK314CYZP9O5NoDrYaP3LsVZ6DmgWiJY0Hb61+ELT EKHvxYX5K5nd3Sq6ZLSKVyXuI8r4GQfyQaeOX1VQM2uT0eexjAexEyp6ixUzJ37KvpMM x6L4GbGJeCRPq47QdiOoSWuZC9Gvekz/PDUKgMEYVmbZQg5HVGFAIXoamaBFnnOML3u6 8lew9rvErcubfTwKYr3waA4w0KvUZe0IZ0s8VCk7QX1tjNkfxnlkGAQIyvm5s1AsjpLY xATmLPfhDd1R1VObta9YrWETFGE/TsvGb5HatsaVXKbKLuYFkacER97DQSvQ9bSN1Jw9 boWA== X-Gm-Message-State: AOAM530AiHt01Q5T265UznoYScP/ahAQwneSAXhVuaTqZa0jR6Iefk93 7juH9BhAytOSk7OfxsL95S0BVq4zeoJVT2+LhlM= X-Google-Smtp-Source: ABdhPJz2o62Ss7oDQTTmGH81wnC7xKIUk4liobRoHw41O4mq9FIBCTf1pmYEiGN0f38RXzhdHHbm8rf3UmXqkLV/6XQ= X-Received: by 2002:adf:f707:: with SMTP id r7mr9141490wrp.390.1592400732305; Wed, 17 Jun 2020 06:32:12 -0700 (PDT) MIME-Version: 1.0 References: <6a2ae3bae4a4ad32bc7caecd8af2655a76a9fb19.1592335579.git.tamas.lengyel@intel.com> <4b06e4f3-2b23-359a-9d80-c881016c0d91@suse.com> <47abe61b-76e1-4491-f539-60c427c2ffc8@suse.com> In-Reply-To: <47abe61b-76e1-4491-f539-60c427c2ffc8@suse.com> From: Tamas K Lengyel Date: Wed, 17 Jun 2020 07:31:36 -0600 Message-ID: Subject: Re: [PATCH for-4.14] x86/hap: use get_gfn_type in hap_update_paging_modes To: Jan Beulich Content-Type: text/plain; charset="UTF-8" X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Tamas K Lengyel , Wei Liu , Andrew Cooper , George Dunlap , Xen-devel , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" On Wed, Jun 17, 2020 at 7:28 AM Jan Beulich wrote: > > On 17.06.2020 15:21, Tamas K Lengyel wrote: > > On Wed, Jun 17, 2020 at 7:04 AM Jan Beulich wrote: > >> > >> On 17.06.2020 15:00, Tamas K Lengyel wrote: > >>> On Wed, Jun 17, 2020 at 3:59 AM Jan Beulich wrote: > >>>> If there are code paths of both kinds, which approach to use in > >>>> vmx_load_pdptrs() may need to be chosen based on what > >>>> paging_locked_by_me() returns. Or perhaps an unlocked query is > >>>> fine in either case? > >>> > >>> Perhaps adjusting vmx_load_pdptrs to chose the unlocked query would be > >>> fine. But at that point what is the reason for having the lock > >>> ordering at all? Why not just have a single recursive lock and avoid > >>> issues like this altogether? > >> > >> With just a single lock, contention problems we already know we > >> have would be even worse. When the current locking model was > >> introduced, there was actually a plan to make gfn_lock() more > >> fine-grained (i.e. not simply "de-generate" to p2m_lock()), for > >> example. > > > > Sigh. Well, I've been checking and adjust vmx_load_pdptrs to use an > > unlocked query doesn't seem as straightforward because, well, there is > > no unlocked version of p2m_get_page_from_gfn which would also do the > > "fixups". > > Which fixups do we need here, in particular? Of course, whenever > any fixups get done, the operation can't be lock-less. > > > What seems redundant to me though is that > > hap_update_paging_modes takes both the p2m_lock via get_gfn PLUS the > > paging_lock. Does it really need to take the paging_lock? > > From mm-locks.h's comments: > > * For HAP, it protects the NPT/EPT tables and mode changes. We do the population of the EPT as part of fork_page() if there was a hole in the p2m when the query was issued using P2M_ALLOC (or P2M_UNSHARE). I checked and without the paging lock held it throws up at hap_alloc's ASSERT.. So yea, currently I don't think we have a better route then what I currently sent in. Perhaps the "hvm_pae_enabled(v) && !hvm_long_mode_active(v)" can be moved into hvm.h and be used by vmx_load_pdptrs as well, making it less fragile in case there is an adjustment to it in the future. Tamas