All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Kossifidis <mick@ics.forth.gr>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: mick@ics.forth.gr, alexandre.ghiti@canonical.com,
	Paul Walmsley <paul.walmsley@sifive.com>,
	aou@eecs.berkeley.edu, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Don't use va_pa_offset on kdump
Date: Mon, 25 Oct 2021 04:00:12 +0300	[thread overview]
Message-ID: <d4d0d13c9fda686575f1040cd16b516f@mailhost.ics.forth.gr> (raw)
In-Reply-To: <mhng-b28fa0cb-24ee-40d1-9f02-619605a4cc9a@palmerdabbelt-glaptop>

Στις 2021-10-23 23:14, Palmer Dabbelt έγραψε:
> On Sat, 09 Oct 2021 06:18:48 PDT (-0700), mick@ics.forth.gr wrote:
>> Στις 2021-10-06 14:13, Alexandre Ghiti έγραψε:
>>>> +
>>>> +       /* This will trigger a jump to CSR_STVEC anyway */
>>>>         jalr    zero, a2, 0
>>> 
>>> The last jump to a2 can be removed since the fault will be triggered
>>> before even reaching this instruction.
>>> 
>> 
>> Just switching SATP to zero doesn't generate a trap unless mstatus.TVM
>> is set (for visualization purposes). The hart will try and execute the
>> next instruction but it's not clear in the spec what happens in case 
>> the
>> code is cached, I don't want to rely solely on STVEC. I prefer having
>> this instruction there, note that some earlier QEMU versions also had
>> this behavior (the original kdump patch didn't set STVEC and it worked
>> fine after setting SATP to zero).
> 
> IIRC this came down to some very specific wording in the spec.
> Something along the lines of the 0 in SATP meaning "no translation",
> SFENCE.VMA ordering translations, and the general "if the spec doesn't
> mention it then it has to work" logic.  I thought I opened a spec
> issue about this for clarification, but I can't find it.
> 

I guess you mean this one:
https://github.com/riscv/riscv-isa-manual/issues/538

I couldn't find anything though regarding cached code, it's not that 
there's going to be a load after setting satp to 0 if the code has been 
cached, so even if the translation is cached we don't have a guarantee 
that the next instruction will result a trap.

> That said, I'm perfectly fine taking the safe approach here as it's
> not like the performance matters here.  Warrants a comment, though.
> 

ACK

> 
> I don't have a v2 in my inbox, did I miss something?  Also, if it's
> just the tags then it's generally not necessary to re-send something.
> The comment does, though.
> 
> LMK if you want me to deal with this, or if there's going to be a v2.
> 
> Thanks!

I'll send a v2 with the tags and the comment.

Regards,
Nick

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Nick Kossifidis <mick@ics.forth.gr>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: mick@ics.forth.gr, alexandre.ghiti@canonical.com,
	Paul Walmsley <paul.walmsley@sifive.com>,
	aou@eecs.berkeley.edu, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Don't use va_pa_offset on kdump
Date: Mon, 25 Oct 2021 04:00:12 +0300	[thread overview]
Message-ID: <d4d0d13c9fda686575f1040cd16b516f@mailhost.ics.forth.gr> (raw)
In-Reply-To: <mhng-b28fa0cb-24ee-40d1-9f02-619605a4cc9a@palmerdabbelt-glaptop>

Στις 2021-10-23 23:14, Palmer Dabbelt έγραψε:
> On Sat, 09 Oct 2021 06:18:48 PDT (-0700), mick@ics.forth.gr wrote:
>> Στις 2021-10-06 14:13, Alexandre Ghiti έγραψε:
>>>> +
>>>> +       /* This will trigger a jump to CSR_STVEC anyway */
>>>>         jalr    zero, a2, 0
>>> 
>>> The last jump to a2 can be removed since the fault will be triggered
>>> before even reaching this instruction.
>>> 
>> 
>> Just switching SATP to zero doesn't generate a trap unless mstatus.TVM
>> is set (for visualization purposes). The hart will try and execute the
>> next instruction but it's not clear in the spec what happens in case 
>> the
>> code is cached, I don't want to rely solely on STVEC. I prefer having
>> this instruction there, note that some earlier QEMU versions also had
>> this behavior (the original kdump patch didn't set STVEC and it worked
>> fine after setting SATP to zero).
> 
> IIRC this came down to some very specific wording in the spec.
> Something along the lines of the 0 in SATP meaning "no translation",
> SFENCE.VMA ordering translations, and the general "if the spec doesn't
> mention it then it has to work" logic.  I thought I opened a spec
> issue about this for clarification, but I can't find it.
> 

I guess you mean this one:
https://github.com/riscv/riscv-isa-manual/issues/538

I couldn't find anything though regarding cached code, it's not that 
there's going to be a load after setting satp to 0 if the code has been 
cached, so even if the translation is cached we don't have a guarantee 
that the next instruction will result a trap.

> That said, I'm perfectly fine taking the safe approach here as it's
> not like the performance matters here.  Warrants a comment, though.
> 

ACK

> 
> I don't have a v2 in my inbox, did I miss something?  Also, if it's
> just the tags then it's generally not necessary to re-send something.
> The comment does, though.
> 
> LMK if you want me to deal with this, or if there's going to be a v2.
> 
> Thanks!

I'll send a v2 with the tags and the comment.

Regards,
Nick

  reply	other threads:[~2021-10-25  1:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-02 12:20 [PATCH] riscv: Don't use va_pa_offset on kdump Nick Kossifidis
2021-10-02 12:20 ` Nick Kossifidis
2021-10-06 11:13 ` Alexandre Ghiti
2021-10-06 11:13   ` Alexandre Ghiti
2021-10-09 13:18   ` Nick Kossifidis
2021-10-09 13:18     ` Nick Kossifidis
2021-10-23 20:14     ` Palmer Dabbelt
2021-10-23 20:14       ` Palmer Dabbelt
2021-10-25  1:00       ` Nick Kossifidis [this message]
2021-10-25  1:00         ` Nick Kossifidis

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=d4d0d13c9fda686575f1040cd16b516f@mailhost.ics.forth.gr \
    --to=mick@ics.forth.gr \
    --cc=alexandre.ghiti@canonical.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --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 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.