[U-Boot] [PATCH] part_efi: fix protective_mbr struct allocation
The calloc() call was allocating space for the sizeof the struct pointer rather than for the struct contents.
Signed-off-by: Hector Palacios hector.palacios@digi.com --- disk/part_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5dfaf490c89a..7fabec059d7a 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) legacy_mbr *p_mbr;
/* Setup the Protective MBR */ - p_mbr = calloc(1, sizeof(p_mbr)); + p_mbr = calloc(1, sizeof(legacy_mbr)); if (p_mbr == NULL) { printf("%s: calloc failed!\n", __func__); return -1;
On Wed, Feb 12, 2014 at 12:40 PM, Hector Palacios hector.palacios@digi.com wrote:
The calloc() call was allocating space for the sizeof the struct pointer rather than for the struct contents.
Signed-off-by: Hector Palacios hector.palacios@digi.com
disk/part_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5dfaf490c89a..7fabec059d7a 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) legacy_mbr *p_mbr;
/* Setup the Protective MBR */
p_mbr = calloc(1, sizeof(p_mbr));
p_mbr = calloc(1, sizeof(legacy_mbr));
What about:
p_mbr = calloc(1, sizeof(*p_mbr)) ?
Regards,
Fabio Estevam
Hi Fabio,
On Wed, 12 Feb 2014 12:43:02 -0200, Fabio Estevam festevam@gmail.com wrote:
On Wed, Feb 12, 2014 at 12:40 PM, Hector Palacios hector.palacios@digi.com wrote:
The calloc() call was allocating space for the sizeof the struct pointer rather than for the struct contents.
Signed-off-by: Hector Palacios hector.palacios@digi.com
disk/part_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5dfaf490c89a..7fabec059d7a 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) legacy_mbr *p_mbr;
/* Setup the Protective MBR */
p_mbr = calloc(1, sizeof(p_mbr));
p_mbr = calloc(1, sizeof(legacy_mbr));What about:
p_mbr = calloc(1, sizeof(*p_mbr)) ?
I don't like the idea of setting p_mbr based on *p_mbr at a time where p_mbr is still undefined. I know that from a C standard perspective this is ok, but I'd rather simply not run any risk and pass sizeof the struct type, not a (non-existent) dereferenced 'value'.
Regards,
Fabio Estevam
Amicalement,
Hi Albert,
On Wed, Feb 12, 2014 at 2:33 PM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What about:
p_mbr = calloc(1, sizeof(*p_mbr)) ?
I don't like the idea of setting p_mbr based on *p_mbr at a time where p_mbr is still undefined. I know that from a C standard perspective this is ok, but I'd rather simply not run any risk and pass sizeof the struct type, not a (non-existent) dereferenced 'value'.
At least in kernel this is the preferred way.
According to Documentation/CodingStyle:
"Chapter 14: Allocating memory
The preferred form for passing a size of a struct is the following:
p = kmalloc(sizeof(*p), ...);
The alternative form where struct name is spelled out hurts readability and introduces an opportunity for a bug when the pointer variable type is changed but the corresponding sizeof that is passed to a memory allocator is not."
Regards,
Fabio Estevam
Hi Fabio,
On Wed, 12 Feb 2014 15:33:17 -0200, Fabio Estevam festevam@gmail.com wrote:
Hi Albert,
On Wed, Feb 12, 2014 at 2:33 PM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
What about:
p_mbr = calloc(1, sizeof(*p_mbr)) ?
I don't like the idea of setting p_mbr based on *p_mbr at a time where p_mbr is still undefined. I know that from a C standard perspective this is ok, but I'd rather simply not run any risk and pass sizeof the struct type, not a (non-existent) dereferenced 'value'.
At least in kernel this is the preferred way.
According to Documentation/CodingStyle:
"Chapter 14: Allocating memory
The preferred form for passing a size of a struct is the following:
p = kmalloc(sizeof(*p), ...);The alternative form where struct name is spelled out hurts readability and introduces an opportunity for a bug when the pointer variable type is changed but the corresponding sizeof that is passed to a memory allocator is not."
I still don't like it, but it makes sense indeed.
Regards,
Fabio Estevam
Amicalement,
Hi Hector,
The calloc() call was allocating space for the sizeof the struct pointer rather than for the struct contents.
Signed-off-by: Hector Palacios hector.palacios@digi.com
disk/part_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5dfaf490c89a..7fabec059d7a 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) legacy_mbr *p_mbr;
/* Setup the Protective MBR */
- p_mbr = calloc(1, sizeof(p_mbr));
- p_mbr = calloc(1, sizeof(legacy_mbr));
Thanks for spotting the error. _Damn_
However, this is not enough.
Since this buffer is passed to mmc for writing (and some targets may use cache) the legacy_mbr shall be defined as:
ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, sizeof(legacy_mbr)); memset(p_mbr, 0, sizeof(legacy_mbr));
Would you like to prepare v2 of this patch or shall I prepare the fix?
if (p_mbr == NULL) { printf("%s: calloc failed!\n", __func__); return -1;
Hi Lukasz,
On 02/12/2014 04:56 PM, Lukasz Majewski wrote:
Hi Hector,
The calloc() call was allocating space for the sizeof the struct pointer rather than for the struct contents.
Signed-off-by: Hector Palacios hector.palacios@digi.com
disk/part_efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5dfaf490c89a..7fabec059d7a 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) legacy_mbr *p_mbr;
/* Setup the Protective MBR */
- p_mbr = calloc(1, sizeof(p_mbr));
- p_mbr = calloc(1, sizeof(legacy_mbr));
Thanks for spotting the error. _Damn_
However, this is not enough.
Since this buffer is passed to mmc for writing (and some targets may use cache) the legacy_mbr shall be defined as:
ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, sizeof(legacy_mbr)); memset(p_mbr, 0, sizeof(legacy_mbr));
Unfortunately this is causing unaligned access in my i.MX6. I'm specifically passing the -mno-unaligned-access when building this file so I guess it has to do with the macro and the packed structure.
Would you like to prepare v2 of this patch or shall I prepare the fix?
if (p_mbr == NULL) { printf("%s: calloc failed!\n", __func__); return -1;
Hi Hector,
On Wed, 12 Feb 2014 17:24:26 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
Unfortunately this is causing unaligned access in my i.MX6. I'm specifically passing the -mno-unaligned-access when building this file so I guess it has to do with the macro and the packed structure.
I don't think this is due to packed structures, because -mno-unaligned-access tells the compiler that unaligned fields in packed structures should be accesses by breaking the unaligned access into smaller aligned ones, so this would not cause an alignment abort.
What can cause an alignment abort despite -mno-unaligned-access is dereferencing a badly aligned pointer, and this is probably what happens here.
Amicalement,
Hi Albert,
On 02/12/2014 05:31 PM, Albert ARIBAUD wrote:
Hi Hector,
On Wed, 12 Feb 2014 17:24:26 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
Unfortunately this is causing unaligned access in my i.MX6. I'm specifically passing the -mno-unaligned-access when building this file so I guess it has to do with the macro and the packed structure.
I don't think this is due to packed structures, because -mno-unaligned-access tells the compiler that unaligned fields in packed structures should be accesses by breaking the unaligned access into smaller aligned ones, so this would not cause an alignment abort.
What can cause an alignment abort despite -mno-unaligned-access is dereferencing a badly aligned pointer, and this is probably what happens here.
Oh, the macro ALLOC_CACHE_ALIGN_BUFFER is returning a correctly aligned value for the pointer: 0x4FD33BA0 The problem is we don't have to free() the pointer anymore at the end of the function.
I'll repost the patch
Hi Hector,
Hi Albert,
On 02/12/2014 05:31 PM, Albert ARIBAUD wrote:
Hi Hector,
On Wed, 12 Feb 2014 17:24:26 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
Unfortunately this is causing unaligned access in my i.MX6. I'm specifically passing the -mno-unaligned-access when building this file so I guess it has to do with the macro and the packed structure.
I don't think this is due to packed structures, because -mno-unaligned-access tells the compiler that unaligned fields in packed structures should be accesses by breaking the unaligned access into smaller aligned ones, so this would not cause an alignment abort.
What can cause an alignment abort despite -mno-unaligned-access is dereferencing a badly aligned pointer, and this is probably what happens here.
Oh, the macro ALLOC_CACHE_ALIGN_BUFFER is returning a correctly aligned value for the pointer: 0x4FD33BA0 The problem is we don't have to free() the pointer anymore at the end of the function.
I'll repost the patch
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Best regards, Lukasz Majewski
Hi Lukasz,
On Wed, 12 Feb 2014 21:45:41 +0100, Lukasz Majewski l.majewski@majess.pl wrote:
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Neither V1 nor V2 have disappeared from patchwork. A "filter" on the subject, selecting "any" state and "both" archived and non-archived patches will return both:
V1: http://patchwork.ozlabs.org/patch/282753/ (New) V2: http://patchwork.ozlabs.org/patch/314717/ (Deferred)
(I think that the patchwork web interface does not make it possible at all to remove a patch anyway)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Best regards, Lukasz Majewski
Amicalement,
Hi Albert, Tom
Hi Lukasz,
On Wed, 12 Feb 2014 21:45:41 +0100, Lukasz Majewski l.majewski@majess.pl wrote:
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Neither V1 nor V2 have disappeared from patchwork. A "filter" on the subject, selecting "any" state and "both" archived and non-archived patches will return both:
V1: http://patchwork.ozlabs.org/patch/282753/ (New) V2: http://patchwork.ozlabs.org/patch/314717/ (Deferred)
(I think that the patchwork web interface does not make it possible at all to remove a patch anyway)
Thanks for pointing out. Now it is perfectly visible :-)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Best regards, Lukasz Majewski
Amicalement,
Hi Lukasz,
On Wed, 19 Feb 2014 09:19:04 +0100, Lukasz Majewski l.majewski@samsung.com wrote:
Hi Albert, Tom
Hi Lukasz,
On Wed, 12 Feb 2014 21:45:41 +0100, Lukasz Majewski l.majewski@majess.pl wrote:
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Neither V1 nor V2 have disappeared from patchwork. A "filter" on the subject, selecting "any" state and "both" archived and non-archived patches will return both:
V1: http://patchwork.ozlabs.org/patch/282753/ (New) V2: http://patchwork.ozlabs.org/patch/314717/ (Deferred)
(I think that the patchwork web interface does not make it possible at all to remove a patch anyway)
Thanks for pointing out. Now it is perfectly visible :-)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Amicalement,
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
Thanks for pointing out. Now it is perfectly visible :-)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
Amicalement,
On 02/19/2014 11:16 AM, Albert ARIBAUD wrote:
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
Thanks for pointing out. Now it is perfectly visible :-)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
I tested Piotr's patch on i.MX6 (armv7) custom board and it is working fine without the -mno-unaligned-access flag.
Tested-by: Hector Palacios hector.palacios@digi.com
Hi Hector,
On Wed, 19 Feb 2014 13:52:07 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
On 02/19/2014 11:16 AM, Albert ARIBAUD wrote:
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
Thanks for pointing out. Now it is perfectly visible :-)
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
I tested Piotr's patch on i.MX6 (armv7) custom board and it is working fine without the -mno-unaligned-access flag.
Tested-by: Hector Palacios hector.palacios@digi.com
You've just Tested-By-ed your own patch, I think.
... but I am the one to blame, and should not have discussed Piotr's patch in this thread.
Amicalement,
Hi Albert,
Hi Hector,
On Wed, 19 Feb 2014 13:52:07 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
On 02/19/2014 11:16 AM, Albert ARIBAUD wrote:
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
Thanks for pointing out. Now it is perfectly visible :-)
> Inclusion of v2 has been postponed since there was a > discussion if we shall allow unaligned access > (-mno-unaligned-access flag) at armv7 (after patches posted > by Tom). > > As fair as I can tell, we will keep the current approach so, > I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
I tested Piotr's patch on i.MX6 (armv7) custom board and it is working fine without the -mno-unaligned-access flag.
Tested-by: Hector Palacios hector.palacios@digi.com
You've just Tested-By-ed your own patch, I think.
Nope.
Patch prepared by Piotr is orthogonal to the one prepared by Hector.
Hector has spotted other mistake at GPT code (made by me). Fix for it has been posted a few days ago:
http://patchwork.ozlabs.org/patch/319914/
... but I am the one to blame, and should not have discussed Piotr's patch in this thread.
Amicalement,
Hi Lukasz,
On Wed, 19 Feb 2014 15:25:37 +0100, Lukasz Majewski l.majewski@samsung.com wrote:
Hi Albert,
Hi Hector,
On Wed, 19 Feb 2014 13:52:07 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
On 02/19/2014 11:16 AM, Albert ARIBAUD wrote:
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
Thanks for pointing out. Now it is perfectly visible :-)
> >> Inclusion of v2 has been postponed since there was a >> discussion if we shall allow unaligned access >> (-mno-unaligned-access flag) at armv7 (after patches posted >> by Tom). >> >> As fair as I can tell, we will keep the current approach so, >> I think that Tom will be willing to pull this patch (v2) now. > > Agreed, but then we should make sure no one has comments on V2 > that they might have withheld due to the initial rejection of > V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
I tested Piotr's patch on i.MX6 (armv7) custom board and it is working fine without the -mno-unaligned-access flag.
Tested-by: Hector Palacios hector.palacios@digi.com
You've just Tested-By-ed your own patch, I think.
Nope.
Patch prepared by Piotr is orthogonal to the one prepared by Hector.
Hector has spotted other mistake at GPT code (made by me). Fix for it has been posted a few days ago:
I did not comment on the relationship between patches, I only commented on the fact that Hector said he has tested Piotr's patch but sent his Tested-by on his own patch thread, not on Piotr's. To verify this, look up
http://patchwork.ozlabs.org/patch/319649/
... which is Hector's patchwork entry and has his own Tested-by, and
http://patchwork.ozlabs.org/patch/314717/
... which is Piotr's patch and does not have Hector's (or anyone's) Tested-by.
Amicalement,
Hi Albert,
Hi Lukasz,
On Wed, 19 Feb 2014 15:25:37 +0100, Lukasz Majewski l.majewski@samsung.com wrote:
Hi Albert,
Hi Hector,
On Wed, 19 Feb 2014 13:52:07 +0100, "Palacios, Hector" Hector.Palacios@digi.com wrote:
On 02/19/2014 11:16 AM, Albert ARIBAUD wrote:
On Wed, 19 Feb 2014 11:08:03 +0100, Albert ARIBAUD
> Thanks for pointing out. Now it is perfectly visible :-) > >> >>> Inclusion of v2 has been postponed since there was a >>> discussion if we shall allow unaligned access >>> (-mno-unaligned-access flag) at armv7 (after patches >>> posted by Tom). >>> >>> As fair as I can tell, we will keep the current approach >>> so, I think that Tom will be willing to pull this patch >>> (v2) now. >> >> Agreed, but then we should make sure no one has comments >> on V2 that they might have withheld due to the initial >> rejection of V2. > > Any comments? > > This patch do fix unaligned access problem on Trats2 > (Exynos4412), when we restore/create GPT, so I would like to > know if there are any new inquires regarding this patch.
Does not seem to be, so I will apply V2.
Correction: I would like it to be applied as per current ARM alignment policy, but this patch is not ARM per se and is in Tom's hands.
Tom, can you apply http://patchwork.ozlabs.org/patch/314717/ ? This would by no means close the discussion I opened, and in the event of a policy change, the patch could always be reverted; meanwhile, it matches our current policy.
I tested Piotr's patch on i.MX6 (armv7) custom board and it is working fine without the -mno-unaligned-access flag.
Tested-by: Hector Palacios hector.palacios@digi.com
You've just Tested-By-ed your own patch, I think.
Nope.
Patch prepared by Piotr is orthogonal to the one prepared by Hector.
Hector has spotted other mistake at GPT code (made by me). Fix for it has been posted a few days ago:
I did not comment on the relationship between patches, I only commented on the fact that Hector said he has tested Piotr's patch but sent his Tested-by on his own patch thread, not on Piotr's. To verify this, look up
http://patchwork.ozlabs.org/patch/319649/
... which is Hector's patchwork entry and has his own Tested-by, and
http://patchwork.ozlabs.org/patch/314717/
... which is Piotr's patch and does not have Hector's (or anyone's) Tested-by.
Hmm. I've misunderstood you a bit.
Anyway thanks for clarification :-).
Amicalement,
On Wed, Feb 19, 2014 at 09:19:04AM +0100, Lukasz Majewski wrote:
Hi Albert, Tom
Hi Lukasz,
On Wed, 12 Feb 2014 21:45:41 +0100, Lukasz Majewski l.majewski@majess.pl wrote:
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Neither V1 nor V2 have disappeared from patchwork. A "filter" on the subject, selecting "any" state and "both" archived and non-archived patches will return both:
V1: http://patchwork.ozlabs.org/patch/282753/ (New) V2: http://patchwork.ozlabs.org/patch/314717/ (Deferred)
(I think that the patchwork web interface does not make it possible at all to remove a patch anyway)
Thanks for pointing out. Now it is perfectly visible :-)
I've also un-deferred, for now, the patch as well.
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Can you give me some steps on how to hit this bug? I believe it's a bug and I believe we need to fix it, I just want to investigate a few things while we've got a trigger case right now. Thanks!
Hi Tom,
On Wed, Feb 19, 2014 at 09:19:04AM +0100, Lukasz Majewski wrote:
Hi Albert, Tom
Hi Lukasz,
On Wed, 12 Feb 2014 21:45:41 +0100, Lukasz Majewski l.majewski@majess.pl wrote:
I think, that the patch for fixing the unaligned access in this function has already been posted by Piotr Wilczek. We have experienced similar issues with Samsung's Exynos4 based targets.
[PATCH V2] disk:efi: avoid unaligned access on efi partition
Despite its disappearance from patchwork it shall be available at mailing list archive.
The v1 can be found at the following link: http://patchwork.ozlabs.org/patch/282753/
Neither V1 nor V2 have disappeared from patchwork. A "filter" on the subject, selecting "any" state and "both" archived and non-archived patches will return both:
V1: http://patchwork.ozlabs.org/patch/282753/ (New) V2: http://patchwork.ozlabs.org/patch/314717/ (Deferred)
(I think that the patchwork web interface does not make it possible at all to remove a patch anyway)
Thanks for pointing out. Now it is perfectly visible :-)
I've also un-deferred, for now, the patch as well.
Inclusion of v2 has been postponed since there was a discussion if we shall allow unaligned access (-mno-unaligned-access flag) at armv7 (after patches posted by Tom).
As fair as I can tell, we will keep the current approach so, I think that Tom will be willing to pull this patch (v2) now.
Agreed, but then we should make sure no one has comments on V2 that they might have withheld due to the initial rejection of V2.
Any comments?
This patch do fix unaligned access problem on Trats2 (Exynos4412), when we restore/create GPT, so I would like to know if there are any new inquires regarding this patch.
Can you give me some steps on how to hit this bug? I believe it's a bug and I believe we need to fix it, I just want to investigate a few things while we've got a trigger case right now. Thanks!
The easiest way is to type:
gpt write mmc 0 $partitions
before that you shall define several uuid's.
For reference please look into the Trats2/Trats code and ./doc/README.gpt
As a side note - I'm using following toolchain: CROSS_COMPILE=/opt/eldk-5.4/armv7a/sysroots/i686-eldk-linux/usr/bin/armv7a-vfp-neon-linux-gnueabi/arm-linux-gnueabi-
It is 4.7.2 from denx.de.
participants (7)
-
Albert ARIBAUD -
Fabio Estevam -
Hector Palacios -
Lukasz Majewski -
Lukasz Majewski -
Palacios, Hector
-
Tom Rini