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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (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 E4F09C433F5 for ; Thu, 6 Jan 2022 14:55:10 +0000 (UTC) Received: from localhost ([::1]:58862 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n5UAY-0001iw-1R for qemu-devel@archiver.kernel.org; Thu, 06 Jan 2022 09:55:10 -0500 Received: from eggs.gnu.org ([209.51.188.92]:33726) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n5TsU-0005lo-Am for qemu-devel@nongnu.org; Thu, 06 Jan 2022 09:36:30 -0500 Received: from 3.mo552.mail-out.ovh.net ([178.33.254.192]:54937) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n5TsQ-0008Hx-To for qemu-devel@nongnu.org; Thu, 06 Jan 2022 09:36:30 -0500 Received: from mxplan5.mail.ovh.net (unknown [10.108.20.54]) by mo552.mail-out.ovh.net (Postfix) with ESMTPS id 8A69E22881; Thu, 6 Jan 2022 14:36:18 +0000 (UTC) Received: from kaod.org (37.59.142.106) by DAG4EX1.mxp5.local (172.16.2.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Thu, 6 Jan 2022 15:36:18 +0100 Authentication-Results: garm.ovh; auth=pass (GARM-106R006afed0a36-fe35-4627-9ee2-d8aa387b1370, 021048AAC49377EB75D2DE2E73CB44671C288654) smtp.auth=clg@kaod.org X-OVh-ClientIp: 82.64.250.170 Message-ID: <6d473893-6c0d-0dbc-7ed5-df034d787f74@kaod.org> Date: Thu, 6 Jan 2022 15:36:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH v2 14/18] pnv_phb4.h: turn phb into a pointer in struct PnvPhb4PecStack Content-Language: en-US To: Daniel Henrique Barboza , References: <20220105212338.49899-1-danielhb413@gmail.com> <20220105212338.49899-15-danielhb413@gmail.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= In-Reply-To: <20220105212338.49899-15-danielhb413@gmail.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [37.59.142.106] X-ClientProxiedBy: DAG5EX1.mxp5.local (172.16.2.41) To DAG4EX1.mxp5.local (172.16.2.31) X-Ovh-Tracer-GUID: 82d86d1b-3b46-402f-ada4-b6c00455708a X-Ovh-Tracer-Id: 18330213435903871968 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvuddrudefledgieehucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecuhedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhepkfffgggfuffvfhfhjggtgfhisehtjeertddtfeejnecuhfhrohhmpeevrogurhhitggpnfgvpgfiohgrthgvrhcuoegtlhhgsehkrghougdrohhrgheqnecuggftrfgrthhtvghrnhephffhleegueektdetffdvffeuieeugfekkeelheelteeftdfgtefffeehueegleehnecukfhppedtrddtrddtrddtpdefjedrheelrddugedvrddutdeinecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmohguvgepshhmthhpohhuthdphhgvlhhopehmgihplhgrnhehrdhmrghilhdrohhvhhdrnhgvthdpihhnvghtpedtrddtrddtrddtpdhmrghilhhfrhhomheptghlgheskhgrohgurdhorhhgpdhrtghpthhtohepuggrvhhiugesghhisghsohhnrdgurhhophgsvggrrhdrihgurdgruh Received-SPF: pass client-ip=178.33.254.192; envelope-from=clg@kaod.org; helo=3.mo552.mail-out.ovh.net X-Spam_score_int: -45 X-Spam_score: -4.6 X-Spam_bar: ---- X-Spam_report: (-4.6 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-2.691, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-ppc@nongnu.org, david@gibson.dropbear.id.au Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 1/5/22 22:23, Daniel Henrique Barboza wrote: > At this moment, stack->phb is the plain PnvPHB4 device itself instead > of a pointer to the device. This will present a problem when adding user > creatable devices because we can't deal with this struct and the > realize() callback from the user creatable device. > > We can't get rid of this attribute, similar to what we did when enabling > pnv-phb3 user creatable devices, because pnv_phb4_update_regions() needs > to access stack->phb to do its job. This function is called twice in > pnv_pec_stk_update_map(), which is one of the nested xscom write > callbacks (via pnv_pec_stk_nest_xscom_write()). In fact, > pnv_pec_stk_update_map() code comment is explicit about how the order of > the unmap/map operations relates with the PHB subregions. > > All of this indicates that this code is tied together in a way that we > either go on a crusade, featuring lots of refactories and redesign and > considerable pain, to decouple stack and phb mapping, or we allow stack > update_map operations to access the associated PHB as it is today even > after introducing pnv-phb4 user devices. > > This patch chooses the latter. Instead of getting rid of stack->phb, > turn it into a PHB pointer. This will allow us to assign an user created > PHB to an existing stack later. > > Signed-off-by: Daniel Henrique Barboza > --- > hw/pci-host/pnv_phb4.c | 2 +- > hw/pci-host/pnv_phb4_pec.c | 2 +- > include/hw/pci-host/pnv_phb4.h | 7 +++++-- > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c > index 4c785bbe4c..9e670e41d2 100644 > --- a/hw/pci-host/pnv_phb4.c > +++ b/hw/pci-host/pnv_phb4.c > @@ -1449,7 +1449,7 @@ type_init(pnv_phb4_register_types); > > void pnv_phb4_update_regions(PnvPhb4PecStack *stack) > { > - PnvPHB4 *phb = &stack->phb; > + PnvPHB4 *phb = stack->phb; > > /* Unmap first always */ > if (memory_region_is_mapped(&phb->mr_regs)) { > diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c > index 0675fc55bc..be6eefdbb1 100644 > --- a/hw/pci-host/pnv_phb4_pec.c > +++ b/hw/pci-host/pnv_phb4_pec.c > @@ -563,7 +563,7 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error **errp) > PHB4_PEC_PCI_STK_REGS_COUNT); > > /* PHB pass-through */ > - pnv_phb4_set_stack_phb_props(stack, &stack->phb); > + pnv_phb4_set_stack_phb_props(stack, stack->phb); > if (!sysbus_realize(SYS_BUS_DEVICE(&stack->phb), errp)) { > return; > } > diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h > index b2c4a6b263..2fb5e119c4 100644 > --- a/include/hw/pci-host/pnv_phb4.h > +++ b/include/hw/pci-host/pnv_phb4.h > @@ -178,8 +178,11 @@ struct PnvPhb4PecStack { > /* The owner PEC */ > PnvPhb4PecState *pec; > > - /* The actual PHB */ > - PnvPHB4 phb; I would keep this object for default_enabled() and rename it to avoid conflicts with the next one : > + /* > + * PHB4 pointer. pnv_phb4_update_regions() needs to access > + * the PHB4 via a PnvPhb4PecStack pointer. > + */ > + PnvPHB4 *phb; and introduce this pointer for dynamic phbs. if default_enabled(), then just make it point to the above instance. Thanks, C. > }; > > struct PnvPhb4PecState { >