the dll dependency of qemu.exe

2021-06-26 Thread Wu, Wentong
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

2021-01-27 Thread Wu, Wentong
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

2020-11-30 Thread Wu, Wentong
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

2020-11-29 Thread Wu, Wentong
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

2020-11-29 Thread Wu, Wentong
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

2020-11-27 Thread Wu, Wentong


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

2020-07-28 Thread Wu, Wentong

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

2020-07-28 Thread Wu, Wentong
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

2020-07-27 Thread Wu, Wentong
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

2020-07-14 Thread Wu, Wentong


> -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

2020-07-14 Thread Wu, Wentong


> -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

2020-07-12 Thread Wu, Wentong
> -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

2020-07-09 Thread Wu, Wentong
> >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

2020-07-06 Thread Wu, Wentong
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

2020-07-05 Thread Wu, Wentong
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

2020-07-05 Thread Wu, Wentong
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

2020-07-05 Thread Wu, Wentong
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

2020-07-05 Thread Wu, Wentong
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

2020-07-03 Thread Wu, Wentong
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

2020-07-03 Thread Wu, Wentong


  -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

2020-07-01 Thread Wu, Wentong
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

2020-06-17 Thread Wu, Wentong

> >
> > 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

2020-06-16 Thread Wu, Wentong

>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

2020-06-12 Thread Wu, Wentong
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

2020-06-11 Thread Wu, Wentong
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

2020-06-09 Thread Wu, Wentong
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

2020-06-07 Thread Wu, Wentong
@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

2020-06-05 Thread Wu, Wentong
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

2020-06-05 Thread Wu, Wentong
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

2019-08-19 Thread Wu, Wentong


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

2019-08-19 Thread Wu, Wentong


Could you please give some comments about this? Thanks a lot!



[Qemu-devel] qemu icount mode timer accuracy

2019-08-08 Thread Wu, Wentong


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]

2019-07-10 Thread Wu, Wentong
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]

2019-07-10 Thread Wu, Wentong


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

2019-06-25 Thread Wu, Wentong
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

2019-06-24 Thread Wu, Wentong
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

2019-06-24 Thread Wu, Wentong
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