[PATCH v6 0/2] xen/pci: implement is_memory_hole for ARM

[PATCH v6 0/2] xen/pci: implement is_memory_hole for ARM

[Rahul Singh]

This patch series is to implement something like is_memory_hole function for
ARM.

Rahul Singh (2):
  xen/arm: pci: modify pci_find_host_bridge_node argument to const pdev
  xen/pci: replace call to is_memory_hole to pci_check_bar

 xen/arch/arm/include/asm/pci.h     |  5 ++-
 xen/arch/arm/pci/pci-host-common.c | 58 ++++++++++++++++++++++++++++--
 xen/arch/x86/include/asm/pci.h     | 10 ++++++
 xen/drivers/passthrough/pci.c      |  8 ++---
 4 files changed, 74 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH v6 1/2] xen/arm: pci: modify pci_find_host_bridge_node argument to const pdev

[Rahul Singh]

Modify pci_find_host_bridge_node argument to const pdev to avoid
converting the dev to pdev in pci_find_host_bridge_node and also
constify the return.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
Changes in v6:
 - no changes
Changes in v5:
 - no changes
Changes in v4:
 - no changes
Changes in v3:
 - no changes
---
---
 xen/arch/arm/include/asm/pci.h     | 3 ++-
 xen/arch/arm/pci/pci-host-common.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 7c7449d64f..80a2431804 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -106,7 +106,8 @@ bool pci_ecam_need_p2m_hwdom_mapping(struct domain *d,
                                      struct pci_host_bridge *bridge,
                                      uint64_t addr);
 struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus);
-struct dt_device_node *pci_find_host_bridge_node(struct device *dev);
+const struct dt_device_node *
+pci_find_host_bridge_node(const struct pci_dev *pdev);
 int pci_get_host_bridge_segment(const struct dt_device_node *node,
                                 uint16_t *segment);
 
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index fd8c0f837a..89ef30028e 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -243,10 +243,10 @@ err_exit:
 /*
  * Get host bridge node given a device attached to it.
  */
-struct dt_device_node *pci_find_host_bridge_node(struct device *dev)
+const struct dt_device_node *
+pci_find_host_bridge_node(const struct pci_dev *pdev)
 {
     struct pci_host_bridge *bridge;
-    struct pci_dev *pdev = dev_to_pci(dev);
 
     bridge = pci_find_host_bridge(pdev->seg, pdev->bus);
     if ( unlikely(!bridge) )
-- 
2.25.1

[PATCH v6 2/2] xen/pci: replace call to is_memory_hole to pci_check_bar

[Rahul Singh]

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.singh@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v6:
 - change from unsigned long to paddr_t
Changes in v5:
 - drop use of PFN_UP and PF_DOWN in case addresses are not aligned.
 - As we drop the PFN_UP and PFN_DOWN we need to use the mfn_to_maddr()
   to get the BAR address without page shift.
 - Add TODO comment for address alignment check for ranges.
 - Added Jan Acked-by for x86 and common code.
Changes in v4:
 - check "s <= e" before callback
 - Add TODO comment for revisiting the function pci_check_bar() when
   ACPI PCI passthrough support is added.
 - Not Added the Jan Acked-by as patch is modified.
Changes in v3:
 - fix minor comments
---
---
 xen/arch/arm/include/asm/pci.h     |  2 ++
 xen/arch/arm/pci/pci-host-common.c | 54 ++++++++++++++++++++++++++++++
 xen/arch/x86/include/asm/pci.h     | 10 ++++++
 xen/drivers/passthrough/pci.c      |  8 ++---
 4 files changed, 70 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..a8ece94303 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.
+ */
+struct pdev_bar_check
+{
+    paddr_t start;
+    paddr_t end;
+    bool is_valid;
+};
+
 /*
  * List for all the pci host bridges.
  */
@@ -363,6 +373,50 @@ int __init pci_host_bridge_mappings(struct domain *d)
     return 0;
 }
 
+/*
+ * TODO: BAR addresses and Root Complex window addresses are not guaranteed
+ * to be page aligned. We should check for alignment but this is not the
+ * right place for alignment check.
+ */
+static int is_bar_valid(const struct dt_device_node *dev,
+                        paddr_t addr, paddr_t len, void *data)
+{
+    struct pdev_bar_check *bar_data = data;
+    paddr_t s = bar_data->start;
+    paddr_t e = bar_data->end;
+
+    if ( (s >= addr) && (e <= (addr + len - 1)) )
+        bar_data->is_valid =  true;
+
+    return 0;
+}
+
+/* TODO: Revisit this function when ACPI PCI passthrough support is added. */
+bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
+{
+    int ret;
+    const struct dt_device_node *dt_node;
+    paddr_t s = mfn_to_maddr(start);
+    paddr_t e = mfn_to_maddr(end);
+    struct pdev_bar_check bar_data =  {
+        .start = s,
+        .end = e,
+        .is_valid = false
+    };
+
+    if ( s >= e )
+        return false;
+
+    dt_node = pci_find_host_bridge_node(pdev);
+    if ( !dt_node )
+        return false;
+
+    ret = dt_for_each_range(dt_node, &is_bar_valid, &bar_data);
+    if ( ret < 0 )
+        return false;
+
+    return bar_data.is_valid;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
index c8e1a9ecdb..f4a58c8acf 100644
--- a/xen/arch/x86/include/asm/pci.h
+++ b/xen/arch/x86/include/asm/pci.h
@@ -57,4 +57,14 @@ static always_inline bool is_pci_passthrough_enabled(void)
 
 void arch_pci_init_pdev(struct pci_dev *pdev);
 
+static inline bool pci_check_bar(const struct pci_dev *pdev,
+                                 mfn_t start, mfn_t end)
+{
+    /*
+     * Check if BAR is not overlapping with any memory region defined
+     * in the memory map.
+     */
+    return is_memory_hole(start, end);
+}
+
 #endif /* __X86_PCI_H__ */
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index cdaf5c247f..149f68bb6e 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -304,8 +304,8 @@ static void check_pdev(const struct pci_dev *pdev)
         if ( rc < 0 )
             /* Unable to size, better leave memory decoding disabled. */
             return;
-        if ( size && !is_memory_hole(maddr_to_mfn(addr),
-                                     maddr_to_mfn(addr + size - 1)) )
+        if ( size && !pci_check_bar(pdev, maddr_to_mfn(addr),
+                                    maddr_to_mfn(addr + size - 1)) )
         {
             /*
              * Return without enabling memory decoding if BAR position is not
@@ -331,8 +331,8 @@ static void check_pdev(const struct pci_dev *pdev)
 
         if ( rc < 0 )
             return;
-        if ( size && !is_memory_hole(maddr_to_mfn(addr),
-                                     maddr_to_mfn(addr + size - 1)) )
+        if ( size && !pci_check_bar(pdev, maddr_to_mfn(addr),
+                                    maddr_to_mfn(addr + size - 1)) )
         {
             printk(warn, &pdev->sbdf, "ROM ", PFN_DOWN(addr),
                    PFN_DOWN(addr + size - 1));
-- 
2.25.1

Re: [PATCH v6 2/2] xen/pci: replace call to is_memory_hole to pci_check_bar

[Stefano Stabellini]

On Fri, 9 Sep 2022, 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.singh@arm.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in v6:
>  - change from unsigned long to paddr_t
> Changes in v5:
>  - drop use of PFN_UP and PF_DOWN in case addresses are not aligned.
>  - As we drop the PFN_UP and PFN_DOWN we need to use the mfn_to_maddr()
>    to get the BAR address without page shift.
>  - Add TODO comment for address alignment check for ranges.
>  - Added Jan Acked-by for x86 and common code.
> Changes in v4:
>  - check "s <= e" before callback
>  - Add TODO comment for revisiting the function pci_check_bar() when
>    ACPI PCI passthrough support is added.
>  - Not Added the Jan Acked-by as patch is modified.
> Changes in v3:
>  - fix minor comments
> ---
> ---
>  xen/arch/arm/include/asm/pci.h     |  2 ++
>  xen/arch/arm/pci/pci-host-common.c | 54 ++++++++++++++++++++++++++++++
>  xen/arch/x86/include/asm/pci.h     | 10 ++++++
>  xen/drivers/passthrough/pci.c      |  8 ++---
>  4 files changed, 70 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..a8ece94303 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.
> + */
> +struct pdev_bar_check
> +{
> +    paddr_t start;
> +    paddr_t end;
> +    bool is_valid;
> +};
> +
>  /*
>   * List for all the pci host bridges.
>   */
> @@ -363,6 +373,50 @@ int __init pci_host_bridge_mappings(struct domain *d)
>      return 0;
>  }
>  
> +/*
> + * TODO: BAR addresses and Root Complex window addresses are not guaranteed
> + * to be page aligned. We should check for alignment but this is not the
> + * right place for alignment check.
> + */
> +static int is_bar_valid(const struct dt_device_node *dev,
> +                        paddr_t addr, paddr_t len, void *data)
> +{
> +    struct pdev_bar_check *bar_data = data;
> +    paddr_t s = bar_data->start;
> +    paddr_t e = bar_data->end;
> +
> +    if ( (s >= addr) && (e <= (addr + len - 1)) )
> +        bar_data->is_valid =  true;
> +
> +    return 0;
> +}
> +
> +/* TODO: Revisit this function when ACPI PCI passthrough support is added. */
> +bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
> +{
> +    int ret;
> +    const struct dt_device_node *dt_node;
> +    paddr_t s = mfn_to_maddr(start);
> +    paddr_t e = mfn_to_maddr(end);
> +    struct pdev_bar_check bar_data =  {
> +        .start = s,
> +        .end = e,
> +        .is_valid = false
> +    };
> +
> +    if ( s >= e )
> +        return false;
> +
> +    dt_node = pci_find_host_bridge_node(pdev);
> +    if ( !dt_node )
> +        return false;
> +
> +    ret = dt_for_each_range(dt_node, &is_bar_valid, &bar_data);
> +    if ( ret < 0 )
> +        return false;
> +
> +    return bar_data.is_valid;
> +}
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
> index c8e1a9ecdb..f4a58c8acf 100644
> --- a/xen/arch/x86/include/asm/pci.h
> +++ b/xen/arch/x86/include/asm/pci.h
> @@ -57,4 +57,14 @@ static always_inline bool is_pci_passthrough_enabled(void)
>  
>  void arch_pci_init_pdev(struct pci_dev *pdev);
>  
> +static inline bool pci_check_bar(const struct pci_dev *pdev,
> +                                 mfn_t start, mfn_t end)
> +{
> +    /*
> +     * Check if BAR is not overlapping with any memory region defined
> +     * in the memory map.
> +     */
> +    return is_memory_hole(start, end);
> +}
> +
>  #endif /* __X86_PCI_H__ */
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index cdaf5c247f..149f68bb6e 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -304,8 +304,8 @@ static void check_pdev(const struct pci_dev *pdev)
>          if ( rc < 0 )
>              /* Unable to size, better leave memory decoding disabled. */
>              return;
> -        if ( size && !is_memory_hole(maddr_to_mfn(addr),
> -                                     maddr_to_mfn(addr + size - 1)) )
> +        if ( size && !pci_check_bar(pdev, maddr_to_mfn(addr),
> +                                    maddr_to_mfn(addr + size - 1)) )
>          {
>              /*
>               * Return without enabling memory decoding if BAR position is not
> @@ -331,8 +331,8 @@ static void check_pdev(const struct pci_dev *pdev)
>  
>          if ( rc < 0 )
>              return;
> -        if ( size && !is_memory_hole(maddr_to_mfn(addr),
> -                                     maddr_to_mfn(addr + size - 1)) )
> +        if ( size && !pci_check_bar(pdev, maddr_to_mfn(addr),
> +                                    maddr_to_mfn(addr + size - 1)) )
>          {
>              printk(warn, &pdev->sbdf, "ROM ", PFN_DOWN(addr),
>                     PFN_DOWN(addr + size - 1));
> -- 
> 2.25.1
>