From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen/dts: Don't translate invalid address Date: Sun, 05 Jan 2014 21:19:39 +0000 Message-ID: <52C9CC6B.7030905@linaro.org> References: <1386901910-1016-1-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vzv6n-00007K-3n for xen-devel@lists.xenproject.org; Sun, 05 Jan 2014 21:19:45 +0000 Received: by mail-lb0-f175.google.com with SMTP id w6so9229000lbh.6 for ; Sun, 05 Jan 2014 13:19:42 -0800 (PST) In-Reply-To: <1386901910-1016-1-git-send-email-julien.grall@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xenproject.org Cc: ian.campbell@citrix.com, patches@linaro.org, Julien Grall , tim@xen.org, stefano.stabellini@citrix.com, tsahee@gmx.com List-Id: xen-devel@lists.xenproject.org Hello all, Any comments on this patch? Sincerely yours, On 12/13/2013 02:31 AM, 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: > Signed-off-by: Julien Grall > > --- > > 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. > --- > xen/common/device_tree.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 84e709d..9b9a348 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -93,7 +93,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 *parent); > 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); > @@ -793,6 +793,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 *parent) > +{ > + /* Root node doesn't have "ranges" property */ > + if ( parent->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(parent, "ranges", NULL) != NULL); > +} > + > static void dt_bus_default_count_cells(const struct dt_device_node *dev, > int *addrc, int *sizec) > { > @@ -819,7 +831,7 @@ static u64 dt_bus_default_map(__be32 *addr, const __be32 *range, > * If the number of address cells is larger than 2 we assume the > * mapping doesn't specify a physical address. Rather, the address > * specifies an identifier that must match exactly. > - */ > + */ > if ( na > 2 && memcmp(range, addr, na * 4) != 0 ) > return DT_BAD_ADDR; > > @@ -856,7 +868,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, > @@ -871,7 +883,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; > } > @@ -890,7 +901,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) ) > @@ -994,6 +1008,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); > @@ -1026,6 +1042,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) ) > { > @@ -1164,6 +1185,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) ) > -- Julien Grall