| Message ID | 20230203171510.2867451-1-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: test CPUJumpCache in tb_jmp_cache_clear_page() | expand |
On 3/2/23 18:15, Eric Auger wrote: > After commit 4e4fa6c12d ("accel/tcg: Complete cpu initialization > before registration"), it looks the CPUJumpCache handle can be NULL. > This causes a SIGSEV when running debug-wp-migration kvm unit test. Do you mean commit a976a99a29 ("include/hw/core: Create struct CPUJumpCache") instead? > At the first place it should be clarified why this TCG code is called > with KVM acceleration. This may hide another bug. > > Fixes: 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration") > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > accel/tcg/cputlb.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c > index 4e040a1cb9..ac0245ee6c 100644 > --- a/accel/tcg/cputlb.c > +++ b/accel/tcg/cputlb.c > @@ -103,6 +103,10 @@ static void tb_jmp_cache_clear_page(CPUState *cpu, target_ulong page_addr) > int i, i0 = tb_jmp_cache_hash_page(page_addr); > CPUJumpCache *jc = cpu->tb_jmp_cache; > > + if (!jc) { unlikely()? > + return; > + } > + > for (i = 0; i < TB_JMP_PAGE_SIZE; i++) { > qatomic_set(&jc->array[i0 + i].tb, NULL); > }
Hi Philippe, On 2/3/23 18:21, Philippe Mathieu-Daudé wrote: > On 3/2/23 18:15, Eric Auger wrote: >> After commit 4e4fa6c12d ("accel/tcg: Complete cpu initialization >> before registration"), it looks the CPUJumpCache handle can be NULL. >> This causes a SIGSEV when running debug-wp-migration kvm unit test. > > Do you mean commit a976a99a29 ("include/hw/core: Create struct > CPUJumpCache") instead? No as reported in https://lore.kernel.org/all/d91ccc02-a963-946d-169a-fd4da2610861@redhat.com/ my bisection pointed to 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration") > >> At the first place it should be clarified why this TCG code is called >> with KVM acceleration. This may hide another bug. >> >> Fixes: 4e4fa6c12d ("accel/tcg: Complete cpu initialization before >> registration") >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> accel/tcg/cputlb.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c >> index 4e040a1cb9..ac0245ee6c 100644 >> --- a/accel/tcg/cputlb.c >> +++ b/accel/tcg/cputlb.c >> @@ -103,6 +103,10 @@ static void tb_jmp_cache_clear_page(CPUState >> *cpu, target_ulong page_addr) >> int i, i0 = tb_jmp_cache_hash_page(page_addr); >> CPUJumpCache *jc = cpu->tb_jmp_cache; >> + if (!jc) { > > unlikely()? yes maybe Thanks Eric > >> + return; >> + } >> + >> for (i = 0; i < TB_JMP_PAGE_SIZE; i++) { >> qatomic_set(&jc->array[i0 + i].tb, NULL); >> } >
On 2/3/23 07:15, Eric Auger wrote: > After commit 4e4fa6c12d ("accel/tcg: Complete cpu initialization > before registration"), it looks the CPUJumpCache handle can be NULL. > This causes a SIGSEV when running debug-wp-migration kvm unit test. > > At the first place it should be clarified why this TCG code is called > with KVM acceleration. This may hide another bug. > > Fixes: 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration") > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > accel/tcg/cputlb.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c > index 4e040a1cb9..ac0245ee6c 100644 > --- a/accel/tcg/cputlb.c > +++ b/accel/tcg/cputlb.c > @@ -103,6 +103,10 @@ static void tb_jmp_cache_clear_page(CPUState *cpu, target_ulong page_addr) > int i, i0 = tb_jmp_cache_hash_page(page_addr); > CPUJumpCache *jc = cpu->tb_jmp_cache; > > + if (!jc) { > + return; > + } While I think we shouldn't arrive here for KVM, it was previously not an error to do so. I'm going to go ahead and queue this while the correct cpregs change gets worked out. r~
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 4e040a1cb9..ac0245ee6c 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -103,6 +103,10 @@ static void tb_jmp_cache_clear_page(CPUState *cpu, target_ulong page_addr) int i, i0 = tb_jmp_cache_hash_page(page_addr); CPUJumpCache *jc = cpu->tb_jmp_cache; + if (!jc) { + return; + } + for (i = 0; i < TB_JMP_PAGE_SIZE; i++) { qatomic_set(&jc->array[i0 + i].tb, NULL); }
After commit 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration"), it looks the CPUJumpCache handle can be NULL. This causes a SIGSEV when running debug-wp-migration kvm unit test. At the first place it should be clarified why this TCG code is called with KVM acceleration. This may hide another bug. Fixes: 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration") Signed-off-by: Eric Auger <eric.auger@redhat.com> --- accel/tcg/cputlb.c | 4 ++++ 1 file changed, 4 insertions(+)