Re: [Xen-devel] [PATCH] xen/dts: Don't translate invalid address

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-12-13 at 02:31 +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@xxxxxxx>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> ---
> 
> 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 )

Calling the parameter "parent" led to me confusedly wondering why it was
the grandparent which mattered. I suppose "parent" in this sense means
the "parent bus" as opposed to parent node? Or does it just fall out of
the fact that in the caller it is the parent which is passed through?

Can we call it something else, like "bus" or "node" or something else
appropriate?

> +        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.
> -     */
> +      */

Spurious w/s change.

Other that those two changes the patch looks good.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel