Re: [PATCH v7 5/7] xen/arm: do not map PCI ECAM and MMIO space to Domain-0's p2m
Hi, Julien! On 08.12.21 19:20, Julien Grall wrote: > Hi Oleksandr, > > On 24/11/2021 07:59, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko >> >> PCI host bridges are special devices in terms of implementing PCI >> passthrough. According to [1] the current implementation depends on >> Domain-0 to perform the initialization of the relevant PCI host >> bridge hardware and perform PCI device enumeration. In order to >> achieve that one of the required changes is to not map all the memory >> ranges in map_range_to_domain as we traverse the device tree on startup >> and perform some additional checks if the range needs to be mapped to >> Domain-0. >> >> The generic PCI host controller device tree binding says [2]: >> - ranges: As described in IEEE Std 1275-1994, but must provide >> at least a definition of non-prefetchable memory. One >> or both of prefetchable Memory and IO Space may also >> be provided. >> >> - reg : The Configuration Space base address and size, as accessed >> from the parent bus. The base address corresponds to >> the first bus in the "bus-range" property. If no >> "bus-range" is specified, this will be bus 0 (the default). >> >> From the above none of the memory ranges from the "ranges" property >> needs to be mapped to Domain-0 at startup as MMIO mapping is going to >> be handled dynamically by vPCI as we assign PCI devices, e.g. each >> device assigned to Domain-0/guest will have its MMIOs mapped/unmapped >> as needed by Xen. >> >> The "reg" property covers not only ECAM space, but may also have other >> then the configuration memory ranges described, for example [3]: >> - reg: Should contain rc_dbi, config registers location and length. >> - reg-names: Must include the following entries: >> "rc_dbi": controller configuration registers; >> "config": PCIe configuration space registers. >> >> This patch makes it possible to not map all the ranges from the >> "ranges" property and also ECAM from the "reg". All the rest from the >> "reg" property still needs to be mapped to Domain-0, so the PCI >> host bridge remains functional in Domain-0. This is done by first >> skipping the mappings while traversing the device tree as it is done for >> usual devices and then by calling a dedicated pci_host_bridge_mappings >> function which only maps MMIOs required by the host bridges leaving the >> regions, needed for vPCI traps, unmapped. >> >> [1] >> https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg00777.html >> [2] >> https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt >> [3] >> https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt >> >> Signed-off-by: Oleksandr Andrushchenko > > Reviewed-by: Julien Grall > > I haven't committed because it is not clear whether this patch depends on > earlier patches that are still under review. Can you advise? I will resend the whole series (leftovers), so no need to commit now Thank you, Oleksandr > > Cheers, >
Re: [PATCH v7 5/7] xen/arm: do not map PCI ECAM and MMIO space to Domain-0's p2m
Hi Oleksandr, On 24/11/2021 07:59, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko PCI host bridges are special devices in terms of implementing PCI passthrough. According to [1] the current implementation depends on Domain-0 to perform the initialization of the relevant PCI host bridge hardware and perform PCI device enumeration. In order to achieve that one of the required changes is to not map all the memory ranges in map_range_to_domain as we traverse the device tree on startup and perform some additional checks if the range needs to be mapped to Domain-0. The generic PCI host controller device tree binding says [2]: - ranges: As described in IEEE Std 1275-1994, but must provide at least a definition of non-prefetchable memory. One or both of prefetchable Memory and IO Space may also be provided. - reg : The Configuration Space base address and size, as accessed from the parent bus. The base address corresponds to the first bus in the "bus-range" property. If no "bus-range" is specified, this will be bus 0 (the default). From the above none of the memory ranges from the "ranges" property needs to be mapped to Domain-0 at startup as MMIO mapping is going to be handled dynamically by vPCI as we assign PCI devices, e.g. each device assigned to Domain-0/guest will have its MMIOs mapped/unmapped as needed by Xen. The "reg" property covers not only ECAM space, but may also have other then the configuration memory ranges described, for example [3]: - reg: Should contain rc_dbi, config registers location and length. - reg-names: Must include the following entries: "rc_dbi": controller configuration registers; "config": PCIe configuration space registers. This patch makes it possible to not map all the ranges from the "ranges" property and also ECAM from the "reg". All the rest from the "reg" property still needs to be mapped to Domain-0, so the PCI host bridge remains functional in Domain-0. This is done by first skipping the mappings while traversing the device tree as it is done for usual devices and then by calling a dedicated pci_host_bridge_mappings function which only maps MMIOs required by the host bridges leaving the regions, needed for vPCI traps, unmapped. [1] https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg00777.html [2] https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Julien Grall I haven't committed because it is not clear whether this patch depends on earlier patches that are still under review. Can you advise? Cheers, -- Julien Grall
[PATCH v7 5/7] xen/arm: do not map PCI ECAM and MMIO space to Domain-0's p2m
From: Oleksandr Andrushchenko
PCI host bridges are special devices in terms of implementing PCI
passthrough. According to [1] the current implementation depends on
Domain-0 to perform the initialization of the relevant PCI host
bridge hardware and perform PCI device enumeration. In order to
achieve that one of the required changes is to not map all the memory
ranges in map_range_to_domain as we traverse the device tree on startup
and perform some additional checks if the range needs to be mapped to
Domain-0.
The generic PCI host controller device tree binding says [2]:
- ranges: As described in IEEE Std 1275-1994, but must provide
at least a definition of non-prefetchable memory. One
or both of prefetchable Memory and IO Space may also
be provided.
- reg : The Configuration Space base address and size, as accessed
from the parent bus. The base address corresponds to
the first bus in the "bus-range" property. If no
"bus-range" is specified, this will be bus 0 (the default).
>From the above none of the memory ranges from the "ranges" property
needs to be mapped to Domain-0 at startup as MMIO mapping is going to
be handled dynamically by vPCI as we assign PCI devices, e.g. each
device assigned to Domain-0/guest will have its MMIOs mapped/unmapped
as needed by Xen.
The "reg" property covers not only ECAM space, but may also have other
then the configuration memory ranges described, for example [3]:
- reg: Should contain rc_dbi, config registers location and length.
- reg-names: Must include the following entries:
"rc_dbi": controller configuration registers;
"config": PCIe configuration space registers.
This patch makes it possible to not map all the ranges from the
"ranges" property and also ECAM from the "reg". All the rest from the
"reg" property still needs to be mapped to Domain-0, so the PCI
host bridge remains functional in Domain-0. This is done by first
skipping the mappings while traversing the device tree as it is done for
usual devices and then by calling a dedicated pci_host_bridge_mappings
function which only maps MMIOs required by the host bridges leaving the
regions, needed for vPCI traps, unmapped.
[1] https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg00777.html
[2]
https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
[3]
https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
Signed-off-by: Oleksandr Andrushchenko
---
Since v7:
- updates in comments and commit message
Since v5:
- remove some need_mapping local variables
- use own_device in handle_device
- add __init for pci_ecam_need_p2m_hwdom_mapping
- make pci_host_bridge_mappings use p2m_mmio_direct_dev directly
Since v4:
- update skip_mapping comment
- add comment why we need to map interrupts to Dom0
Since v3:
- pass struct map_range_data to map_dt_irq_to_domain
- remove redundant check from map_range_to_domain
- fix handle_device's .skip_mapping
Since v2:
- removed check in map_range_to_domain for PCI_DEV
and moved it to handle_device, so the code is
simpler
- s/map_pci_bridge/skip_mapping
- extended comment in pci_host_bridge_mappings
- minor code restructure in construct_dom0
- s/.need_p2m_mapping/.need_p2m_hwdom_mapping and related
callbacks
- unsigned int i; in pci_host_bridge_mappings
Since v1:
- Added better description of why and what needs to be mapped into
Domain-0's p2m and what doesn't
- Do not do any mappings for PCI devices while traversing the DT
- Walk all the bridges and make required mappings in one go
---
xen/arch/arm/domain_build.c| 66 +-
xen/arch/arm/pci/ecam.c| 14 +++
xen/arch/arm/pci/pci-host-common.c | 50 ++
xen/arch/arm/pci/pci-host-zynqmp.c | 1 +
xen/include/asm-arm/pci.h | 10 +
xen/include/asm-arm/setup.h| 13 ++
6 files changed, 126 insertions(+), 28 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c83c02ab8ac6..47c884cca2c3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -51,12 +51,6 @@ static int __init parse_dom0_mem(const char *s)
}
custom_param("dom0_mem", parse_dom0_mem);
-struct map_range_data
-{
-struct domain *d;
-p2m_type_t p2mt;
-};
-
/* Override macros from asm/page.h to make them work with mfn_t */
#undef virt_to_mfn
#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
@@ -1720,10 +1714,10 @@ static int __init map_dt_irq_to_domain(const struct
dt_device_node *dev,
const struct dt_irq *dt_irq,
void *data)
{
-struct domain *d = data;
+struct map_range_data *mr_data = data;
+struct domain *d = mr_data->d;
unsigned int irq = dt_irq->irq;
int res;
-bool need_mapping = !dt_device_for_passthrough(dev);
if ( irq < NR_LOCAL_IRQS )
