diff for duplicates of <20171214154518.GX910@cbox> diff --git a/a/1.txt b/N1/1.txt index d880e5d..bba224a 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -48,14 +48,14 @@ On Thu, Dec 14, 2017 at 11:28:04PM +0800, Jia He wrote: > >>problematic? > >> > >> -> >>vtimer_save_state (vtimer->loaded = false, cntv_ctl is 0) +> >>vtimer_save_state?????????? (vtimer->loaded = false, cntv_ctl is 0) > >> -> >>kvm_arch_timer_handler (read cntv_ctl and set vtimer->cnt_ctl = 0) +> >>kvm_arch_timer_handler????????(read cntv_ctl and set vtimer->cnt_ctl = 0) > >> -> >>vtimer_restore_state (write vtimer->cnt_ctl to cntv_ctl, +> >>vtimer_restore_state ? ? ? ? ?? (write vtimer->cnt_ctl to cntv_ctl, > >>then cntv_ctl will > >> -> >> be 0 forever) +> >> ??? ??? ??? ??? ?? ? ? be 0 forever) > >> > >> > >>If above analysis is reasonable @@ -71,21 +71,21 @@ On Thu, Dec 14, 2017 at 11:28:04PM +0800, Jia He wrote: > >>+++ b/virt/kvm/arm/arch_timer.c > >>@@ -99,7 +99,7 @@ static irqreturn_t kvm_arch_timer_handler(int irq, > >>void *dev_id) -> >> } -> >> vtimer = vcpu_vtimer(vcpu); +> >> ??????? } +> >> ??????? vtimer = vcpu_vtimer(vcpu); > >> -> >>- if (!vtimer->irq.level) { -> >>+ if (vtimer->loaded && !vtimer->irq.level) { -> >> vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl); -> >> if (kvm_timer_irq_can_fire(vtimer)) -> >> kvm_timer_update_irq(vcpu, true, vtimer); +> >>-?????? if (!vtimer->irq.level) { +> >>+?????? if (vtimer->loaded && !vtimer->irq.level) { +> >> ??????????????? vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl); +> >> ??????????????? if (kvm_timer_irq_can_fire(vtimer)) +> >> ??????????????????????? kvm_timer_update_irq(vcpu, true, vtimer); > >> > >There's nothing really wrong with that patch, I just didn't think it > >would be necessary, as we really shouldn't see interrupts if the timer > >is not loaded. Can you confirm that a WARN_ON(!vtimer->loaded) in > >kvm_arch_timer_handler() gives you a splat? > Please see the WARN_ON result (without my patch) -> [ 72.171706] WARNING: CPU: 24 PID: 1768 at +> [?? 72.171706] WARNING: CPU: 24 PID: 1768 at > arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:101 > kvm_arch_timer_handler+0xc0/0xc8 > @@ -99,7 +99,7 @@ On Thu, Dec 14, 2017 at 11:28:04PM +0800, Jia He wrote: > > /* Disable the virtual timer */ > > write_sysreg_el0(0, cntv_ctl); > >+ isb(); -> No luck， the bug is still there +> No luck? the bug is still there > ok, so this is a slightly different approach to what you were trying to diff --git a/a/content_digest b/N1/content_digest index 0e3a125..db33f78 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,15 +3,10 @@ "ref\0dc95b58c-ee6c-e5c7-1f37-8f69c789a1fc@gmail.com\0" "ref\020171214130954.GV910@cbox\0" "ref\05615f3e1-756e-0537-f0b6-20ae8626ac87@gmail.com\0" - "From\0Christoffer Dall <christoffer.dall@linaro.org>\0" - "Subject\0Re: [PATCH] KVM: arm/arm64: don't set vtimer->cnt_ctl in kvm_arch_timer_handler\0" + "From\0christoffer.dall@linaro.org (Christoffer Dall)\0" + "Subject\0[PATCH] KVM: arm/arm64: don't set vtimer->cnt_ctl in kvm_arch_timer_handler\0" "Date\0Thu, 14 Dec 2017 16:45:18 +0100\0" - "To\0Jia He <hejianet@gmail.com>\0" - "Cc\0Marc Zyngier <marc.zyngier@arm.com>" - linux-arm-kernel@lists.infradead.org - kvmarm@lists.cs.columbia.edu - linux-kernel@vger.kernel.org - " Jia He <jia.he@hxt-semitech.com>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "On Thu, Dec 14, 2017 at 11:28:04PM +0800, Jia He wrote:\n" @@ -64,14 +59,14 @@ "> >>problematic?\n" "> >>\n" "> >>\n" - "> >>vtimer_save_state\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240 (vtimer->loaded = false, cntv_ctl is 0)\n" + "> >>vtimer_save_state?????????? (vtimer->loaded = false, cntv_ctl is 0)\n" "> >>\n" - "> >>kvm_arch_timer_handler\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240(read cntv_ctl and set vtimer->cnt_ctl = 0)\n" + "> >>kvm_arch_timer_handler????????(read cntv_ctl and set vtimer->cnt_ctl = 0)\n" "> >>\n" - "> >>vtimer_restore_state \302\240 \302\240 \302\240 \302\240 \302\240\302\240 (write vtimer->cnt_ctl to cntv_ctl,\n" + "> >>vtimer_restore_state ? ? ? ? ?? (write vtimer->cnt_ctl to cntv_ctl,\n" "> >>then cntv_ctl will\n" "> >>\n" - "> >> \302\240\302\240\302\240 \302\240\302\240\302\240 \302\240\302\240\302\240 \302\240\302\240\302\240 \302\240\302\240 \302\240 \302\240 be 0 forever)\n" + "> >> ??? ??? ??? ??? ?? ? ? be 0 forever)\n" "> >>\n" "> >>\n" "> >>If above analysis is reasonable\n" @@ -87,21 +82,21 @@ "> >>+++ b/virt/kvm/arm/arch_timer.c\n" "> >>@@ -99,7 +99,7 @@ static irqreturn_t kvm_arch_timer_handler(int irq,\n" "> >>void *dev_id)\n" - "> >> \302\240\302\240\302\240\302\240\302\240\302\240\302\240 }\n" - "> >> \302\240\302\240\302\240\302\240\302\240\302\240\302\240 vtimer = vcpu_vtimer(vcpu);\n" + "> >> ??????? }\n" + "> >> ??????? vtimer = vcpu_vtimer(vcpu);\n" "> >>\n" - "> >>-\302\240\302\240\302\240\302\240\302\240\302\240 if (!vtimer->irq.level) {\n" - "> >>+\302\240\302\240\302\240\302\240\302\240\302\240 if (vtimer->loaded && !vtimer->irq.level) {\n" - "> >> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240 vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);\n" - "> >> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240 if (kvm_timer_irq_can_fire(vtimer))\n" - "> >> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240 kvm_timer_update_irq(vcpu, true, vtimer);\n" + "> >>-?????? if (!vtimer->irq.level) {\n" + "> >>+?????? if (vtimer->loaded && !vtimer->irq.level) {\n" + "> >> ??????????????? vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);\n" + "> >> ??????????????? if (kvm_timer_irq_can_fire(vtimer))\n" + "> >> ??????????????????????? kvm_timer_update_irq(vcpu, true, vtimer);\n" "> >>\n" "> >There's nothing really wrong with that patch, I just didn't think it\n" "> >would be necessary, as we really shouldn't see interrupts if the timer\n" "> >is not loaded. Can you confirm that a WARN_ON(!vtimer->loaded) in\n" "> >kvm_arch_timer_handler() gives you a splat?\n" "> Please see the WARN_ON result (without my patch)\n" - "> [\302\240\302\240 72.171706] WARNING: CPU: 24 PID: 1768 at\n" + "> [?? 72.171706] WARNING: CPU: 24 PID: 1768 at\n" "> arch/arm64/kvm/../../../virt/kvm/arm/arch_timer.c:101\n" "> kvm_arch_timer_handler+0xc0/0xc8\n" "> \n" @@ -115,7 +110,7 @@ "> > \t/* Disable the virtual timer */\n" "> > \twrite_sysreg_el0(0, cntv_ctl);\n" "> >+\tisb();\n" - "> No luck\357\274\214 the bug is still there\n" + "> No luck? the bug is still there\n" "> \n" "\n" "ok, so this is a slightly different approach to what you were trying to\n" @@ -221,4 +216,4 @@ "Thanks,\n" -Christoffer -e1af975562954610013692b8219bfe97ead36c818d0dfcc4ffbfacbdd3c5d171 +b18e6f5dac64486bd213cadda54135cd701528d0ee9b733dfc11bb035f260a83
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.