On 11.09.21 00:41, Julien Grall wrote: > > > On Fri, 10 Sep 2021, 21:30 Stefano Stabellini, <[email protected] > <mailto:[email protected]>> 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 > <[email protected] <mailto:[email protected]>> > > > > > > > > > > > > > > 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 > > > > > > > <[email protected] > <mailto:[email protected]>> > > > > > > > --- > > > > > > > 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 1. Yes, currently we map MMIOs to Dom0 from the beginning (the whole aperture actually)
2. When a PCIe device assigned to a DomU we do unmap the relevant MMIOs from Dom0 and map them to 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. > Ok, I'll do that > > > 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. > It is already in vPCI code (with non-identity mappings in the PCI devices passthrough on Arm, part 3)
