[PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Chuanhong Guo]

It's reported that current memory detection code occasionally detects
larger memory under some bootloaders.
Current memory detection code tests whether address space wraps around
on KSEG0, which is unreliable because it's cached.

Rewrite memory size detection to perform the same test on KSEG1 instead.
While at it, this patch also does the following two things:
1. use a fixed pattern instead of a random function pointer as the magic
   value.
2. add an additional memory write and a second comparison as part of the
   test to prevent possible smaller memory detection result due to
   leftover values in memory.

Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index d6efffd4dd20..12c8808e0dea 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -22,7 +22,9 @@
 
 #include "common.h"
 
-static void *detect_magic __initdata = detect_memory_region;
+#define MT7621_MEM_TEST_PATTERN         0xaa5555aa
+
+static u32 detect_magic __initdata;
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
@@ -58,24 +60,32 @@ phys_addr_t mips_cpc_default_phys_base(void)
 	panic("Cannot detect cpc address");
 }
 
+static bool __init mt7621_addr_wraparound_test(phys_addr_t size)
+{
+	void *dm = (void *)KSEG1ADDR(&detect_magic);
+
+	if (CPHYSADDR(dm + size) >= MT7621_LOWMEM_MAX_SIZE)
+		return true;
+	__raw_writel(MT7621_MEM_TEST_PATTERN, dm);
+	if (__raw_readl(dm) != __raw_readl(dm + size))
+		return false;
+	__raw_writel(!MT7621_MEM_TEST_PATTERN, dm);
+	return __raw_readl(dm) == __raw_readl(dm + size);
+}
+
 static void __init mt7621_memory_detect(void)
 {
-	void *dm = &detect_magic;
 	phys_addr_t size;
 
-	for (size = 32 * SZ_1M; size < 256 * SZ_1M; size <<= 1) {
-		if (!__builtin_memcmp(dm, dm + size, sizeof(detect_magic)))
-			break;
+	for (size = 32 * SZ_1M; size <= 256 * SZ_1M; size <<= 1) {
+		if (mt7621_addr_wraparound_test(size)) {
+			memblock_add(MT7621_LOWMEM_BASE, size);
+			return;
+		}
 	}
 
-	if ((size == 256 * SZ_1M) &&
-	    (CPHYSADDR(dm + size) < MT7621_LOWMEM_MAX_SIZE) &&
-	    __builtin_memcmp(dm, dm + size, sizeof(detect_magic))) {
-		memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
-		memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
-	} else {
-		memblock_add(MT7621_LOWMEM_BASE, size);
-	}
+	memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
+	memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
 }
 
 void __init ralink_of_remap(void)
-- 
2.34.1

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Ilya Lipnitskiy]

Hi Guo,

On Thu, Feb 10, 2022 at 4:14 PM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> It's reported that current memory detection code occasionally detects
> larger memory under some bootloaders.
> Current memory detection code tests whether address space wraps around
> on KSEG0, which is unreliable because it's cached.
>
> Rewrite memory size detection to perform the same test on KSEG1 instead.
> While at it, this patch also does the following two things:
> 1. use a fixed pattern instead of a random function pointer as the magic
>    value.
> 2. add an additional memory write and a second comparison as part of the
>    test to prevent possible smaller memory detection result due to
>    leftover values in memory.
>
> Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
> Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> index d6efffd4dd20..12c8808e0dea 100644
> --- a/arch/mips/ralink/mt7621.c
> +++ b/arch/mips/ralink/mt7621.c
> @@ -22,7 +22,9 @@
>
>  #include "common.h"
>
> -static void *detect_magic __initdata = detect_memory_region;
> +#define MT7621_MEM_TEST_PATTERN         0xaa5555aa
> +
> +static u32 detect_magic __initdata;
>
>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  {
> @@ -58,24 +60,32 @@ phys_addr_t mips_cpc_default_phys_base(void)
>         panic("Cannot detect cpc address");
>  }
>
> +static bool __init mt7621_addr_wraparound_test(phys_addr_t size)
> +{
> +       void *dm = (void *)KSEG1ADDR(&detect_magic);
> +
> +       if (CPHYSADDR(dm + size) >= MT7621_LOWMEM_MAX_SIZE)
> +               return true;
> +       __raw_writel(MT7621_MEM_TEST_PATTERN, dm);
> +       if (__raw_readl(dm) != __raw_readl(dm + size))
> +               return false;
> +       __raw_writel(!MT7621_MEM_TEST_PATTERN, dm);
> +       return __raw_readl(dm) == __raw_readl(dm + size);
> +}
> +
>  static void __init mt7621_memory_detect(void)
>  {
> -       void *dm = &detect_magic;
>         phys_addr_t size;
>
> -       for (size = 32 * SZ_1M; size < 256 * SZ_1M; size <<= 1) {
> -               if (!__builtin_memcmp(dm, dm + size, sizeof(detect_magic)))
> -                       break;
> +       for (size = 32 * SZ_1M; size <= 256 * SZ_1M; size <<= 1) {
> +               if (mt7621_addr_wraparound_test(size)) {
> +                       memblock_add(MT7621_LOWMEM_BASE, size);
> +                       return;
> +               }
>         }
>
> -       if ((size == 256 * SZ_1M) &&
> -           (CPHYSADDR(dm + size) < MT7621_LOWMEM_MAX_SIZE) &&
> -           __builtin_memcmp(dm, dm + size, sizeof(detect_magic))) {
> -               memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
> -               memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
> -       } else {
> -               memblock_add(MT7621_LOWMEM_BASE, size);
> -       }
> +       memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
> +       memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
>  }
>
>  void __init ralink_of_remap(void)
> --
> 2.34.1
>
Thanks for your change. I think this will also fix
https://lore.kernel.org/lkml/202201191557.OISJHNMi-lkp@intel.com/
since you are removing __builtin_memcmp usage.

Ilya

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Sergio Paracuellos]

Hi Guo,

On Fri, Feb 11, 2022 at 4:56 AM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> It's reported that current memory detection code occasionally detects
> larger memory under some bootloaders.
> Current memory detection code tests whether address space wraps around
> on KSEG0, which is unreliable because it's cached.
>
> Rewrite memory size detection to perform the same test on KSEG1 instead.
> While at it, this patch also does the following two things:
> 1. use a fixed pattern instead of a random function pointer as the magic
>    value.
> 2. add an additional memory write and a second comparison as part of the
>    test to prevent possible smaller memory detection result due to
>    leftover values in memory.
>
> Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
> Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

Thanks! Nothing seems to be broken after these changes, at least for
my boards :)

Tested-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

Best regards,
    Sergio Paracuellos

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Rui Salvaterra]

Hi, Guo,

On Fri, 11 Feb 2022 at 00:14, Chuanhong Guo <gch981213@gmail.com> wrote:
>
> It's reported that current memory detection code occasionally detects
> larger memory under some bootloaders.
> Current memory detection code tests whether address space wraps around
> on KSEG0, which is unreliable because it's cached.
>
> Rewrite memory size detection to perform the same test on KSEG1 instead.
> While at it, this patch also does the following two things:
> 1. use a fixed pattern instead of a random function pointer as the magic
>    value.
> 2. add an additional memory write and a second comparison as part of the
>    test to prevent possible smaller memory detection result due to
>    leftover values in memory.

[patch snipped]

No issues here, and it does seem to have fixed the RAM size detection
on my Redmi AC2100. It's always been a very sporadic failure here, so
I'll be keeping an eye on it to see if there are any further
misdetections. :)
In any case, feel free to add my

Tested-by: Rui Salvaterra <rsalvaterra@gmail.com>

Thanks,
Rui

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Chuanhong Guo]

 Hi!

On Fri, Feb 11, 2022 at 8:14 AM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> It's reported that current memory detection code occasionally detects
> larger memory under some bootloaders.
> Current memory detection code tests whether address space wraps around
> on KSEG0, which is unreliable because it's cached.
>
> Rewrite memory size detection to perform the same test on KSEG1 instead.
> While at it, this patch also does the following two things:
> 1. use a fixed pattern instead of a random function pointer as the magic
>    value.
> 2. add an additional memory write and a second comparison as part of the
>    test to prevent possible smaller memory detection result due to
>    leftover values in memory.
>
> Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")

I misplaced a bracket in this Fixes tag.

> Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> index d6efffd4dd20..12c8808e0dea 100644
> --- a/arch/mips/ralink/mt7621.c
> +++ b/arch/mips/ralink/mt7621.c
> @@ -22,7 +22,9 @@
>
>  #include "common.h"
>
> -static void *detect_magic __initdata = detect_memory_region;
> +#define MT7621_MEM_TEST_PATTERN         0xaa5555aa
> +
> +static u32 detect_magic __initdata;
>
>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  {
> @@ -58,24 +60,32 @@ phys_addr_t mips_cpc_default_phys_base(void)
>         panic("Cannot detect cpc address");
>  }
>
> +static bool __init mt7621_addr_wraparound_test(phys_addr_t size)
> +{
> +       void *dm = (void *)KSEG1ADDR(&detect_magic);
> +
> +       if (CPHYSADDR(dm + size) >= MT7621_LOWMEM_MAX_SIZE)
> +               return true;
> +       __raw_writel(MT7621_MEM_TEST_PATTERN, dm);
> +       if (__raw_readl(dm) != __raw_readl(dm + size))
> +               return false;
> +       __raw_writel(!MT7621_MEM_TEST_PATTERN, dm);

Someone on Github notified me that this second test pattern is incorrect.
I actually mean to use ~MT7621_MEM_TEST_PATTERN here.

I'll send a v2 fixing both issues.

--
Regards,
Chuanhong Guo

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Thomas Bogendoerfer]

On Fri, Feb 11, 2022 at 08:13:44AM +0800, Chuanhong Guo wrote:
> It's reported that current memory detection code occasionally detects
> larger memory under some bootloaders.
> Current memory detection code tests whether address space wraps around
> on KSEG0, which is unreliable because it's cached.
> 
> Rewrite memory size detection to perform the same test on KSEG1 instead.
> While at it, this patch also does the following two things:
> 1. use a fixed pattern instead of a random function pointer as the magic
>    value.
> 2. add an additional memory write and a second comparison as part of the
>    test to prevent possible smaller memory detection result due to
>    leftover values in memory.
> 
> Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
> Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
>  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)

applied to mips-fixes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Chuanhong Guo]

Hi!

On Thu, Feb 17, 2022 at 3:57 AM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Fri, Feb 11, 2022 at 08:13:44AM +0800, Chuanhong Guo wrote:
> > It's reported that current memory detection code occasionally detects
> > larger memory under some bootloaders.
> > Current memory detection code tests whether address space wraps around
> > on KSEG0, which is unreliable because it's cached.
> >
> > Rewrite memory size detection to perform the same test on KSEG1 instead.
> > While at it, this patch also does the following two things:
> > 1. use a fixed pattern instead of a random function pointer as the magic
> >    value.
> > 2. add an additional memory write and a second comparison as part of the
> >    test to prevent possible smaller memory detection result due to
> >    leftover values in memory.
> >
> > Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
> > Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> > Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> > ---
> >  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
> >  1 file changed, 23 insertions(+), 13 deletions(-)
>
> applied to mips-fixes.

Oops.

As I mentioned in a previous mail, this patch has two cosmetic problems:
1. misplaced bracket in commit message "Fixes" tag
2. incorrect second test pattern: I meant to flip all the bits in the
first pattern,
   but I used "!" instead of "~". Any value will work just fine but it
looks weird
   to construct a zero using !MT7621_MEM_TEST_PATTERN.

Should I send a second patch to fix this patch or send a v2 of the
original patch?

-- 
Regards,
Chuanhong Guo

Re: [PATCH] MIPS: ralink: mt7621: do memory detection on KSEG1

[Thomas Bogendoerfer]

On Thu, Feb 17, 2022 at 12:06:09PM +0800, Chuanhong Guo wrote:
> Hi!
> 
> On Thu, Feb 17, 2022 at 3:57 AM Thomas Bogendoerfer
> <tsbogend@alpha.franken.de> wrote:
> >
> > On Fri, Feb 11, 2022 at 08:13:44AM +0800, Chuanhong Guo wrote:
> > > It's reported that current memory detection code occasionally detects
> > > larger memory under some bootloaders.
> > > Current memory detection code tests whether address space wraps around
> > > on KSEG0, which is unreliable because it's cached.
> > >
> > > Rewrite memory size detection to perform the same test on KSEG1 instead.
> > > While at it, this patch also does the following two things:
> > > 1. use a fixed pattern instead of a random function pointer as the magic
> > >    value.
> > > 2. add an additional memory write and a second comparison as part of the
> > >    test to prevent possible smaller memory detection result due to
> > >    leftover values in memory.
> > >
> > > Fixes: 139c949f7f0a MIPS: ("ralink: mt7621: add memory detection support")
> > > Reported-by: Rui Salvaterra <rsalvaterra@gmail.com>
> > > Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> > > ---
> > >  arch/mips/ralink/mt7621.c | 36 +++++++++++++++++++++++-------------
> > >  1 file changed, 23 insertions(+), 13 deletions(-)
> >
> > applied to mips-fixes.
> 
> Oops.
> 
> As I mentioned in a previous mail, this patch has two cosmetic problems:
> 1. misplaced bracket in commit message "Fixes" tag
> 2. incorrect second test pattern: I meant to flip all the bits in the
> first pattern,
>    but I used "!" instead of "~". Any value will work just fine but it
> looks weird
>    to construct a zero using !MT7621_MEM_TEST_PATTERN.
> 
> Should I send a second patch to fix this patch or send a v2 of the
> original patch?

a second patch please.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]