All of lore.kernel.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] arm64: regard FDT_SW_MAGIC as good fdt magic
Date: Thu, 1 Sep 2016 12:21:13 +0100	[thread overview]
Message-ID: <20160901112113.GA4220@leverpostej> (raw)
In-Reply-To: <a6a8826c-3006-ae43-ef4c-8049ef93cd9f@zoho.com>

On Thu, Sep 01, 2016 at 06:58:29PM +0800, zijun_hu wrote:
> From: zijun_hu <zijun_hu@htc.com>
> 
> regard FDT_SW_MAGIC as good fdt magic during mapping fdt area
> see fdt_check_header() for details

It looks like we should only see FDT_SW_MAGIC for a FDT that was in the
process of being created, but was not finished. So I'm somewhat confused
as to why fdt_check_header() would allow this.

Neither ePAPR nor the new devicetree spec define FDT_SW_MAGIC. They both
only define 0xd00dfeed as a valid magic value. In libfdt, FDT_SW_MAGIC
is an internal constant, and it looks like fdt_check_header() simply
accepts this for convenience within libfdt.

Given all of that, it looks like the kernel should *not* accept
FDT_SW_MAGIC in any case.

Why do you think this is necessary? Have you seen a problem in practice?

> --- a/scripts/dtc/libfdt/fdt.h
> +++ b/scripts/dtc/libfdt/fdt.h
> @@ -92,7 +92,8 @@ struct fdt_property {
>  
>  #endif /* !__ASSEMBLY */
>  
> -#define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
> +/* 4: version, 4: total size */
> +#define FDT_MAGIC	((fdt32_t)0xd00dfeed)
>  #define FDT_TAGSIZE	sizeof(fdt32_t)
>  
>  #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 59ca33976e56..6998f9249183 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -54,6 +54,8 @@
>  #include "libfdt_env.h"
>  #include "fdt.h"
>  
> +#define FDT_SW_MAGIC           (~FDT_MAGIC)
> +
>  #define FDT_FIRST_SUPPORTED_VERSION	0x10
>  #define FDT_LAST_SUPPORTED_VERSION	0x11
>  
> diff --git a/scripts/dtc/libfdt/libfdt_internal.h b/scripts/dtc/libfdt/libfdt_internal.h
> index 02cfa6fb612d..f4efde0119f2 100644
> --- a/scripts/dtc/libfdt/libfdt_internal.h
> +++ b/scripts/dtc/libfdt/libfdt_internal.h
> @@ -90,6 +90,4 @@ static inline struct fdt_reserve_entry *_fdt_mem_rsv_w(void *fdt, int n)
>  	return (void *)(uintptr_t)_fdt_mem_rsv(fdt, n);
>  }
>  
> -#define FDT_SW_MAGIC		(~FDT_MAGIC)
> -
>  #endif /* _LIBFDT_INTERNAL_H */

Regardless of the above, changes to libfdt must happen in the upstream
libfdt codebase first.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
To: zijun_hu <zijun_hu-ytc+IHgoah0@public.gmane.org>
Cc: catalin.marinas-5wv7dgnIgG8@public.gmane.org,
	will.deacon-5wv7dgnIgG8@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	zijun_hu-TYsT/PRPW1c@public.gmane.org
Subject: Re: [PATCH 2/2] arm64: regard FDT_SW_MAGIC as good fdt magic
Date: Thu, 1 Sep 2016 12:21:13 +0100	[thread overview]
Message-ID: <20160901112113.GA4220@leverpostej> (raw)
In-Reply-To: <a6a8826c-3006-ae43-ef4c-8049ef93cd9f-ytc+IHgoah0@public.gmane.org>

On Thu, Sep 01, 2016 at 06:58:29PM +0800, zijun_hu wrote:
> From: zijun_hu <zijun_hu-TYsT/PRPW1c@public.gmane.org>
> 
> regard FDT_SW_MAGIC as good fdt magic during mapping fdt area
> see fdt_check_header() for details

It looks like we should only see FDT_SW_MAGIC for a FDT that was in the
process of being created, but was not finished. So I'm somewhat confused
as to why fdt_check_header() would allow this.

Neither ePAPR nor the new devicetree spec define FDT_SW_MAGIC. They both
only define 0xd00dfeed as a valid magic value. In libfdt, FDT_SW_MAGIC
is an internal constant, and it looks like fdt_check_header() simply
accepts this for convenience within libfdt.

Given all of that, it looks like the kernel should *not* accept
FDT_SW_MAGIC in any case.

Why do you think this is necessary? Have you seen a problem in practice?

> --- a/scripts/dtc/libfdt/fdt.h
> +++ b/scripts/dtc/libfdt/fdt.h
> @@ -92,7 +92,8 @@ struct fdt_property {
>  
>  #endif /* !__ASSEMBLY */
>  
> -#define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
> +/* 4: version, 4: total size */
> +#define FDT_MAGIC	((fdt32_t)0xd00dfeed)
>  #define FDT_TAGSIZE	sizeof(fdt32_t)
>  
>  #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 59ca33976e56..6998f9249183 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -54,6 +54,8 @@
>  #include "libfdt_env.h"
>  #include "fdt.h"
>  
> +#define FDT_SW_MAGIC           (~FDT_MAGIC)
> +
>  #define FDT_FIRST_SUPPORTED_VERSION	0x10
>  #define FDT_LAST_SUPPORTED_VERSION	0x11
>  
> diff --git a/scripts/dtc/libfdt/libfdt_internal.h b/scripts/dtc/libfdt/libfdt_internal.h
> index 02cfa6fb612d..f4efde0119f2 100644
> --- a/scripts/dtc/libfdt/libfdt_internal.h
> +++ b/scripts/dtc/libfdt/libfdt_internal.h
> @@ -90,6 +90,4 @@ static inline struct fdt_reserve_entry *_fdt_mem_rsv_w(void *fdt, int n)
>  	return (void *)(uintptr_t)_fdt_mem_rsv(fdt, n);
>  }
>  
> -#define FDT_SW_MAGIC		(~FDT_MAGIC)
> -
>  #endif /* _LIBFDT_INTERNAL_H */

Regardless of the above, changes to libfdt must happen in the upstream
libfdt codebase first.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: zijun_hu <zijun_hu@zoho.com>
Cc: catalin.marinas@arm.com, will.deacon@arm.com, robh+dt@kernel.org,
	frowand.list@gmail.com, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	zijun_hu@htc.com
Subject: Re: [PATCH 2/2] arm64: regard FDT_SW_MAGIC as good fdt magic
Date: Thu, 1 Sep 2016 12:21:13 +0100	[thread overview]
Message-ID: <20160901112113.GA4220@leverpostej> (raw)
In-Reply-To: <a6a8826c-3006-ae43-ef4c-8049ef93cd9f@zoho.com>

On Thu, Sep 01, 2016 at 06:58:29PM +0800, zijun_hu wrote:
> From: zijun_hu <zijun_hu@htc.com>
> 
> regard FDT_SW_MAGIC as good fdt magic during mapping fdt area
> see fdt_check_header() for details

It looks like we should only see FDT_SW_MAGIC for a FDT that was in the
process of being created, but was not finished. So I'm somewhat confused
as to why fdt_check_header() would allow this.

Neither ePAPR nor the new devicetree spec define FDT_SW_MAGIC. They both
only define 0xd00dfeed as a valid magic value. In libfdt, FDT_SW_MAGIC
is an internal constant, and it looks like fdt_check_header() simply
accepts this for convenience within libfdt.

Given all of that, it looks like the kernel should *not* accept
FDT_SW_MAGIC in any case.

Why do you think this is necessary? Have you seen a problem in practice?

> --- a/scripts/dtc/libfdt/fdt.h
> +++ b/scripts/dtc/libfdt/fdt.h
> @@ -92,7 +92,8 @@ struct fdt_property {
>  
>  #endif /* !__ASSEMBLY */
>  
> -#define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
> +/* 4: version, 4: total size */
> +#define FDT_MAGIC	((fdt32_t)0xd00dfeed)
>  #define FDT_TAGSIZE	sizeof(fdt32_t)
>  
>  #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 59ca33976e56..6998f9249183 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -54,6 +54,8 @@
>  #include "libfdt_env.h"
>  #include "fdt.h"
>  
> +#define FDT_SW_MAGIC           (~FDT_MAGIC)
> +
>  #define FDT_FIRST_SUPPORTED_VERSION	0x10
>  #define FDT_LAST_SUPPORTED_VERSION	0x11
>  
> diff --git a/scripts/dtc/libfdt/libfdt_internal.h b/scripts/dtc/libfdt/libfdt_internal.h
> index 02cfa6fb612d..f4efde0119f2 100644
> --- a/scripts/dtc/libfdt/libfdt_internal.h
> +++ b/scripts/dtc/libfdt/libfdt_internal.h
> @@ -90,6 +90,4 @@ static inline struct fdt_reserve_entry *_fdt_mem_rsv_w(void *fdt, int n)
>  	return (void *)(uintptr_t)_fdt_mem_rsv(fdt, n);
>  }
>  
> -#define FDT_SW_MAGIC		(~FDT_MAGIC)
> -
>  #endif /* _LIBFDT_INTERNAL_H */

Regardless of the above, changes to libfdt must happen in the upstream
libfdt codebase first.

Thanks,
Mark.

  reply	other threads:[~2016-09-01 11:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-01 10:58 [PATCH 2/2] arm64: regard FDT_SW_MAGIC as good fdt magic zijun_hu
2016-09-01 10:58 ` zijun_hu
2016-09-01 10:58 ` zijun_hu
2016-09-01 11:21 ` Mark Rutland [this message]
2016-09-01 11:21   ` Mark Rutland
2016-09-01 11:21   ` Mark Rutland
2016-09-01 16:03   ` zijun_hu
2016-09-01 16:03     ` zijun_hu
2016-09-01 16:45     ` Mark Rutland
2016-09-01 16:45       ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160901112113.GA4220@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.