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 E1AC2C001DE for ; Thu, 10 Aug 2023 17:46:58 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qU9k5-0000lX-4Q; Thu, 10 Aug 2023 13:46:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qU9k3-0000jf-Bg for qemu-devel@nongnu.org; Thu, 10 Aug 2023 13:46:35 -0400 Received: from mail-lj1-x234.google.com ([2a00:1450:4864:20::234]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qU9k0-0005p7-VU for qemu-devel@nongnu.org; Thu, 10 Aug 2023 13:46:35 -0400 Received: by mail-lj1-x234.google.com with SMTP id 38308e7fff4ca-2b9c907bc68so18295491fa.2 for ; Thu, 10 Aug 2023 10:46:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1691689591; x=1692294391; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=VkqxCtuC2URbYPpM1G53qKid4kFeBBRfoZGZKOvydnY=; b=YrztI+uMDQ/CLHBwzbnRyqB2kAOBe1PFiuOC+VADWx3H8fzyfV/nOQA4Q2grVEbyhV g601zEzuzMcelMZjI970l488LPyFQ5DTHr8b4wBuz2zKeiKq4G0TBs5CHQSaLBnA32ys 2NQVr9uqbXsBklc1e5rvecSH85hnPsfj3LxnSkVWEojs3ydYJIUGOzfQDi3MpSehYRjr vHB6NZDze0vSkkOl/qYnKs6zu9PygQ52GcLtDEK19OmCf0vW23EzYWV6/P6SXE4gZbYU Vxkqe7h9Ffaazq3gBuTVeMIQj76lj51QzBwoJB4KL1v4aEqn+ZLdNq9K3XC35SAWHTQ3 9IGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691689591; x=1692294391; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VkqxCtuC2URbYPpM1G53qKid4kFeBBRfoZGZKOvydnY=; b=Xve+zL85f7gVQe2J/DnzYK5DxjSfMEkRcMJ9VLZYJr/f3ciunGVadidvZxBJQWKKsT SaRjjDph0/EVKOvYE+QmruSEaKy9tWnyk+Rc3gdUxbwD6LIiWYTWXj26tXqODocSVpQq COqTXbuJlzc/PsLatcZ2NMa+y4NzCo/5MyedEKhGHRwiqURom7BgHUjTuj+XDOuFTP0C yeMJ1KnSypT2q3IYRMRdlVstxk+jEZt1UyPSL3qZ4hmvLBRRs7aCGdv8t1mMeNKZ3v9O 7i5urrbmygCAeAxtSdn6O43EVbU/bOcL6S7daZ8luKxptsM6cGeePTCi9hPvjtbejWi3 Jl9w== X-Gm-Message-State: AOJu0YyU8OUyh+fESCORsEFgIAoS/K8/GmWJ5SG5b+tMiyYeX8MFTpop 53KQ8QoUsKuEQ1XYD5YZelb25EKM0xkaWtzqHcg1lA== X-Google-Smtp-Source: AGHT+IFG9CSe/O0ZLGXkTJlpCXMZ2KCYGHatmiqfBYsBLmMLk1cBeyeuB2WPL/Ch6f2sZfRtpoWK9FUdOlpgm5TC7+4= X-Received: by 2002:a2e:720d:0:b0:2b9:7513:3596 with SMTP id n13-20020a2e720d000000b002b975133596mr2691234ljc.5.1691689590617; Thu, 10 Aug 2023 10:46:30 -0700 (PDT) MIME-Version: 1.0 References: <20230809102257.25121-1-jason.chien@sifive.com> <20230809172413-mutt-send-email-mst@kernel.org> <20230810134315-mutt-send-email-mst@kernel.org> In-Reply-To: <20230810134315-mutt-send-email-mst@kernel.org> From: Jason Chien Date: Fri, 11 Aug 2023 01:46:19 +0800 Message-ID: Subject: Re: [PATCH] hw/pci-host: Allow extended config space access for Designware PCIe host To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org, Peter Maydell , Andrey Smirnov , Greg Kurz , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Igor Mammedov , qemu-arm@nongnu.org Content-Type: multipart/alternative; boundary="0000000000001e9bcc0602952d5a" Received-SPF: pass client-ip=2a00:1450:4864:20::234; envelope-from=jason.chien@sifive.com; helo=mail-lj1-x234.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --0000000000001e9bcc0602952d5a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable the patch link: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg02162.html On Fri, Aug 11, 2023 at 1:44=E2=80=AFAM Michael S. Tsirkin = wrote: > On Fri, Aug 11, 2023 at 01:22:08AM +0800, Jason Chien wrote: > > As far as I know, the order issue is caused by nested device > realization. In > > this case, realizing TYPE_DESIGNWARE_PCIE_HOST will also > > realize TYPE_DESIGNWARE_PCIE_ROOT(see designware_pcie_host_realize()). > > device_set_realized() is the function that realizing a device must go > through, > > and this function first realizes the device by dc->realize() and then > realizes > > the device's child bus by qbus_realize(). Whether there is any child bu= s > of the > > device may depend on dc->realize(). The realization flow will be like a > > recursive call to device_set_realized(). More precisely, the flow in > this case > > is: qdev_realize() --> ... --> FIRST device_set_realized() --> FIRST dc= -> > > realize() --> ... --> designware_pcie_host_realize() --> qdev_realize() > --> ... > > --> SECOND device_set_realized() --> SECOND dc->realize() --> ... --> > > designware_pcie_root_realize() --> ...--> back to the SECOND > > device_set_realized() --> SECOND qbus_realize() the CHILD bus "dw-pcie" > --> ... > > --> back to the FIRST device_set_realized() --> FIRST qbus_realize() th= e > PARENT > > bus "pcie". > > > > I also found this patch that solves the same bus issue. > > Which patch? > > > Do you have any suggestions on the order of realization? Thanks! > > > I see. It's not easy to fix. Worth thinking about but I guess your > patch is ok for now. > > > On Thu, Aug 10, 2023 at 5:24=E2=80=AFAM Michael S. Tsirkin > wrote: > > > > On Wed, Aug 09, 2023 at 10:22:50AM +0000, Jason Chien wrote: > > > In pcie_bus_realize(), a root bus is realized as a PCIe bus and a > > non-root > > > bus is realized as a PCIe bus if its parent bus is a PCIe bus. > However, > > > the child bus "dw-pcie" is realized before the parent bus "pcie" > which is > > > the root PCIe bus. Thus, the extended configuration space is not > > accessible > > > on "dw-pcie". The issue can be resolved by adding the > > > PCI_BUS_EXTENDED_CONFIG_SPACE flag to "pcie" before "dw-pcie" is > > realized. > > > > > > Signed-off-by: Jason Chien > > > > I think we should fix the order of initialization rather than > > hack around it. > > > > > --- > > > hw/pci-host/designware.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c > > > index 9e183caa48..388d252ee2 100644 > > > --- a/hw/pci-host/designware.c > > > +++ b/hw/pci-host/designware.c > > > @@ -694,6 +694,7 @@ static void > designware_pcie_host_realize(DeviceState > > *dev, Error **errp) > > > &s->pci.io, > > > 0, 4, > > > TYPE_PCIE_BUS); > > > + pci->bus->flags |=3D PCI_BUS_EXTENDED_CONFIG_SPACE; > > > > > > memory_region_init(&s->pci.address_space_root, > > > OBJECT(s), > > > -- > > > 2.17.1 > > > > > > --0000000000001e9bcc0602952d5a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, Aug 11, 2023 at= 1:44=E2=80=AFAM Michael S. Tsirkin <m= st@redhat.com> wrote:
On Fri, Aug 11, 2023 at 01:22:08AM +0800, Jason Chien wrote: > As far as I know, the order issue is caused by nested device realizati= on. In
> this case, realizing=C2=A0TYPE_DESIGNWARE_PCIE_HOST will also
> realize=C2=A0TYPE_DESIGNWARE_PCIE_ROOT(see designware_pcie_host_realiz= e()).
> device_set_realized() is the function that realizing a device must go = through,
> and this function first realizes the device by dc->realize() and th= en realizes
> the device's child bus by qbus_realize(). Whether there is any chi= ld bus of the
> device may depend=C2=A0on dc->realize(). The realization flow will = be like a
> recursive call to=C2=A0device_set_realized(). More precisely, the flow= in this case
> is: qdev_realize() --> ... --> FIRST device_set_realized() -->= ; FIRST dc->
> realize() --> ... -->=C2=A0designware_pcie_host_realize() -->= =C2=A0qdev_realize() --> ...
> --> SECOND device_set_realized() --> SECOND dc->realize() --&= gt; ... -->
> =C2=A0designware_pcie_root_realize() --> ...--> back to the SECO= ND
> device_set_realized() --> SECOND qbus_realize() the CHILD bus "= ;dw-pcie" --> ...
> --> back to the FIRST device_set_realized() -->=C2=A0FIRST qbus_= realize() the PARENT
> bus "pcie".
>
> I also found this patch=C2=A0that solves the same bus issue.

Which patch?

> Do you have any suggestions on the order of realization? Thanks!


I see. It's not easy to fix. Worth thinking about but I guess your
patch is ok for now.

> On Thu, Aug 10, 2023 at 5:24=E2=80=AFAM Michael S. Tsirkin <mst@redhat.com> wrote:<= br> >
>=C2=A0 =C2=A0 =C2=A0On Wed, Aug 09, 2023 at 10:22:50AM +0000, Jason Chi= en wrote:
>=C2=A0 =C2=A0 =C2=A0> In pcie_bus_realize(), a root bus is realized = as a PCIe bus and a
>=C2=A0 =C2=A0 =C2=A0non-root
>=C2=A0 =C2=A0 =C2=A0> bus is realized as a PCIe bus if its parent bu= s is a PCIe bus. However,
>=C2=A0 =C2=A0 =C2=A0> the child bus "dw-pcie" is realized = before the parent bus "pcie" which is
>=C2=A0 =C2=A0 =C2=A0> the root PCIe bus. Thus, the extended configur= ation space is not
>=C2=A0 =C2=A0 =C2=A0accessible
>=C2=A0 =C2=A0 =C2=A0> on "dw-pcie". The issue can be resol= ved by adding the
>=C2=A0 =C2=A0 =C2=A0> PCI_BUS_EXTENDED_CONFIG_SPACE flag to "pc= ie" before "dw-pcie" is
>=C2=A0 =C2=A0 =C2=A0realized.
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> Signed-off-by: Jason Chien <jason.chien@sifive.com>=
>
>=C2=A0 =C2=A0 =C2=A0I think we should fix the order of initialization r= ather than
>=C2=A0 =C2=A0 =C2=A0hack around it.
>
>=C2=A0 =C2=A0 =C2=A0> ---
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 hw/pci-host/designware.c | 1 +
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 1 file changed, 1 insertion(+)
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> diff --git a/hw/pci-host/designware.c b/hw/pci= -host/designware.c
>=C2=A0 =C2=A0 =C2=A0> index 9e183caa48..388d252ee2 100644
>=C2=A0 =C2=A0 =C2=A0> --- a/hw/pci-host/designware.c
>=C2=A0 =C2=A0 =C2=A0> +++ b/hw/pci-host/designware.c
>=C2=A0 =C2=A0 =C2=A0> @@ -694,6 +694,7 @@ static void designware_pci= e_host_realize(DeviceState
>=C2=A0 =C2=A0 =C2=A0*dev, Error **errp)
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0&s->pci.io,
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A00, 4,
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0TYPE_PCIE_BUS);
>=C2=A0 =C2=A0 =C2=A0> +=C2=A0 =C2=A0 pci->bus->flags |=3D PCI_= BUS_EXTENDED_CONFIG_SPACE;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 memory_region_init(&s-= >pci.address_space_root,
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0OBJECT(s),
>=C2=A0 =C2=A0 =C2=A0> --
>=C2=A0 =C2=A0 =C2=A0> 2.17.1
>
>

--0000000000001e9bcc0602952d5a--