the dll dependency of qemu.exe
Hi, I build qemu with MingW, and after the linking, I find the qemu.exe depends on some dlls like below, I understand the glib dependencies, but why qemu uses libffi-6.dll, libiconv-2.dll, libpcre-1.dll libssp-0.dll, libintl-8.dll, libpixman-1-0.dll, zlib1.dll? I just want to use TCG function and don't want so much dll dependencies, can I remove those dependencies by disable some configurations? libffi-6.dll libgio-2.0-0.dll libgmodule-2.0-0.dll libiconv-2.dll libpcre-1.dll libssp-0.dll libgcc_s_seh-1.dll libglib-2.0-0.dll libgobject-2.0-0.dll libintl-8.dll libpixman-1-0.dll libwinpthread-1.dll zlib1.dll Thanks Wentong
seems currently QEMU doesn't support file backend for RAM memory region on Windows
There is a doc about the API of memory-mapped file on Windows https://docs.microsoft.com/en-us/previous-versions/ms810613(v=msdn.10)?redirectedfrom=MSDN, not sure anyone is working on it. Thanks Wentong
RE: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper
On Monday, November 30, 2020 5:54 PM, Peter Maydell wrote: > On Mon, 30 Nov 2020 at 05:41, Wu, Wentong wrote: > > Reviewed and tested. > > Thanks! Can I put that in as Reviewed-by/Tested-by lines? Sure and my pleasure, thanks Peter! > > -- PMM
RE: [PATCH v2 2/3] target/nios2: Move nios2_check_interrupts() into target/nios2
On Monday, November 30, 2020 1:40 AM, Peter Maydell wrote: > The function nios2_check_interrupts)() looks only at CPU-internal state; it > belongs in target/nios2, not hw/nios2. Move it into the same file as its only > caller, so it can just be local to that file. > > This removes the only remaining code from cpu_pic.c, so we can delete that > file > entirely. > > Signed-off-by: Peter Maydell > Reviewed-by: Philippe Mathieu-Daudé > --- > target/nios2/cpu.h | 2 -- > hw/nios2/cpu_pic.c | 36 > target/nios2/op_helper.c | 9 + > hw/nios2/meson.build | 2 +- > 4 files changed, 10 insertions(+), 39 deletions(-) delete mode 100644 > hw/nios2/cpu_pic.c Reviewed and tested.
RE: [PATCH v2 1/3] target/nios2: Move IIC code into CPU object proper
On Monday, November 30, 2020 1:40 AM, Peter Maydell wrote: > The Nios2 architecture supports two different interrupt controller > options: > > * The IIC (Internal Interrupt Controller) is part of the CPU itself; >it has 32 IRQ input lines and no NMI support. Interrupt status is >queried and controlled via the CPU's ipending and istatus >registers. > > * The EIC (External Interrupt Controller) interface allows the CPU >to connect to an external interrupt controller. The interface >allows the interrupt controller to present a packet of information >containing: > - handler address > - interrupt level > - register set > - NMI mode > > QEMU does not model an EIC currently. We do model the IIC, but its > implementation is split across code in hw/nios2/cpu_pic.c and > hw/intc/nios2_iic.c. The code in those two files has no state of its own -- > the IIC > state is in the Nios2CPU state struct. > > Because CPU objects now inherit (indirectly) from TYPE_DEVICE, they can have > GPIO input lines themselves, so we can implement the IIC directly in the CPU > object the same way that real hardware does. > > Create named "IRQ" GPIO inputs to the Nios2 CPU object, and make the only > user of the IIC wire up directly to those instead. > > Note that the old code had an "NMI" concept which was entirely unused and > also as far as I can see not architecturally correct, since only the EIC has a > concept of an NMI. > > This fixes a Coverity-reported trivial memory leak of the IRQ array allocated > in > nios2_cpu_pic_init(). > > Fixes: Coverity CID 1421916 > Signed-off-by: Peter Maydell > --- > target/nios2/cpu.h| 1 - > hw/intc/nios2_iic.c | 95 --- > hw/nios2/10m50_devboard.c | 13 +- > hw/nios2/cpu_pic.c| 31 - > target/nios2/cpu.c| 30 + > MAINTAINERS | 1 - > hw/intc/meson.build | 1 - > 7 files changed, 32 insertions(+), 140 deletions(-) delete mode 100644 > hw/intc/nios2_iic.c Reviewed and tested.
RE: [PATCH 1/2] target/nios2: Move cpu_pic code into CPU object proper
On 11/28/20 3:13 AM, Peter Maydell wrote: > The nios2 code uses an old style of interrupt handling, where a separate > standalone set of qemu_irqs invoke a function > nios2_pic_cpu_handler() which signals the interrupt to the CPU proper by > directly calling cpu_interrupt() and cpu_reset_interrupt(). > Because CPU objects now inherit (indirectly) from TYPE_DEVICE, they can have > GPIO input lines themselves, and the neater modern way to implement this is > to simply have the CPU object itself provide the input IRQ lines. > > Create named "NMI" and "IRQ" GPIO inputs to the Nios2 CPU object, and make > the only user of nios2_cpu_pic_init() wire up directly to those instead. > > This fixes a Coverity-reported trivial memory leak of the IRQ array allocated > in nios2_cpu_pic_init(). > > Fixes: Coverity CID 1421916 > Signed-off-by: Peter Maydell > --- > target/nios2/cpu.h| 1 - > hw/nios2/10m50_devboard.c | 8 +++- > hw/nios2/cpu_pic.c| 31 --- > target/nios2/cpu.c| 34 ++ > 4 files changed, 37 insertions(+), 37 deletions(-) > > diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h index > 86bbe1d8670..b7efb54ba7e 100644 > --- a/target/nios2/cpu.h > +++ b/target/nios2/cpu.h > @@ -201,7 +201,6 @@ void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr > addr, >MMUAccessType access_type, > int mmu_idx, uintptr_t retaddr); > > -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu); void > nios2_check_interrupts(CPUNios2State *env); > > void do_nios2_semihosting(CPUNios2State *env); diff --git > a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c index > 5c13b74306f..ac1993e8c08 100644 > --- a/hw/nios2/10m50_devboard.c > +++ b/hw/nios2/10m50_devboard.c > @@ -52,7 +52,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine) > ram_addr_t tcm_size = 0x1000;/* 1 kiB, but QEMU limit is 4 kiB */ > ram_addr_t ram_base = 0x0800; > ram_addr_t ram_size = 0x0800; > -qemu_irq *cpu_irq, irq[32]; > +qemu_irq irq[32]; >int i; > > /* Physical TCM (tb_ram_1k) with alias at 0xc000 */ @@ -76,14 +76,12 > @@ static void nios2_10m50_ghrd_init(MachineState *machine) > /* Create CPU -- FIXME */ > cpu = NIOS2_CPU(cpu_create(TYPE_NIOS2_CPU)); > > -/* Register: CPU interrupt controller (PIC) */ > -cpu_irq = nios2_cpu_pic_init(cpu); > - > /* Register: Internal Interrupt Controller (IIC) */ >dev = qdev_new("altera,iic"); > object_property_add_const_link(OBJECT(dev), "cpu", OBJECT(cpu)); > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal); > -sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irq[0]); > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, > + qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", 0)); > for (i = 0; i < 32; i++) { >irq[i] = qdev_get_gpio_in(dev, i); > } > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index > 5ea7e52ab83..3fb621c5c85 100644 > --- a/hw/nios2/cpu_pic.c > +++ b/hw/nios2/cpu_pic.c > @@ -26,32 +26,6 @@ > > #include "boot.h" > > -static void nios2_pic_cpu_handler(void *opaque, int irq, int level) -{ > -Nios2CPU *cpu = opaque; > -CPUNios2State *env = >env; > -CPUState *cs = CPU(cpu); > -int type = irq ? CPU_INTERRUPT_NMI : CPU_INTERRUPT_HARD; > - > -if (type == CPU_INTERRUPT_HARD) { > -env->irq_pending = level; > - > -if (level && (env->regs[CR_STATUS] & CR_STATUS_PIE)) { > -env->irq_pending = 0; > -cpu_interrupt(cs, type); > -} else if (!level) { > -env->irq_pending = 0; > -cpu_reset_interrupt(cs, type); > -} > -} else { > -if (level) { > -cpu_interrupt(cs, type); > -} else { > -cpu_reset_interrupt(cs, type); > -} > -} > -} > - > void nios2_check_interrupts(CPUNios2State *env) { > if (env->irq_pending && > @@ -60,8 +34,3 @@ void nios2_check_interrupts(CPUNios2State *env) >cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); > } > } > - > -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu) -{ > -return qemu_allocate_irqs(nios2_pic_cpu_handler, cpu, 2); > -} > diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c index > 8f7011fcb92..4b21e7c6d1c 100644 > --- a/target/nios2/cpu.c > +++ b/target/nios2/cpu.c > @@ -64,6 +64,37 @@ static void nios2_cpu_reset(DeviceState *dev) #endif } > > +#ifndef CONFIG_USER_ONLY > +static void nios2_cpu_set_nmi(void *opaque, int irq, int level) { > +Nios2CPU *cpu = opaque; > +CPUState *cs = CPU(cpu); > + > +if (level) { > +cpu_interrupt(cs, CPU_INTERRUPT_NMI); > +} else { > +cpu_reset_interrupt(cs, CPU_INTERRUPT_NMI); > +} > +} > + > +static void nios2_cpu_set_irq(void *opaque, int irq, int level) { > +Nios2CPU *cpu = opaque; > +CPUNios2State *env = >env; > +CPUState *cs =
RE: qemu icount to run guest SMP code
Thanks for the reply. > > We are trying to run guest SMP code with qemu icount mode, but based on my > > current understanding I don’t think we can do that, because with icount > > enabled, the multi cpus will be simulated in round-robin way(tcg kick vcpu > > timer, or current cpu exit in order to handle interrupt or the ending of > > the current execution translationblock) with the single vCPU thread, so > > qemu is not running guest code in parallel as real hardware does, if guest > > code has the assumption cores run in parallel it will cause unexpected > > behavior. > > Timing of the emulated CPUs will always vary from that of real hardware to > some extent. I understand that, but at least we can get the deterministic result on load heavily PC for emulated single core system if we can adjust the shift value to the level of hardware frequency. And it will not work if icount qemu need communicate with other real hardware, for example via TCP protocol. > In general you can't expect QEMU's simulation to be accurate to the level > that it will correctly run guest code that's looking carefully at the level > of parallelism between multiple cores (whether using -icount or not.) Not sure without icount(maybe it will be accurate if only QEMU is running on a powerful PC), but I can understand it's not accurate with icount enabled, the reason is as you mentioned below, there is the possibility that we have to spin to wait another core, so the icount timer will be not accurate. > > SMP mode with icount (ie without MTTCG) will run all vCPUs on one thread, but > since we always round-robin between them well-behaved guest code will make > forward progress and will not notice any major differences between this and > real parallel execution. (Sometimes it might spin a little more if it has a > busy-loop waiting for another core, but only until the round-robin kicks in > and runs the other core.) Yes, agree with "well-behaved guest code will make forward progress", please correct me if anything wrong. BR
qemu icount to run guest SMP code
Hi, We are trying to run guest SMP code with qemu icount mode, but based on my current understanding I don't think we can do that, because with icount enabled, the multi cpus will be simulated in round-robin way(tcg kick vcpu timer, or current cpu exit in order to handle interrupt or the ending of the current execution translationblock) with the single vCPU thread, so qemu is not running guest code in parallel as real hardware does, if guest code has the assumption cores run in parallel it will cause unexpected behavior. I'm fresh man to QEMU and not sure whether the understanding is correct or not, so appreciate any comment, thanks a lot! BR.
qemu icount to run guest SMP code
Hi, We are trying to run guest SMP code with qemu icount mode, but based on my current understanding I don't think we can do that, because with icount enabled, the multi cpus will be simulated in round-robin way(tcg kick vcpu timer, or current cpu exit in order to handle interrupt or the ending of the current execution translationblock) with the single vCPU thread, so qemu is not running guest code in parallel as real hardware does, if guest code has the assumption cores run in parallel it will cause unexpected behavior. Appreciate any comment, thanks a lot! BR, Wentong
RE: [PULL 00/25] target-arm queue
> -Original Message- > From: Peter Maydell > Sent: Tuesday, July 14, 2020 10:55 PM > To: Wu, Wentong > Cc: QEMU Developers > Subject: Re: [PULL 00/25] target-arm queue > > On Tue, 14 Jul 2020 at 15:52, Wu, Wentong wrote: > > > > > On Mon, 13 Jul 2020 at 15:11, Peter Maydell > > > wrote: > > > > > > > > target-arm queue: > > > > * hw/arm/bcm2836: Remove unused 'cpu_type' field > > > > * target/arm: Fix mtedesc for do_mem_zpz > > > > * Add the ability to change the FEC PHY MDIO device number on > > > > i.MX25/i.MX6/i.MX7 > > > > * target/arm: Don't do raw writes for PMINTENCLR > > > > * virtio-iommu: Fix coverity issue in > > > > virtio_iommu_handle_command() > > > > * build: Fix various issues with building on Haiku > > > > * target/nios2: fix wrctl behaviour when using icount > > > > * hw/arm/tosa: Encapsulate misc GPIO handling in a device > > > > * hw/arm/palm.c: Encapsulate misc GPIO handling in a device > > > > * hw/arm/aspeed: Do not create and attach empty SD cards by > > > > default > > > > > > > > > Applied, thanks. > > > > > > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 > > > for any user-visible changes. > > > > Who will be responsible updating the changelog? Patch author or the > > person who has the special access for that wiki page? > > Usually it's the person who sends the pullrequest (me in this case), unless > they specifically ask a patch author to write some changelog text. In this > case I didn't think anything in this set of patches needed a changelog entry > except for the empty-SD-card change. Thanks for the detail, I asked this because I don't want to break any working process in case patch author should do that. Now I understand it and no text worth adding, thanks again! > If you think there's some text worth adding I can add it for you. > > The changelog wiki page, incidentally, can be edited by anybody with a wiki > account. We don't have an automatic account-creation process because it was > heavily hit by spammers, but anybody with an existing wiki account can create > one for developers who want one. > thanks > -- PMM
RE: [PULL 00/25] target-arm queue
> -Original Message- > From: Qemu-devel On > Behalf Of Peter Maydell > Sent: Monday, July 13, 2020 11:59 PM > To: QEMU Developers > Subject: Re: [PULL 00/25] target-arm queue > > On Mon, 13 Jul 2020 at 15:11, Peter Maydell wrote: > > > > Last lot of target-arm changes to squeeze in before rc1: > > * various minor Arm bug fixes > > * David Carlier's Haiku build portability fixes > > * Wentong Wu's fixes for icount handling in the nios2 target > > > > The following changes since commit 00ce6c36b35e0eb8cc5d68a28f288a6335848813: > > > > Merge remote-tracking branch > > 'remotes/huth-gitlab/tags/pull-request-2020-07-13' into staging (2020-07-13 > > 13:01:30 +0100) > > > > are available in the Git repository at: > > > > https://git.linaro.org/people/pmaydell/qemu-arm.git > > tags/pull-target-arm-20200713 > > > > for you to fetch changes up to 756f739b1682bf131994ec96dad7fbdf8b54493a: > > > > hw/arm/aspeed: Do not create and attach empty SD cards by default > > (2020-07-13 14:36:12 +0100) > > > > > > target-arm queue: > > * hw/arm/bcm2836: Remove unused 'cpu_type' field > > * target/arm: Fix mtedesc for do_mem_zpz > > * Add the ability to change the FEC PHY MDIO device number on > > i.MX25/i.MX6/i.MX7 > > * target/arm: Don't do raw writes for PMINTENCLR > > * virtio-iommu: Fix coverity issue in virtio_iommu_handle_command() > > * build: Fix various issues with building on Haiku > > * target/nios2: fix wrctl behaviour when using icount > > * hw/arm/tosa: Encapsulate misc GPIO handling in a device > > * hw/arm/palm.c: Encapsulate misc GPIO handling in a device > > * hw/arm/aspeed: Do not create and attach empty SD cards by default > > > Applied, thanks. > > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 > for any user-visible changes. Who will be responsible updating the changelog? Patch author or the person who has the special access for that wiki page? Thanks > -- PMM
RE: [PATCH v2 1/4] target/nios2: add DISAS_NORETURN case for nothing more to generate
> -Original Message- > From: Peter Maydell > Sent: Sunday, July 12, 2020 2:50 AM > To: Wu, Wentong > Cc: QEMU Developers ; QEMU Trivial > ; Chris Wulff ; Marek Vasut > > Subject: Re: [PATCH v2 1/4] target/nios2: add DISAS_NORETURN case for nothing > more to generate > > On Fri, 10 Jul 2020 at 16:46, Wentong Wu wrote: > > > > Add DISAS_NORETURN case for nothing more to generate because at > > runtime execution will never return from some helper call. And at the > > same time replace DISAS_UPDATE in t_gen_helper_raise_exception and > > gen_exception with the newly added DISAS_NORETURN. > > > > Signed-off-by: Wentong Wu > > Hi; I'm going to pick these up and get them into master. > > A couple of notes below for if you plan to submit more patches to QEMU in > future: these are really just minor workflow things, but they do help make > our lives easier in getting code submissions into the tree. Thanks Peter, I will follow the process to submit more patches to QEMU project, and I really learn a lot! Thanks > If people provide you with a Reviewed-by: tag for a patch, and you don't > change it when you send out an updated version, it's helpful if you include > that tag in the commit message of the revised version you send out. This > saves people having to remember whether they'd reviewed something or not, and > means that when applying I don't have to go back and look at old versions to > see who reviewed what. > > Patch series are much easier for our tooling to deal with if you send them > out with a cover letter email (a 0/n email which all the other emails are > followups to; git format-patch has a '--cover-letter' option which will do > the right thing here). > > We document this kind of workflow stuff here: > https://wiki.qemu.org/Contribute/SubmitAPatch > > thanks > -- PMM
Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
> >On Mon, 29 Jun 2020 at 09:17, Wentong Wu wrote: > > > > wrctl instruction on nios2 target will cause checking cpu > > interrupt but tcg_handle_interrupt() will call cpu_abort() > > if the CPU gets an interrupt while it's not in 'can do IO' > > state, so add gen_io_start around wrctl instruction. Also > > at the same time, end the onging TB with DISAS_UPDATE. > > > > Signed-off-by: Wentong Wu > > --- > > target/nios2/translate.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c > > index 83c10eb2..51347ada 100644 > > --- a/target/nios2/translate.c > > +++ b/target/nios2/translate.c > > @@ -32,6 +32,7 @@ > > #include "exec/cpu_ldst.h" > > #include "exec/translator.h" > > #include "qemu/qemu-print.h" > > +#include "exec/gen-icount.h" > > > > /* is_jmp field values */ > > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > > uint32_t flags) > > /* If interrupts were enabled using WRCTL, trigger them. */ > > #if !defined(CONFIG_USER_ONLY) > > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > > +gen_io_start(); > > +} > > gen_helper_check_interrupts(dc->cpu_env); > > +dc->is_jmp = DISAS_UPDATE; > > } > > #endif > > } > > Reviewed-by: Peter Maydell Hi Peter, Will this be merged? If not, I would like to follow any suggestions, thanks Thanks > > though as Richard notes ideally the interrupt handling code here should > be rewritten because the check_interrupts helper is a very weird way > to implement things. > > thanks > -- PMM
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Hi, I think we can get this patch series merged first in order to get qemu_nios2 working with icount, actually we are blocked by it for some time. BTW if maintainers(Chris Wulff and Marek Vasut) don't have time for the re-work, I'd like to take it. Thanks > -Original Message- > From: Peter Maydell > Sent: Monday, July 6, 2020 1:10 AM > To: Wu, Wentong > Cc: QEMU Developers ; QEMU Trivial > ; Chris Wulff ; Marek Vasut > > Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl > instruction > > On Mon, 29 Jun 2020 at 09:17, Wentong Wu wrote: > > > > wrctl instruction on nios2 target will cause checking cpu interrupt > > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > > interrupt while it's not in 'can do IO' > > state, so add gen_io_start around wrctl instruction. Also at the same > > time, end the onging TB with DISAS_UPDATE. > > > > Signed-off-by: Wentong Wu > > --- > > target/nios2/translate.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > > 83c10eb2..51347ada 100644 > > --- a/target/nios2/translate.c > > +++ b/target/nios2/translate.c > > @@ -32,6 +32,7 @@ > > #include "exec/cpu_ldst.h" > > #include "exec/translator.h" > > #include "qemu/qemu-print.h" > > +#include "exec/gen-icount.h" > > > > /* is_jmp field values */ > > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > > uint32_t flags) > > /* If interrupts were enabled using WRCTL, trigger them. */ #if > > !defined(CONFIG_USER_ONLY) > > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > > +gen_io_start(); > > +} > > gen_helper_check_interrupts(dc->cpu_env); > > +dc->is_jmp = DISAS_UPDATE; > > } > > #endif > > } > > Reviewed-by: Peter Maydell > > though as Richard notes ideally the interrupt handling code here should be > rewritten because the check_interrupts helper is a very weird way to > implement things. > > thanks > -- PMM
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Maybe below patch will reduce some overhead, because currently it will exit to main loop to handle interrupt but if with (env->regs[CR_STATUS] & CR_STATUS_PIE) = False, it does nothing except set env->irq_pending again. diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index 1c1989d5..5ea7e52a 100644 --- a/hw/nios2/cpu_pic.c +++ b/hw/nios2/cpu_pic.c @@ -54,7 +54,8 @@ static void nios2_pic_cpu_handler(void *opaque, int irq, int level) void nios2_check_interrupts(CPUNios2State *env) { -if (env->irq_pending) { +if (env->irq_pending && +(env->regs[CR_STATUS] & CR_STATUS_PIE)) { env->irq_pending = 0; cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); } -Original Message- From: Richard Henderson Sent: Friday, July 3, 2020 2:54 AM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: qemu-triv...@nongnu.org; ma...@denx.de; crwu...@gmail.com; peter.mayd...@linaro.org Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction On 6/29/20 9:05 AM, Wentong Wu wrote: > wrctl instruction on nios2 target will cause checking cpu interrupt > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > interrupt while it's not in 'can do IO' > state, so add gen_io_start around wrctl instruction. Also at the same > time, end the onging TB with DISAS_UPDATE. > > Signed-off-by: Wentong Wu > --- > target/nios2/translate.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > 83c10eb2..51347ada 100644 > --- a/target/nios2/translate.c > +++ b/target/nios2/translate.c > @@ -32,6 +32,7 @@ > #include "exec/cpu_ldst.h" > #include "exec/translator.h" > #include "qemu/qemu-print.h" > +#include "exec/gen-icount.h" > > /* is_jmp field values */ > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > uint32_t flags) > /* If interrupts were enabled using WRCTL, trigger them. */ #if > !defined(CONFIG_USER_ONLY) > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > +gen_io_start(); > +} > gen_helper_check_interrupts(dc->cpu_env); > +dc->is_jmp = DISAS_UPDATE; > } > #endif This isn't right. Not so much the gen_io_start portion, but the entire existence of helper_check_interrupt. The correct way to acknowledge interrupts after changing an interrupt mask bit is to exit the TB back to the cpu main loop. Which you are doing here with DISAS_UPDATE, so that part is fine. (Although you could merge that into the switch statement above.) Looking at nios_pic_cpu_handler, there are two other bugs: 1) Get rid of env->irq_pending and use cpu_interrupt/cpu_reset_interrupt instead. 2) Do not check env->regs[CR_STATUS] & CR_STATUS_PIE. That variable does not belong to the pic and should not be checked there. The check belongs in nios2_cpu_exec_interrupt, and is in fact already there. r~
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Thanks, I think we can get this series merged currently. -Original Message- From: Peter Maydell Sent: Monday, July 6, 2020 1:10 AM To: Wu, Wentong Cc: QEMU Developers ; QEMU Trivial ; Chris Wulff ; Marek Vasut Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction On Mon, 29 Jun 2020 at 09:17, Wentong Wu wrote: > > wrctl instruction on nios2 target will cause checking cpu interrupt > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > interrupt while it's not in 'can do IO' > state, so add gen_io_start around wrctl instruction. Also at the same > time, end the onging TB with DISAS_UPDATE. > > Signed-off-by: Wentong Wu > --- > target/nios2/translate.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > 83c10eb2..51347ada 100644 > --- a/target/nios2/translate.c > +++ b/target/nios2/translate.c > @@ -32,6 +32,7 @@ > #include "exec/cpu_ldst.h" > #include "exec/translator.h" > #include "qemu/qemu-print.h" > +#include "exec/gen-icount.h" > > /* is_jmp field values */ > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > uint32_t flags) > /* If interrupts were enabled using WRCTL, trigger them. */ #if > !defined(CONFIG_USER_ONLY) > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > +gen_io_start(); > +} > gen_helper_check_interrupts(dc->cpu_env); > +dc->is_jmp = DISAS_UPDATE; > } > #endif > } Reviewed-by: Peter Maydell though as Richard notes ideally the interrupt handling code here should be rewritten because the check_interrupts helper is a very weird way to implement things. thanks -- PMM
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Correct the format > -Original Message- > From: Richard Henderson > Sent: Friday, July 3, 2020 2:54 AM > To: Wu, Wentong ; qemu-devel@nongnu.org > Cc: qemu-triv...@nongnu.org; ma...@denx.de; crwu...@gmail.com; > peter.mayd...@linaro.org > Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl > instruction > > On 6/29/20 9:05 AM, Wentong Wu wrote: > > wrctl instruction on nios2 target will cause checking cpu interrupt > > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > > interrupt while it's not in 'can do IO' > > state, so add gen_io_start around wrctl instruction. Also at the same > > time, end the onging TB with DISAS_UPDATE. > > > > Signed-off-by: Wentong Wu > > --- > > target/nios2/translate.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > > 83c10eb2..51347ada 100644 > > --- a/target/nios2/translate.c > > +++ b/target/nios2/translate.c > > @@ -32,6 +32,7 @@ > > #include "exec/cpu_ldst.h" > > #include "exec/translator.h" > > #include "qemu/qemu-print.h" > > +#include "exec/gen-icount.h" > > > > /* is_jmp field values */ > > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > > uint32_t flags) > > /* If interrupts were enabled using WRCTL, trigger them. */ #if > > !defined(CONFIG_USER_ONLY) > > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > > +gen_io_start(); > > +} > > gen_helper_check_interrupts(dc->cpu_env); > > +dc->is_jmp = DISAS_UPDATE; > > } > > #endif > > This isn't right. Not so much the gen_io_start portion, but the entire > existence of helper_check_interrupt. > > The correct way to acknowledge interrupts after changing an interrupt mask > bit is to exit the TB back to the cpu main loop. > Which you are doing here with DISAS_UPDATE, so that part is fine. (Although > you could merge that into the switch statement above.) > > Looking at nios_pic_cpu_handler, there are two other bugs: > > 1) Get rid of env->irq_pending and use cpu_interrupt/cpu_reset_interrupt > instead. IMHO, we need env->irq_pending, because if ((env->regs[CR_STATUS] & CR_STATUS_PIE) == False), it can indicate there is interrupt pending, we can exit to main loop to handle the interrupt at once if guest code enable irq later. > 2) Do not check env->regs[CR_STATUS] & CR_STATUS_PIE. That variable does not > belong to the pic and should not be checked there. The check belongs in > nios2_cpu_exec_interrupt, and is in fact already there. But that will cause some assert happen if device trigger irq with env->regs[CR_STATUS] & CR_STATUS_PIE = False BR
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Correct the format > -Original Message- > From: Richard Henderson > Sent: Friday, July 3, 2020 2:54 AM > To: Wu, Wentong ; qemu-devel@nongnu.org > Cc: qemu-triv...@nongnu.org; ma...@denx.de; crwu...@gmail.com; > peter.mayd...@linaro.org > Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl > instruction > > On 6/29/20 9:05 AM, Wentong Wu wrote: > > wrctl instruction on nios2 target will cause checking cpu interrupt > > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > > interrupt while it's not in 'can do IO' > > state, so add gen_io_start around wrctl instruction. Also at the same > > time, end the onging TB with DISAS_UPDATE. > > > > Signed-off-by: Wentong Wu > > --- > > target/nios2/translate.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > > 83c10eb2..51347ada 100644 > > --- a/target/nios2/translate.c > > +++ b/target/nios2/translate.c > > @@ -32,6 +32,7 @@ > > #include "exec/cpu_ldst.h" > > #include "exec/translator.h" > > #include "qemu/qemu-print.h" > > +#include "exec/gen-icount.h" > > > > /* is_jmp field values */ > > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > > uint32_t flags) > > /* If interrupts were enabled using WRCTL, trigger them. */ #if > > !defined(CONFIG_USER_ONLY) > > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > > +gen_io_start(); > > +} > > gen_helper_check_interrupts(dc->cpu_env); > > +dc->is_jmp = DISAS_UPDATE; > > } > > #endif > > This isn't right. Not so much the gen_io_start portion, but the entire > existence of helper_check_interrupt. > > The correct way to acknowledge interrupts after changing an interrupt mask > bit is to exit the TB back to the cpu main loop. > Which you are doing here with DISAS_UPDATE, so that part is fine. (Although > you could merge that into the switch statement above.) > > Looking at nios_pic_cpu_handler, there are two other bugs: > > 1) Get rid of env->irq_pending and use cpu_interrupt/cpu_reset_interrupt > instead. IMHO, we need env->irq_pending, because if ((env->regs[CR_STATUS] & CR_STATUS_PIE) == False), it can indicate there is interrupt pending, we can exit to main loop to handle the interrupt at once if guest code enable irq later. > 2) Do not check env->regs[CR_STATUS] & CR_STATUS_PIE. That variable does not > belong to the pic and should not be checked there. The check belongs in > nios2_cpu_exec_interrupt, and is in fact already there. But that will cause some assert happen if device trigger irq with env->regs[CR_STATUS] & CR_STATUS_PIE = False BR
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
HI Peter, Could you please take a look at this patch which is following your pervious suggestion? Thanks -Original Message- From: Wu, Wentong Sent: Tuesday, June 30, 2020 12:06 AM To: qemu-devel@nongnu.org Cc: qemu-triv...@nongnu.org; crwu...@gmail.com; ma...@denx.de; peter.mayd...@linaro.org; Wu, Wentong Subject: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction wrctl instruction on nios2 target will cause checking cpu interrupt but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an interrupt while it's not in 'can do IO' state, so add gen_io_start around wrctl instruction. Also at the same time, end the onging TB with DISAS_UPDATE. Signed-off-by: Wentong Wu --- target/nios2/translate.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target/nios2/translate.c b/target/nios2/translate.c index 83c10eb2..51347ada 100644 --- a/target/nios2/translate.c +++ b/target/nios2/translate.c @@ -32,6 +32,7 @@ #include "exec/cpu_ldst.h" #include "exec/translator.h" #include "qemu/qemu-print.h" +#include "exec/gen-icount.h" /* is_jmp field values */ #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, uint32_t flags) /* If interrupts were enabled using WRCTL, trigger them. */ #if !defined(CONFIG_USER_ONLY) if ((instr.imm5 + CR_BASE) == CR_STATUS) { +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { +gen_io_start(); +} gen_helper_check_interrupts(dc->cpu_env); +dc->is_jmp = DISAS_UPDATE; } #endif } -- 2.21.3
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
-Original Message- From: Richard Henderson Sent: Friday, July 3, 2020 2:54 AM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: qemu-triv...@nongnu.org; ma...@denx.de; crwu...@gmail.com; peter.mayd...@linaro.org Subject: Re: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction On 6/29/20 9:05 AM, Wentong Wu wrote: > wrctl instruction on nios2 target will cause checking cpu interrupt > but tcg_handle_interrupt() will call cpu_abort() if the CPU gets an > interrupt while it's not in 'can do IO' > state, so add gen_io_start around wrctl instruction. Also at the same > time, end the onging TB with DISAS_UPDATE. > > Signed-off-by: Wentong Wu mailto:wentong...@intel.com>> > --- > target/nios2/translate.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > 83c10eb2..51347ada 100644 > --- a/target/nios2/translate.c > +++ b/target/nios2/translate.c > @@ -32,6 +32,7 @@ > #include "exec/cpu_ldst.h" > #include "exec/translator.h" > #include "qemu/qemu-print.h" > +#include "exec/gen-icount.h" > > /* is_jmp field values */ > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, uint32_t flags) > /* If interrupts were enabled using WRCTL, trigger them. */ #if > !defined(CONFIG_USER_ONLY) > if ((instr.imm5 + CR_BASE) == CR_STATUS) { > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > +gen_io_start(); > +} > gen_helper_check_interrupts(dc->cpu_env); > +dc->is_jmp = DISAS_UPDATE; > } > #endif This isn't right. Not so much the gen_io_start portion, but the entire existence of helper_check_interrupt. The correct way to acknowledge interrupts after changing an interrupt mask bit is to exit the TB back to the cpu main loop. Which you are doing here with DISAS_UPDATE, so that part is fine. (Although you could merge that into the switch statement above.) Looking at nios_pic_cpu_handler, there are two other bugs: 1) Get rid of env->irq_pending and use cpu_interrupt/cpu_reset_interrupt instead IMHO, we need env->irq_pending, because if ((env->regs[CR_STATUS] & CR_STATUS_PIE) == False), it can indicate there is interrupt pending, we can exit to main loop to handle the interrupt at once if guest code enable irq later. 2) Do not check env->regs[CR_STATUS] & CR_STATUS_PIE. That variable does not belong to the pic and should not be checked there. The check belongs in nios2_cpu_exec_interrupt, and is in fact already there. But that will cause some assert happen if device trigger irq with env->regs[CR_STATUS] & CR_STATUS_PIE = False BR
RE: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction
Hi, Could you please take a look the new patch? Thanks > -Original Message- > Subject: [PATCH 3/3] target/nios2: Use gen_io_start around wrctl instruction > wrctl instruction on nios2 target will cause checking cpu interrupt but > tcg_handle_interrupt() will call cpu_abort() if the CPU gets an interrupt > while it's not in 'can do IO' > state, so add gen_io_start around wrctl instruction. Also at the same time, > end the onging TB with DISAS_UPDATE. > Signed-off-by: Wentong Wu > --- > target/nios2/translate.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c index > 83c10eb2..51347ada 100644 > --- a/target/nios2/translate.c > +++ b/target/nios2/translate.c > @@ -32,6 +32,7 @@ > #include "exec/cpu_ldst.h" > #include "exec/translator.h" > #include "qemu/qemu-print.h" > +#include "exec/gen-icount.h" > /* is_jmp field values */ > #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ > @@ -518,7 +519,11 @@ static void wrctl(DisasContext *dc, uint32_t code, > uint32_t flags) > /* If interrupts were enabled using WRCTL, trigger them. */ #if > !defined(CONFIG_USER_ONLY) >if ((instr.imm5 + CR_BASE) == CR_STATUS) { > +if (tb_cflags(dc->tb) & CF_USE_ICOUNT) { > +gen_io_start(); > +} > gen_helper_check_interrupts(dc->cpu_env); > +dc->is_jmp = DISAS_UPDATE; >} > #endif > } > -- > 2.21.3
RE: [PATCH] hw/nios2: Update interrupt request when CR_STATUS_PIE disabled
> > > > Update interrupt request when external interupt pends for STATUS_PIE > > disabled. Otherwise on icount enabled nios2 target there will be cpu > > abort when guest code changes state register with wrctl instruction. > > > > Signed-off-by: Wentong Wu > > --- > > hw/nios2/cpu_pic.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index > > 1c1989d5..2abc8fa8 100644 > > --- a/hw/nios2/cpu_pic.c > > +++ b/hw/nios2/cpu_pic.c > > @@ -42,6 +42,8 @@ static void nios2_pic_cpu_handler(void *opaque, int irq, > > int level) > >} else if (!level) { > > env->irq_pending = 0; > > cpu_reset_interrupt(cs, type); > > +} else { > > +cs->interrupt_request |= type; > >} > Thanks for the clarification in your other email about the issue you're > trying to address: Thanks for the review! > > I’m running icount mode on qemu_nios2 with customized platform but cpu > > abort happened(qemu: fatal: Raised interrupt while not in I/O > > function) when guest code changes state register with wrctl > > instruction add some debug code finding that it’s caused by the > > interrupt_request mismatch. > I don't think the change you've made is the correct fix. > Setting cs->interrupt_request like this is pretty much the same thing that > calling cpu_interrupt() does, so what your patch is doing is essentially > "ignore the status.PIE bit and always deliver interrupts", which isn't how > the hardware behaves. > The assertion you've run into is saying "some instruction caused us to take > an interrupt, but it wasn't marked up to indicate that it might cause a side > effect". (This only matters in icount mode, where we insist that we never get > unexpected sideeffects like this.) If the guest writes to status.PIE to > unmask interrupts that's the kind of thing that will cause an interrupt to be > taken, so the problem really here is that the nios2 translate.c code hasn't > indicated that this insn can do this. > The right fix here will be that target/nios2/translate.c needs to do this: > if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { >gen_io_start(); > } > before generating code for an insn like this one, and then > if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) { >gen_io_end(); > } > after it. (Compare the xtensa target which does a similar kind of thing for > its interrupt handling.) For wrctl to STATUS it should I think also end the > TB, because we want to actually take any pending interrupt now, not in a few > instructions time when the next branch comes along. > The fact that the current nios2 code has no calls to > gen_io_start() in it at all (apart from one in boilerplate > code) suggests to me that this target is simply broken for use with -icount > at the moment. There may well be other bugs of a similar kind where > particular insns that cause interrupts or touch devices (any equivalent to the > x86 in/out insns, for instance) also need to be marked up as IO. Thanks for the detail, you are right, understand this more. Thanks > (Beyond that, the way that nios2_check_interrupts() works looks weird; in an > ideal world it would be rewritten to work in a way that's more in line with > how we'd write that kind of code today. It should be possible to get it to > work with icount without getting into that kind of refactoring/rework, > though.) > thanks > -- PMM
RE: [PATCH] hw/nios2: Update interrupt request when CR_STATUS_PIE disabled
>Hi, >On 6/12/20 3:43 PM, Wu, Wentong wrote: > > Hi, > >Can any body help review this patch ? Thanks in advance! > You just sent this patch yesterday... Please give reviewers more time. > See: > https://wiki.qemu.org/Contribute/SubmitAPatch#Participating_in_Code_Review > In particular: > https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_been_ignored > > > > BR > > > > -Original Message- > > From: Wu, Wentong > > Sent: Thursday, June 11, 2020 4:13 PM > > To: qemu-devel@nongnu.org > > Cc: qemu-triv...@nongnu.org; crwu...@gmail.com; ma...@denx.de; > > th...@redhat.com; Wu, Wentong > > Subject: [PATCH] hw/nios2: Update interrupt request when CR_STATUS_PIE > > disabled > > > > Update interrupt request when external interupt pends for STATUS_PIE > > disabled. Otherwise on icount enabled nios2 target there will be cpu abort > > when guest code changes state register with wrctl instruction. > It'd help if you provide more information, what code where you testing, how > you ran QEMU, enough for reviewers to reproduce the issue you had and test if > your patch indeed resolves the issue you described. Hi, I’m running icount mode on qemu_nios2 with customized platform(https://github.com/zephyrproject-rtos/sdk-ng/blob/master/meta-zephyr-sdk/recipes-devtools/qemu/files/0001-qemu-nios2-Add-Altera-MAX-10-board-support-for-Zephy.patch, almost same with 10m50_devboard) but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O function) when guest code changes state register with wrctl instruction add some debug code finding that it’s caused by the interrupt_request mismatch. Hope that will be helpful. > Regards, > Phil. > > > > Signed-off-by: Wentong Wu > > --- > > hw/nios2/cpu_pic.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index > > 1c1989d5..2abc8fa8 100644 > > --- a/hw/nios2/cpu_pic.c > > +++ b/hw/nios2/cpu_pic.c > > @@ -42,6 +42,8 @@ static void nios2_pic_cpu_handler(void *opaque, int irq, > > int level) > > } else if (!level) { > > env->irq_pending = 0; > >cpu_reset_interrupt(cs, type); > > +} else { > > +cs->interrupt_request |= type; > >} > > } else { > >if (level) { > > -- > > 2.21.3 > > > >
RE: [PATCH] hw/nios2: Update interrupt request when CR_STATUS_PIE disabled
Hi, Can any body help review this patch ? Thanks in advance! BR -Original Message- From: Wu, Wentong Sent: Thursday, June 11, 2020 4:13 PM To: qemu-devel@nongnu.org Cc: qemu-triv...@nongnu.org; crwu...@gmail.com; ma...@denx.de; th...@redhat.com; Wu, Wentong Subject: [PATCH] hw/nios2: Update interrupt request when CR_STATUS_PIE disabled Update interrupt request when external interupt pends for STATUS_PIE disabled. Otherwise on icount enabled nios2 target there will be cpu abort when guest code changes state register with wrctl instruction. Signed-off-by: Wentong Wu --- hw/nios2/cpu_pic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index 1c1989d5..2abc8fa8 100644 --- a/hw/nios2/cpu_pic.c +++ b/hw/nios2/cpu_pic.c @@ -42,6 +42,8 @@ static void nios2_pic_cpu_handler(void *opaque, int irq, int level) } else if (!level) { env->irq_pending = 0; cpu_reset_interrupt(cs, type); +} else { +cs->interrupt_request |= type; } } else { if (level) { -- 2.21.3
RE: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled
HI Thomas, Thanks for the help, I updated the patch in the new thread https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg03103.html, hoping that follows QEMU's working style, Thanks again. BR, Wentong -Original Message- From: Thomas Huth Sent: Wednesday, June 10, 2020 9:29 PM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: Chris Wulff ; Marek Vasut Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled On 09/06/2020 10.39, Wu, Wentong wrote: > Hi @Thomas Huth, > It's my first time to send patch in qemu community, not sure if there is > something wrong sending patch like below and I'm happy to receive any > suggestions. And by the way, could you please help review the patch? Hi, it would be good if you could send the patch as plain text e-mail, not as HTML mail, otherwise it's impossible to apply it with "git am" or "patch". If you can, try to use "git send-email" to send out patches. Also see https://wiki.qemu.org/Contribute/SubmitAPatch for some more details. Technically, I don't have a clue about nios2, so sorry, I can't help reviewing it. But it's a very small patch, so maybe send the plain-text mail with CC: to qemu-triv...@nongnu.org - if Chris or Marek provide an Reviewed-by or Acked-by then, it should get merged with the next trivial pull request. HTH, Thomas > Thanks > > -Original Message- > From: Thomas Huth > Sent: Friday, June 5, 2020 3:07 PM > To: Wu, Wentong ; qemu-devel@nongnu.org > Cc: Chris Wulff ; Marek Vasut > Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE > disabled > > On 05/06/2020 07.59, Wu, Wentong wrote: >> Hi all, >> >> I’m running icount mode on qemu_nios2 with customized >> platform(almost same with 10m50_devboard), >> >> but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O >> function) when guest code changes >> >> state register with wrctl instruction, add some debug code finding >> that it’s caused by the interrupt_request >> >> mismatch, so I made a patch as below, not sure if it’s right, hope I >> can have some discussion with maintainers > > Hi, > > please have a look at the MAINTAINERS file in the main directory of the > sources, you can find the corresponding maintainers there. Thus if you have > questions related to nios2, please make sure to put Chris and Marek into CC: > so that your patch gets the right attention! > > Thanks, > Thomas > > > >> commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master) >> >> Author: Wentong Wu >> >> Date: Fri Jun 5 09:29:43 2020 -0400 >> >> >> >> hw: nios2: update interrupt_request when CR_STATUS_PIE disabled >> >> >> >> Update interrupt_request when external interupt pends for >> STATUS_PIE >> >> disabled. Otherwise on icount enabled nios2 target there will be >> cpu >> >> abort when guest code changes state register with wrctl instruction. >> >> >> >> Signed-off-by: Wentong Wu >> >> >> >> diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c >> >> index 1c1989d5..b04db4d7 100644 >> >> --- a/hw/nios2/cpu_pic.c >> >> +++ b/hw/nios2/cpu_pic.c >> >> @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int >> irq, int level) >> >> } else if (!level) { >> >> env->irq_pending = 0; >> >> cpu_reset_interrupt(cs, type); >> >> - } >> >> + } else { >> >> + cs->interrupt_request |= type; >> >> + } >> >> } else { >> >> if (level) { >> >> cpu_interrupt(cs, type); >> >
RE: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled
Hi @Thomas Huth, It's my first time to send patch in qemu community, not sure if there is something wrong sending patch like below and I'm happy to receive any suggestions. And by the way, could you please help review the patch? Thanks -Original Message- From: Thomas Huth Sent: Friday, June 5, 2020 3:07 PM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: Chris Wulff ; Marek Vasut Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled On 05/06/2020 07.59, Wu, Wentong wrote: > Hi all, > > I’m running icount mode on qemu_nios2 with customized platform(almost > same with 10m50_devboard), > > but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O > function) when guest code changes > > state register with wrctl instruction, add some debug code finding > that it’s caused by the interrupt_request > > mismatch, so I made a patch as below, not sure if it’s right, hope I > can have some discussion with maintainers Hi, please have a look at the MAINTAINERS file in the main directory of the sources, you can find the corresponding maintainers there. Thus if you have questions related to nios2, please make sure to put Chris and Marek into CC: so that your patch gets the right attention! Thanks, Thomas > commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master) > > Author: Wentong Wu > > Date: Fri Jun 5 09:29:43 2020 -0400 > > > > hw: nios2: update interrupt_request when CR_STATUS_PIE disabled > > > > Update interrupt_request when external interupt pends for > STATUS_PIE > > disabled. Otherwise on icount enabled nios2 target there will be > cpu > > abort when guest code changes state register with wrctl instruction. > > > > Signed-off-by: Wentong Wu > > > > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c > > index 1c1989d5..b04db4d7 100644 > > --- a/hw/nios2/cpu_pic.c > > +++ b/hw/nios2/cpu_pic.c > > @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int > irq, int level) > > } else if (!level) { > > env->irq_pending = 0; > > cpu_reset_interrupt(cs, type); > > - } > > + } else { > > + cs->interrupt_request |= type; > > + } > > } else { > > if (level) { > > cpu_interrupt(cs, type); >
RE: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled
@Chris Wulff @Marek Vasut could you please take a look this issue and patch? Thanks in advance! -Original Message- From: Thomas Huth Sent: Friday, June 5, 2020 3:07 PM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: Chris Wulff ; Marek Vasut Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled On 05/06/2020 07.59, Wu, Wentong wrote: > Hi all, > > I’m running icount mode on qemu_nios2 with customized platform(almost > same with 10m50_devboard), > > but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O > function) when guest code changes > > state register with wrctl instruction, add some debug code finding > that it’s caused by the interrupt_request > > mismatch, so I made a patch as below, not sure if it’s right, hope I > can have some discussion with maintainers Hi, please have a look at the MAINTAINERS file in the main directory of the sources, you can find the corresponding maintainers there. Thus if you have questions related to nios2, please make sure to put Chris and Marek into CC: so that your patch gets the right attention! Thanks, Thomas > commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master) > > Author: Wentong Wu > > Date: Fri Jun 5 09:29:43 2020 -0400 > > > > hw: nios2: update interrupt_request when CR_STATUS_PIE disabled > > > > Update interrupt_request when external interupt pends for > STATUS_PIE > > disabled. Otherwise on icount enabled nios2 target there will be > cpu > > abort when guest code changes state register with wrctl instruction. > > > > Signed-off-by: Wentong Wu > > > > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c > > index 1c1989d5..b04db4d7 100644 > > --- a/hw/nios2/cpu_pic.c > > +++ b/hw/nios2/cpu_pic.c > > @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int > irq, int level) > > } else if (!level) { > > env->irq_pending = 0; > > cpu_reset_interrupt(cs, type); > > - } > > + } else { > > + cs->interrupt_request |= type; > > + } > > } else { > > if (level) { > > cpu_interrupt(cs, type); >
RE: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled
Thanks Thomas! @Chris Wulff @Marek Vasut could you please give some suggestions? Thanks -Original Message- From: Thomas Huth Sent: Friday, June 5, 2020 3:07 PM To: Wu, Wentong ; qemu-devel@nongnu.org Cc: Chris Wulff ; Marek Vasut Subject: Re: [RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled On 05/06/2020 07.59, Wu, Wentong wrote: > Hi all, > > I’m running icount mode on qemu_nios2 with customized platform(almost > same with 10m50_devboard), > > but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O > function) when guest code changes > > state register with wrctl instruction, add some debug code finding > that it’s caused by the interrupt_request > > mismatch, so I made a patch as below, not sure if it’s right, hope I > can have some discussion with maintainers Hi, please have a look at the MAINTAINERS file in the main directory of the sources, you can find the corresponding maintainers there. Thus if you have questions related to nios2, please make sure to put Chris and Marek into CC: so that your patch gets the right attention! Thanks, Thomas > commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master) > > Author: Wentong Wu > > Date: Fri Jun 5 09:29:43 2020 -0400 > > > > hw: nios2: update interrupt_request when CR_STATUS_PIE disabled > > > > Update interrupt_request when external interupt pends for > STATUS_PIE > > disabled. Otherwise on icount enabled nios2 target there will be > cpu > > abort when guest code changes state register with wrctl instruction. > > > > Signed-off-by: Wentong Wu > > > > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c > > index 1c1989d5..b04db4d7 100644 > > --- a/hw/nios2/cpu_pic.c > > +++ b/hw/nios2/cpu_pic.c > > @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int > irq, int level) > > } else if (!level) { > > env->irq_pending = 0; > > cpu_reset_interrupt(cs, type); > > - } > > + } else { > > + cs->interrupt_request |= type; > > + } > > } else { > > if (level) { > > cpu_interrupt(cs, type); >
[RFC] hw: nios2: update interrupt_request when STATUS_PIE disabled
Hi all, I'm running icount mode on qemu_nios2 with customized platform(almost same with 10m50_devboard), but cpu abort happened(qemu: fatal: Raised interrupt while not in I/O function) when guest code changes state register with wrctl instruction, add some debug code finding that it's caused by the interrupt_request mismatch, so I made a patch as below, not sure if it's right, hope I can have some discussion with maintainers first! Thanks a lot! commit efdb3da4e145a7a34ba8b3ab1cdcfc346ae20a11 (HEAD -> master) Author: Wentong Wu Date: Fri Jun 5 09:29:43 2020 -0400 hw: nios2: update interrupt_request when CR_STATUS_PIE disabled Update interrupt_request when external interupt pends for STATUS_PIE disabled. Otherwise on icount enabled nios2 target there will be cpu abort when guest code changes state register with wrctl instruction. Signed-off-by: Wentong Wu diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index 1c1989d5..b04db4d7 100644 --- a/hw/nios2/cpu_pic.c +++ b/hw/nios2/cpu_pic.c @@ -42,7 +42,9 @@ static void nios2_pic_cpu_handler(void *opaque, int irq, int level) } else if (!level) { env->irq_pending = 0; cpu_reset_interrupt(cs, type); -} +} else { +cs->interrupt_request |= type; + } } else { if (level) { cpu_interrupt(cs, type);
[Qemu-devel] qemu icount mode timer accuracy
Could anyone please give some comments? Thanks in advance! Hi, Recently I'm working to enable Qemu icount mode with TCG, with source code review I found that Qemu can give deterministic execution for guest code timeout. But for exact time point for guest OS, I have a question: For armv7m_systick.c example, guest OS will use systick_read which will call "t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); " to calculate his exact time point, and qemu_clock_get_ns will use qemu_icount. But from qemu_tcg_rr_cpu_thread_fn { prepare_icount_for_run(cpu); r = tcg_cpu_exec(cpu); process_icount_data(cpu);}, we know qemu just update qemu_icount value after tcg_cpu_exec, so for each tcg_cpu_exec execution there is the same qemu_icount value, and then guest code will get the same time point for that one tcg execution. Can someone confirm that?
[Qemu-devel] qemu icount mode timer accuracy
Could you please give some comments about this? Thanks a lot!
[Qemu-devel] qemu icount mode timer accuracy
Hi, Recently I'm working to enable Qemu icount mode with TCG, with source code review I found that Qemu can give deterministic execution for guest code timeout. But for exact time point for guest OS, I have a question: For armv7m_systick.c example, guest OS will use systick_read which will call "t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); " to calculate his exact time point, and qemu_clock_get_ns will use qemu_icount. But from qemu_tcg_rr_cpu_thread_fn { prepare_icount_for_run(cpu); r = tcg_cpu_exec(cpu); process_icount_data(cpu);}, we know qemu just update qemu_icount value after tcg_cpu_exec, so for each tcg_cpu_exec execution there is the same qemu_icount value, and then guest code will get the same time point for that one tcg execution. Can someone confirm that? Thanks
Re: [Qemu-devel] in icount mode guest OS can't boot on qemu_x86 when shift value range in [0, 3]
Add more info: the qemu binary is qemu-system-i386 and it's TCG mode, not kvm. From: Wu, Wentong Sent: Wednesday, July 10, 2019 3:01 PM To: 'qemu-devel@nongnu.org' Subject: in icount mode guest OS can't boot on qemu_x86 when shift value range in [0, 3] Hi all, I'm running a rtos on qemu_x86 with icount mode enabled, shift value located in [0, 3], there is very high possibility that the guest os(the rtos) can't boot up. Does anyone have any idea for this issue? And this issue blocked me long time, appreciate any input? Thanks
[Qemu-devel] in icount mode guest OS can't boot on qemu_x86 when shift value range in [0, 3]
Hi all, I'm running a rtos on qemu_x86 with icount mode enabled, shift value located in [0, 3], there is very high possibility that the guest os(the rtos) can't boot up. Does anyone have any idea for this issue? And this issue blocked me long time, appreciate any input? Thanks
Re: [Qemu-devel] icount mode
Hi Alex, If there is something wrong, please tell me. I really like to have some discusses with Qemu developers about icount mode. Thanks a lot! Wentong Wu -Original Message- From: Wu, Wentong Sent: Tuesday, June 25, 2019 4:44 AM To: 'Alex Bennée' ; qemu-devel@nongnu.org Subject: RE: [Qemu-devel] icount mode Hi Alex, Thanks for your reply. For the different frequencies, please see below code in armv7m_systick.c and mps2.c first, the s->reload will be set by the guest os code according to the CPU's frequency which will be SYSCLK_FRQ, and s->tick will be set as "s->tick += (s->reload + 1) * systick_scale(s);", it means the frequency of this timer which I called qemu timer will be NANOSECONDS_PER_SECOND. static void systick_reload(SysTickState *s, int reset) { /* The Cortex-M3 Devices Generic User Guide says that "When the * ENABLE bit is set to 1, the counter loads the RELOAD value from the * SYST RVR register and then counts down". So, we need to check the * ENABLE bit before reloading the value. */ trace_systick_reload(); if ((s->control & SYSTICK_ENABLE) == 0) { return; } if (reset) { s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); } s->tick += (s->reload + 1) * systick_scale(s); timer_mod(s->timer, s->tick); } static void mps2_common_init(MachineState *machine) { ... system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ; ... } But for below code, it will use qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) to get the current time which will be calculated by 2^N ns * instruction counter, but this frequency will be NANOSECONDS_PER_SECOND / 2^N. Below code is an example two different frequencies are used, actually in cpus.c, qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL) will use the qemu timer(freq is NANOSECONDS_PER_SECOND), and cpu_icount_to_ns will calcaute time with frequency NANOSECONDS_PER_SECOND / 2^N. static void systick_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { SysTickState *s = opaque; trace_systick_write(addr, value, size); switch (addr) { case 0x0: /* SysTick Control and Status. */ { uint32_t oldval = s->control; s->control &= 0xfff8; s->control |= value & 7; if ((oldval ^ value) & SYSTICK_ENABLE) { int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); if (value & SYSTICK_ENABLE) { if (s->tick) { s->tick += now; timer_mod(s->timer, s->tick); } else { systick_reload(s, 1); } } else { timer_del(s->timer); s->tick -= now; if (s->tick < 0) { s->tick = 0; } } } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) { /* This is a hack. Force the timer to be reloaded when the reference clock is changed. */ systick_reload(s, 1); } break; } case 0x4: /* SysTick Reload Value. */ s->reload = value; break; .. Yes, I'm for the for determinism, in my guest image there are some testing cases for timer system which locate in a small rtos. And for shift value, I mean it seems shift value impact system greatly, for the same one guest image and different shift value in count mode(-icount shift=4,align=off,sleep=off -rtc clock=vm) give very different accuracy for guest timer. So my question is how to calculate the shift value for the end user. Thanks again for your help. Thanks -Original Message- From: Qemu-devel [mailto:qemu-devel-bounces+wentong.wu=intel@nongnu.org] On Behalf Of Alex Bennée Sent: Monday, June 24, 2019 11:48 PM To: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] icount mode Wu, Wentong writes: > Hi all, > > Recently I'm using Qemu TCG icount mode, from the code I found Qemu > timers run at 1GHz, and for ArmV7M example, there will be conversion Are you talking about: #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */ because as far as I can tell that only affects the scaling factors applied to PMU counters. The internal counters (CNTFRQ_EL0 and friends) are hardwired to: /* Scale factor for generic timers, ie number of ns per tick. * This gives a 62.5MHz timer. */ #define GTIMER_SCALE 16 but this only affects the nominal rate the counters expire at. Software could attempt to reprogram it and the emulation will read-as-written but it won't actually change anything. However this only affects the clocks - it implies nothing about how fast the core may be executing. In fact unless you are using icount we will just run as fast a possible. > factor from q
Re: [Qemu-devel] icount mode
Hi Alex, Thanks for your reply. For the different frequencies, please see below code in armv7m_systick.c and mps2.c first, the s->reload will be set by the guest os code according to the CPU's frequency which will be SYSCLK_FRQ, and s->tick will be set as "s->tick += (s->reload + 1) * systick_scale(s);", it means the frequency of this timer which I called qemu timer will be NANOSECONDS_PER_SECOND. static void systick_reload(SysTickState *s, int reset) { /* The Cortex-M3 Devices Generic User Guide says that "When the * ENABLE bit is set to 1, the counter loads the RELOAD value from the * SYST RVR register and then counts down". So, we need to check the * ENABLE bit before reloading the value. */ trace_systick_reload(); if ((s->control & SYSTICK_ENABLE) == 0) { return; } if (reset) { s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); } s->tick += (s->reload + 1) * systick_scale(s); timer_mod(s->timer, s->tick); } static void mps2_common_init(MachineState *machine) { ... system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ; ... } But for below code, it will use qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) to get the current time which will be calculated by 2^N ns * instruction counter, but this frequency will be NANOSECONDS_PER_SECOND / 2^N. Below code is an example two different frequencies are used, actually in cpus.c, qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL) will use the qemu timer(freq is NANOSECONDS_PER_SECOND), and cpu_icount_to_ns will calcaute time with frequency NANOSECONDS_PER_SECOND / 2^N. static void systick_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { SysTickState *s = opaque; trace_systick_write(addr, value, size); switch (addr) { case 0x0: /* SysTick Control and Status. */ { uint32_t oldval = s->control; s->control &= 0xfff8; s->control |= value & 7; if ((oldval ^ value) & SYSTICK_ENABLE) { int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); if (value & SYSTICK_ENABLE) { if (s->tick) { s->tick += now; timer_mod(s->timer, s->tick); } else { systick_reload(s, 1); } } else { timer_del(s->timer); s->tick -= now; if (s->tick < 0) { s->tick = 0; } } } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) { /* This is a hack. Force the timer to be reloaded when the reference clock is changed. */ systick_reload(s, 1); } break; } case 0x4: /* SysTick Reload Value. */ s->reload = value; break; .. Yes, I'm for the for determinism, in my guest image there are some testing cases for timer system which locate in a small rtos. And for shift value, I mean it seems shift value impact system greatly, for the same one guest image and different shift value in count mode(-icount shift=4,align=off,sleep=off -rtc clock=vm) give very different accuracy for guest timer. So my question is how to calculate the shift value for the end user. Thanks again for your help. Thanks -Original Message- From: Qemu-devel [mailto:qemu-devel-bounces+wentong.wu=intel@nongnu.org] On Behalf Of Alex Bennée Sent: Monday, June 24, 2019 11:48 PM To: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] icount mode Wu, Wentong writes: > Hi all, > > Recently I'm using Qemu TCG icount mode, from the code I found Qemu > timers run at 1GHz, and for ArmV7M example, there will be conversion Are you talking about: #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */ because as far as I can tell that only affects the scaling factors applied to PMU counters. The internal counters (CNTFRQ_EL0 and friends) are hardwired to: /* Scale factor for generic timers, ie number of ns per tick. * This gives a 62.5MHz timer. */ #define GTIMER_SCALE 16 but this only affects the nominal rate the counters expire at. Software could attempt to reprogram it and the emulation will read-as-written but it won't actually change anything. However this only affects the clocks - it implies nothing about how fast the core may be executing. In fact unless you are using icount we will just run as fast a possible. > factor from qemu timer to SysTick frequency which will be calculated > by NANOSECONDS_PER_SECOND / SYSCLK_FRQ. You need to be a little more precise here. ARM systems vary in the number of timer sources they have. The qemu timers are an internal implementation detail for providing a way to track time. The value of SYSCLK_FRQ
[Qemu-devel] icount mode
Hi all, Recently I'm using Qemu TCG icount mode, from the code I found Qemu timers run at 1GHz, and for ArmV7M example, there will be conversion factor from qemu timer to SysTick frequency which will be calculated by NANOSECONDS_PER_SECOND / SYSCLK_FRQ. But the shift value also define the target cpu frequency(2^N ns /one instruction), and both frequencies will be used together to calculate the guest timer, so I think there is buggy because of the different frequency, can anyone give some explanation for this? Thanks a lot! Also can anyone give some hints about how to give the shift value when use icount TCG mode? Thanks Wentong Wu