All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS: Fixed __debug_virt_addr_valid()
@ 2022-07-07 21:52 Florian Fainelli
  2022-07-08 11:58 ` Serge Semin
  2022-07-11  8:38 ` Thomas Bogendoerfer
  0 siblings, 2 replies; 10+ messages in thread
From: Florian Fainelli @ 2022-07-07 21:52 UTC (permalink / raw)
  To: linux-mips
  Cc: gerg, fancer.lancer, Florian Fainelli, Thomas Bogendoerfer, open list

It is permissible for kernel code to call virt_to_phys() against virtual
addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
types. Add a final condition that ensures that the virtual address is
below KSEG2.

Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/mm/physaddr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
index a1ced5e44951..a82f8f57a652 100644
--- a/arch/mips/mm/physaddr.c
+++ b/arch/mips/mm/physaddr.c
@@ -5,6 +5,7 @@
 #include <linux/mmdebug.h>
 #include <linux/mm.h>
 
+#include <asm/addrspace.h>
 #include <asm/sections.h>
 #include <asm/io.h>
 #include <asm/page.h>
@@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
 	if (x == MAX_DMA_ADDRESS)
 		return true;
 
-	return false;
+	return KSEGX(x) < KSEG2;
 }
 
 phys_addr_t __virt_to_phys(volatile const void *x)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-07 21:52 [PATCH] MIPS: Fixed __debug_virt_addr_valid() Florian Fainelli
@ 2022-07-08 11:58 ` Serge Semin
  2022-07-12 16:28   ` Florian Fainelli
  2022-07-11  8:38 ` Thomas Bogendoerfer
  1 sibling, 1 reply; 10+ messages in thread
From: Serge Semin @ 2022-07-08 11:58 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-mips, gerg, Thomas Bogendoerfer, open list

On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> It is permissible for kernel code to call virt_to_phys() against virtual
> addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> types. Add a final condition that ensures that the virtual address is
> below KSEG2.
> 
> Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/mips/mm/physaddr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> index a1ced5e44951..a82f8f57a652 100644
> --- a/arch/mips/mm/physaddr.c
> +++ b/arch/mips/mm/physaddr.c
> @@ -5,6 +5,7 @@
>  #include <linux/mmdebug.h>
>  #include <linux/mm.h>
>  
> +#include <asm/addrspace.h>
>  #include <asm/sections.h>
>  #include <asm/io.h>
>  #include <asm/page.h>
> @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
>  	if (x == MAX_DMA_ADDRESS)
>  		return true;
>  

> -	return false;
> +	return KSEGX(x) < KSEG2;

With this do we really need the high_memory-based conditionals in this
method?

If the line above is the only way to take the uncached segment into
account then can we reduce the whole method to:
static inline bool __debug_virt_addr_valid {
	return x >= PAGE_OFFSET && KSEGX(x) < KSEG2;
}
?

Though this still may be invalid for EVA systems, like malta (see
arch/mips/include/asm/mach-malta/spaces.h).

Note AFAICS if EVA is enabled, highmem is implied to be disabled (see
the CPU_MIPS32_3_5_EVA config utilization and HIGHMEM config
dependencies). Thus all the memory is supposed to be linearly mapped
in that case.

-Sergey

>  }
>  
>  phys_addr_t __virt_to_phys(volatile const void *x)
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-07 21:52 [PATCH] MIPS: Fixed __debug_virt_addr_valid() Florian Fainelli
  2022-07-08 11:58 ` Serge Semin
@ 2022-07-11  8:38 ` Thomas Bogendoerfer
  2022-07-11 10:40   ` Serge Semin
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2022-07-11  8:38 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-mips, gerg, fancer.lancer, open list

On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> It is permissible for kernel code to call virt_to_phys() against virtual
> addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> types. Add a final condition that ensures that the virtual address is
> below KSEG2.
> 
> Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/mips/mm/physaddr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> index a1ced5e44951..a82f8f57a652 100644
> --- a/arch/mips/mm/physaddr.c
> +++ b/arch/mips/mm/physaddr.c
> @@ -5,6 +5,7 @@
>  #include <linux/mmdebug.h>
>  #include <linux/mm.h>
>  
> +#include <asm/addrspace.h>
>  #include <asm/sections.h>
>  #include <asm/io.h>
>  #include <asm/page.h>
> @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
>  	if (x == MAX_DMA_ADDRESS)
>  		return true;
>  
> -	return false;
> +	return KSEGX(x) < KSEG2;
>  }
>  
>  phys_addr_t __virt_to_phys(volatile const void *x)
> -- 
> 2.25.1

applied to mips-next.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-11  8:38 ` Thomas Bogendoerfer
@ 2022-07-11 10:40   ` Serge Semin
  2022-07-11 11:27     ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Serge Semin @ 2022-07-11 10:40 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: Florian Fainelli, linux-mips, gerg, open list

On Mon, Jul 11, 2022 at 10:38:48AM +0200, Thomas Bogendoerfer wrote:
> On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> > It is permissible for kernel code to call virt_to_phys() against virtual
> > addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> > types. Add a final condition that ensures that the virtual address is
> > below KSEG2.
> > 
> > Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> >  arch/mips/mm/physaddr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> > index a1ced5e44951..a82f8f57a652 100644
> > --- a/arch/mips/mm/physaddr.c
> > +++ b/arch/mips/mm/physaddr.c
> > @@ -5,6 +5,7 @@
> >  #include <linux/mmdebug.h>
> >  #include <linux/mm.h>
> >  
> > +#include <asm/addrspace.h>
> >  #include <asm/sections.h>
> >  #include <asm/io.h>
> >  #include <asm/page.h>
> > @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
> >  	if (x == MAX_DMA_ADDRESS)
> >  		return true;
> >  
> > -	return false;
> > +	return KSEGX(x) < KSEG2;
> >  }
> >  
> >  phys_addr_t __virt_to_phys(volatile const void *x)
> > -- 
> > 2.25.1
> 

> applied to mips-next.

Are you sure it was ready to be applied?
Link: https://lore.kernel.org/linux-mips/20220708115851.ejsooiilxcopkoei@mobilestation/

-Sergey

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-11 10:40   ` Serge Semin
@ 2022-07-11 11:27     ` Thomas Bogendoerfer
  2022-07-11 11:29       ` Serge Semin
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Bogendoerfer @ 2022-07-11 11:27 UTC (permalink / raw)
  To: Serge Semin; +Cc: Florian Fainelli, linux-mips, gerg, open list

On Mon, Jul 11, 2022 at 01:40:52PM +0300, Serge Semin wrote:
> On Mon, Jul 11, 2022 at 10:38:48AM +0200, Thomas Bogendoerfer wrote:
> > On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> > > It is permissible for kernel code to call virt_to_phys() against virtual
> > > addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> > > types. Add a final condition that ensures that the virtual address is
> > > below KSEG2.
> > > 
> > > Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > >  arch/mips/mm/physaddr.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> > > index a1ced5e44951..a82f8f57a652 100644
> > > --- a/arch/mips/mm/physaddr.c
> > > +++ b/arch/mips/mm/physaddr.c
> > > @@ -5,6 +5,7 @@
> > >  #include <linux/mmdebug.h>
> > >  #include <linux/mm.h>
> > >  
> > > +#include <asm/addrspace.h>
> > >  #include <asm/sections.h>
> > >  #include <asm/io.h>
> > >  #include <asm/page.h>
> > > @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
> > >  	if (x == MAX_DMA_ADDRESS)
> > >  		return true;
> > >  
> > > -	return false;
> > > +	return KSEGX(x) < KSEG2;
> > >  }
> > >  
> > >  phys_addr_t __virt_to_phys(volatile const void *x)
> > > -- 
> > > 2.25.1
> > 
> 
> > applied to mips-next.
> 
> Are you sure it was ready to be applied?
> Link: https://lore.kernel.org/linux-mips/20220708115851.ejsooiilxcopkoei@mobilestation/

your comment sounded like optimizing, which can be done later on, so
I assumed it ready.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-11 11:27     ` Thomas Bogendoerfer
@ 2022-07-11 11:29       ` Serge Semin
  2022-07-12  0:02         ` Florian Fainelli
  0 siblings, 1 reply; 10+ messages in thread
From: Serge Semin @ 2022-07-11 11:29 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: Florian Fainelli, linux-mips, gerg, open list

On Mon, Jul 11, 2022 at 01:27:40PM +0200, Thomas Bogendoerfer wrote:
> On Mon, Jul 11, 2022 at 01:40:52PM +0300, Serge Semin wrote:
> > On Mon, Jul 11, 2022 at 10:38:48AM +0200, Thomas Bogendoerfer wrote:
> > > On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> > > > It is permissible for kernel code to call virt_to_phys() against virtual
> > > > addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> > > > types. Add a final condition that ensures that the virtual address is
> > > > below KSEG2.
> > > > 
> > > > Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> > > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > > > ---
> > > >  arch/mips/mm/physaddr.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> > > > index a1ced5e44951..a82f8f57a652 100644
> > > > --- a/arch/mips/mm/physaddr.c
> > > > +++ b/arch/mips/mm/physaddr.c
> > > > @@ -5,6 +5,7 @@
> > > >  #include <linux/mmdebug.h>
> > > >  #include <linux/mm.h>
> > > >  
> > > > +#include <asm/addrspace.h>
> > > >  #include <asm/sections.h>
> > > >  #include <asm/io.h>
> > > >  #include <asm/page.h>
> > > > @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
> > > >  	if (x == MAX_DMA_ADDRESS)
> > > >  		return true;
> > > >  
> > > > -	return false;
> > > > +	return KSEGX(x) < KSEG2;
> > > >  }
> > > >  
> > > >  phys_addr_t __virt_to_phys(volatile const void *x)
> > > > -- 
> > > > 2.25.1
> > > 
> > 
> > > applied to mips-next.
> > 
> > Are you sure it was ready to be applied?
> > Link: https://lore.kernel.org/linux-mips/20220708115851.ejsooiilxcopkoei@mobilestation/
> 

> your comment sounded like optimizing, which can be done later on, so
> I assumed it ready.

What about Malta and EVA?

-Sergey

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-11 11:29       ` Serge Semin
@ 2022-07-12  0:02         ` Florian Fainelli
  2022-07-12  7:30           ` Thomas Bogendoerfer
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2022-07-12  0:02 UTC (permalink / raw)
  To: Serge Semin, Thomas Bogendoerfer; +Cc: linux-mips, gerg, open list



On 7/11/2022 4:29 AM, Serge Semin wrote:
> On Mon, Jul 11, 2022 at 01:27:40PM +0200, Thomas Bogendoerfer wrote:
>> On Mon, Jul 11, 2022 at 01:40:52PM +0300, Serge Semin wrote:
>>> On Mon, Jul 11, 2022 at 10:38:48AM +0200, Thomas Bogendoerfer wrote:
>>>> On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
>>>>> It is permissible for kernel code to call virt_to_phys() against virtual
>>>>> addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
>>>>> types. Add a final condition that ensures that the virtual address is
>>>>> below KSEG2.
>>>>>
>>>>> Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>> ---
>>>>>   arch/mips/mm/physaddr.c | 3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
>>>>> index a1ced5e44951..a82f8f57a652 100644
>>>>> --- a/arch/mips/mm/physaddr.c
>>>>> +++ b/arch/mips/mm/physaddr.c
>>>>> @@ -5,6 +5,7 @@
>>>>>   #include <linux/mmdebug.h>
>>>>>   #include <linux/mm.h>
>>>>>   
>>>>> +#include <asm/addrspace.h>
>>>>>   #include <asm/sections.h>
>>>>>   #include <asm/io.h>
>>>>>   #include <asm/page.h>
>>>>> @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
>>>>>   	if (x == MAX_DMA_ADDRESS)
>>>>>   		return true;
>>>>>   
>>>>> -	return false;
>>>>> +	return KSEGX(x) < KSEG2;
>>>>>   }
>>>>>   
>>>>>   phys_addr_t __virt_to_phys(volatile const void *x)
>>>>> -- 
>>>>> 2.25.1
>>>>
>>>
>>>> applied to mips-next.
>>>
>>> Are you sure it was ready to be applied?
>>> Link: https://lore.kernel.org/linux-mips/20220708115851.ejsooiilxcopkoei@mobilestation/
>>
> 
>> your comment sounded like optimizing, which can be done later on, so
>> I assumed it ready.
> 
> What about Malta and EVA?

Sergey's comment definitively need to be addressed, and I would not see 
the point in making an incremental change to a wrong fix. Can you drop 
that patch for now? Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-12  0:02         ` Florian Fainelli
@ 2022-07-12  7:30           ` Thomas Bogendoerfer
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Bogendoerfer @ 2022-07-12  7:30 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Serge Semin, linux-mips, gerg, open list

On Mon, Jul 11, 2022 at 05:02:51PM -0700, Florian Fainelli wrote:
> On 7/11/2022 4:29 AM, Serge Semin wrote:
> > On Mon, Jul 11, 2022 at 01:27:40PM +0200, Thomas Bogendoerfer wrote:
> > > On Mon, Jul 11, 2022 at 01:40:52PM +0300, Serge Semin wrote:
> > > > On Mon, Jul 11, 2022 at 10:38:48AM +0200, Thomas Bogendoerfer wrote:
> > > > > On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> > > > > > It is permissible for kernel code to call virt_to_phys() against virtual
> > > > > > addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> > > > > > types. Add a final condition that ensures that the virtual address is
> > > > > > below KSEG2.
> > > > > > 
> > > > > > Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> > > > > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > > > > > ---
> > > > > >   arch/mips/mm/physaddr.c | 3 ++-
> > > > > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> > > > > > index a1ced5e44951..a82f8f57a652 100644
> > > > > > --- a/arch/mips/mm/physaddr.c
> > > > > > +++ b/arch/mips/mm/physaddr.c
> > > > > > @@ -5,6 +5,7 @@
> > > > > >   #include <linux/mmdebug.h>
> > > > > >   #include <linux/mm.h>
> > > > > > +#include <asm/addrspace.h>
> > > > > >   #include <asm/sections.h>
> > > > > >   #include <asm/io.h>
> > > > > >   #include <asm/page.h>
> > > > > > @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
> > > > > >   	if (x == MAX_DMA_ADDRESS)
> > > > > >   		return true;
> > > > > > -	return false;
> > > > > > +	return KSEGX(x) < KSEG2;
> > > > > >   }
> > > > > >   phys_addr_t __virt_to_phys(volatile const void *x)
> > > > > > -- 
> > > > > > 2.25.1
> > > > > 
> > > > 
> > > > > applied to mips-next.
> > > > 
> > > > Are you sure it was ready to be applied?
> > > > Link: https://lore.kernel.org/linux-mips/20220708115851.ejsooiilxcopkoei@mobilestation/
> > > 
> > 
> > > your comment sounded like optimizing, which can be done later on, so
> > > I assumed it ready.
> > 
> > What about Malta and EVA?
> 
> Sergey's comment definitively need to be addressed, and I would not see the
> point in making an incremental change to a wrong fix. Can you drop that
> patch for now? Thanks!

dropped.

Thomas.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-08 11:58 ` Serge Semin
@ 2022-07-12 16:28   ` Florian Fainelli
  2022-07-13  7:01     ` Serge Semin
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2022-07-12 16:28 UTC (permalink / raw)
  To: Serge Semin; +Cc: linux-mips, gerg, Thomas Bogendoerfer, open list

On 7/8/22 04:58, Serge Semin wrote:
> On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
>> It is permissible for kernel code to call virt_to_phys() against virtual
>> addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
>> types. Add a final condition that ensures that the virtual address is
>> below KSEG2.
>>
>> Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   arch/mips/mm/physaddr.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
>> index a1ced5e44951..a82f8f57a652 100644
>> --- a/arch/mips/mm/physaddr.c
>> +++ b/arch/mips/mm/physaddr.c
>> @@ -5,6 +5,7 @@
>>   #include <linux/mmdebug.h>
>>   #include <linux/mm.h>
>>   
>> +#include <asm/addrspace.h>
>>   #include <asm/sections.h>
>>   #include <asm/io.h>
>>   #include <asm/page.h>
>> @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
>>   	if (x == MAX_DMA_ADDRESS)
>>   		return true;
>>   
> 
>> -	return false;
>> +	return KSEGX(x) < KSEG2;
> 
> With this do we really need the high_memory-based conditionals in this
> method?
> 
> If the line above is the only way to take the uncached segment into
> account then can we reduce the whole method to:
> static inline bool __debug_virt_addr_valid {
> 	return x >= PAGE_OFFSET && KSEGX(x) < KSEG2;
> }
> ?
> 
> Though this still may be invalid for EVA systems, like malta (see
> arch/mips/include/asm/mach-malta/spaces.h).
> 
> Note AFAICS if EVA is enabled, highmem is implied to be disabled (see
> the CPU_MIPS32_3_5_EVA config utilization and HIGHMEM config
> dependencies). Thus all the memory is supposed to be linearly mapped
> in that case.

OK, so if all of the memory is linearly mapped, then I am not too sure 
what we will be able to check, which is in essence pretty similar to 
what happens on MIPS64, right?

Maybe DEBUG_VIRTUAL should depend on !EVA as well?
-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS: Fixed __debug_virt_addr_valid()
  2022-07-12 16:28   ` Florian Fainelli
@ 2022-07-13  7:01     ` Serge Semin
  0 siblings, 0 replies; 10+ messages in thread
From: Serge Semin @ 2022-07-13  7:01 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-mips, gerg, Thomas Bogendoerfer, open list

On Tue, Jul 12, 2022 at 09:28:02AM -0700, Florian Fainelli wrote:
> On 7/8/22 04:58, Serge Semin wrote:
> > On Thu, Jul 07, 2022 at 02:52:36PM -0700, Florian Fainelli wrote:
> > > It is permissible for kernel code to call virt_to_phys() against virtual
> > > addresses that are in KSEG0 or KSEG1 and we need to be dealing with both
> > > types. Add a final condition that ensures that the virtual address is
> > > below KSEG2.
> > > 
> > > Fixes: dfad83cb7193 ("MIPS: Add support for CONFIG_DEBUG_VIRTUAL")
> > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > >   arch/mips/mm/physaddr.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/mips/mm/physaddr.c b/arch/mips/mm/physaddr.c
> > > index a1ced5e44951..a82f8f57a652 100644
> > > --- a/arch/mips/mm/physaddr.c
> > > +++ b/arch/mips/mm/physaddr.c
> > > @@ -5,6 +5,7 @@
> > >   #include <linux/mmdebug.h>
> > >   #include <linux/mm.h>
> > > +#include <asm/addrspace.h>
> > >   #include <asm/sections.h>
> > >   #include <asm/io.h>
> > >   #include <asm/page.h>
> > > @@ -30,7 +31,7 @@ static inline bool __debug_virt_addr_valid(unsigned long x)
> > >   	if (x == MAX_DMA_ADDRESS)
> > >   		return true;
> > 
> > > -	return false;
> > > +	return KSEGX(x) < KSEG2;
> > 
> > With this do we really need the high_memory-based conditionals in this
> > method?
> > 
> > If the line above is the only way to take the uncached segment into
> > account then can we reduce the whole method to:
> > static inline bool __debug_virt_addr_valid {
> > 	return x >= PAGE_OFFSET && KSEGX(x) < KSEG2;
> > }
> > ?
> > 
> > Though this still may be invalid for EVA systems, like malta (see
> > arch/mips/include/asm/mach-malta/spaces.h).
> > 
> > Note AFAICS if EVA is enabled, highmem is implied to be disabled (see
> > the CPU_MIPS32_3_5_EVA config utilization and HIGHMEM config
> > dependencies). Thus all the memory is supposed to be linearly mapped
> > in that case.
> 

> OK, so if all of the memory is linearly mapped, then I am not too sure what
> we will be able to check, which is in essence pretty similar to what happens
> on MIPS64, right?

Essence is right, but in general situation is more complicated.
Basically EVA (seems like a Bible reference...) provides a way to
change the traditional MIPS memory address space to pretty much any
within 4GB virtual-to-physical address mapping. Most importantly it
can be used to eliminate the limitation of having just 512MB of the
directly mapped memory in kernel.

Anyway neither MIPS HIGHMEM kernel config nor the Malta's EVA mode
don't imply having high-memory enabled in case of EVAs. Most likely
the constraint has been set due to the MIPS arch code too much relying
on the traditional address space mapping. So the high-memory part just
wasn't fixed to be properly working in that case, while the CPU
MMU-type segments can still be defined for EVA. As the comment in the
Malta spaces.h header file says HIGHMEM_START is preserved just for
the correct high-mem macros arithmetics.

Just to note. Lack of high-memory in case of EVA is a big drawback
because some physical memory gets to be unavailable. At the very least
512MB segment must be preserved for the uncached kernel region for
MMIOs. Not to say that XPA won't work without high-memory. So it's
either XPA (greater than 4GB physical memory) or EVA (up to 3.5GB
directly mapped kernel memory). So annoying.

> 
> Maybe DEBUG_VIRTUAL should depend on !EVA as well?

In that case the debug-version of __phys_addr_symbol() will be
unavailable too. I would rather suggest to fix the
__debug_virt_addr_valid() method implementation to at least returning
always true in case of EVA or !HIGHMEM.

-Sergey

> -- 
> Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-13  7:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 21:52 [PATCH] MIPS: Fixed __debug_virt_addr_valid() Florian Fainelli
2022-07-08 11:58 ` Serge Semin
2022-07-12 16:28   ` Florian Fainelli
2022-07-13  7:01     ` Serge Semin
2022-07-11  8:38 ` Thomas Bogendoerfer
2022-07-11 10:40   ` Serge Semin
2022-07-11 11:27     ` Thomas Bogendoerfer
2022-07-11 11:29       ` Serge Semin
2022-07-12  0:02         ` Florian Fainelli
2022-07-12  7:30           ` Thomas Bogendoerfer

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.