Hi Rahul,
On 01/09/2022 10:29, Rahul Singh wrote:
is_memory_hole was implemented for x86 and not for ARM when introduced.
Replace is_memory_hole call to pci_check_bar as function should check
if device BAR is in defined memory range. Also, add an implementation
for ARM which is required for PCI passthrough.
On x86, pci_check_bar will call is_memory_hole which will check if BAR
is not overlapping with any memory region defined in the memory map.
On ARM, pci_check_bar will go through the host bridge ranges and check
if the BAR is in the range of defined ranges.
Signed-off-by: Rahul Singh <rahul.si...@arm.com>
---
Changes in v3:
- fix minor comments
---
xen/arch/arm/include/asm/pci.h | 2 ++
xen/arch/arm/pci/pci-host-common.c | 43 ++++++++++++++++++++++++++++++
xen/arch/x86/include/asm/pci.h | 10 +++++++
xen/drivers/passthrough/pci.c | 8 +++---
4 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 80a2431804..8cb46f6b71 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -126,6 +126,8 @@ int pci_host_iterate_bridges_and_count(struct domain *d,
int pci_host_bridge_mappings(struct domain *d);
+bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end);
+
#else /*!CONFIG_HAS_PCI*/
struct arch_pci_dev { };
diff --git a/xen/arch/arm/pci/pci-host-common.c
b/xen/arch/arm/pci/pci-host-common.c
index 89ef30028e..0eb121666d 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -24,6 +24,16 @@
#include <asm/setup.h>
+/*
+ * struct to hold pci device bar.
+ */
I find this comment a bit misleading. What you are storing is a
candidate region. IOW, this may or may not be a PCI device bar.
Given the current use below, I would rename the structure to something
more specific like: pdev_bar_check.
+struct pdev_bar
+{
+ mfn_t start;
+ mfn_t end;
+ bool is_valid;
+};
+
/*
* List for all the pci host bridges.
*/
@@ -363,6 +373,39 @@ int __init pci_host_bridge_mappings(struct domain *d)
return 0;
}
+static int is_bar_valid(const struct dt_device_node *dev,
+ uint64_t addr, uint64_t len, void *data)
+{
+ struct pdev_bar *bar_data = data;
+ unsigned long s = mfn_x(bar_data->start);
+ unsigned long e = mfn_x(bar_data->end);
+
+ if ( (s <= e) && (s >= PFN_DOWN(addr)) && (e <= PFN_UP(addr + len - 1)) )
AFAICT 's' and 'e' are provided by pci_check_bar() and will never
change. So can we move the check 's <= e' outside of the callback?
+ bar_data->is_valid = true;
+
+ return 0;
+}
+
+bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
+{
Other than the current calls in check_pdev(), do you have plan to use it
in more places? The reason I am asking it is this function is
non-trivial on Arm (dt_for_each_range() is quite complex).
Cheers,
--
Julien Grall
