From: Palmer Dabbelt <palmer@sifive.com>
To: anup@brainfault.org
Cc: Anup Patel <Anup.Patel@wdc.com>,
linux-kernel@vger.kernel.org,
Christoph Hellwig <hch@infradead.org>,
Atish Patra <Atish.Patra@wdc.com>,
Alistair Francis <Alistair.Francis@wdc.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
linux-riscv@lists.infradead.org
Subject: Re: [PATCH] RISC-V: Fix FIXMAP area corruption on RV32 systems
Date: Mon, 26 Aug 2019 14:17:41 -0700 (PDT) [thread overview]
Message-ID: <mhng-7a475a74-60cb-4e3f-bcae-6cd6299bb173@palmer-si-x1c4> (raw)
In-Reply-To: <CAAhSdy1arxoekV4p3so=2PtTtBCvT61sz+uDbaZ=e11p7b5DXg@mail.gmail.com>
On Sun, 18 Aug 2019 21:49:01 PDT (-0700), anup@brainfault.org wrote:
> On Sun, Aug 18, 2019 at 11:49 PM Christoph Hellwig <hch@infradead.org> wrote:
>>
>> > +#define FIXADDR_TOP (VMALLOC_START)
>>
>> Nit: no need for the braces, the definitions below don't use it
>> either.
>
> Sure, I will update and send v2 soon.
>
>>
>> > +#ifdef CONFIG_64BIT
>> > +#define FIXADDR_SIZE PMD_SIZE
>> > +#else
>> > +#define FIXADDR_SIZE PGDIR_SIZE
>> > +#endif
>> > +#define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE)
>> > +
>> > /*
>> > - * Task size is 0x4000000000 for RV64 or 0xb800000 for RV32.
>> > + * Task size is 0x4000000000 for RV64 or 0x9fc00000 for RV32.
>> > * Note that PGDIR_SIZE must evenly divide TASK_SIZE.
>> > */
>> > #ifdef CONFIG_64BIT
>> > #define TASK_SIZE (PGDIR_SIZE * PTRS_PER_PGD / 2)
>> > #else
>> > -#define TASK_SIZE VMALLOC_START
>> > +#define TASK_SIZE FIXADDR_START
>> > #endif
>>
>> Mentioning the addresses is a little weird. IMHO this would be
>> a much nicer place to explain the high-level memory layout, including
>> maybe a little ASCII art. Also we could have one #ifdef CONFIG_64BIT
>> for both related values. Last but not least instead of saying that
>> something should be dividable it would be nice to have a BUILD_BUG_ON
>> to enforce it.
>>
>> Either way we are late in the cycle, so I guess this is ok for now:
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>
>> But I'd love to see this area improved a little further as it is full
>> of mine fields.
>
> I agree with you. We also have Sparsemem and KASAN patches which
> touch virtual memory layout so it's important to have virtual memory layout
> documented clearly. I can add the required documentation as separate patch.
Documentation is great, but if we document something that is broken then it's
still broken :)
I think this needs to just be redone -- we keep running into issues here and
fixing them, but there are probably more issues and it'll probably be faster to
just think through the memory map than to keep fixing bugs as they crop up.
This was one of the areas of the port I didn't rewrite as part of the upstream
submission process, and as a result it's pretty crusty.
> I think the best place to add ASCII art would be asm/pgtable.h where all
> virtual memory related defines are placed. Suggestions??
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2019-08-26 21:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-16 11:49 [PATCH] RISC-V: Fix FIXMAP area corruption on RV32 systems Anup Patel
2019-08-16 17:57 ` Alistair Francis
2019-08-18 18:19 ` Christoph Hellwig
2019-08-19 4:49 ` Anup Patel
2019-08-26 21:17 ` Palmer Dabbelt [this message]
2019-08-26 21:30 ` Alistair Francis
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=mhng-7a475a74-60cb-4e3f-bcae-6cd6299bb173@palmer-si-x1c4 \
--to=palmer@sifive.com \
--cc=Alistair.Francis@wdc.com \
--cc=Anup.Patel@wdc.com \
--cc=Atish.Patra@wdc.com \
--cc=anup@brainfault.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=paul.walmsley@sifive.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).