diff mbox

[v2] xen/dts: Don't translate invalid address

Message ID 1389026178-8792-1-git-send-email-julien.grall@linaro.org
State Accepted, archived
Headers show

Commit Message

Julien Grall Jan. 6, 2014, 4:36 p.m. UTC
ePAR specifies that if the property "ranges" doesn't exist in a bus node:

"it is assumed that no mapping exists between children of node and the parent
address space".

Modify dt_number_of_address to check if the list of ranges are valid. Return
0 (ie there is zero range) if the list is not valid.

This patch has been tested on the Arndale where the bug can occur with the
'/hdmi' node.

Reported-by: <tsahee@gmx.com>
Signed-off-by: Julien Grall <julien.grall@linaro.org>

---

This patch is a bug fix for Xen 4.4. Without it, Xen can't boot on the Arndale
because it's unable to translate a wrong address.

    Changes in v2:
        - "parent" name is very confusing, rename it in "node"
        - remove spurious change
---
 xen/common/device_tree.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

Comments

Ian Campbell Jan. 7, 2014, 1:54 p.m. UTC | #1
On Mon, 2014-01-06 at 16:36 +0000, Julien Grall wrote:
> ePAR specifies that if the property "ranges" doesn't exist in a bus node:
> 
> "it is assumed that no mapping exists between children of node and the parent
> address space".
> 
> Modify dt_number_of_address to check if the list of ranges are valid. Return
> 0 (ie there is zero range) if the list is not valid.
> 
> This patch has been tested on the Arndale where the bug can occur with the
> '/hdmi' node.
> 
> Reported-by: <tsahee@gmx.com>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> This patch is a bug fix for Xen 4.4. Without it, Xen can't boot on the Arndale
> because it's unable to translate a wrong address.

RM hat: Arndale is one of our main platforms today, so not working out
of the box is a pretty serious issue. The risk of course is that we
break things on some other platform. I think we know that none of the
other platforms have this problem, since otherwise Xen wouldn't boot on
them. Given the ePAPR specification trying to translate these addresses
is totally meaningless so it seems unlikely that stopping trying to
translate them would cause an issue. So I think this is OK.

Applied.
Ian Campbell Jan. 8, 2014, 5:45 p.m. UTC | #2
On Tue, 2014-01-07 at 13:54 +0000, Ian Campbell wrote:
> Applied.

Except I seem to have failed to actually do it... I've put this back in
my queue and will pick it up on my next attempt...

Ian.
Ian Campbell Jan. 9, 2014, 3:50 p.m. UTC | #3
On Wed, 2014-01-08 at 17:45 +0000, Ian Campbell wrote:
> On Tue, 2014-01-07 at 13:54 +0000, Ian Campbell wrote:
> > Applied.
> 
> Except I seem to have failed to actually do it... I've put this back in
> my queue and will pick it up on my next attempt...

Really done this time...
diff mbox

Patch

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index c35aee1..fa9d06c 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -83,7 +83,7 @@  struct dt_bus
 {
     const char *name;
     const char *addresses;
-    int (*match)(const struct dt_device_node *parent);
+    bool_t (*match)(const struct dt_device_node *node);
     void (*count_cells)(const struct dt_device_node *child,
                         int *addrc, int *sizec);
     u64 (*map)(__be32 *addr, const __be32 *range, int na, int ns, int pna);
@@ -783,6 +783,18 @@  int dt_n_size_cells(const struct dt_device_node *np)
 /*
  * Default translator (generic bus)
  */
+static bool_t dt_bus_default_match(const struct dt_device_node *node)
+{
+    /* Root node doesn't have "ranges" property */
+    if ( node->parent == NULL )
+        return 1;
+
+    /* The default bus is only used when the "ranges" property exists.
+     * Otherwise we can't translate the address
+     */
+    return (dt_get_property(node, "ranges", NULL) != NULL);
+}
+
 static void dt_bus_default_count_cells(const struct dt_device_node *dev,
                                 int *addrc, int *sizec)
 {
@@ -846,7 +858,7 @@  static const struct dt_bus dt_busses[] =
     {
         .name = "default",
         .addresses = "reg",
-        .match = NULL,
+        .match = dt_bus_default_match,
         .count_cells = dt_bus_default_count_cells,
         .map = dt_bus_default_map,
         .translate = dt_bus_default_translate,
@@ -861,7 +873,6 @@  static const struct dt_bus *dt_match_bus(const struct dt_device_node *np)
     for ( i = 0; i < ARRAY_SIZE(dt_busses); i++ )
         if ( !dt_busses[i].match || dt_busses[i].match(np) )
             return &dt_busses[i];
-    BUG();
 
     return NULL;
 }
@@ -880,7 +891,10 @@  static const __be32 *dt_get_address(const struct dt_device_node *dev,
     parent = dt_get_parent(dev);
     if ( parent == NULL )
         return NULL;
+
     bus = dt_match_bus(parent);
+    if ( !bus )
+        return NULL;
     bus->count_cells(dev, &na, &ns);
 
     if ( !DT_CHECK_ADDR_COUNT(na) )
@@ -984,6 +998,8 @@  static u64 __dt_translate_address(const struct dt_device_node *dev,
     if ( parent == NULL )
         goto bail;
     bus = dt_match_bus(parent);
+    if ( !bus )
+        goto bail;
 
     /* Count address cells & copy address locally */
     bus->count_cells(dev, &na, &ns);
@@ -1016,6 +1032,11 @@  static u64 __dt_translate_address(const struct dt_device_node *dev,
 
         /* Get new parent bus and counts */
         pbus = dt_match_bus(parent);
+        if ( pbus == NULL )
+        {
+            dt_printk("DT: %s is not a valid bus\n", parent->full_name);
+            break;
+        }
         pbus->count_cells(dev, &pna, &pns);
         if ( !DT_CHECK_COUNTS(pna, pns) )
         {
@@ -1154,6 +1175,8 @@  unsigned int dt_number_of_address(const struct dt_device_node *dev)
         return 0;
 
     bus = dt_match_bus(parent);
+    if ( !bus )
+        return 0;
     bus->count_cells(dev, &na, &ns);
 
     if ( !DT_CHECK_COUNTS(na, ns) )