[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 10/11] xen/arm: Do not map PCI ECAM space to Domain-0's p2m
> >> >> 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 Sorry for pasting here quite a big patch, but I feel I need clarification if this is the way we want it. Please note I had to modify setup.h. From 6eee96bc046efb41ec25f87362b1f6e973a4e6c2 Mon Sep 17 00:00:00 2001 From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> Date: Tue, 14 Sep 2021 12:14:43 +0300 Subject: [PATCH] Fixes: a57dc84da5fd ("xen/arm: Do not map PCI ECAM space to Domain-0's p2m") Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> --- xen/arch/arm/domain_build.c | 37 +++++++++++++-------- xen/arch/arm/pci/ecam.c | 11 +++---- xen/arch/arm/pci/pci-host-common.c | 53 ++++++++++++++++++++++-------- xen/include/asm-arm/pci.h | 18 ++++------ xen/include/asm-arm/setup.h | 13 ++++++++ 5 files changed, 86 insertions(+), 46 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 76f5b513280c..b4bfda9d5b5a 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -10,7 +10,6 @@ #include <asm/regs.h> #include <xen/errno.h> #include <xen/err.h> -#include <xen/device_tree.h> #include <xen/libfdt/libfdt.h> #include <xen/guest_access.h> #include <xen/iocap.h> @@ -47,12 +46,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)) @@ -1228,9 +1221,8 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev, return 0; } -static int __init map_range_to_domain(const struct dt_device_node *dev, - u64 addr, u64 len, - void *data) +int __init map_range_to_domain(const struct dt_device_node *dev, + u64 addr, u64 len, void *data) { struct map_range_data *mr_data = data; struct domain *d = mr_data->d; @@ -1257,8 +1249,10 @@ 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); +#ifdef CONFIG_HAS_PCI + if ( (device_get_class(dev) == DEVICE_PCI) && !mr_data->map_pci_bridge ) + need_mapping = false; +#endif if ( need_mapping ) { @@ -1293,7 +1287,11 @@ static int __init map_device_children(struct domain *d, const struct dt_device_node *dev, p2m_type_t p2mt) { - struct map_range_data mr_data = { .d = d, .p2mt = p2mt }; + struct map_range_data mr_data = { + .d = d, + .p2mt = p2mt, + .map_pci_bridge = false + }; int ret; if ( dt_device_type_is_equal(dev, "pci") ) @@ -1425,7 +1423,11 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, /* Give permission and map MMIOs */ for ( i = 0; i < naddr; i++ ) { - struct map_range_data mr_data = { .d = d, .p2mt = p2mt }; + struct map_range_data mr_data = { + .d = d, + .p2mt = p2mt, + .map_pci_bridge = false + }; res = dt_device_get_address(dev, i, &addr, &size); if ( res ) { @@ -2594,7 +2596,14 @@ static int __init construct_dom0(struct domain *d) return rc; if ( acpi_disabled ) + { rc = prepare_dtb_hwdom(d, &kinfo); +#ifdef CONFIG_HAS_PCI + if ( rc < 0 ) + return rc; + rc = pci_host_bridge_mappings(d, p2m_mmio_direct_c); +#endif + } else rc = prepare_acpi(d, &kinfo); diff --git a/xen/arch/arm/pci/ecam.c b/xen/arch/arm/pci/ecam.c index d32efb7fcbd0..e08b9c6909b6 100644 --- a/xen/arch/arm/pci/ecam.c +++ b/xen/arch/arm/pci/ecam.c @@ -52,17 +52,14 @@ static int pci_ecam_register_mmio_handler(struct domain *d, return 0; } -static int pci_ecam_need_p2m_mapping(struct domain *d, - struct pci_host_bridge *bridge, - uint64_t addr, uint64_t len) +static bool pci_ecam_need_p2m_mapping(struct domain *d, + struct pci_host_bridge *bridge, + uint64_t addr) { struct pci_config_window *cfg = bridge->sysdata; - if ( !is_hardware_domain(d) ) - return true; - /* - * We do not want ECAM address space to be mapped in domain's p2m, + * We do not want ECAM address space to be mapped in Domain-0's p2m, * so we can trap access to it. */ return cfg->phys_addr != addr; diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c index c04be636452d..74077dec8c72 100644 --- a/xen/arch/arm/pci/pci-host-common.c +++ b/xen/arch/arm/pci/pci-host-common.c @@ -25,6 +25,7 @@ #include <xen/init.h> #include <xen/pci.h> #include <asm/pci.h> +#include <asm/setup.h> #include <xen/rwlock.h> #include <xen/sched.h> #include <xen/vmap.h> @@ -335,25 +336,51 @@ int pci_host_iterate_bridges(struct domain *d, return 0; } -bool pci_host_bridge_need_p2m_mapping(struct domain *d, - const struct dt_device_node *node, - uint64_t addr, uint64_t len) +int __init pci_host_bridge_mappings(struct domain *d, p2m_type_t p2mt) { struct pci_host_bridge *bridge; + struct map_range_data mr_data = { + .d = d, + .p2mt = p2mt, + .map_pci_bridge = true + }; + /* + * For each PCI host bridge we need to only map those ranges + * which are used by Domain-0 to properly initialize the bridge, + * e.g. we do not want to map ECAM configuration space which lives in + * "reg" or "assigned-addresses" device tree property. + * Neither we want to map any of the MMIO ranges found in the "ranges" + * device tree property. + */ list_for_each_entry( bridge, &pci_host_bridges, node ) { - if ( bridge->dt_node != node ) - continue; - - if ( !bridge->ops->need_p2m_mapping ) - return true; - - return bridge->ops->need_p2m_mapping(d, bridge, addr, len); + const struct dt_device_node *dev = bridge->dt_node; + int i; + + for ( i = 0; i < dt_number_of_address(dev); i++ ) + { + uint64_t addr, size; + int err; + + err = dt_device_get_address(dev, i, &addr, &size); + if ( err ) + { + printk(XENLOG_ERR "Unable to retrieve address %u for %s\n", + i, dt_node_full_name(dev)); + return err; + } + + if ( bridge->ops->need_p2m_mapping(d, bridge, addr) ) + { + err = map_range_to_domain(dev, addr, size, &mr_data); + if ( err ) + return err; + } + } } - printk(XENLOG_ERR "Unable to find PCI bridge for %s segment %d, addr %lx\n", - node->full_name, bridge->segment, addr); - return true; + + return 0; } /* diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h index 9c28a4bdc4b7..97fbaac01370 100644 --- a/xen/include/asm-arm/pci.h +++ b/xen/include/asm-arm/pci.h @@ -21,6 +21,8 @@ #ifdef CONFIG_HAS_PCI +#include <asm/p2m.h> + #define pci_to_dev(pcidev) (&(pcidev)->arch.dev) #define PRI_pci "%04x:%02x:%02x.%u" @@ -82,8 +84,9 @@ struct pci_ops { int (*register_mmio_handler)(struct domain *d, struct pci_host_bridge *bridge, const struct mmio_handler_ops *ops); - int (*need_p2m_mapping)(struct domain *d, struct pci_host_bridge *bridge, - uint64_t addr, uint64_t len); + bool (*need_p2m_mapping)(struct domain *d, + struct pci_host_bridge *bridge, + uint64_t addr); }; /* @@ -117,19 +120,10 @@ struct dt_device_node *pci_find_host_bridge_node(struct device *dev); int pci_host_iterate_bridges(struct domain *d, int (*clb)(struct domain *d, struct pci_host_bridge *bridge)); -bool pci_host_bridge_need_p2m_mapping(struct domain *d, - const struct dt_device_node *node, - uint64_t addr, uint64_t len); +int pci_host_bridge_mappings(struct domain *d, p2m_type_t p2mt); #else /*!CONFIG_HAS_PCI*/ struct arch_pci_dev { }; -static inline bool -pci_host_bridge_need_p2m_mapping(struct domain *d, - const struct dt_device_node *node, - uint64_t addr, uint64_t len) -{ - return true; -} #endif /*!CONFIG_HAS_PCI*/ #endif /* __ARM_PCI_H__ */ diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index c4b6af602995..a1c31c0bb024 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -2,6 +2,8 @@ #define __ARM_SETUP_H_ #include <public/version.h> +#include <asm/p2m.h> +#include <xen/device_tree.h> #define MIN_FDT_ALIGN 8 #define MAX_FDT_SIZE SZ_2M @@ -76,6 +78,14 @@ struct bootinfo { #endif }; +struct map_range_data +{ + struct domain *d; + p2m_type_t p2mt; + /* Set if mappings for PCI host bridges must not be skipped. */ + bool map_pci_bridge; +}; + extern struct bootinfo bootinfo; extern domid_t max_init_domid; @@ -123,6 +133,9 @@ void device_tree_get_reg(const __be32 **cell, u32 address_cells, u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name, u32 dflt); +int map_range_to_domain(const struct dt_device_node *dev, + u64 addr, u64 len, void *data); + #endif /* * Local variables: -- 2.25.1 With the patch above I have the following log in Domain-0: generic-armv8-xt-dom0 login: root root@generic-armv8-xt-dom0:~# (XEN) *** Serial input to Xen (type 'CTRL-a' three times to switch input) (XEN) ==== PCI devices ==== (XEN) ==== segment 0000 ==== (XEN) 0000:03:00.0 - d0 - node -1 (XEN) 0000:02:02.0 - d0 - node -1 (XEN) 0000:02:01.0 - d0 - node -1 (XEN) 0000:02:00.0 - d0 - node -1 (XEN) 0000:01:00.0 - d0 - node -1 (XEN) 0000:00:00.0 - d0 - node -1 (XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input) root@generic-armv8-xt-dom0:~# modprobe e1000e [ 46.104729] e1000e: Intel(R) PRO/1000 Network Driver [ 46.105479] e1000e: Copyright(c) 1999 - 2015 Intel Corporation. [ 46.107297] e1000e 0000:03:00.0: enabling device (0000 -> 0002) (XEN) map [e0000, e001f] -> 0xe0000 for d0 (XEN) map [e0020, e003f] -> 0xe0020 for d0 [ 46.178513] e1000e 0000:03:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode [ 46.189668] pci_msi_setup_msi_irqs [ 46.191016] nwl_compose_msi_msg msg at fe440000 (XEN) traps.c:2014:d0v0 HSR=0x00000093810047 pc=0xffff8000104b4b00 gva=0xffff800010fa5000 gpa=0x000000e0040000 [ 46.200455] Unhandled fault at 0xffff800010fa5000 [snip] [ 46.233079] Call trace: [ 46.233559] __pci_write_msi_msg+0x70/0x180 [ 46.234149] pci_msi_domain_write_msg+0x28/0x30 [ 46.234869] msi_domain_activate+0x5c/0x88 From the above you can see that BARs are mapped for Domain-0 now only when an assigned PCI device gets enabled in Domain-0. Another thing to note is that we crash on MSI-X access as BARs are mapped to the domain while enabling memory decoding in the COMMAND register, but MSI-X are handled differently, e.g. we have MSI-X holes in the mappings. This is because before this change the whole PCI aperture was mapped into Domain-0 and it is not. Thus, MSI-X holes are left unmapped now and there was no need to do so, e.g. they were always mapped into Domain-0 and emulated for guests. Please note that one cannot use xl pci-attach in this case to attach the PCI device in question to Domain-0 as (please see the log) that device is already attached. Neither it can be detached and re-attached. So, without mapping MSI-X holes for Domain-0 the device becomes unusable in Domain-0. At the same time the device can be successfully passed to DomU. Julien, Stefano! Please let me know how can we proceed with this. Thank you, Oleksandr
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |