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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B6EF7C5AD4C for ; Thu, 23 Nov 2023 10:59:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gAEsPx7qCh17yWary0zvStESnzaodBN6UfPyyOlUlMU=; b=kFkxSVNVp4akBA MyRWnNPQb5aUzMThhD610HocGWum4Uz+Uyt+wTL3ULNrBPJCiVvgK5WdmTb7olaqEZ5gXGR/dnUbK B42kWBjrKXIOWasJedYTEhM+1w9b3zllWaL3oK5ANVujUBArUbUJIM/YiFAivw9Ho0tbawNFwr8cW cxUG/1Y/izo/K3ojHz95Y35rstIHXqAbavWzNHHlR+Qo5VmBCPmWS4wrUp3SfZlfvvTe+DQGOhrRl 0e+YNAMvYSw/Q3Yy99zS9uBffJ8oliALJGTc8/Q9wWewst5uZvKFOl5YymYR/MktEiKK2dxcK3e/G Qqp+gvvBK6kaxoox6j4Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r67Pi-004Rf8-0g; Thu, 23 Nov 2023 10:58:30 +0000 Received: from mail-wm1-x332.google.com ([2a00:1450:4864:20::332]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r67Pf-004Rdo-1P for linux-arm-kernel@lists.infradead.org; Thu, 23 Nov 2023 10:58:28 +0000 Received: by mail-wm1-x332.google.com with SMTP id 5b1f17b1804b1-40b367a0a12so29825e9.1 for ; Thu, 23 Nov 2023 02:58:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1700737102; x=1701341902; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=tbD+CZLI2OJnj2XtkqlWGnXInTbmUcqYNyaWG++MW1o=; b=Yqr4Fm0dslbuZ6yYUPe74DFsQisrovOE829w+8H3vQbHvbUTBsqJkob0KQ+KWp6bLH ZMxT81FG7FxHHkimV7XejwTRVsHfOSbL2x/ILgTxeG2Ti3yNOg0xKlVOfwn7YSvNgZ28 ZdQNV7HxenC8KhXXhqEIPuderkMT5UQQm7hUl8WMZIqRM1xBmvTt+m1zxCf8wU8SGyh3 O8CgS9b3yhx9LECOpoaCxn/E88xymFBqex8VLsYsWKxSWgdJvMSZuCOq48EQdAOWeThV qVvrcHwJveGpy4DlJLkaLp1SzuaY1k1HOSd3v38faxVd9++6LL1BJtofR1PJ4K6aQ2PH jdUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700737102; x=1701341902; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=tbD+CZLI2OJnj2XtkqlWGnXInTbmUcqYNyaWG++MW1o=; b=Vb2bajwcK1EI3UwJTlhDSpJbMuw23Dk3Wvsw+3oSmQtmdcaRa/Mmfj70KAjO4tDa6e hqG4tCAjAMECdnoP8g1nqM+BRNJ+E2LoGcAv608IwyxNluvxYVDGH0T43QZFcLHY1837 TQ4RnsQzrECO18TbZ+fJZzVqKntOrUlDvc7EsKQ+z9haRXnWTINeJqTvafqXKkZIZ7s5 WIKo4QaE1WRn4LzOPE2mRtjNxXCEk2wKrSH1dE/aIDjMMxq8Ylht82xVzQXBiJstlfzX CAsXuESs9oZvIx3Tv8+u/cmLl2sKrCC0QTtwQ7SEebF4xcxYi45nSnHrCc3CGBS3HJuE 9b5A== X-Gm-Message-State: AOJu0YyEuybGA24JVIUPRvNjDLYCJDOfM79oeBMiQazcl1+Aytd0IbEn COPTxRP7kAtBOMKDjhPKy8kkJw== X-Google-Smtp-Source: AGHT+IECpNi7V/qSvW7pVQ/CJFY4C0ksNj+vFUmeqFagu+qFkUWQoOPShJMnthXGa96ey5y1uPJSiw== X-Received: by 2002:a05:600c:3b13:b0:40a:483f:f828 with SMTP id m19-20020a05600c3b1300b0040a483ff828mr251605wms.4.1700737102403; Thu, 23 Nov 2023 02:58:22 -0800 (PST) Received: from google.com (110.121.148.146.bc.googleusercontent.com. [146.148.121.110]) by smtp.gmail.com with ESMTPSA id k11-20020adfe3cb000000b00332d3c78e11sm1305204wrm.85.2023.11.23.02.58.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Nov 2023 02:58:21 -0800 (PST) Date: Thu, 23 Nov 2023 10:58:20 +0000 From: Sebastian Ene To: Oliver Upton Cc: will@kernel.org, James Morse , Suzuki K Poulose , Zenghui Yu , catalin.marinas@arm.com, mark.rutland@arm.com, akpm@linux-foundation.org, maz@kernel.org, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-team@android.com, vdonnefort@google.com, qperret@google.com, smostafa@google.com Subject: Re: [PATCH v3 10/10] arm64: ptdump: Add support for guest stage-2 pagetables dumping Message-ID: References: <20231115171639.2852644-2-sebastianene@google.com> <20231115171639.2852644-12-sebastianene@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231123_025827_490069_7C188671 X-CRM114-Status: GOOD ( 24.47 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Nov 22, 2023 at 11:35:57PM +0000, Oliver Upton wrote: > On Wed, Nov 15, 2023 at 05:16:40PM +0000, Sebastian Ene wrote: > > +struct ptdump_registered_guest { > > + struct list_head reg_list; > > + struct ptdump_info info; > > + struct kvm_pgtable_snapshot snapshot; > > + rwlock_t *lock; > > +}; > > Why can't we just store a pointer directly to struct kvm in ::private? I don't think it will work unless we expect a struct kvm_pgtable in stage2_ptdump_walk file_priv field. I think it is a good ideea and will simplify things a little bit dropping kvm_pgtable_snapshot from here. The current code has some fileds that are reduntant (the priv pointers) because I also made this to work with protected guests where we can't access their pagetables directly. > Also, shouldn't you take a reference on struct kvm when the file is > opened to protect against VM teardown? > I am not sure about the need could you please elaborate a bit ? On VM teardown we expect ptdump_unregister_guest_stage2 to be invoked. > > +static LIST_HEAD(ptdump_guest_list); > > +static DEFINE_MUTEX(ptdump_list_lock); > > What is the list for? > I am keeping a list of registered guests with ptdump and the lock should protect the list against concurent VM teardowns. > > static phys_addr_t ptdump_host_pa(void *addr) > > { > > return __pa(addr); > > @@ -757,6 +768,63 @@ static void stage2_ptdump_walk(struct seq_file *s, struct ptdump_info *info) > > kvm_pgtable_walk(pgtable, start_ipa, end_ipa, &walker); > > } > > > > +static void guest_stage2_ptdump_walk(struct seq_file *s, > > + struct ptdump_info *info) > > +{ > > + struct ptdump_info_file_priv *f_priv = > > + container_of(info, struct ptdump_info_file_priv, info); > > + struct ptdump_registered_guest *guest = info->priv; > > + > > + f_priv->file_priv = &guest->snapshot; > > + > > + read_lock(guest->lock); > > + stage2_ptdump_walk(s, info); > > + read_unlock(guest->lock); > > Taking the mmu lock for read allows other table walkers to add new > mappings and adjust the granularity of existing ones. Should this > instead take the mmu lock for write? > Thanks for pointing our, this is exactly what I was trying to avoid, so yes I should use the write mmu lock in this case. > > +} > > + > > +int ptdump_register_guest_stage2(struct kvm *kvm) > > +{ > > + struct ptdump_registered_guest *guest; > > + struct kvm_s2_mmu *mmu = &kvm->arch.mmu; > > + struct kvm_pgtable *pgt = mmu->pgt; > > + > > + guest = kzalloc(sizeof(struct ptdump_registered_guest), GFP_KERNEL); > > You want GFP_KERNEL_ACCOUNT here. > Right, thanks this is because it is an untrusted allocation triggered from userspace. > -- > Thanks, > Oliver Thank you, Seb _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel