All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
To: kvm-riscv@lists.infradead.org
Subject: [PATCH 1/1] target/riscv: enable floating point unit
Date: Wed, 18 Sep 2024 15:49:39 +0200	[thread overview]
Message-ID: <f1e41b95-c499-4e06-91cb-006dcd9d29e6@canonical.com> (raw)
In-Reply-To: <CAFEAcA-L7sQfK6MNt1ZbZqUMk+TJor=uD3Jj-Pc6Vy9j9JHhYQ@mail.gmail.com>

On 18.09.24 15:12, Peter Maydell wrote:
> On Wed, 18 Sept 2024 at 14:06, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>> Thanks Peter for looking into this.
>>
>> QEMU's cpu_synchronize_all_post_init() and
>> do_kvm_cpu_synchronize_post_reset() both end up in
>> kvm_arch_put_registers() and that is long after Linux
>> kvm_arch_vcpu_create() has been setting some FPU state. See the output
>> below.
>>
>> kvm_arch_put_registers() copies the CSRs by calling
>> kvm_riscv_put_regs_csr(). Here we can find:
>>
>>       KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
>>
>> This call enables or disables the FPU according to the value of
>> env->mstatus.
>>
>> So we need to set the desired state of the floating point unit in QEMU.
>> And this is what the current patch does both for TCG and KVM.
> 
> If it does this for both TCG and KVM then I don't understand
> this bit from the commit message:
> 
> # Without this patch EDK II with TLS enabled crashes when hitting the first
> # floating point instruction while running QEMU with --accel kvm and runs
> # fine with --accel tcg.
> 
> Shouldn't this guest crash the same way with both KVM and TCG without
> this patch, because the FPU state is the same for both?
> 
> -- PMM

By default `qemu-system-riscv64 --accel tcg` runs OpenSBI as firmware 
which enables the FPU.

If you would choose a different SBI implementation which does not enable 
the FPU you could experience the same crash.

Best regards

Heinrich


WARNING: multiple messages have this Message-ID (diff)
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	Bin Meng <bmeng.cn@gmail.com>, Weiwei Li <liwei1518@gmail.com>,
	Daniel Henrique Barboza <dbarboza@ventanamicro.com>,
	Liu Zhiwei <zhiwei_liu@linux.alibaba.com>,
	qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	kvm@vger.kernel.org, kvm-riscv@lists.infradead.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	Andrew Jones <ajones@ventanamicro.com>
Subject: Re: [PATCH 1/1] target/riscv: enable floating point unit
Date: Wed, 18 Sep 2024 15:49:39 +0200	[thread overview]
Message-ID: <f1e41b95-c499-4e06-91cb-006dcd9d29e6@canonical.com> (raw)
In-Reply-To: <CAFEAcA-L7sQfK6MNt1ZbZqUMk+TJor=uD3Jj-Pc6Vy9j9JHhYQ@mail.gmail.com>

On 18.09.24 15:12, Peter Maydell wrote:
> On Wed, 18 Sept 2024 at 14:06, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>> Thanks Peter for looking into this.
>>
>> QEMU's cpu_synchronize_all_post_init() and
>> do_kvm_cpu_synchronize_post_reset() both end up in
>> kvm_arch_put_registers() and that is long after Linux
>> kvm_arch_vcpu_create() has been setting some FPU state. See the output
>> below.
>>
>> kvm_arch_put_registers() copies the CSRs by calling
>> kvm_riscv_put_regs_csr(). Here we can find:
>>
>>       KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
>>
>> This call enables or disables the FPU according to the value of
>> env->mstatus.
>>
>> So we need to set the desired state of the floating point unit in QEMU.
>> And this is what the current patch does both for TCG and KVM.
> 
> If it does this for both TCG and KVM then I don't understand
> this bit from the commit message:
> 
> # Without this patch EDK II with TLS enabled crashes when hitting the first
> # floating point instruction while running QEMU with --accel kvm and runs
> # fine with --accel tcg.
> 
> Shouldn't this guest crash the same way with both KVM and TCG without
> this patch, because the FPU state is the same for both?
> 
> -- PMM

By default `qemu-system-riscv64 --accel tcg` runs OpenSBI as firmware 
which enables the FPU.

If you would choose a different SBI implementation which does not enable 
the FPU you could experience the same crash.

Best regards

Heinrich

WARNING: multiple messages have this Message-ID (diff)
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	Bin Meng <bmeng.cn@gmail.com>, Weiwei Li <liwei1518@gmail.com>,
	Daniel Henrique Barboza <dbarboza@ventanamicro.com>,
	Liu Zhiwei <zhiwei_liu@linux.alibaba.com>,
	qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	kvm@vger.kernel.org, kvm-riscv@lists.infradead.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	Andrew Jones <ajones@ventanamicro.com>
Subject: Re: [PATCH 1/1] target/riscv: enable floating point unit
Date: Wed, 18 Sep 2024 15:49:39 +0200	[thread overview]
Message-ID: <f1e41b95-c499-4e06-91cb-006dcd9d29e6@canonical.com> (raw)
In-Reply-To: <CAFEAcA-L7sQfK6MNt1ZbZqUMk+TJor=uD3Jj-Pc6Vy9j9JHhYQ@mail.gmail.com>

On 18.09.24 15:12, Peter Maydell wrote:
> On Wed, 18 Sept 2024 at 14:06, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>> Thanks Peter for looking into this.
>>
>> QEMU's cpu_synchronize_all_post_init() and
>> do_kvm_cpu_synchronize_post_reset() both end up in
>> kvm_arch_put_registers() and that is long after Linux
>> kvm_arch_vcpu_create() has been setting some FPU state. See the output
>> below.
>>
>> kvm_arch_put_registers() copies the CSRs by calling
>> kvm_riscv_put_regs_csr(). Here we can find:
>>
>>       KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
>>
>> This call enables or disables the FPU according to the value of
>> env->mstatus.
>>
>> So we need to set the desired state of the floating point unit in QEMU.
>> And this is what the current patch does both for TCG and KVM.
> 
> If it does this for both TCG and KVM then I don't understand
> this bit from the commit message:
> 
> # Without this patch EDK II with TLS enabled crashes when hitting the first
> # floating point instruction while running QEMU with --accel kvm and runs
> # fine with --accel tcg.
> 
> Shouldn't this guest crash the same way with both KVM and TCG without
> this patch, because the FPU state is the same for both?
> 
> -- PMM

By default `qemu-system-riscv64 --accel tcg` runs OpenSBI as firmware 
which enables the FPU.

If you would choose a different SBI implementation which does not enable 
the FPU you could experience the same crash.

Best regards

Heinrich

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

  reply	other threads:[~2024-09-18 13:49 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16 18:16 [PATCH 1/1] target/riscv: enable floating point unit Heinrich Schuchardt
2024-09-17 12:13 ` Andrew Jones
2024-09-17 13:28   ` Heinrich Schuchardt
2024-09-17 14:49     ` Andrew Jones
2024-09-17 16:45       ` Heinrich Schuchardt
2024-09-17 16:45         ` Heinrich Schuchardt
2024-09-17 16:45         ` Heinrich Schuchardt
2024-09-18  6:05         ` Andrew Jones
2024-09-18  6:05           ` Andrew Jones
2024-09-18  6:05           ` Andrew Jones
2024-09-18 11:10           ` Peter Maydell
2024-09-18 11:10             ` Peter Maydell
2024-09-18 11:10             ` Peter Maydell
2024-09-18 13:06             ` Heinrich Schuchardt
2024-09-18 13:06               ` Heinrich Schuchardt
2024-09-18 13:06               ` Heinrich Schuchardt
2024-09-18 13:12               ` Peter Maydell
2024-09-18 13:12                 ` Peter Maydell
2024-09-18 13:12                 ` Peter Maydell
2024-09-18 13:49                 ` Heinrich Schuchardt [this message]
2024-09-18 13:49                   ` Heinrich Schuchardt
2024-09-18 13:49                   ` Heinrich Schuchardt
2024-09-18 13:56                   ` Andrew Jones
2024-09-18 13:56                     ` Andrew Jones
2024-09-18 13:56                     ` Andrew Jones
2024-09-18 15:49                     ` Heinrich Schuchardt
2024-09-18 15:49                       ` Heinrich Schuchardt
2024-09-18 15:49                       ` Heinrich Schuchardt
2024-09-18 15:32                   ` Peter Maydell
2024-09-18 15:32                     ` Peter Maydell
2024-09-18 15:32                     ` Peter Maydell
2024-09-18 16:27                     ` Heinrich Schuchardt
2024-09-18 16:27                       ` Heinrich Schuchardt
2024-09-18 16:27                       ` Heinrich Schuchardt
2024-10-08  0:45                     ` Alistair Francis
2024-10-08  0:45                       ` Alistair Francis
2024-10-08  0:45                       ` Alistair Francis
2024-09-18 14:38 ` Richard Henderson

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=f1e41b95-c499-4e06-91cb-006dcd9d29e6@canonical.com \
    --to=heinrich.schuchardt@canonical.com \
    --cc=kvm-riscv@lists.infradead.org \
    /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.