* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-16 17:22 phantom
2022-03-29 23:15 ` Atish Patra
0 siblings, 1 reply; 32+ messages in thread
From: phantom @ 2022-03-16 17:22 UTC (permalink / raw)
To: idan.horowitz; +Cc: qemu-riscv
Here is a test case for this patch. I used to submit this bug on https://bugs.launchpad.net/qemu/+bug/1906516
sfence.vma will flush the tlb, so after this instruction, the translation block should be end.
The following code will only work in single step mode:
```
relocate:
li a0, OFFSET
la t0, 1f
add t0, t0, a0
csrw stvec, t0
la t0, early_pgtbl
srl t0, t0, PAGE_SHIFT
li t1, SATP_SV39
or t0, t1, t0
csrw satp, t0
1:
sfence.vma
la t0, trap_s
csrw stvec, t0
ret
```
In this code, I want to relocate pc to virtual address with the OFFSET prefix.
Before writing to satp, pc run at physic address, stvec has been set to label 1
with a virtual prefix and virtual address has been mapping in early_pgtbl,
after writing satp, qemu will throw a page fault, and pc will set to virtual
address of label 1.
The problem is that, in this situation, the translation block will not end after
sfence.vma, and stvec will be set to trap_s,
```
----------------
IN:
Priv: 1; Virt: 0
0x00000000800000dc: 00a080b3 add ra,ra,a0
0x00000000800000e0: 00007297 auipc t0,28672 # 0x800070e0
0x00000000800000e4: f2028293 addi t0,t0,-224
0x00000000800000e8: 00c2d293 srli t0,t0,12
0x00000000800000ec: fff0031b addiw t1,zero,-1
0x00000000800000f0: 03f31313 slli t1,t1,63
0x00000000800000f4: 005362b3 or t0,t1,t0
0x00000000800000f8: 18029073 csrrw zero,satp,t0
----------------
IN:
Priv: 1; Virt: 0
0x00000000800000fc: 12000073 sfence.vma zero,zero
0x0000000080000100: 00000297 auipc t0,0 # 0x80000100
0x0000000080000104: 1c828293 addi t0,t0,456
0x0000000080000108: 10529073 csrrw zero,stvec,t0
riscv_raise_exception: 12
riscv_raise_exception: 12
riscv_raise_exception: 12
riscv_raise_exception: 12
...
```
So, the program will crash. And the program will only work in single step mode:
```
----------------
IN:
Priv: 1; Virt: 0
0x00000000800000f8: 18029073 csrrw zero,satp,t0
----------------
IN:
Priv: 1; Virt: 0
0x00000000800000fc: 12000073 sfence.vma zero,zero
riscv_raise_exception: 12
----------------
IN:
Priv: 1; Virt: 0
0xffffffff800000fc: 12000073 sfence.vma zero,zero
----------------
IN:
Priv: 1; Virt: 0
0xffffffff80000100: 00000297 auipc t0,0 # 0xffffffff80000100
```
The pc will set to label 1, instead of trap_s.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-16 17:22 [PATCH] target/riscv: Exit current TB after an sfence.vma phantom
@ 2022-03-29 23:15 ` Atish Patra
0 siblings, 0 replies; 32+ messages in thread
From: Atish Patra @ 2022-03-29 23:15 UTC (permalink / raw)
To: phantom
Cc: open list:RISC-V, Alistair Francis, idan.horowitz,
qemu-devel@nongnu.org Developers
On Wed, Mar 16, 2022 at 10:23 AM <phantom@zju.edu.cn> wrote:
>
> Here is a test case for this patch. I used to submit this bug on https://bugs.launchpad.net/qemu/+bug/1906516
>
> sfence.vma will flush the tlb, so after this instruction, the translation block should be end.
> The following code will only work in single step mode:
> ```
> relocate:
> li a0, OFFSET
>
> la t0, 1f
> add t0, t0, a0
> csrw stvec, t0
>
> la t0, early_pgtbl
> srl t0, t0, PAGE_SHIFT
> li t1, SATP_SV39
> or t0, t1, t0
>
> csrw satp, t0
> 1:
> sfence.vma
> la t0, trap_s
> csrw stvec, t0
> ret
> ```
>
> In this code, I want to relocate pc to virtual address with the OFFSET prefix.
> Before writing to satp, pc run at physic address, stvec has been set to label 1
> with a virtual prefix and virtual address has been mapping in early_pgtbl,
> after writing satp, qemu will throw a page fault, and pc will set to virtual
> address of label 1.
>
> The problem is that, in this situation, the translation block will not end after
> sfence.vma, and stvec will be set to trap_s,
>
> ```
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000dc: 00a080b3 add ra,ra,a0
> 0x00000000800000e0: 00007297 auipc t0,28672 # 0x800070e0
> 0x00000000800000e4: f2028293 addi t0,t0,-224
> 0x00000000800000e8: 00c2d293 srli t0,t0,12
> 0x00000000800000ec: fff0031b addiw t1,zero,-1
> 0x00000000800000f0: 03f31313 slli t1,t1,63
> 0x00000000800000f4: 005362b3 or t0,t1,t0
> 0x00000000800000f8: 18029073 csrrw zero,satp,t0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000fc: 12000073 sfence.vma zero,zero
> 0x0000000080000100: 00000297 auipc t0,0 # 0x80000100
> 0x0000000080000104: 1c828293 addi t0,t0,456
> 0x0000000080000108: 10529073 csrrw zero,stvec,t0
>
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> ...
> ```
>
> So, the program will crash. And the program will only work in single step mode:
> ```
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000f8: 18029073 csrrw zero,satp,t0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000fc: 12000073 sfence.vma zero,zero
>
> riscv_raise_exception: 12
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0xffffffff800000fc: 12000073 sfence.vma zero,zero
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0xffffffff80000100: 00000297 auipc t0,0 # 0xffffffff80000100
>
> ```
> The pc will set to label 1, instead of trap_s.
+qemu-dev and Alistair
This is in for-next on Alistair's tree and fails to boot the kernel
with the following error (found -d in_asm mode).
Reverting the patch solves the issue.
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201040: 18051073 csrrw zero,satp,a0
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201044: Address 0x80201044 is out of bounds.
0x0000000080201049: Address 0x80201049 is out of bounds.
0x000000008020104e: Address 0x8020104e is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201050: Address 0x80201050 is out of bounds.
0x0000000080201055: Address 0x80201055 is out of bounds.
0x000000008020105a: Address 0x8020105a is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x000000008020105c: Address 0x8020105c is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
--
Regards,
Atish
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-29 23:15 ` Atish Patra
0 siblings, 0 replies; 32+ messages in thread
From: Atish Patra @ 2022-03-29 23:15 UTC (permalink / raw)
To: phantom
Cc: idan.horowitz, open list:RISC-V, Alistair Francis,
qemu-devel@nongnu.org Developers
On Wed, Mar 16, 2022 at 10:23 AM <phantom@zju.edu.cn> wrote:
>
> Here is a test case for this patch. I used to submit this bug on https://bugs.launchpad.net/qemu/+bug/1906516
>
> sfence.vma will flush the tlb, so after this instruction, the translation block should be end.
> The following code will only work in single step mode:
> ```
> relocate:
> li a0, OFFSET
>
> la t0, 1f
> add t0, t0, a0
> csrw stvec, t0
>
> la t0, early_pgtbl
> srl t0, t0, PAGE_SHIFT
> li t1, SATP_SV39
> or t0, t1, t0
>
> csrw satp, t0
> 1:
> sfence.vma
> la t0, trap_s
> csrw stvec, t0
> ret
> ```
>
> In this code, I want to relocate pc to virtual address with the OFFSET prefix.
> Before writing to satp, pc run at physic address, stvec has been set to label 1
> with a virtual prefix and virtual address has been mapping in early_pgtbl,
> after writing satp, qemu will throw a page fault, and pc will set to virtual
> address of label 1.
>
> The problem is that, in this situation, the translation block will not end after
> sfence.vma, and stvec will be set to trap_s,
>
> ```
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000dc: 00a080b3 add ra,ra,a0
> 0x00000000800000e0: 00007297 auipc t0,28672 # 0x800070e0
> 0x00000000800000e4: f2028293 addi t0,t0,-224
> 0x00000000800000e8: 00c2d293 srli t0,t0,12
> 0x00000000800000ec: fff0031b addiw t1,zero,-1
> 0x00000000800000f0: 03f31313 slli t1,t1,63
> 0x00000000800000f4: 005362b3 or t0,t1,t0
> 0x00000000800000f8: 18029073 csrrw zero,satp,t0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000fc: 12000073 sfence.vma zero,zero
> 0x0000000080000100: 00000297 auipc t0,0 # 0x80000100
> 0x0000000080000104: 1c828293 addi t0,t0,456
> 0x0000000080000108: 10529073 csrrw zero,stvec,t0
>
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> riscv_raise_exception: 12
> ...
> ```
>
> So, the program will crash. And the program will only work in single step mode:
> ```
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000f8: 18029073 csrrw zero,satp,t0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x00000000800000fc: 12000073 sfence.vma zero,zero
>
> riscv_raise_exception: 12
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0xffffffff800000fc: 12000073 sfence.vma zero,zero
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0xffffffff80000100: 00000297 auipc t0,0 # 0xffffffff80000100
>
> ```
> The pc will set to label 1, instead of trap_s.
+qemu-dev and Alistair
This is in for-next on Alistair's tree and fails to boot the kernel
with the following error (found -d in_asm mode).
Reverting the patch solves the issue.
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201040: 18051073 csrrw zero,satp,a0
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201044: Address 0x80201044 is out of bounds.
0x0000000080201049: Address 0x80201049 is out of bounds.
0x000000008020104e: Address 0x8020104e is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201050: Address 0x80201050 is out of bounds.
0x0000000080201055: Address 0x80201055 is out of bounds.
0x000000008020105a: Address 0x8020105a is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x000000008020105c: Address 0x8020105c is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
--
Regards,
Atish
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-29 23:15 ` Atish Patra
@ 2022-03-30 6:15 ` Idan Horowitz
-1 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 6:15 UTC (permalink / raw)
To: Atish Patra
Cc: Alistair Francis, phantom, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 02:16, Atish Patra <atishp@atishpatra.org> wrote:
>
> This is in for-next on Alistair's tree and fails to boot the kernel
> with the following error (found -d in_asm mode).
> Reverting the patch solves the issue.
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201040: 18051073 csrrw zero,satp,a0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201044: Address 0x80201044 is out of bounds.
>
> 0x0000000080201049: Address 0x80201049 is out of bounds.
>
> 0x000000008020104e: Address 0x8020104e is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201050: Address 0x80201050 is out of bounds.
>
> 0x0000000080201055: Address 0x80201055 is out of bounds.
>
> 0x000000008020105a: Address 0x8020105a is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x000000008020105c: Address 0x8020105c is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> --
> Regards,
> Atish
Do you have more specific information about which kernel image doesn't boot?
The errors you're seeing simply mean that these addresses are not
translated by the new address translation context set by the write to
the satp.
To be honest I don't immediately see how this could be caused by the
patch, as it modifies the behaviour of the sfence.vma instruction, and
there are none in your trace.
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 6:15 ` Idan Horowitz
0 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 6:15 UTC (permalink / raw)
To: Atish Patra
Cc: phantom, open list:RISC-V, Alistair Francis,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 02:16, Atish Patra <atishp@atishpatra.org> wrote:
>
> This is in for-next on Alistair's tree and fails to boot the kernel
> with the following error (found -d in_asm mode).
> Reverting the patch solves the issue.
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201040: 18051073 csrrw zero,satp,a0
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201044: Address 0x80201044 is out of bounds.
>
> 0x0000000080201049: Address 0x80201049 is out of bounds.
>
> 0x000000008020104e: Address 0x8020104e is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x0000000080201050: Address 0x80201050 is out of bounds.
>
> 0x0000000080201055: Address 0x80201055 is out of bounds.
>
> 0x000000008020105a: Address 0x8020105a is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> ----------------
> IN:
> Priv: 1; Virt: 0
> 0x000000008020105c: Address 0x8020105c is out of bounds.
>
> Disassembler disagrees with translator over instruction decoding
> Please report this to qemu-devel@nongnu.org
>
> --
> Regards,
> Atish
Do you have more specific information about which kernel image doesn't boot?
The errors you're seeing simply mean that these addresses are not
translated by the new address translation context set by the write to
the satp.
To be honest I don't immediately see how this could be caused by the
patch, as it modifies the behaviour of the sfence.vma instruction, and
there are none in your trace.
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 6:15 ` Idan Horowitz
@ 2022-03-30 7:28 ` Atish Patra
-1 siblings, 0 replies; 32+ messages in thread
From: Atish Patra @ 2022-03-30 7:28 UTC (permalink / raw)
To: Idan Horowitz
Cc: Alistair Francis, phantom, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Tue, Mar 29, 2022 at 11:15 PM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> On Wed, 30 Mar 2022 at 02:16, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > This is in for-next on Alistair's tree and fails to boot the kernel
> > with the following error (found -d in_asm mode).
> > Reverting the patch solves the issue.
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201040: 18051073 csrrw zero,satp,a0
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201044: Address 0x80201044 is out of bounds.
> >
> > 0x0000000080201049: Address 0x80201049 is out of bounds.
> >
> > 0x000000008020104e: Address 0x8020104e is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201050: Address 0x80201050 is out of bounds.
> >
> > 0x0000000080201055: Address 0x80201055 is out of bounds.
> >
> > 0x000000008020105a: Address 0x8020105a is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x000000008020105c: Address 0x8020105c is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > --
> > Regards,
> > Atish
>
> Do you have more specific information about which kernel image doesn't boot?
I tested on v5.17 built from defconfig for rv64.
> The errors you're seeing simply mean that these addresses are not
> translated by the new address translation context set by the write to
> the satp.
> To be honest I don't immediately see how this could be caused by the
> patch, as it modifies the behaviour of the sfence.vma instruction, and
> there are none in your trace.
>
There was a sfence.vma. I just did not share the detailed trace before.
Here is the kernel code executing sfence.vma
https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
Here is the detailed trace that should provide more information.
------------------------------------------------------------------------------------------------------------------------------
----------------
IN:
Priv: 1; Virt: 0
0x0000000080a04664: 70e2 ld ra,56(sp)
0x0000000080a04666: 7442 ld s0,48(sp)
0x0000000080a04668: 74a2 ld s1,40(sp)
0x0000000080a0466a: 7902 ld s2,32(sp)
0x0000000080a0466c: 69e2 ld s3,24(sp)
0x0000000080a0466e: 6a42 ld s4,16(sp)
0x0000000080a04670: 6aa2 ld s5,8(sp)
0x0000000080a04672: 6121 addi sp,sp,64
0x0000000080a04674: 8082 ret
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201132: 00a05517 auipc a0,10506240
# 0x80c06132
0x0000000080201136: ece50513 addi a0,a0,-306
0x000000008020113a: ec7ff0ef jal ra,-314
# 0x80201000
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201000: 00d95597 auipc a1,14241792
# 0x80f96000
0x0000000080201004: 38858593 addi a1,a1,904
0x0000000080201008: 658c ld a1,8(a1)
0x000000008020100a: fffff617 auipc a2,-4096
# 0x8020000a
0x000000008020100e: ff660613 addi a2,a2,-10
0x0000000080201012: 8d91 sub a1,a1,a2
0x0000000080201014: 90ae add ra,ra,a1
0x0000000080201016: 00000617 auipc a2,0
# 0x80201016
0x000000008020101a: 02e60613 addi a2,a2,46
0x000000008020101e: 962e add a2,a2,a1
0x0000000080201020: 10561073 csrrw zero,stvec,a2
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201024: 00c55613 srli a2,a0,12
0x0000000080201028: 83018593 addi a1,gp,-2000
0x000000008020102c: 618c ld a1,0(a1)
0x000000008020102e: 8e4d or a2,a2,a1
0x0000000080201030: 010f7517 auipc a0,17788928
# 0x812f8030
0x0000000080201034: fd050513 addi a0,a0,-48
0x0000000080201038: 8131 srli a0,a0,12
0x000000008020103a: 8d4d or a0,a0,a1
0x000000008020103c: 12000073 sfence.vma zero,zero
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201040: 18051073 csrrw zero,satp,a0
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201044: Address 0x80201044 is out of bounds.
0x0000000080201049: Address 0x80201049 is out of bounds.
0x000000008020104e: Address 0x8020104e is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201050: Address 0x80201050 is out of bounds.
0x0000000080201055: Address 0x80201055 is out of bounds.
0x000000008020105a: Address 0x8020105a is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x000000008020105c: Address 0x8020105c is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
------------------------------------------------------------------------------------------------------------------------------
> Idan Horowitz
--
Regards,
Atish
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 7:28 ` Atish Patra
0 siblings, 0 replies; 32+ messages in thread
From: Atish Patra @ 2022-03-30 7:28 UTC (permalink / raw)
To: Idan Horowitz
Cc: phantom, open list:RISC-V, Alistair Francis,
qemu-devel@nongnu.org Developers
On Tue, Mar 29, 2022 at 11:15 PM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> On Wed, 30 Mar 2022 at 02:16, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > This is in for-next on Alistair's tree and fails to boot the kernel
> > with the following error (found -d in_asm mode).
> > Reverting the patch solves the issue.
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201040: 18051073 csrrw zero,satp,a0
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201044: Address 0x80201044 is out of bounds.
> >
> > 0x0000000080201049: Address 0x80201049 is out of bounds.
> >
> > 0x000000008020104e: Address 0x8020104e is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x0000000080201050: Address 0x80201050 is out of bounds.
> >
> > 0x0000000080201055: Address 0x80201055 is out of bounds.
> >
> > 0x000000008020105a: Address 0x8020105a is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > ----------------
> > IN:
> > Priv: 1; Virt: 0
> > 0x000000008020105c: Address 0x8020105c is out of bounds.
> >
> > Disassembler disagrees with translator over instruction decoding
> > Please report this to qemu-devel@nongnu.org
> >
> > --
> > Regards,
> > Atish
>
> Do you have more specific information about which kernel image doesn't boot?
I tested on v5.17 built from defconfig for rv64.
> The errors you're seeing simply mean that these addresses are not
> translated by the new address translation context set by the write to
> the satp.
> To be honest I don't immediately see how this could be caused by the
> patch, as it modifies the behaviour of the sfence.vma instruction, and
> there are none in your trace.
>
There was a sfence.vma. I just did not share the detailed trace before.
Here is the kernel code executing sfence.vma
https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
Here is the detailed trace that should provide more information.
------------------------------------------------------------------------------------------------------------------------------
----------------
IN:
Priv: 1; Virt: 0
0x0000000080a04664: 70e2 ld ra,56(sp)
0x0000000080a04666: 7442 ld s0,48(sp)
0x0000000080a04668: 74a2 ld s1,40(sp)
0x0000000080a0466a: 7902 ld s2,32(sp)
0x0000000080a0466c: 69e2 ld s3,24(sp)
0x0000000080a0466e: 6a42 ld s4,16(sp)
0x0000000080a04670: 6aa2 ld s5,8(sp)
0x0000000080a04672: 6121 addi sp,sp,64
0x0000000080a04674: 8082 ret
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201132: 00a05517 auipc a0,10506240
# 0x80c06132
0x0000000080201136: ece50513 addi a0,a0,-306
0x000000008020113a: ec7ff0ef jal ra,-314
# 0x80201000
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201000: 00d95597 auipc a1,14241792
# 0x80f96000
0x0000000080201004: 38858593 addi a1,a1,904
0x0000000080201008: 658c ld a1,8(a1)
0x000000008020100a: fffff617 auipc a2,-4096
# 0x8020000a
0x000000008020100e: ff660613 addi a2,a2,-10
0x0000000080201012: 8d91 sub a1,a1,a2
0x0000000080201014: 90ae add ra,ra,a1
0x0000000080201016: 00000617 auipc a2,0
# 0x80201016
0x000000008020101a: 02e60613 addi a2,a2,46
0x000000008020101e: 962e add a2,a2,a1
0x0000000080201020: 10561073 csrrw zero,stvec,a2
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201024: 00c55613 srli a2,a0,12
0x0000000080201028: 83018593 addi a1,gp,-2000
0x000000008020102c: 618c ld a1,0(a1)
0x000000008020102e: 8e4d or a2,a2,a1
0x0000000080201030: 010f7517 auipc a0,17788928
# 0x812f8030
0x0000000080201034: fd050513 addi a0,a0,-48
0x0000000080201038: 8131 srli a0,a0,12
0x000000008020103a: 8d4d or a0,a0,a1
0x000000008020103c: 12000073 sfence.vma zero,zero
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201040: 18051073 csrrw zero,satp,a0
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201044: Address 0x80201044 is out of bounds.
0x0000000080201049: Address 0x80201049 is out of bounds.
0x000000008020104e: Address 0x8020104e is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x0000000080201050: Address 0x80201050 is out of bounds.
0x0000000080201055: Address 0x80201055 is out of bounds.
0x000000008020105a: Address 0x8020105a is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
----------------
IN:
Priv: 1; Virt: 0
0x000000008020105c: Address 0x8020105c is out of bounds.
Disassembler disagrees with translator over instruction decoding
Please report this to qemu-devel@nongnu.org
------------------------------------------------------------------------------------------------------------------------------
> Idan Horowitz
--
Regards,
Atish
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 7:28 ` Atish Patra
@ 2022-03-30 7:35 ` Idan Horowitz
-1 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 7:35 UTC (permalink / raw)
To: Atish Patra
Cc: Alistair Francis, phantom, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>
> I tested on v5.17 built from defconfig for rv64.
>
> Here is the kernel code executing sfence.vma
> https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>
I believe this is a kernel bug and not a QEMU one. They perform a
write to the SATP with the same ASID as the one used before (0) and
then expect it to be used, without performing an sfence.vma following
it.
This was exposed by my change, as previously the write to the satp was
performed in the same TB block as the sfence.vma *before it*, which
meant the TLB was not filled in between the previous sfence and the
write to SATP following it.
I was able to reproduce the issue with the Fedora Rawhide image in the
wiki, and I was able to resolve it by artificially forcing a TLB flush
following all writes to SATP.
I think the correct course of action here is to:
1. Report the issue to the linux kernel mailing list and/or contribute
a patch that adds said missing sfence.vma following the SATP write.
(Atish: Are you able to test if adding an sfence.vma in your kernel
build fixes the issue?)
2. Restore the patch
>
> --
> Regards,
> Atish
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 7:35 ` Idan Horowitz
0 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 7:35 UTC (permalink / raw)
To: Atish Patra
Cc: phantom, open list:RISC-V, Alistair Francis,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>
> I tested on v5.17 built from defconfig for rv64.
>
> Here is the kernel code executing sfence.vma
> https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>
I believe this is a kernel bug and not a QEMU one. They perform a
write to the SATP with the same ASID as the one used before (0) and
then expect it to be used, without performing an sfence.vma following
it.
This was exposed by my change, as previously the write to the satp was
performed in the same TB block as the sfence.vma *before it*, which
meant the TLB was not filled in between the previous sfence and the
write to SATP following it.
I was able to reproduce the issue with the Fedora Rawhide image in the
wiki, and I was able to resolve it by artificially forcing a TLB flush
following all writes to SATP.
I think the correct course of action here is to:
1. Report the issue to the linux kernel mailing list and/or contribute
a patch that adds said missing sfence.vma following the SATP write.
(Atish: Are you able to test if adding an sfence.vma in your kernel
build fixes the issue?)
2. Restore the patch
>
> --
> Regards,
> Atish
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 7:35 ` Idan Horowitz
@ 2022-03-30 12:38 ` phantom
-1 siblings, 0 replies; 32+ messages in thread
From: phantom @ 2022-03-30 12:38 UTC (permalink / raw)
To: Idan Horowitz; +Cc: Alistair.Francis, qemu-riscv, qemu-devel
I agree with you partly, my test case is actually from linux kernel, I notice
the strange sfence.vma before write satp during write our teaching kernel.
I think, the strange code is used to bypass the qemu bug that Idan patched.
Because in hardware, if the stap is empty, sfence.vma will do nothing.
And that's why nobody report it.
Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
So, the kernel author reodered write stap and sfence.vma to make sfence.vma
place in the same BB with write satp, instead of the following write stvec.
(If don't reorder, sfence.vma will place in the same BB with write stvec,
that will crash kernel, see my origin analysis).
However, in hardware, since tlb is empty, put the first sfence.vma before or
after write satp is not really matters.
In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
write stap to invalid qemu's translation cache.
Jinyan
> -----Original Messages-----
> From: "Idan Horowitz" <idan.horowitz@gmail.com>
> Sent Time: 2022-03-30 15:35:19 (Wednesday)
> To: "Atish Patra" <atishp@atishpatra.org>
> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>
> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > I tested on v5.17 built from defconfig for rv64.
> >
> > Here is the kernel code executing sfence.vma
> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
> >
>
> I believe this is a kernel bug and not a QEMU one. They perform a
> write to the SATP with the same ASID as the one used before (0) and
> then expect it to be used, without performing an sfence.vma following
> it.
> This was exposed by my change, as previously the write to the satp was
> performed in the same TB block as the sfence.vma *before it*, which
> meant the TLB was not filled in between the previous sfence and the
> write to SATP following it.
> I was able to reproduce the issue with the Fedora Rawhide image in the
> wiki, and I was able to resolve it by artificially forcing a TLB flush
> following all writes to SATP.
> I think the correct course of action here is to:
> 1. Report the issue to the linux kernel mailing list and/or contribute
> a patch that adds said missing sfence.vma following the SATP write.
> (Atish: Are you able to test if adding an sfence.vma in your kernel
> build fixes the issue?)
> 2. Restore the patch
>
> >
> > --
> > Regards,
> > Atish
>
> Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 12:38 ` phantom
0 siblings, 0 replies; 32+ messages in thread
From: phantom @ 2022-03-30 12:38 UTC (permalink / raw)
To: Idan Horowitz; +Cc: qemu-riscv, Alistair.Francis, qemu-devel
I agree with you partly, my test case is actually from linux kernel, I notice
the strange sfence.vma before write satp during write our teaching kernel.
I think, the strange code is used to bypass the qemu bug that Idan patched.
Because in hardware, if the stap is empty, sfence.vma will do nothing.
And that's why nobody report it.
Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
So, the kernel author reodered write stap and sfence.vma to make sfence.vma
place in the same BB with write satp, instead of the following write stvec.
(If don't reorder, sfence.vma will place in the same BB with write stvec,
that will crash kernel, see my origin analysis).
However, in hardware, since tlb is empty, put the first sfence.vma before or
after write satp is not really matters.
In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
write stap to invalid qemu's translation cache.
Jinyan
> -----Original Messages-----
> From: "Idan Horowitz" <idan.horowitz@gmail.com>
> Sent Time: 2022-03-30 15:35:19 (Wednesday)
> To: "Atish Patra" <atishp@atishpatra.org>
> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>
> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > I tested on v5.17 built from defconfig for rv64.
> >
> > Here is the kernel code executing sfence.vma
> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
> >
>
> I believe this is a kernel bug and not a QEMU one. They perform a
> write to the SATP with the same ASID as the one used before (0) and
> then expect it to be used, without performing an sfence.vma following
> it.
> This was exposed by my change, as previously the write to the satp was
> performed in the same TB block as the sfence.vma *before it*, which
> meant the TLB was not filled in between the previous sfence and the
> write to SATP following it.
> I was able to reproduce the issue with the Fedora Rawhide image in the
> wiki, and I was able to resolve it by artificially forcing a TLB flush
> following all writes to SATP.
> I think the correct course of action here is to:
> 1. Report the issue to the linux kernel mailing list and/or contribute
> a patch that adds said missing sfence.vma following the SATP write.
> (Atish: Are you able to test if adding an sfence.vma in your kernel
> build fixes the issue?)
> 2. Restore the patch
>
> >
> > --
> > Regards,
> > Atish
>
> Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 12:38 ` phantom
(?)
@ 2022-03-30 16:11 ` Palmer Dabbelt
-1 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 16:11 UTC (permalink / raw)
To: phantom, linux-riscv
Cc: idan.horowitz, Alistair Francis, qemu-riscv, qemu-devel
[re-ordering the top post]
+linux-riscv, as this may very well be a kernel bug
On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>> -----Original Messages-----
>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>> To: "Atish Patra" <atishp@atishpatra.org>
>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>
>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>> >
>> > I tested on v5.17 built from defconfig for rv64.
>> >
>> > Here is the kernel code executing sfence.vma
>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>> >
>>
>> I believe this is a kernel bug and not a QEMU one. They perform a
>> write to the SATP with the same ASID as the one used before (0) and
I seem to remember ASID 0 being a special one, with some global-ness,
but I couldn't find that in a quick poke into the spec. As below, I'm
going to read through this a bit more...
(Also, I haven't had any coffee yet)
>> then expect it to be used, without performing an sfence.vma following
>> it.
>> This was exposed by my change, as previously the write to the satp was
>> performed in the same TB block as the sfence.vma *before it*, which
>> meant the TLB was not filled in between the previous sfence and the
>> write to SATP following it.
>> I was able to reproduce the issue with the Fedora Rawhide image in the
>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>> following all writes to SATP.
>> I think the correct course of action here is to:
>> 1. Report the issue to the linux kernel mailing list and/or contribute
>> a patch that adds said missing sfence.vma following the SATP write.
>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>> build fixes the issue?)
If it's a kernel bug we should fix it, but I'm not entirely convinced
that's the case. I can confirm that the following makes Linux boot
again
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index ec07f991866a..83373c2bd7d0 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -121,6 +121,7 @@ relocate:
or a0, a0, a1
sfence.vma
csrw CSR_SATP, a0
+ sfence.vma
.align 2
1:
/* Set trap vector to spin forever to help debug */
It's been a while since I read through the rules here so I'm going to go
read them again, but IIRC that shouldn't be necessary: that first
sfence.vma should be sufficiently global to ensure all prior writes to
the page tables are visible, regardless of what's in SATP. That said, I
remember there being a lot of subtly here in the spec wording so I'm
going to go read the spec again.
>> 2. Restore the patch
Presumably you mean "revert" here? That might be the right way to go,
just to avoid breaking users (even if we fix the kernel bug, it'll take
a while to get everyone to update). That said, this smells like the
sort of thing that's going to crop up at arbitrary times in dynamic
systems so while a revert looks like it'd work around the boot issue we
might be making more headaches for folks down the road.
> I agree with you partly, my test case is actually from linux kernel, I notice
> the strange sfence.vma before write satp during write our teaching kernel.
> I think, the strange code is used to bypass the qemu bug that Idan patched.
> Because in hardware, if the stap is empty, sfence.vma will do nothing.
> And that's why nobody report it.
IIUC the sfence.vma before the SATP write is very explicitly necessary:
without that fence old mappings could be utilized directly after the
SATP write, so we might not even be able to fetch the next instruction.
> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
> place in the same BB with write satp, instead of the following write stvec.
> (If don't reorder, sfence.vma will place in the same BB with write stvec,
> that will crash kernel, see my origin analysis).
>
> However, in hardware, since tlb is empty, put the first sfence.vma before or
> after write satp is not really matters.
> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
> write stap to invalid qemu's translation cache.
The ISA manual is quite explicit about SATP not enforcing these
orderings. If what I remember about ASID 0 is true then I do think we'd
need one, though, to avoid the aliasing -- hopefully I'll make a bit
more sense soon, though...
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 16:11 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 16:11 UTC (permalink / raw)
To: phantom, linux-riscv
Cc: Alistair Francis, idan.horowitz, qemu-riscv, qemu-devel
[re-ordering the top post]
+linux-riscv, as this may very well be a kernel bug
On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>> -----Original Messages-----
>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>> To: "Atish Patra" <atishp@atishpatra.org>
>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>
>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>> >
>> > I tested on v5.17 built from defconfig for rv64.
>> >
>> > Here is the kernel code executing sfence.vma
>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>> >
>>
>> I believe this is a kernel bug and not a QEMU one. They perform a
>> write to the SATP with the same ASID as the one used before (0) and
I seem to remember ASID 0 being a special one, with some global-ness,
but I couldn't find that in a quick poke into the spec. As below, I'm
going to read through this a bit more...
(Also, I haven't had any coffee yet)
>> then expect it to be used, without performing an sfence.vma following
>> it.
>> This was exposed by my change, as previously the write to the satp was
>> performed in the same TB block as the sfence.vma *before it*, which
>> meant the TLB was not filled in between the previous sfence and the
>> write to SATP following it.
>> I was able to reproduce the issue with the Fedora Rawhide image in the
>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>> following all writes to SATP.
>> I think the correct course of action here is to:
>> 1. Report the issue to the linux kernel mailing list and/or contribute
>> a patch that adds said missing sfence.vma following the SATP write.
>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>> build fixes the issue?)
If it's a kernel bug we should fix it, but I'm not entirely convinced
that's the case. I can confirm that the following makes Linux boot
again
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index ec07f991866a..83373c2bd7d0 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -121,6 +121,7 @@ relocate:
or a0, a0, a1
sfence.vma
csrw CSR_SATP, a0
+ sfence.vma
.align 2
1:
/* Set trap vector to spin forever to help debug */
It's been a while since I read through the rules here so I'm going to go
read them again, but IIRC that shouldn't be necessary: that first
sfence.vma should be sufficiently global to ensure all prior writes to
the page tables are visible, regardless of what's in SATP. That said, I
remember there being a lot of subtly here in the spec wording so I'm
going to go read the spec again.
>> 2. Restore the patch
Presumably you mean "revert" here? That might be the right way to go,
just to avoid breaking users (even if we fix the kernel bug, it'll take
a while to get everyone to update). That said, this smells like the
sort of thing that's going to crop up at arbitrary times in dynamic
systems so while a revert looks like it'd work around the boot issue we
might be making more headaches for folks down the road.
> I agree with you partly, my test case is actually from linux kernel, I notice
> the strange sfence.vma before write satp during write our teaching kernel.
> I think, the strange code is used to bypass the qemu bug that Idan patched.
> Because in hardware, if the stap is empty, sfence.vma will do nothing.
> And that's why nobody report it.
IIUC the sfence.vma before the SATP write is very explicitly necessary:
without that fence old mappings could be utilized directly after the
SATP write, so we might not even be able to fetch the next instruction.
> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
> place in the same BB with write satp, instead of the following write stvec.
> (If don't reorder, sfence.vma will place in the same BB with write stvec,
> that will crash kernel, see my origin analysis).
>
> However, in hardware, since tlb is empty, put the first sfence.vma before or
> after write satp is not really matters.
> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
> write stap to invalid qemu's translation cache.
The ISA manual is quite explicit about SATP not enforcing these
orderings. If what I remember about ASID 0 is true then I do think we'd
need one, though, to avoid the aliasing -- hopefully I'll make a bit
more sense soon, though...
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 16:11 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 16:11 UTC (permalink / raw)
To: phantom, linux-riscv
Cc: idan.horowitz, Alistair Francis, qemu-riscv, qemu-devel
[re-ordering the top post]
+linux-riscv, as this may very well be a kernel bug
On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>> -----Original Messages-----
>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>> To: "Atish Patra" <atishp@atishpatra.org>
>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>
>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>> >
>> > I tested on v5.17 built from defconfig for rv64.
>> >
>> > Here is the kernel code executing sfence.vma
>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>> >
>>
>> I believe this is a kernel bug and not a QEMU one. They perform a
>> write to the SATP with the same ASID as the one used before (0) and
I seem to remember ASID 0 being a special one, with some global-ness,
but I couldn't find that in a quick poke into the spec. As below, I'm
going to read through this a bit more...
(Also, I haven't had any coffee yet)
>> then expect it to be used, without performing an sfence.vma following
>> it.
>> This was exposed by my change, as previously the write to the satp was
>> performed in the same TB block as the sfence.vma *before it*, which
>> meant the TLB was not filled in between the previous sfence and the
>> write to SATP following it.
>> I was able to reproduce the issue with the Fedora Rawhide image in the
>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>> following all writes to SATP.
>> I think the correct course of action here is to:
>> 1. Report the issue to the linux kernel mailing list and/or contribute
>> a patch that adds said missing sfence.vma following the SATP write.
>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>> build fixes the issue?)
If it's a kernel bug we should fix it, but I'm not entirely convinced
that's the case. I can confirm that the following makes Linux boot
again
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index ec07f991866a..83373c2bd7d0 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -121,6 +121,7 @@ relocate:
or a0, a0, a1
sfence.vma
csrw CSR_SATP, a0
+ sfence.vma
.align 2
1:
/* Set trap vector to spin forever to help debug */
It's been a while since I read through the rules here so I'm going to go
read them again, but IIRC that shouldn't be necessary: that first
sfence.vma should be sufficiently global to ensure all prior writes to
the page tables are visible, regardless of what's in SATP. That said, I
remember there being a lot of subtly here in the spec wording so I'm
going to go read the spec again.
>> 2. Restore the patch
Presumably you mean "revert" here? That might be the right way to go,
just to avoid breaking users (even if we fix the kernel bug, it'll take
a while to get everyone to update). That said, this smells like the
sort of thing that's going to crop up at arbitrary times in dynamic
systems so while a revert looks like it'd work around the boot issue we
might be making more headaches for folks down the road.
> I agree with you partly, my test case is actually from linux kernel, I notice
> the strange sfence.vma before write satp during write our teaching kernel.
> I think, the strange code is used to bypass the qemu bug that Idan patched.
> Because in hardware, if the stap is empty, sfence.vma will do nothing.
> And that's why nobody report it.
IIUC the sfence.vma before the SATP write is very explicitly necessary:
without that fence old mappings could be utilized directly after the
SATP write, so we might not even be able to fetch the next instruction.
> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
> place in the same BB with write satp, instead of the following write stvec.
> (If don't reorder, sfence.vma will place in the same BB with write stvec,
> that will crash kernel, see my origin analysis).
>
> However, in hardware, since tlb is empty, put the first sfence.vma before or
> after write satp is not really matters.
> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
> write stap to invalid qemu's translation cache.
The ISA manual is quite explicit about SATP not enforcing these
orderings. If what I remember about ASID 0 is true then I do think we'd
need one, though, to avoid the aliasing -- hopefully I'll make a bit
more sense soon, though...
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 16:11 ` Palmer Dabbelt
(?)
@ 2022-03-30 17:06 ` Palmer Dabbelt
-1 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 17:06 UTC (permalink / raw)
To: phantom
Cc: linux-riscv, idan.horowitz, Alistair Francis, qemu-riscv, qemu-devel
On Wed, 30 Mar 2022 09:11:18 PDT (-0700), Palmer Dabbelt wrote:
> [re-ordering the top post]
>
> +linux-riscv, as this may very well be a kernel bug
>
> On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>>> -----Original Messages-----
>>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>>> To: "Atish Patra" <atishp@atishpatra.org>
>>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>>
>>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>>> >
>>> > I tested on v5.17 built from defconfig for rv64.
>>> >
>>> > Here is the kernel code executing sfence.vma
>>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>>> >
>>>
>>> I believe this is a kernel bug and not a QEMU one. They perform a
>>> write to the SATP with the same ASID as the one used before (0) and
>
> I seem to remember ASID 0 being a special one, with some global-ness,
> but I couldn't find that in a quick poke into the spec. As below, I'm
> going to read through this a bit more...
>
> (Also, I haven't had any coffee yet)
>
>>> then expect it to be used, without performing an sfence.vma following
>>> it.
>>> This was exposed by my change, as previously the write to the satp was
>>> performed in the same TB block as the sfence.vma *before it*, which
>>> meant the TLB was not filled in between the previous sfence and the
>>> write to SATP following it.
>>> I was able to reproduce the issue with the Fedora Rawhide image in the
>>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>>> following all writes to SATP.
>>> I think the correct course of action here is to:
>>> 1. Report the issue to the linux kernel mailing list and/or contribute
>>> a patch that adds said missing sfence.vma following the SATP write.
>>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>>> build fixes the issue?)
>
> If it's a kernel bug we should fix it, but I'm not entirely convinced
> that's the case. I can confirm that the following makes Linux boot
> again
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index ec07f991866a..83373c2bd7d0 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -121,6 +121,7 @@ relocate:
> or a0, a0, a1
> sfence.vma
> csrw CSR_SATP, a0
> + sfence.vma
> .align 2
> 1:
> /* Set trap vector to spin forever to help debug */
>
> It's been a while since I read through the rules here so I'm going to go
> read them again, but IIRC that shouldn't be necessary: that first
> sfence.vma should be sufficiently global to ensure all prior writes to
> the page tables are visible, regardless of what's in SATP. That said, I
> remember there being a lot of subtly here in the spec wording so I'm
> going to go read the spec again.
>
>>> 2. Restore the patch
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
>> I agree with you partly, my test case is actually from linux kernel, I notice
>> the strange sfence.vma before write satp during write our teaching kernel.
>> I think, the strange code is used to bypass the qemu bug that Idan patched.
>> Because in hardware, if the stap is empty, sfence.vma will do nothing.
>> And that's why nobody report it.
>
> IIUC the sfence.vma before the SATP write is very explicitly necessary:
> without that fence old mappings could be utilized directly after the
> SATP write, so we might not even be able to fetch the next instruction.
>
>> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
>> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
>> place in the same BB with write satp, instead of the following write stvec.
>> (If don't reorder, sfence.vma will place in the same BB with write stvec,
>> that will crash kernel, see my origin analysis).
>>
>> However, in hardware, since tlb is empty, put the first sfence.vma before or
>> after write satp is not really matters.
>> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
>> write stap to invalid qemu's translation cache.
>
> The ISA manual is quite explicit about SATP not enforcing these
> orderings. If what I remember about ASID 0 is true then I do think we'd
> need one, though, to avoid the aliasing -- hopefully I'll make a bit
> more sense soon, though...
OK, I must have just been crazy -- there's nothing special about ASID=0.
Looks to me like flushing the TLB on SATP writes is necessary, I just
sent a patch with a more concrete description of why.
Thanks!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 17:06 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 17:06 UTC (permalink / raw)
To: phantom
Cc: Alistair Francis, linux-riscv, idan.horowitz, qemu-riscv, qemu-devel
On Wed, 30 Mar 2022 09:11:18 PDT (-0700), Palmer Dabbelt wrote:
> [re-ordering the top post]
>
> +linux-riscv, as this may very well be a kernel bug
>
> On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>>> -----Original Messages-----
>>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>>> To: "Atish Patra" <atishp@atishpatra.org>
>>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>>
>>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>>> >
>>> > I tested on v5.17 built from defconfig for rv64.
>>> >
>>> > Here is the kernel code executing sfence.vma
>>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>>> >
>>>
>>> I believe this is a kernel bug and not a QEMU one. They perform a
>>> write to the SATP with the same ASID as the one used before (0) and
>
> I seem to remember ASID 0 being a special one, with some global-ness,
> but I couldn't find that in a quick poke into the spec. As below, I'm
> going to read through this a bit more...
>
> (Also, I haven't had any coffee yet)
>
>>> then expect it to be used, without performing an sfence.vma following
>>> it.
>>> This was exposed by my change, as previously the write to the satp was
>>> performed in the same TB block as the sfence.vma *before it*, which
>>> meant the TLB was not filled in between the previous sfence and the
>>> write to SATP following it.
>>> I was able to reproduce the issue with the Fedora Rawhide image in the
>>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>>> following all writes to SATP.
>>> I think the correct course of action here is to:
>>> 1. Report the issue to the linux kernel mailing list and/or contribute
>>> a patch that adds said missing sfence.vma following the SATP write.
>>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>>> build fixes the issue?)
>
> If it's a kernel bug we should fix it, but I'm not entirely convinced
> that's the case. I can confirm that the following makes Linux boot
> again
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index ec07f991866a..83373c2bd7d0 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -121,6 +121,7 @@ relocate:
> or a0, a0, a1
> sfence.vma
> csrw CSR_SATP, a0
> + sfence.vma
> .align 2
> 1:
> /* Set trap vector to spin forever to help debug */
>
> It's been a while since I read through the rules here so I'm going to go
> read them again, but IIRC that shouldn't be necessary: that first
> sfence.vma should be sufficiently global to ensure all prior writes to
> the page tables are visible, regardless of what's in SATP. That said, I
> remember there being a lot of subtly here in the spec wording so I'm
> going to go read the spec again.
>
>>> 2. Restore the patch
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
>> I agree with you partly, my test case is actually from linux kernel, I notice
>> the strange sfence.vma before write satp during write our teaching kernel.
>> I think, the strange code is used to bypass the qemu bug that Idan patched.
>> Because in hardware, if the stap is empty, sfence.vma will do nothing.
>> And that's why nobody report it.
>
> IIUC the sfence.vma before the SATP write is very explicitly necessary:
> without that fence old mappings could be utilized directly after the
> SATP write, so we might not even be able to fetch the next instruction.
>
>> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
>> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
>> place in the same BB with write satp, instead of the following write stvec.
>> (If don't reorder, sfence.vma will place in the same BB with write stvec,
>> that will crash kernel, see my origin analysis).
>>
>> However, in hardware, since tlb is empty, put the first sfence.vma before or
>> after write satp is not really matters.
>> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
>> write stap to invalid qemu's translation cache.
>
> The ISA manual is quite explicit about SATP not enforcing these
> orderings. If what I remember about ASID 0 is true then I do think we'd
> need one, though, to avoid the aliasing -- hopefully I'll make a bit
> more sense soon, though...
OK, I must have just been crazy -- there's nothing special about ASID=0.
Looks to me like flushing the TLB on SATP writes is necessary, I just
sent a patch with a more concrete description of why.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 17:06 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-30 17:06 UTC (permalink / raw)
To: phantom
Cc: linux-riscv, idan.horowitz, Alistair Francis, qemu-riscv, qemu-devel
On Wed, 30 Mar 2022 09:11:18 PDT (-0700), Palmer Dabbelt wrote:
> [re-ordering the top post]
>
> +linux-riscv, as this may very well be a kernel bug
>
> On Wed, 30 Mar 2022 05:38:30 PDT (-0700), phantom@zju.edu.cn wrote:
>>> -----Original Messages-----
>>> From: "Idan Horowitz" <idan.horowitz@gmail.com>
>>> Sent Time: 2022-03-30 15:35:19 (Wednesday)
>>> To: "Atish Patra" <atishp@atishpatra.org>
>>> Cc: phantom@zju.edu.cn, "open list:RISC-V" <qemu-riscv@nongnu.org>, "Alistair Francis" <Alistair.Francis@wdc.com>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
>>> Subject: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
>>>
>>> On Wed, 30 Mar 2022 at 10:28, Atish Patra <atishp@atishpatra.org> wrote:
>>> >
>>> > I tested on v5.17 built from defconfig for rv64.
>>> >
>>> > Here is the kernel code executing sfence.vma
>>> > https://elixir.bootlin.com/linux/v5.17/source/arch/riscv/kernel/head.S#L122
>>> >
>>>
>>> I believe this is a kernel bug and not a QEMU one. They perform a
>>> write to the SATP with the same ASID as the one used before (0) and
>
> I seem to remember ASID 0 being a special one, with some global-ness,
> but I couldn't find that in a quick poke into the spec. As below, I'm
> going to read through this a bit more...
>
> (Also, I haven't had any coffee yet)
>
>>> then expect it to be used, without performing an sfence.vma following
>>> it.
>>> This was exposed by my change, as previously the write to the satp was
>>> performed in the same TB block as the sfence.vma *before it*, which
>>> meant the TLB was not filled in between the previous sfence and the
>>> write to SATP following it.
>>> I was able to reproduce the issue with the Fedora Rawhide image in the
>>> wiki, and I was able to resolve it by artificially forcing a TLB flush
>>> following all writes to SATP.
>>> I think the correct course of action here is to:
>>> 1. Report the issue to the linux kernel mailing list and/or contribute
>>> a patch that adds said missing sfence.vma following the SATP write.
>>> (Atish: Are you able to test if adding an sfence.vma in your kernel
>>> build fixes the issue?)
>
> If it's a kernel bug we should fix it, but I'm not entirely convinced
> that's the case. I can confirm that the following makes Linux boot
> again
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index ec07f991866a..83373c2bd7d0 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -121,6 +121,7 @@ relocate:
> or a0, a0, a1
> sfence.vma
> csrw CSR_SATP, a0
> + sfence.vma
> .align 2
> 1:
> /* Set trap vector to spin forever to help debug */
>
> It's been a while since I read through the rules here so I'm going to go
> read them again, but IIRC that shouldn't be necessary: that first
> sfence.vma should be sufficiently global to ensure all prior writes to
> the page tables are visible, regardless of what's in SATP. That said, I
> remember there being a lot of subtly here in the spec wording so I'm
> going to go read the spec again.
>
>>> 2. Restore the patch
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
>> I agree with you partly, my test case is actually from linux kernel, I notice
>> the strange sfence.vma before write satp during write our teaching kernel.
>> I think, the strange code is used to bypass the qemu bug that Idan patched.
>> Because in hardware, if the stap is empty, sfence.vma will do nothing.
>> And that's why nobody report it.
>
> IIUC the sfence.vma before the SATP write is very explicitly necessary:
> without that fence old mappings could be utilized directly after the
> SATP write, so we might not even be able to fetch the next instruction.
>
>> Before patch, qemu won't end a BB after sfence (but jump and CSR write do).
>> So, the kernel author reodered write stap and sfence.vma to make sfence.vma
>> place in the same BB with write satp, instead of the following write stvec.
>> (If don't reorder, sfence.vma will place in the same BB with write stvec,
>> that will crash kernel, see my origin analysis).
>>
>> However, in hardware, since tlb is empty, put the first sfence.vma before or
>> after write satp is not really matters.
>> In qemu, as Atish's log shown, we should do a imply invisible sfence.vma after
>> write stap to invalid qemu's translation cache.
>
> The ISA manual is quite explicit about SATP not enforcing these
> orderings. If what I remember about ASID 0 is true then I do think we'd
> need one, though, to avoid the aliasing -- hopefully I'll make a bit
> more sense soon, though...
OK, I must have just been crazy -- there's nothing special about ASID=0.
Looks to me like flushing the TLB on SATP writes is necessary, I just
sent a patch with a more concrete description of why.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 16:11 ` Palmer Dabbelt
(?)
@ 2022-03-30 17:10 ` Idan Horowitz
-1 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 17:10 UTC (permalink / raw)
To: Palmer Dabbelt, linux-riscv
Cc: phantom, Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
The opposite in fact, I did not suggest to revert it, but rather undo
the revert (as Alistair already removed it from the apply-next tree),
since my original patch fixes buggy behaviour that is blocking the
testing of some embedded software on QEMU.
Idan Horowitz
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 17:10 ` Idan Horowitz
0 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 17:10 UTC (permalink / raw)
To: Palmer Dabbelt, linux-riscv
Cc: open list:RISC-V, phantom, Alistair Francis,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
The opposite in fact, I did not suggest to revert it, but rather undo
the revert (as Alistair already removed it from the apply-next tree),
since my original patch fixes buggy behaviour that is blocking the
testing of some embedded software on QEMU.
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-30 17:10 ` Idan Horowitz
0 siblings, 0 replies; 32+ messages in thread
From: Idan Horowitz @ 2022-03-30 17:10 UTC (permalink / raw)
To: Palmer Dabbelt, linux-riscv
Cc: phantom, Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
>
> Presumably you mean "revert" here? That might be the right way to go,
> just to avoid breaking users (even if we fix the kernel bug, it'll take
> a while to get everyone to update). That said, this smells like the
> sort of thing that's going to crop up at arbitrary times in dynamic
> systems so while a revert looks like it'd work around the boot issue we
> might be making more headaches for folks down the road.
>
The opposite in fact, I did not suggest to revert it, but rather undo
the revert (as Alistair already removed it from the apply-next tree),
since my original patch fixes buggy behaviour that is blocking the
testing of some embedded software on QEMU.
Idan Horowitz
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-30 17:10 ` Idan Horowitz
(?)
@ 2022-03-31 3:23 ` Alistair Francis
-1 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 3:23 UTC (permalink / raw)
To: Idan Horowitz
Cc: Palmer Dabbelt, linux-riscv, open list:RISC-V, phantom,
Alistair Francis, qemu-devel@nongnu.org Developers
On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> >
> > Presumably you mean "revert" here? That might be the right way to go,
> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> > a while to get everyone to update). That said, this smells like the
> > sort of thing that's going to crop up at arbitrary times in dynamic
> > systems so while a revert looks like it'd work around the boot issue we
> > might be making more headaches for folks down the road.
> >
>
> The opposite in fact, I did not suggest to revert it, but rather undo
> the revert (as Alistair already removed it from the apply-next tree),
> since my original patch fixes buggy behaviour that is blocking the
> testing of some embedded software on QEMU.
So, this is a little tricky.
We want to apply the fix, but that will break current users.
Once the fix is merged into Linux we can apply it here. That should
hopefully be right at the start of the 7.1 QEMU development window,
which should give time for the fix to propagate into stable kernels
and not break too many people by the time QEMU is released.
Alistair
>
> Idan Horowitz
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 3:23 ` Alistair Francis
0 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 3:23 UTC (permalink / raw)
To: Idan Horowitz
Cc: phantom, open list:RISC-V, qemu-devel@nongnu.org Developers,
Palmer Dabbelt, Alistair Francis, linux-riscv
On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> >
> > Presumably you mean "revert" here? That might be the right way to go,
> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> > a while to get everyone to update). That said, this smells like the
> > sort of thing that's going to crop up at arbitrary times in dynamic
> > systems so while a revert looks like it'd work around the boot issue we
> > might be making more headaches for folks down the road.
> >
>
> The opposite in fact, I did not suggest to revert it, but rather undo
> the revert (as Alistair already removed it from the apply-next tree),
> since my original patch fixes buggy behaviour that is blocking the
> testing of some embedded software on QEMU.
So, this is a little tricky.
We want to apply the fix, but that will break current users.
Once the fix is merged into Linux we can apply it here. That should
hopefully be right at the start of the 7.1 QEMU development window,
which should give time for the fix to propagate into stable kernels
and not break too many people by the time QEMU is released.
Alistair
>
> Idan Horowitz
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 3:23 ` Alistair Francis
0 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 3:23 UTC (permalink / raw)
To: Idan Horowitz
Cc: Palmer Dabbelt, linux-riscv, open list:RISC-V, phantom,
Alistair Francis, qemu-devel@nongnu.org Developers
On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>
> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> >
> > Presumably you mean "revert" here? That might be the right way to go,
> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> > a while to get everyone to update). That said, this smells like the
> > sort of thing that's going to crop up at arbitrary times in dynamic
> > systems so while a revert looks like it'd work around the boot issue we
> > might be making more headaches for folks down the road.
> >
>
> The opposite in fact, I did not suggest to revert it, but rather undo
> the revert (as Alistair already removed it from the apply-next tree),
> since my original patch fixes buggy behaviour that is blocking the
> testing of some embedded software on QEMU.
So, this is a little tricky.
We want to apply the fix, but that will break current users.
Once the fix is merged into Linux we can apply it here. That should
hopefully be right at the start of the 7.1 QEMU development window,
which should give time for the fix to propagate into stable kernels
and not break too many people by the time QEMU is released.
Alistair
>
> Idan Horowitz
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-31 3:23 ` Alistair Francis
(?)
@ 2022-03-31 4:36 ` Palmer Dabbelt
-1 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 4:36 UTC (permalink / raw)
To: alistair23
Cc: idan.horowitz, linux-riscv, qemu-riscv, phantom,
Alistair Francis, qemu-devel
On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>>
>> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >
>> >
>> > Presumably you mean "revert" here? That might be the right way to go,
>> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> > a while to get everyone to update). That said, this smells like the
>> > sort of thing that's going to crop up at arbitrary times in dynamic
>> > systems so while a revert looks like it'd work around the boot issue we
>> > might be making more headaches for folks down the road.
>> >
>>
>> The opposite in fact, I did not suggest to revert it, but rather undo
>> the revert (as Alistair already removed it from the apply-next tree),
>> since my original patch fixes buggy behaviour that is blocking the
>> testing of some embedded software on QEMU.
Ah, sorry -- the QEMU tree I was looking at still had the patch in
there, must have just been an old one.
> So, this is a little tricky.
>
> We want to apply the fix, but that will break current users.
>
> Once the fix is merged into Linux we can apply it here. That should
> hopefully be right at the start of the 7.1 QEMU development window,
> which should give time for the fix to propagate into stable kernels
> and not break too many people by the time QEMU is released.
If you think this is a Linux bug then that makes sense, but I think this
is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
make it to lore.
I also think the bug will manifest without the TB exit patch, maybe in
single-step mode and definately if we happen to exit the TB at that
point for other reasons. Assuming my reasoning is correct in that
patch, we may also be hitting this as arbitrary corruption anywhere.
I'd started to write up a "QEMU errata" Linux patch for this, but then
convinced myself that just adding the sfence.vma was insufficient.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 4:36 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 4:36 UTC (permalink / raw)
To: alistair23
Cc: phantom, qemu-riscv, qemu-devel, Alistair Francis, linux-riscv,
idan.horowitz
On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>>
>> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >
>> >
>> > Presumably you mean "revert" here? That might be the right way to go,
>> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> > a while to get everyone to update). That said, this smells like the
>> > sort of thing that's going to crop up at arbitrary times in dynamic
>> > systems so while a revert looks like it'd work around the boot issue we
>> > might be making more headaches for folks down the road.
>> >
>>
>> The opposite in fact, I did not suggest to revert it, but rather undo
>> the revert (as Alistair already removed it from the apply-next tree),
>> since my original patch fixes buggy behaviour that is blocking the
>> testing of some embedded software on QEMU.
Ah, sorry -- the QEMU tree I was looking at still had the patch in
there, must have just been an old one.
> So, this is a little tricky.
>
> We want to apply the fix, but that will break current users.
>
> Once the fix is merged into Linux we can apply it here. That should
> hopefully be right at the start of the 7.1 QEMU development window,
> which should give time for the fix to propagate into stable kernels
> and not break too many people by the time QEMU is released.
If you think this is a Linux bug then that makes sense, but I think this
is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
make it to lore.
I also think the bug will manifest without the TB exit patch, maybe in
single-step mode and definately if we happen to exit the TB at that
point for other reasons. Assuming my reasoning is correct in that
patch, we may also be hitting this as arbitrary corruption anywhere.
I'd started to write up a "QEMU errata" Linux patch for this, but then
convinced myself that just adding the sfence.vma was insufficient.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 4:36 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 4:36 UTC (permalink / raw)
To: alistair23
Cc: idan.horowitz, linux-riscv, qemu-riscv, phantom,
Alistair Francis, qemu-devel
On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>>
>> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >
>> >
>> > Presumably you mean "revert" here? That might be the right way to go,
>> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> > a while to get everyone to update). That said, this smells like the
>> > sort of thing that's going to crop up at arbitrary times in dynamic
>> > systems so while a revert looks like it'd work around the boot issue we
>> > might be making more headaches for folks down the road.
>> >
>>
>> The opposite in fact, I did not suggest to revert it, but rather undo
>> the revert (as Alistair already removed it from the apply-next tree),
>> since my original patch fixes buggy behaviour that is blocking the
>> testing of some embedded software on QEMU.
Ah, sorry -- the QEMU tree I was looking at still had the patch in
there, must have just been an old one.
> So, this is a little tricky.
>
> We want to apply the fix, but that will break current users.
>
> Once the fix is merged into Linux we can apply it here. That should
> hopefully be right at the start of the 7.1 QEMU development window,
> which should give time for the fix to propagate into stable kernels
> and not break too many people by the time QEMU is released.
If you think this is a Linux bug then that makes sense, but I think this
is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
make it to lore.
I also think the bug will manifest without the TB exit patch, maybe in
single-step mode and definately if we happen to exit the TB at that
point for other reasons. Assuming my reasoning is correct in that
patch, we may also be hitting this as arbitrary corruption anywhere.
I'd started to write up a "QEMU errata" Linux patch for this, but then
convinced myself that just adding the sfence.vma was insufficient.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-31 4:36 ` Palmer Dabbelt
(?)
@ 2022-03-31 5:13 ` Alistair Francis
-1 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 5:13 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Idan Horowitz, linux-riscv, open list:RISC-V, phantom,
Alistair Francis, qemu-devel@nongnu.org Developers
On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
> >>
> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> >
> >> >
> >> > Presumably you mean "revert" here? That might be the right way to go,
> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> >> > a while to get everyone to update). That said, this smells like the
> >> > sort of thing that's going to crop up at arbitrary times in dynamic
> >> > systems so while a revert looks like it'd work around the boot issue we
> >> > might be making more headaches for folks down the road.
> >> >
> >>
> >> The opposite in fact, I did not suggest to revert it, but rather undo
> >> the revert (as Alistair already removed it from the apply-next tree),
> >> since my original patch fixes buggy behaviour that is blocking the
> >> testing of some embedded software on QEMU.
>
> Ah, sorry -- the QEMU tree I was looking at still had the patch in
> there, must have just been an old one.
>
> > So, this is a little tricky.
> >
> > We want to apply the fix, but that will break current users.
> >
> > Once the fix is merged into Linux we can apply it here. That should
> > hopefully be right at the start of the 7.1 QEMU development window,
> > which should give time for the fix to propagate into stable kernels
> > and not break too many people by the time QEMU is released.
>
> If you think this is a Linux bug then that makes sense, but I think this
> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
> make it to lore.
Ah whoops. I saw the patch but didn't read it, then I assumed it was a
Linux bug from your diff earlier.
>
> I also think the bug will manifest without the TB exit patch, maybe in
> single-step mode and definately if we happen to exit the TB at that
> point for other reasons. Assuming my reasoning is correct in that
> patch, we may also be hitting this as arbitrary corruption anywhere.
> I'd started to write up a "QEMU errata" Linux patch for this, but then
> convinced myself that just adding the sfence.vma was insufficient.
Yeah, looking at it now I agree, I'll send a PR for 7.0.
Alistair
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 5:13 ` Alistair Francis
0 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 5:13 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: phantom, open list:RISC-V, qemu-devel@nongnu.org Developers,
Alistair Francis, linux-riscv, Idan Horowitz
On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
> >>
> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> >
> >> >
> >> > Presumably you mean "revert" here? That might be the right way to go,
> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> >> > a while to get everyone to update). That said, this smells like the
> >> > sort of thing that's going to crop up at arbitrary times in dynamic
> >> > systems so while a revert looks like it'd work around the boot issue we
> >> > might be making more headaches for folks down the road.
> >> >
> >>
> >> The opposite in fact, I did not suggest to revert it, but rather undo
> >> the revert (as Alistair already removed it from the apply-next tree),
> >> since my original patch fixes buggy behaviour that is blocking the
> >> testing of some embedded software on QEMU.
>
> Ah, sorry -- the QEMU tree I was looking at still had the patch in
> there, must have just been an old one.
>
> > So, this is a little tricky.
> >
> > We want to apply the fix, but that will break current users.
> >
> > Once the fix is merged into Linux we can apply it here. That should
> > hopefully be right at the start of the 7.1 QEMU development window,
> > which should give time for the fix to propagate into stable kernels
> > and not break too many people by the time QEMU is released.
>
> If you think this is a Linux bug then that makes sense, but I think this
> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
> make it to lore.
Ah whoops. I saw the patch but didn't read it, then I assumed it was a
Linux bug from your diff earlier.
>
> I also think the bug will manifest without the TB exit patch, maybe in
> single-step mode and definately if we happen to exit the TB at that
> point for other reasons. Assuming my reasoning is correct in that
> patch, we may also be hitting this as arbitrary corruption anywhere.
> I'd started to write up a "QEMU errata" Linux patch for this, but then
> convinced myself that just adding the sfence.vma was insufficient.
Yeah, looking at it now I agree, I'll send a PR for 7.0.
Alistair
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 5:13 ` Alistair Francis
0 siblings, 0 replies; 32+ messages in thread
From: Alistair Francis @ 2022-03-31 5:13 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Idan Horowitz, linux-riscv, open list:RISC-V, phantom,
Alistair Francis, qemu-devel@nongnu.org Developers
On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
> >>
> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> >
> >> >
> >> > Presumably you mean "revert" here? That might be the right way to go,
> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
> >> > a while to get everyone to update). That said, this smells like the
> >> > sort of thing that's going to crop up at arbitrary times in dynamic
> >> > systems so while a revert looks like it'd work around the boot issue we
> >> > might be making more headaches for folks down the road.
> >> >
> >>
> >> The opposite in fact, I did not suggest to revert it, but rather undo
> >> the revert (as Alistair already removed it from the apply-next tree),
> >> since my original patch fixes buggy behaviour that is blocking the
> >> testing of some embedded software on QEMU.
>
> Ah, sorry -- the QEMU tree I was looking at still had the patch in
> there, must have just been an old one.
>
> > So, this is a little tricky.
> >
> > We want to apply the fix, but that will break current users.
> >
> > Once the fix is merged into Linux we can apply it here. That should
> > hopefully be right at the start of the 7.1 QEMU development window,
> > which should give time for the fix to propagate into stable kernels
> > and not break too many people by the time QEMU is released.
>
> If you think this is a Linux bug then that makes sense, but I think this
> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
> make it to lore.
Ah whoops. I saw the patch but didn't read it, then I assumed it was a
Linux bug from your diff earlier.
>
> I also think the bug will manifest without the TB exit patch, maybe in
> single-step mode and definately if we happen to exit the TB at that
> point for other reasons. Assuming my reasoning is correct in that
> patch, we may also be hitting this as arbitrary corruption anywhere.
> I'd started to write up a "QEMU errata" Linux patch for this, but then
> convinced myself that just adding the sfence.vma was insufficient.
Yeah, looking at it now I agree, I'll send a PR for 7.0.
Alistair
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
2022-03-31 5:13 ` Alistair Francis
(?)
@ 2022-03-31 19:54 ` Palmer Dabbelt
-1 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 19:54 UTC (permalink / raw)
To: alistair23
Cc: idan.horowitz, linux-riscv, qemu-riscv, phantom,
Alistair Francis, qemu-devel
On Wed, 30 Mar 2022 22:13:39 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
>> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>> >>
>> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >> >
>> >> >
>> >> > Presumably you mean "revert" here? That might be the right way to go,
>> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> >> > a while to get everyone to update). That said, this smells like the
>> >> > sort of thing that's going to crop up at arbitrary times in dynamic
>> >> > systems so while a revert looks like it'd work around the boot issue we
>> >> > might be making more headaches for folks down the road.
>> >> >
>> >>
>> >> The opposite in fact, I did not suggest to revert it, but rather undo
>> >> the revert (as Alistair already removed it from the apply-next tree),
>> >> since my original patch fixes buggy behaviour that is blocking the
>> >> testing of some embedded software on QEMU.
>>
>> Ah, sorry -- the QEMU tree I was looking at still had the patch in
>> there, must have just been an old one.
>>
>> > So, this is a little tricky.
>> >
>> > We want to apply the fix, but that will break current users.
>> >
>> > Once the fix is merged into Linux we can apply it here. That should
>> > hopefully be right at the start of the 7.1 QEMU development window,
>> > which should give time for the fix to propagate into stable kernels
>> > and not break too many people by the time QEMU is released.
>>
>> If you think this is a Linux bug then that makes sense, but I think this
>> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
>> make it to lore.
>
> Ah whoops. I saw the patch but didn't read it, then I assumed it was a
> Linux bug from your diff earlier.
No problem, that was the first thing I sent in the morning so I doubt it
made any sense.
>
>>
>> I also think the bug will manifest without the TB exit patch, maybe in
>> single-step mode and definately if we happen to exit the TB at that
>> point for other reasons. Assuming my reasoning is correct in that
>> patch, we may also be hitting this as arbitrary corruption anywhere.
>> I'd started to write up a "QEMU errata" Linux patch for this, but then
>> convinced myself that just adding the sfence.vma was insufficient.
>
> Yeah, looking at it now I agree, I'll send a PR for 7.0.
Thanks!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 19:54 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 19:54 UTC (permalink / raw)
To: alistair23
Cc: phantom, qemu-riscv, qemu-devel, Alistair Francis, linux-riscv,
idan.horowitz
On Wed, 30 Mar 2022 22:13:39 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
>> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>> >>
>> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >> >
>> >> >
>> >> > Presumably you mean "revert" here? That might be the right way to go,
>> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> >> > a while to get everyone to update). That said, this smells like the
>> >> > sort of thing that's going to crop up at arbitrary times in dynamic
>> >> > systems so while a revert looks like it'd work around the boot issue we
>> >> > might be making more headaches for folks down the road.
>> >> >
>> >>
>> >> The opposite in fact, I did not suggest to revert it, but rather undo
>> >> the revert (as Alistair already removed it from the apply-next tree),
>> >> since my original patch fixes buggy behaviour that is blocking the
>> >> testing of some embedded software on QEMU.
>>
>> Ah, sorry -- the QEMU tree I was looking at still had the patch in
>> there, must have just been an old one.
>>
>> > So, this is a little tricky.
>> >
>> > We want to apply the fix, but that will break current users.
>> >
>> > Once the fix is merged into Linux we can apply it here. That should
>> > hopefully be right at the start of the 7.1 QEMU development window,
>> > which should give time for the fix to propagate into stable kernels
>> > and not break too many people by the time QEMU is released.
>>
>> If you think this is a Linux bug then that makes sense, but I think this
>> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
>> make it to lore.
>
> Ah whoops. I saw the patch but didn't read it, then I assumed it was a
> Linux bug from your diff earlier.
No problem, that was the first thing I sent in the morning so I doubt it
made any sense.
>
>>
>> I also think the bug will manifest without the TB exit patch, maybe in
>> single-step mode and definately if we happen to exit the TB at that
>> point for other reasons. Assuming my reasoning is correct in that
>> patch, we may also be hitting this as arbitrary corruption anywhere.
>> I'd started to write up a "QEMU errata" Linux patch for this, but then
>> convinced myself that just adding the sfence.vma was insufficient.
>
> Yeah, looking at it now I agree, I'll send a PR for 7.0.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH] target/riscv: Exit current TB after an sfence.vma
@ 2022-03-31 19:54 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2022-03-31 19:54 UTC (permalink / raw)
To: alistair23
Cc: idan.horowitz, linux-riscv, qemu-riscv, phantom,
Alistair Francis, qemu-devel
On Wed, 30 Mar 2022 22:13:39 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Mar 31, 2022 at 2:36 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Wed, 30 Mar 2022 20:23:21 PDT (-0700), alistair23@gmail.com wrote:
>> > On Thu, Mar 31, 2022 at 3:11 AM Idan Horowitz <idan.horowitz@gmail.com> wrote:
>> >>
>> >> On Wed, 30 Mar 2022 at 19:11, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >> >
>> >> >
>> >> > Presumably you mean "revert" here? That might be the right way to go,
>> >> > just to avoid breaking users (even if we fix the kernel bug, it'll take
>> >> > a while to get everyone to update). That said, this smells like the
>> >> > sort of thing that's going to crop up at arbitrary times in dynamic
>> >> > systems so while a revert looks like it'd work around the boot issue we
>> >> > might be making more headaches for folks down the road.
>> >> >
>> >>
>> >> The opposite in fact, I did not suggest to revert it, but rather undo
>> >> the revert (as Alistair already removed it from the apply-next tree),
>> >> since my original patch fixes buggy behaviour that is blocking the
>> >> testing of some embedded software on QEMU.
>>
>> Ah, sorry -- the QEMU tree I was looking at still had the patch in
>> there, must have just been an old one.
>>
>> > So, this is a little tricky.
>> >
>> > We want to apply the fix, but that will break current users.
>> >
>> > Once the fix is merged into Linux we can apply it here. That should
>> > hopefully be right at the start of the 7.1 QEMU development window,
>> > which should give time for the fix to propagate into stable kernels
>> > and not break too many people by the time QEMU is released.
>>
>> If you think this is a Linux bug then that makes sense, but I think this
>> is a QEMU bug -- I sent a patch, not sure if it went through as it didn't
>> make it to lore.
>
> Ah whoops. I saw the patch but didn't read it, then I assumed it was a
> Linux bug from your diff earlier.
No problem, that was the first thing I sent in the morning so I doubt it
made any sense.
>
>>
>> I also think the bug will manifest without the TB exit patch, maybe in
>> single-step mode and definately if we happen to exit the TB at that
>> point for other reasons. Assuming my reasoning is correct in that
>> patch, we may also be hitting this as arbitrary corruption anywhere.
>> I'd started to write up a "QEMU errata" Linux patch for this, but then
>> convinced myself that just adding the sfence.vma was insufficient.
>
> Yeah, looking at it now I agree, I'll send a PR for 7.0.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2022-03-31 19:56 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 17:22 [PATCH] target/riscv: Exit current TB after an sfence.vma phantom
2022-03-29 23:15 ` Atish Patra
2022-03-29 23:15 ` Atish Patra
2022-03-30 6:15 ` Idan Horowitz
2022-03-30 6:15 ` Idan Horowitz
2022-03-30 7:28 ` Atish Patra
2022-03-30 7:28 ` Atish Patra
2022-03-30 7:35 ` Idan Horowitz
2022-03-30 7:35 ` Idan Horowitz
2022-03-30 12:38 ` phantom
2022-03-30 12:38 ` phantom
2022-03-30 16:11 ` Palmer Dabbelt
2022-03-30 16:11 ` Palmer Dabbelt
2022-03-30 16:11 ` Palmer Dabbelt
2022-03-30 17:06 ` Palmer Dabbelt
2022-03-30 17:06 ` Palmer Dabbelt
2022-03-30 17:06 ` Palmer Dabbelt
2022-03-30 17:10 ` Idan Horowitz
2022-03-30 17:10 ` Idan Horowitz
2022-03-30 17:10 ` Idan Horowitz
2022-03-31 3:23 ` Alistair Francis
2022-03-31 3:23 ` Alistair Francis
2022-03-31 3:23 ` Alistair Francis
2022-03-31 4:36 ` Palmer Dabbelt
2022-03-31 4:36 ` Palmer Dabbelt
2022-03-31 4:36 ` Palmer Dabbelt
2022-03-31 5:13 ` Alistair Francis
2022-03-31 5:13 ` Alistair Francis
2022-03-31 5:13 ` Alistair Francis
2022-03-31 19:54 ` Palmer Dabbelt
2022-03-31 19:54 ` Palmer Dabbelt
2022-03-31 19:54 ` Palmer Dabbelt
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.