linux-kernel - Re: [PATCH v5 04/22] sh: Use P1SEGADDR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160706145328.GA15995@brightrain.aerifal.cx>
Date:	Wed, 6 Jul 2016 10:53:28 -0400
From:	Rich Felker <dalias@...c.org>
To:	Yoshinori Sato <ysato@...rs.sourceforge.jp>
Cc:	linux-sh@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 04/22] sh: Use P1SEGADDR

On Wed, Jul 06, 2016 at 11:11:44PM +0900, Yoshinori Sato wrote:
> On Mon, 04 Jul 2016 10:48:52 +0900,
> Rich Felker wrote:
> > 
> > On Mon, Jul 04, 2016 at 01:46:24AM +0900, Yoshinori Sato wrote:
> > > FDT address is P1SEG. So not virtual address.
> > > 
> > > Signed-off-by: Yoshinori Sato <ysato@...rs.sourceforge.jp>
> > > ---
> > >  arch/sh/kernel/setup.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
> > > index 86f2792..8e3b099 100644
> > > --- a/arch/sh/kernel/setup.c
> > > +++ b/arch/sh/kernel/setup.c
> > > @@ -254,7 +254,7 @@ void __ref sh_fdt_init(phys_addr_t dt_phys)
> > >  #ifdef CONFIG_USE_BUILTIN_DTB
> > >  	dt_virt = __dtb_start;
> > >  #else
> > > -	dt_virt = phys_to_virt(dt_phys);
> > > +	dt_virt = (void *)P1SEGADDR(dt_phys);
> > >  #endif
> > >  
> > >  	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
> > > -- 
> > 
> > I don't think this change is correct, and I'm not sure what the
> > motivation is. It certainly can't work with !CONFIG_29BIT, and likely
> > can't work on nommu either (it won't work on J2). Maybe we have
> > different ideas about the sort of physical address the boot loader is
> > expected to pass; I would expect it to be something that, when passed
> > to phys_to_virt, yields an address the kernel can use to access the
> > memory. This does not necessarily mean it's MMU-mapped memory; it
> > could be (and in practice will be, I think) an address in the P1
> > segment obtained by adding PAGE_OFFSET (see asm/page.h).
> 
> Hmm...
> It's better to pass a virtual address in bootloader.

I think we're just having a miscommunication on what "physical
address" vs "virtual address" means. I wouldn't call logical addresses
in the P1 segment "virtual" because they're not remapped by the MMU.
Could you provide an example showing the type of address your
bootloader is currently passing to the kernel and why it needs to be
mapped by P1SEGADDR rather than phys_to_virt?

Rich