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=-9.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 5871DC433EF for ; Fri, 10 Sep 2021 21:42:22 +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 05CD0611F0 for ; Fri, 10 Sep 2021 21:42:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 05CD0611F0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.184808.333562 (Exim 4.92) (envelope-from ) id 1mOoHd-0000bW-64; Fri, 10 Sep 2021 21:42:05 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 184808.333562; Fri, 10 Sep 2021 21:42:05 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mOoHd-0000bP-2l; Fri, 10 Sep 2021 21:42:05 +0000 Received: by outflank-mailman (input) for mailman id 184808; Fri, 10 Sep 2021 21:42:04 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mOoHc-0000bJ-6G for xen-devel@lists.xenproject.org; Fri, 10 Sep 2021 21:42:04 +0000 Received: from mail-ed1-x529.google.com (unknown [2a00:1450:4864:20::529]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 2ad851b2-c48d-47c6-b7e3-1883313dcaba; Fri, 10 Sep 2021 21:42:02 +0000 (UTC) Received: by mail-ed1-x529.google.com with SMTP id i6so4465210edu.1 for ; Fri, 10 Sep 2021 14:42:02 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 2ad851b2-c48d-47c6-b7e3-1883313dcaba DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uxfgzMlBFAsHdhhTIbAJ202DoS4PD95dqeSuumClICc=; b=CDlVhSmKEkEpemhEEIUguzMtHEWu7Hew4ok5miKSF51mlWb545f9B2GCwSXDee/fHs Xa8idfAqsIAceYz/GnAekbIeS0G1rNycwqEFbYeDXhncp9sTKuH0SzW/HFG92Ruxs3QL 4auIXFJcTIiE6wTd96MlzgdUFlZy7/Vn1pKoxN49RsDllefS8cK+7XuJcvyXRu97sxbh i764o9joy/hhbSfl80wLD01zeUBpyHIFTVSazl4QZb66QA0QDQJZXsQ4Ocd9dqsaxJyu /Gh/C2eOf1X1MXDjWGBGdH/J/HBGRXVxwJlPgHyt9BOCQNNlx6NhR++iie56v/In95HX fSEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uxfgzMlBFAsHdhhTIbAJ202DoS4PD95dqeSuumClICc=; b=XO4+KtZQ2mUmZdJzm/Z405US40dBpSz/veHpeFbEIcBLcNZi7102L7mjEHS5WlAtW0 nmO2bVEy9ZcC9w/1VWLUEgM3Z+ZojP6PBaN84tb9LsS3pQP+BpVxBMelxRRy43sgH7W0 ozoevnnxaE/n0MRbrpa50dRwQc8NqEOAk2dzilnRqELtRiYNepBYtns5skcslKLJM3xk +YycA3wVF8FE6ISAppEWx2FrnP7cAvRhO3i0ilk6Z/gTkK6UsthUwoYF7/qHnzq4ciZC j6RHTlgn2vl69EZri4i/4MvTbMf22ZqOH5uZmHeOmNEHTZtg4nmlvshA014/QV44xdwW xaEA== X-Gm-Message-State: AOAM531QtzJPiMEdz0ZWZvUFxcHruN5HlVvuLeMnQgUN6YsrnWF/j4vl mqG7URgqcIzTm1ahZM2Ue8kPbuJwghzppOGCvZo= X-Google-Smtp-Source: ABdhPJyCdqe8GSTIHTaW1fwa31u0Mcm70AZKWMewBDoxVL/QCqLfLagQP6Q0tnGo5YEHm6msc3A7oivOfl/YHGTtYtE= X-Received: by 2002:aa7:df92:: with SMTP id b18mr4863250edy.47.1631310121526; Fri, 10 Sep 2021 14:42:01 -0700 (PDT) MIME-Version: 1.0 References: <20210903083347.131786-1-andr2000@gmail.com> <20210903083347.131786-11-andr2000@gmail.com> <35f7faf6-db90-f279-8ed1-fa4ba96812fb@xen.org> <468a868c-1183-e05f-0c92-3cba172cecb3@epam.com> <9ec2c22c-b834-1c87-71af-236e13538c4a@xen.org> <15a930ff-77d5-b962-b346-c586a2769009@epam.com> <684b3534-40eb-add7-f46e-c6258904757b@xen.org> In-Reply-To: From: Julien Grall Date: Fri, 10 Sep 2021 22:41:50 +0100 Message-ID: Subject: Re: [PATCH 10/11] xen/arm: Do not map PCI ECAM space to Domain-0's p2m To: Stefano Stabellini Cc: Oleksandr Andrushchenko , Oleksandr Andrushchenko , xen-devel , Oleksandr Tyshchenko , Volodymyr Babchuk , Artem Mygaiev , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Bertrand Marquis , Rahul Singh Content-Type: multipart/alternative; boundary="0000000000004fe6de05cbaafc09" --0000000000004fe6de05cbaafc09 Content-Type: text/plain; charset="UTF-8" On Fri, 10 Sep 2021, 21:30 Stefano Stabellini, wrote: > On Fri, 10 Sep 2021, Julien Grall wrote: > > On 10/09/2021 15:01, Oleksandr Andrushchenko wrote: > > > On 10.09.21 16:18, Julien Grall wrote: > > > > On 10/09/2021 13:37, Oleksandr Andrushchenko wrote: > > > > > Hi, Julien! > > > > > > > > Hi Oleksandr, > > > > > > > > > On 09.09.21 20:58, Julien Grall wrote: > > > > > > On 03/09/2021 09:33, Oleksandr Andrushchenko wrote: > > > > > > > From: Oleksandr Andrushchenko < > oleksandr_andrushchenko@epam.com> > > > > > > > > > > > > > > Host bridge controller's ECAM space is mapped into Domain-0's > p2m, > > > > > > > thus it is not possible to trap the same for vPCI via MMIO > handlers. > > > > > > > For this to work we need to not map those while constructing > the > > > > > > > domain. > > > > > > > > > > > > > > Note, that during Domain-0 creation there is no pci_dev yet > > > > > > > allocated for > > > > > > > host bridges, thus we cannot match PCI host and its associated > > > > > > > bridge by SBDF. Use dt_device_node field for checks instead. > > > > > > > > > > > > > > Signed-off-by: Oleksandr Andrushchenko > > > > > > > > > > > > > > --- > > > > > > > xen/arch/arm/domain_build.c | 3 +++ > > > > > > > xen/arch/arm/pci/ecam.c | 17 +++++++++++++++++ > > > > > > > xen/arch/arm/pci/pci-host-common.c | 22 > ++++++++++++++++++++++ > > > > > > > xen/include/asm-arm/pci.h | 12 ++++++++++++ > > > > > > > 4 files changed, 54 insertions(+) > > > > > > > > > > > > > > diff --git a/xen/arch/arm/domain_build.c > > > > > > > b/xen/arch/arm/domain_build.c > > > > > > > index da427f399711..76f5b513280c 100644 > > > > > > > --- a/xen/arch/arm/domain_build.c > > > > > > > +++ b/xen/arch/arm/domain_build.c > > > > > > > @@ -1257,6 +1257,9 @@ static int __init > map_range_to_domain(const > > > > > > > struct dt_device_node *dev, > > > > > > > } > > > > > > > } > > > > > > > + if ( need_mapping && (device_get_class(dev) == > DEVICE_PCI) > > > > > > > ) > + need_mapping = > pci_host_bridge_need_p2m_mapping(d, dev, > > > > > > addr, len); > > > > > > > > > > > > AFAICT, with device_get_class(dev), you know whether the > hostbridge is > > > > > > used by Xen. Therefore, I would expect that we don't want to map > all > > > > > > the regions of the hostbridges in dom0 (including the BARs). > > > > > > > > > > > > Can you clarify it? > > > > > We only want to trap ECAM, not MMIOs and any other memory regions > as the > > > > > bridge is > > > > > > > > > > initialized and used by Domain-0 completely. > > > > > > > > What do you mean by "used by Domain-0 completely"? The hostbridge is > owned > > > > by Xen so I don't think we can let dom0 access any MMIO regions by > > > > default. > > > > > > Now it's my time to ask why do you think Xen owns the bridge? > > > > > > All the bridges are passed to Domain-0, are fully visible to Domain-0, > > > initialized in Domain-0. > > > > > > Enumeration etc. is done in Domain-0. So how comes that Xen owns the > bridge? > > > In what way it does? > > > > > > Xen just accesses the ECAM when it needs it, but that's it. Xen traps > ECAM > > > access - that is true. > > > > > > But it in no way uses MMIOs etc. of the bridge - those under direct > control > > > of Domain-0 > > > > So I looked on the snipped of the design document you posted. I think > you are > > instead referring to a different part: > > > > " PCI-PCIe enumeration is a process of detecting devices connected to its > > host. > > It is the responsibility of the hardware domain or boot firmware to do > the PCI > > enumeration and configure the BAR, PCI capabilities, and MSI/MSI-X > > configuration." > > > > But I still don't see why it means we have to map the MMIOs right now. > Dom0 > > should not need to access the MMIOs (aside the hostbridge registers) > until the > > BARs are configured. > > This is true especially when we are going to assign a specific PCIe > device to a DomU. In that case, the related MMIO regions are going to be > mapped to the DomU and it would be a bad idea to also keep them mapped > in Dom0. Once we do PCIe assignment, the MMIO region of the PCIe device > being assigned should only be mapped in the DomU, right? > That's actually a good point. This is a recipe for disaster because dom0 and the domU may map the BARs with conflicting caching attributes. So we ought to unmap the BAR from dom0. When the device is assigned to the domU > If so, it would be better if the MMIO region in question was never > mapped into Dom0 at all rather having to unmap it. > > With the current approach, given that the entire PCIe aperture is mapped > to Dom0 since boot, we would have to identify the relevant subset region > and unmap it from Dom0 when doing assignment. --0000000000004fe6de05cbaafc09 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Fri, 10 Sep 2021, 21:30 Stefano Stabellini, <sstabellini@kernel.org> wrote= :
On Fri, 10 Sep 2021, Julien Grall= wrote:
> On 10/09/2021 15:01, Oleksandr Andrushchenko wrote:
> > On 10.09.21 16:18, Julien Grall wrote:
> > > On 10/09/2021 13:37, Oleksandr Andrushchenko wrote:
> > > > Hi, Julien!
> > >
> > > Hi Oleksandr,
> > >
> > > > On 09.09.21 20:58, Julien Grall wrote:
> > > > > On 03/09/2021 09:33, Oleksandr Andrushchenko wrote= :
> > > > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> > > > > >
> > > > > > Host bridge controller's ECAM space is ma= pped into Domain-0's p2m,
> > > > > > thus it is not possible to trap the same for = vPCI via MMIO handlers.
> > > > > > For this to work we need to not map those whi= le constructing the
> > > > > > domain.
> > > > > >
> > > > > > Note, that during Domain-0 creation there is = no pci_dev yet
> > > > > > allocated for
> > > > > > host bridges, thus we cannot match PCI host a= nd its associated
> > > > > > bridge by SBDF. Use dt_device_node field for = checks instead.
> > > > > >
> > > > > > Signed-off-by: Oleksandr Andrushchenko
> > > > > > <oleksandr_andrushchenko@epa= m.com>
> > > > > > ---
> > > > > >=C2=A0 =C2=A0=C2=A0 xen/arch/arm/domain_build.= c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 3 +++
> > > > > >=C2=A0 =C2=A0=C2=A0 xen/arch/arm/pci/ecam.c=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 17 ++++++= +++++++++++
> > > > > >=C2=A0 =C2=A0=C2=A0 xen/arch/arm/pci/pci-host-= common.c | 22 ++++++++++++++++++++++
> > > > > >=C2=A0 =C2=A0=C2=A0 xen/include/asm-arm/pci.h= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 12 ++++++++++++ > > > > > >=C2=A0 =C2=A0=C2=A0 4 files changed, 54 insert= ions(+)
> > > > > >
> > > > > > diff --git a/xen/arch/arm/domain_build.c
> > > > > > b/xen/arch/arm/domain_build.c
> > > > > > index da427f399711..76f5b513280c 100644
> > > > > > --- a/xen/arch/arm/domain_build.c
> > > > > > +++ b/xen/arch/arm/domain_build.c
> > > > > > @@ -1257,6 +1257,9 @@ static int __init map_r= ange_to_domain(const
> > > > > > struct dt_device_node *dev,
> > > > > >=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 }<= br> > > > > > >=C2=A0 =C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0 if ( n= eed_mapping && (device_get_class(dev) =3D=3D DEVICE_PCI)
> > > > > > ) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 need_mapping =3D pci_host_bridge_need_p2m_mapping(d, dev,
> > > > > addr, len);
> > > > >
> > > > > AFAICT, with device_get_class(dev), you know wheth= er the hostbridge is
> > > > > used by Xen. Therefore, I would expect that we don= 't want to map all
> > > > > the regions of the hostbridges in dom0 (including = the BARs).
> > > > >
> > > > > Can you clarify it?
> > > > We only want to trap ECAM, not MMIOs and any other memo= ry regions as the
> > > > bridge is
> > > >
> > > > initialized and used by Domain-0 completely.
> > >
> > > What do you mean by "used by Domain-0 completely"?= The hostbridge is owned
> > > by Xen so I don't think we can let dom0 access any MMIO = regions by
> > > default.
> >
> > Now it's my time to ask why do you think Xen owns the bridge?=
> >
> > All the bridges are passed to Domain-0, are fully visible to Doma= in-0,
> > initialized in Domain-0.
> >
> > Enumeration etc. is done in Domain-0. So how comes that Xen owns = the bridge?
> > In what way it does?
> >
> > Xen just accesses the ECAM when it needs it, but that's it. X= en traps ECAM
> > access - that is true.
> >
> > But it in no way uses MMIOs etc. of the bridge - those under dire= ct control
> > of Domain-0
>
> So I looked on the snipped of the design document you posted. I think = you are
> instead referring to a different part:
>
> " PCI-PCIe enumeration is a process of detecting devices connecte= d to its
> host.
> It is the responsibility of the hardware domain or boot firmware to do= the PCI
> enumeration and configure the BAR, PCI capabilities, and MSI/MSI-X
> configuration."
>
> But I still don't see why it means we have to map the MMIOs right = now. Dom0
> should not need to access the MMIOs (aside the hostbridge registers) u= ntil the
> BARs are configured.

This is true especially when we are going to assign a specific PCIe
device to a DomU. In that case, the related MMIO regions are going to be mapped to the DomU and it would be a bad idea to also keep them mapped
in Dom0. Once we do PCIe assignment, the MMIO region of the PCIe device
being assigned should only be mapped in the DomU, right?

That's actually= a good point. This is a recipe for disaster because dom0 and the domU may = map the BARs with conflicting caching attributes.
So we ought to unmap the BAR from dom0. When the = device is assigned to the domU=C2=A0


If so, it would be better if the MMIO region in question was never
mapped into Dom0 at all rather having to unmap it.

With the current approach, given that the entire PCIe aperture is mapped to Dom0 since boot, we would have to identify the relevant subset region and unmap it from Dom0 when doing assignment.
--0000000000004fe6de05cbaafc09--