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 AFF05C48260 for ; Tue, 13 Feb 2024 17:10:52 +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=mlTdQmjuPYeCTBupReFOK2Ob9HBAW9k5BfuL/lvB53A=; b=tQdceEAPv7IqPP Gz7lVhfivRoyLQJx/1hrdNkl3AOReb+OlrmUGHkIe9Gs7okmHauf2B5nU4RMI9weKeNVRdGyc/tKj WUqamrPCy+IJndSLRcAW9479qMUivL74zVO1F1V7CNdn5tjrfxPKBvcivCQ00CoHStTourOy2vanY W5UBdatgPDh6CWmqMJ44z5/NA6sBG1vRXztF5uY667GeStYflPcF3VAmbszr4iyNgYru//EsGtcMR W2qRztqdiHew332f1BZvSPfabQwfCSukFEZbmsPMRiuvfNBAX7vtD6JBAi6lw2SLNyIXyXq6PVSQg cLrX5b559GZzJpr3h33Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rZwIt-0000000A63Q-0kr1; Tue, 13 Feb 2024 17:10:43 +0000 Received: from out-188.mta1.migadu.com ([95.215.58.188]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rZwIq-0000000A62P-1cRV for linux-arm-kernel@lists.infradead.org; Tue, 13 Feb 2024 17:10:42 +0000 Date: Tue, 13 Feb 2024 17:10:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1707844237; 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: in-reply-to:in-reply-to:references:references; bh=XY+IFiGgHmCeoZd/AEfpX4a3LV5h8HfarWWWxCHWdMw=; b=pSy1UV+lUx8nXsiVimgjdtfBP9tXfN5OBdGUJVJ0i/FeU/0/+KkWuHcwJEhXxFvakfaq7U a3h+psZeEeZcRpXELh0tU+MNZmimtkdImBoUpExbCKGUDxnm+gynK0YxqGdmzBGNbI/yD5 niqgaKB2KlZFrMEr1kLiySYvt1gVcWQ= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Sebastian Ene Cc: catalin.marinas@arm.com, gshan@redhat.com, james.morse@arm.com, mark.rutland@arm.com, maz@kernel.org, rananta@google.com, ricarkol@google.com, ryan.roberts@arm.com, shahuang@redhat.com, suzuki.poulose@arm.com, will@kernel.org, yuzenghui@huawei.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-team@android.com, vdonnefort@google.com Subject: Re: [PATCH v5 4/4] KVM: arm64: Initialize the ptdump parser with stage-2 attributes Message-ID: References: <20240207144832.1017815-2-sebastianene@google.com> <20240207144832.1017815-6-sebastianene@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240213_091040_666683_66193B5B X-CRM114-Status: GOOD ( 27.28 ) 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 Tue, Feb 13, 2024 at 04:59:01PM +0000, Sebastian Ene wrote: > On Tue, Feb 13, 2024 at 12:42:42AM +0000, Oliver Upton wrote: > > On Wed, Feb 07, 2024 at 02:48:33PM +0000, Sebastian Ene wrote: [...] > > > + > > > + snprintf(marker_msg, MARKER_MSG_LEN, "IPA bits %2u start lvl %1d", > > > + pgtable->ia_bits, pgtable->start_level); > > > + > > > + ipa_addr_marker[0].name = marker_msg; > > > > Is the dynamic name worth the added complexity? I see nothing wrong with > > exposing additional debugfs files for simple attributes like the IPA > > range and page table levels. > > > > I know it isn't *that* much, just looking for every opportunity to > > simplify further. > > > > We can keep them separate, I have no strong opinion about this. I think > this was Vincent's, original suggestion to have them so I will check with > him as well. Well, if we get to the place where there's a single struct containing all of the required data upfront then this becomes less of an issue. This is useful information still, so let's see if we can go about it the other way. > > > + ret = kvm_ptdump_parser_init(&parser_state, mmu->pgt, m); > > > + if (ret) > > > + return ret; > > > + > > > > Can this be done at open(), or am I missing something? > > > > I guess we can do this in open() but then we will have to add again that > struct that wraps some ptdump specific state tracking. It seemed a bit cleaner in > this way. What do you think ? Allocating something that looks like an iterator end embedding it in ->private isn't too uncommon. > > > write_lock(&guest_kvm->mmu_lock); > > > ret = kvm_ptdump_show_common(m, mmu->pgt, &parser_state); > > > write_unlock(&guest_kvm->mmu_lock); > > > > > > + kvm_ptdump_parser_teardown(&parser_state); > > > > Same question here, can this happen at close()? I guess you'll need a > > struct to encapsulate pg_state and a pointer to the VM at least. > > > > Right, I tried to avoid using a separate struct as we discussed in v4. Sorry, I hope I didn't confuse you in my prior feedback. What I had issue with was the multiple layers of function ptr / ops structs for managing the file interface. I have zero concerns with organizing the _data_ for the walk this way. > > Actually, come to think of it, if you embed all of the data you need for > > the walker into a structure you can just do a single allocation for it > > upfront. -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel