[Qemu-devel] [Bug 965867] Re: 9p virtual file system on qemu slow
One of possible problems could be a block size. In this case I am using ZFS with raidZ 4+1 drives. Each drive has 4Kb block. So optimal block size is 16384 bytes. By optimizing block size it possible to improve performance 10 folds but 9p stably provides 10 folds worse performance than native writes. Some extra tests: - VM - mapped $ dd if=/dev/zero of=test count=10 10+0 records in 10+0 records out 5120 bytes (51 MB) copied, 20.7879 s, 2.5 MB/s $ dd if=/dev/zero of=test count=10 bs=16384 10+0 records in 10+0 records out 163840 bytes (1.6 GB) copied, 74.4378 s, 22.0 MB/s Host: $ dd if=/dev/zero of=test count=10 10+0 records in 10+0 records out 5120 bytes (51 MB) copied, 1.60118 s, 32.0 MB/s $ dd if=/dev/zero of=test count=10 bs=16384 10+0 records in 10+0 records out 163840 bytes (1.6 GB) copied, 4.89932 s, 334 MB/s Iggy: I has issue with permission in passthrough mode. Can you give an idea how to setup permissions in this mode? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/965867 Title: 9p virtual file system on qemu slow Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Confirmed Bug description: Hi, The 9p virtual file system is slow. Several examples below: - Host for the first time: $ time ls bam.unsorted/ ... real0m0.084s user0m0.000s sys 0m0.028s -- Host second and following: real0m0.009s user0m0.000s sys 0m0.008s -- VM for the first time: $ time ls bam.unsorted/ real0m23.141s user0m0.064s sys 0m2.156s -- VM for the second time real0m3.643s user0m0.024s sys 0m0.424s Copy on host: $ time cp 5173T.root.bak test.tmp real0m30.346s user0m0.004s sys 0m5.324s $ ls -lahs test.tmp 2.7G -rw--- 1 oneadmin cloud 2.7G Mar 26 21:47 test.tmp --- $ copy on VM for the same file time cp 5173T.root.bak test.tmp real5m46.978s user0m0.352s sys 1m38.962s To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/965867/+subscriptions
[Qemu-devel] [Bug 965867] Re: 9p virtual file system on qemu slow
>>>Can you try with security_model=passthrough? It provides the same results, see below: $ dd if=/dev/zero of=test count=10 10+0 records in 10+0 records out 5120 bytes (51 MB) copied, 19.8581 s, 2.6 MB/s $ dd if=/dev/zero of=test count=10 bs=16384 10+0 records in 10+0 records out 163840 bytes (1.6 GB) copied, 72.3009 s, 22.7 MB/s -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/965867 Title: 9p virtual file system on qemu slow Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Confirmed Bug description: Hi, The 9p virtual file system is slow. Several examples below: - Host for the first time: $ time ls bam.unsorted/ ... real0m0.084s user0m0.000s sys 0m0.028s -- Host second and following: real0m0.009s user0m0.000s sys 0m0.008s -- VM for the first time: $ time ls bam.unsorted/ real0m23.141s user0m0.064s sys 0m2.156s -- VM for the second time real0m3.643s user0m0.024s sys 0m0.424s Copy on host: $ time cp 5173T.root.bak test.tmp real0m30.346s user0m0.004s sys 0m5.324s $ ls -lahs test.tmp 2.7G -rw--- 1 oneadmin cloud 2.7G Mar 26 21:47 test.tmp --- $ copy on VM for the same file time cp 5173T.root.bak test.tmp real5m46.978s user0m0.352s sys 1m38.962s To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/965867/+subscriptions
[Qemu-devel] [Bug 965867] Re: 9p virtual file system on qemu slow
Hi Mohan, this parameter provide significant improvement in big file access/write: VirtFS on /srv/shared type 9p (rw,trans=virtio,version=9p2000.L,msize=262144) $ dd if=/dev/zero of=test count=10 bs=16384 10+0 records in 10+0 records out 163840 bytes (1.6 GB) copied, 36.3589 s, 45.1 MB/s l$ dd if=/dev/zero of=test count=10 10+0 records in 10+0 records out 5120 bytes (51 MB) copied, 25.6597 s, 2.0 MB/s $ dd if=/dev/zero of=test count=1024 bs=262144 1024+0 records in 1024+0 records out 268435456 bytes (268 MB) copied, 3.41936 s, 78.5 MB/s Speed of copy for large file ~45MB/s (read and write from the same disk). - But Host: time ls -lahs bam.unsorted/ many files: real0m0.053s user0m0.004s sys 0m0.036s VM: real0m4.449s user0m0.012s sys 0m0.136s So we have delays on the first file access. Is it possible to resolve this issue? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/965867 Title: 9p virtual file system on qemu slow Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Confirmed Bug description: Hi, The 9p virtual file system is slow. Several examples below: - Host for the first time: $ time ls bam.unsorted/ ... real0m0.084s user0m0.000s sys 0m0.028s -- Host second and following: real0m0.009s user0m0.000s sys 0m0.008s -- VM for the first time: $ time ls bam.unsorted/ real0m23.141s user0m0.064s sys 0m2.156s -- VM for the second time real0m3.643s user0m0.024s sys 0m0.424s Copy on host: $ time cp 5173T.root.bak test.tmp real0m30.346s user0m0.004s sys 0m5.324s $ ls -lahs test.tmp 2.7G -rw--- 1 oneadmin cloud 2.7G Mar 26 21:47 test.tmp --- $ copy on VM for the same file time cp 5173T.root.bak test.tmp real5m46.978s user0m0.352s sys 1m38.962s To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/965867/+subscriptions
[Qemu-devel] [Bug 899664] [NEW] Bad internet performance for Host to Guest or Guest to Host
Public bug reported: Hi, Internet performance for Host to Quest is low. The speed Guest to same Guest is 11.3 Gbits/sec The speed Host to same Host is similar (9.8-11 Gbits/sec) But the speed from Guest to Host is slow and around 1Gbit/sec. In the reality traffic never leave a Host. I expected that in this case speed should be close to Host to Host. It is important for virtual infrastructure when you have several Guests on a same Host. Guest to Guest on a same host has speed around 1 Gbits/sec too. Below you can find testes and additional information: = biouml@biouml-db:~$ iperf -c 192.168.2.31 -t 30 #Guest to Guest Client connecting to 192.168.2.31, TCP port 5001 TCP window size: 49.7 KByte (default) [ 3] local 192.168.2.31 port 52170 connected with 192.168.2.31 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-30.0 sec 39.6 GBytes 11.3 Gbits/sec biouml@biouml-db:~$ iperf -c 192.168.2.11 -t 30 # Guest to Host Client connecting to 192.168.2.11, TCP port 5001 TCP window size: 43.4 KByte (default) [ 3] local 192.168.2.31 port 47148 connected with 192.168.2.11 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-30.0 sec 3.69 GBytes 1.06 Gbits/sec biouml@biouml-db:~$ root@s2-8core:~# iperf -c 192.168.2.30 -t 30 #host to guest Client connecting to 192.168.2.30, TCP port 5001 TCP window size: 16.0 KByte (default) [ 3] local 192.168.2.11 port 57403 connected with 192.168.2.30 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-30.0 sec 5.47 GBytes 1.57 Gbits/sec == root@s2-8core:~# iperf -c 192.168.2.11 -t 30 #host to host Client connecting to 192.168.2.11, TCP port 5001 TCP window size: 49.7 KByte (default) [ 3] local 192.168.2.11 port 38313 connected with 192.168.2.11 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-30.0 sec 34.5 GBytes 9.87 Gbits/sec root@s2-8core:~# I am using virtio drivers and virtual machine was started with following parameters: /usr/bin/kvm -S -M pc-1.0 -enable-kvm -m 4096 -smp 4,sockets=4,cores=1,threads=1 -name one-25 -uuid d1e125ee-d692-4598-3a75-441cd79a513a -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/one-25.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -drive file=/var/lib/one/OpenNebula/var//25/images/disk.0,if=none,id=drive-virtio-disk0,format=raw,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/var/lib/one/OpenNebula/var//25/images/disk.1,if=none,id=drive-virtio-disk3,format=raw -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk3,id=virtio-disk3 -netdev tap,fd=19,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=02:00:c0:a8:02:02,bus=pci.0,addr=0x3 -usb -device usb-mouse,id=input0 -vnc 0.0.0.0:98 -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 = Qemu version: root@s2-8core:~# /usr/bin/kvm --version QEMU emulator version 0.15.92, Copyright (c) 2003-2008 Fabrice Bellard root@s2-8core:~# ls -la /usr/bin/kvm lrwxrwxrwx 1 root root 27 2011-11-07 17:34 /usr/bin/kvm -> /usr/bin/qemu-system-x86_64 == Bridge configuration: root@s2-8core:~# cat /etc/network/interfaces auto lo iface lo inet loopback auto eth0 iface eth0 inet manual auto eth1 iface eth1 inet manual auto br0 iface br0 inet static address 192.168.2.11 network 192.168.2.0 netmask 255.255.255.0 broadcast 192.168.2.255 gateway 192.168.2.1 bridge_ports eth0 bridge_stp on bridge_fd 0 bridge_maxwait 0 root@s2-8core:~# root@s2-8core:~# brctl show bridge name bridge id STP enabled interfaces br0 8000.001e8cec6113 yes eth0 vnet0 virbr0 8000. yes root@s2-8core:~# brctl --version bridge-utils, 1.5 ===
Re: [Qemu-devel] [PATCH] eepro100: prevent an infinite loop over same command block
I will try to test the PoC on real e100. But this work may need some more time. 发自我的 iPhone > 在 2015年10月20日,上午11:04,Jason Wang 写道: > > > >> On 10/17/2015 07:35 PM, Peter Maydell wrote: >>> On 16 October 2015 at 22:37, Stefan Weil wrote: >>> Maybe real hardware will run an endless loop? >>> Or the "endless" loop is terminated because the driver >>> changes the link while the loop is running? >>> >>> The goal of eepro100.c should be emulation of the >>> real hardware, even of a potential design weakness. >> I agree in general, but we need to be sure that if we're >> letting the guest put a device into an infinite loop this >> doesn't lock up the whole VM (ie preventing the user >> from using the qemu monitor or otherwise rebooting it). >> >> thanks >> -- PMM > > Yes, so the reproducer needs to be tested on real hardware. > > Qinghao: > > Any chance to test the reproducer on real e100? If not, I will try to > find one to test. > > Thanks
[Qemu-devel] [Bug 636095] [NEW] tap downscript is not executed when exiting qemu through "quit" monitor command
Public bug reported: When you tell qemu to shutdown using the "quit" monitor command, the downscript of the tap interface is not executed. To reproduce: Create the test script /tmp/qemu-ifdown-test.sh : == #!/bin/bash touch /tmp/is_this_working == Run: == # chmod +x /tmp/qemu-ifdown-test.sh # qemu-system-x86_64 -daemonize -net nic -net tap,script=/etc/qemu-ifup,downscript=/tmp/qemu-ifdown-test.sh -monitor unix:/tmp/monitor.socket,nowait,server VNC server running on `127.0.0.1:5900' # nc -U /tmp/monitor.socket QEMU 0.12.5 monitor - type 'help' for more information (qemu) quit quit # ls /tmp/is* ls: cannot access /tmp/is*: No such file or directory == If I quit qemu by sending a SIGTERM instead of using the "quit" command, the downscript does get executed: == # qemu-system-x86_64 -daemonize -net nic -net tap,script=/etc/qemu-ifup,downscript=/tmp/qemu-ifdown-test.sh -monitor unix:/tmp/monitor.socket,nowait,server VNC server running on `127.0.0.1:5900' # killall qemu-system-x86_64 # ls /tmp/is* /tmp/is_this_working == Issue occurs with both 0.12.3 and 0.12.5 ** Affects: qemu Importance: Undecided Status: New -- tap downscript is not executed when exiting qemu through "quit" monitor command https://bugs.launchpad.net/bugs/636095 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: When you tell qemu to shutdown using the "quit" monitor command, the downscript of the tap interface is not executed. To reproduce: Create the test script /tmp/qemu-ifdown-test.sh : == #!/bin/bash touch /tmp/is_this_working == Run: == # chmod +x /tmp/qemu-ifdown-test.sh # qemu-system-x86_64 -daemonize -net nic -net tap,script=/etc/qemu-ifup,downscript=/tmp/qemu-ifdown-test.sh -monitor unix:/tmp/monitor.socket,nowait,server VNC server running on `127.0.0.1:5900' # nc -U /tmp/monitor.socket QEMU 0.12.5 monitor - type 'help' for more information (qemu) quit quit # ls /tmp/is* ls: cannot access /tmp/is*: No such file or directory == If I quit qemu by sending a SIGTERM instead of using the "quit" command, the downscript does get executed: == # qemu-system-x86_64 -daemonize -net nic -net tap,script=/etc/qemu-ifup,downscript=/tmp/qemu-ifdown-test.sh -monitor unix:/tmp/monitor.socket,nowait,server VNC server running on `127.0.0.1:5900' # killall qemu-system-x86_64 # ls /tmp/is* /tmp/is_this_working == Issue occurs with both 0.12.3 and 0.12.5
[PATCH] virtio-vga, stubs: Fix qemu failing on virio-vga with blobs on when qemu compiled with modules enabled
>From f972dab518c6581a83f397c35b3d09b2ef6de674 Mon Sep 17 00:00:00 2001 From: max Date: Sun, 22 Aug 2021 14:02:48 +0300 Subject: [PATCH] virtio-vga, stubs: Fix qemu failing on virio-vga with blobs on when qemu compiled with modules enabled Resolves: https://gitlab.com/qemu-project/qemu/-/issues/553 Signed-off-by: max --- stubs/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stubs/meson.build b/stubs/meson.build index d3fa864..beb737f 100644 --- a/stubs/meson.build +++ b/stubs/meson.build @@ -53,7 +53,7 @@ if have_system stub_ss.add(files('semihost.c')) stub_ss.add(files('usb-dev-stub.c')) stub_ss.add(files('xen-hw-stub.c')) - stub_ss.add(files('virtio-gpu-udmabuf.c')) + stub_ss.add(when: 'CONFIG_LINUX', if_false: files('virtio-gpu-udmabuf.c')) else stub_ss.add(files('qdev.c')) endif -- 2.32.0
Re: [Qemu-devel] For all targets and machine types: "start to monitor" smoke test
On Tue, Aug 7, 2012 at 11:26 PM, Markus Armbruster wrote: > Very basic smoke test: start QEMU with -monitor stdio, quit immediately. > Wouldn't it be nice if that worked for all targets and machine types? > > Many targets have mandatory options (fun oxymoron), such as -kernel or > -pflash. Can't stop me, I just try a bunch until something works. > > Many targets expect various files to be present, and some of them need > to have the right size. Can't stop me, I hack up the file loaders until > it works (silly patch appended). To do this right, we'd need the > required files or suitable mock-ups in-tree. [...] > Summary of results: > > * Bad unexplained > qemu-system-xtensaeb lx60 > qemu-system-xtensaeb lx200 > qemu-system-xtensaeb sim > Unable to find CPU definition > > I'm not saying these are all busted. If you know how to "start to > monitor" one of these, let us know. For xtensa the default CPU name is the same for both big-endian and little-endian binaries, but actually it denotes little-endian CPU not supported by xtensaeb. It will start with the explicit -cpu fsf. I can post a patch to make fsf the default CPU type in big-endian case. -- Thanks. -- Max
[Qemu-devel] [PATCH] target-xtensa: make default CPU depend on target endianness
This makes usable default for -cpu option both for qemu-system-xtensa and qemu-system-xtensaeb fixing the following error: $ qemu-system-xtensaeb -M sim Unable to find CPU definition Signed-off-by: Max Filippov --- hw/xtensa_lx60.c|6 +++--- hw/xtensa_sim.c |4 ++-- target-xtensa/cpu.h |6 ++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/xtensa_lx60.c b/hw/xtensa_lx60.c index 152eed9..26a0c75 100644 --- a/hw/xtensa_lx60.c +++ b/hw/xtensa_lx60.c @@ -173,7 +173,7 @@ static void lx_init(const LxBoardDesc *board, int n; if (!cpu_model) { -cpu_model = "dc232b"; +cpu_model = XTENSA_DEFAULT_CPU_MODEL; } for (n = 0; n < smp_cpus; n++) { @@ -300,14 +300,14 @@ static void xtensa_lx200_init(ram_addr_t ram_size, static QEMUMachine xtensa_lx60_machine = { .name = "lx60", -.desc = "lx60 EVB (dc232b)", +.desc = "lx60 EVB (" XTENSA_DEFAULT_CPU_MODEL ")", .init = xtensa_lx60_init, .max_cpus = 4, }; static QEMUMachine xtensa_lx200_machine = { .name = "lx200", -.desc = "lx200 EVB (dc232b)", +.desc = "lx200 EVB (" XTENSA_DEFAULT_CPU_MODEL ")", .init = xtensa_lx200_init, .max_cpus = 4, }; diff --git a/hw/xtensa_sim.c b/hw/xtensa_sim.c index 1ce07fb..ed38bd4 100644 --- a/hw/xtensa_sim.c +++ b/hw/xtensa_sim.c @@ -102,7 +102,7 @@ static void xtensa_sim_init(ram_addr_t ram_size, const char *initrd_filename, const char *cpu_model) { if (!cpu_model) { -cpu_model = "dc232b"; +cpu_model = XTENSA_DEFAULT_CPU_MODEL; } sim_init(ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); @@ -110,7 +110,7 @@ static void xtensa_sim_init(ram_addr_t ram_size, static QEMUMachine xtensa_sim_machine = { .name = "sim", -.desc = "sim machine (dc232b)", +.desc = "sim machine (" XTENSA_DEFAULT_CPU_MODEL ")", .init = xtensa_sim_init, .max_cpus = 4, }; diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index f7db116..177094a 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -351,6 +351,12 @@ typedef struct CPUXtensaState { #define cpu_signal_handler cpu_xtensa_signal_handler #define cpu_list xtensa_cpu_list +#ifdef TARGET_WORDS_BIGENDIAN +#define XTENSA_DEFAULT_CPU_MODEL "fsf" +#else +#define XTENSA_DEFAULT_CPU_MODEL "dc232b" +#endif + XtensaCPU *cpu_xtensa_init(const char *cpu_model); static inline CPUXtensaState *cpu_init(const char *cpu_model) -- 1.7.7.6
[Qemu-devel] [PATCH] target-xtensa: make 'sim' to be the default machine
This fixes the following error: $ qemu-system-xtensa -cpu help Segmentation fault Signed-off-by: Max Filippov --- hw/xtensa_sim.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/xtensa_sim.c b/hw/xtensa_sim.c index ed38bd4..831460b 100644 --- a/hw/xtensa_sim.c +++ b/hw/xtensa_sim.c @@ -111,6 +111,7 @@ static void xtensa_sim_init(ram_addr_t ram_size, static QEMUMachine xtensa_sim_machine = { .name = "sim", .desc = "sim machine (" XTENSA_DEFAULT_CPU_MODEL ")", +.is_default = true, .init = xtensa_sim_init, .max_cpus = 4, }; -- 1.7.7.6
Re: [Qemu-devel] [PATCH] target-xtensa: make 'sim' to be the default machine
On Thu, Aug 9, 2012 at 11:43 AM, Markus Armbruster wrote: > Max Filippov writes: > >> This fixes the following error: >> >> $ qemu-system-xtensa -cpu help >> Segmentation fault > > main() attempts to cope with "no machine found", it just screws it up: > > if (machine->hw_version) { > qemu_set_version(machine->hw_version); > } > [...] > if (machine == NULL) { > fprintf(stderr, "No machine found.\n"); > exit(1); > } > > Probably broken in commit 93bfef4c, cc'ing its author. > > Of course, I don't mind you picking a default machine for xtensa, if > that's what you want. I guess it would make no harm. Also this way it will less likely break next time, and I remember Blue once wondered why xtensa doesn't have the default machine. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH 23/23] xtensa: Suppress unused default drives
On Thu, Aug 9, 2012 at 5:31 PM, Markus Armbruster wrote: > Cc: Max Filippov > > Suppress default floppy, CD-ROM and SD card drives for machines lx60, > lx200, sim. > > Signed-off-by: Markus Armbruster Acked-by: Max Filippov Though I'd agree with Andreas about inverting default device creation logic. -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 8:36 PM, Steven wrote: > On Thu, Aug 16, 2012 at 4:02 AM, 陳韋任 (Wei-Ren Chen) > wrote: >>> I would like to is there any function that could log the register >>> content of the guest machine, like "info registers" in the qemu >>> monitor mode. >> >> Why not check how "info registes" be implemented in QEMU? ;) >> I guess you just have to log env->regs or something like that. > Thanks for pointing this out. > I would like to get a trace of guest memory access. So I can not use > "info registers". > What I want to do is that when tcg fetches a load instruction at > disas_insns(), the guest memory address should be calculated. For No, you don't want this, because the same translated code may be invoked multiple times with different values in registers. > example, the tb has an instruction of mov 0x4(%ebx) %eax. > To calculate the address of 0x4(%ebx), I need to know the value of %ebx. > Is this correct? Thanks. Why don't you just instrument actual memory access functions in softmmu_template.h ? -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 9:29 PM, Steven wrote: > On Thu, Aug 16, 2012 at 1:00 PM, Max Filippov wrote: >> On Thu, Aug 16, 2012 at 8:36 PM, Steven wrote: >>> On Thu, Aug 16, 2012 at 4:02 AM, 陳韋任 (Wei-Ren Chen) >>> wrote: >>>>> I would like to is there any function that could log the register >>>>> content of the guest machine, like "info registers" in the qemu >>>>> monitor mode. >>>> >>>> Why not check how "info registes" be implemented in QEMU? ;) >>>> I guess you just have to log env->regs or something like that. >>> Thanks for pointing this out. >>> I would like to get a trace of guest memory access. So I can not use >>> "info registers". >>> What I want to do is that when tcg fetches a load instruction at >>> disas_insns(), the guest memory address should be calculated. For >> >> No, you don't want this, because the same translated code may be >> invoked multiple times with different values in registers. >> >>> example, the tb has an instruction of mov 0x4(%ebx) %eax. >>> To calculate the address of 0x4(%ebx), I need to know the value of %ebx. >>> Is this correct? Thanks. >> >> Why don't you just instrument actual memory access functions in >> softmmu_template.h ? > But this code only touches the s->pc. For registers in the load > instruction, it won't generate the memory access code. So I need to > add code to some function to get the guest memory address access. Take a close look at DATA_TYPE glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_ulong addr, int mmu_idx) and void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM target_ulong addr, DATA_TYPE val, int mmu_idx) At runtime they get addr, this is the virtual address of the memory access. This file is included several times to instantiate these functions for different memory access types. A set of macros manipulates access size and whether it is code or data access. -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 9:37 PM, Max Filippov wrote: > On Thu, Aug 16, 2012 at 9:29 PM, Steven wrote: >> On Thu, Aug 16, 2012 at 1:00 PM, Max Filippov wrote: >>> On Thu, Aug 16, 2012 at 8:36 PM, Steven wrote: >>>> On Thu, Aug 16, 2012 at 4:02 AM, 陳韋任 (Wei-Ren Chen) >>>> wrote: >>>>>> I would like to is there any function that could log the register >>>>>> content of the guest machine, like "info registers" in the qemu >>>>>> monitor mode. >>>>> >>>>> Why not check how "info registes" be implemented in QEMU? ;) >>>>> I guess you just have to log env->regs or something like that. >>>> Thanks for pointing this out. >>>> I would like to get a trace of guest memory access. So I can not use >>>> "info registers". >>>> What I want to do is that when tcg fetches a load instruction at >>>> disas_insns(), the guest memory address should be calculated. For >>> >>> No, you don't want this, because the same translated code may be >>> invoked multiple times with different values in registers. >>> >>>> example, the tb has an instruction of mov 0x4(%ebx) %eax. >>>> To calculate the address of 0x4(%ebx), I need to know the value of %ebx. >>>> Is this correct? Thanks. >>> >>> Why don't you just instrument actual memory access functions in >>> softmmu_template.h ? >> But this code only touches the s->pc. For registers in the load >> instruction, it won't generate the memory access code. So I need to >> add code to some function to get the guest memory address access. > > Take a close look at > > DATA_TYPE > glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM >target_ulong addr, >int mmu_idx) > > and > > void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM > target_ulong addr, > DATA_TYPE val, > int mmu_idx) > > At runtime they get addr, this is the virtual address of the memory access. > This file is included several times to instantiate these functions for > different memory access types. > A set of macros manipulates access size and whether it is code or data access. But maybe I got you wrong and by What I want to do is that when tcg fetches a load instruction at disas_insns(), the guest memory address should be calculated. you meant that you need to record code address that made an access, not the accessed data address? -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 9:49 PM, Steven wrote: > On Thu, Aug 16, 2012 at 1:43 PM, Max Filippov wrote: >> On Thu, Aug 16, 2012 at 9:37 PM, Max Filippov wrote: >>> On Thu, Aug 16, 2012 at 9:29 PM, Steven wrote: >>>> On Thu, Aug 16, 2012 at 1:00 PM, Max Filippov wrote: >>>>> On Thu, Aug 16, 2012 at 8:36 PM, Steven wrote: >>>>>> On Thu, Aug 16, 2012 at 4:02 AM, 陳韋任 (Wei-Ren Chen) >>>>>> wrote: >>>>>>>> I would like to is there any function that could log the register >>>>>>>> content of the guest machine, like "info registers" in the qemu >>>>>>>> monitor mode. >>>>>>> >>>>>>> Why not check how "info registes" be implemented in QEMU? ;) >>>>>>> I guess you just have to log env->regs or something like that. >>>>>> Thanks for pointing this out. >>>>>> I would like to get a trace of guest memory access. So I can not use >>>>>> "info registers". >>>>>> What I want to do is that when tcg fetches a load instruction at >>>>>> disas_insns(), the guest memory address should be calculated. For >>>>> >>>>> No, you don't want this, because the same translated code may be >>>>> invoked multiple times with different values in registers. >>>>> >>>>>> example, the tb has an instruction of mov 0x4(%ebx) %eax. >>>>>> To calculate the address of 0x4(%ebx), I need to know the value of %ebx. >>>>>> Is this correct? Thanks. >>>>> >>>>> Why don't you just instrument actual memory access functions in >>>>> softmmu_template.h ? >>>> But this code only touches the s->pc. For registers in the load >>>> instruction, it won't generate the memory access code. So I need to >>>> add code to some function to get the guest memory address access. >>> >>> Take a close look at >>> >>> DATA_TYPE >>> glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM >>>target_ulong addr, >>>int mmu_idx) >>> >>> and >>> >>> void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM >>> target_ulong >>> addr, >>> DATA_TYPE val, >>> int mmu_idx) >>> >>> At runtime they get addr, this is the virtual address of the memory access. >>> This file is included several times to instantiate these functions for >>> different memory access types. >>> A set of macros manipulates access size and whether it is code or data >>> access. >> >> But maybe I got you wrong and by >> >> What I want to do is that when tcg fetches a load instruction at >> disas_insns(), the guest memory address should be calculated. >> >> you meant that you need to record code address that made an access, >> not the accessed data address? >> > I want to get the guest memory address in the instruction mov > 0x4(%ebx) %eax, whic is 0x4(%ebx). > Since %ebx is not resolved until the execution time, the code in > softmmu_header.h does not generate any hit or miss information. > Do you know any place that I could resolve the memory access address? Thanks. I see that with the following patch diff --git a/softmmu_template.h b/softmmu_template.h index b8bd700..2d02133 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_phys_addr_t ioaddr; uintptr_t retaddr; +fprintf(stderr, "%s: %08x\n", __func__, addr); /* test if there is match for unaligned or IO access */ /* XXX: could done more in memory macro in a non portable way */ index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); I get some memory accesses logged, but not all. That's due to fast path in tcg_out_qemu_ld in case there's TLB hit. I guess you can play with tcg_out_qemu_ld and make it produce a call to a helper function, like qemu_ld_helpers, that will print addresses for all memory access attempts. -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 10:31 PM, Max Filippov wrote: > On Thu, Aug 16, 2012 at 9:49 PM, Steven wrote: >> On Thu, Aug 16, 2012 at 1:43 PM, Max Filippov wrote: >>> On Thu, Aug 16, 2012 at 9:37 PM, Max Filippov wrote: >>>> On Thu, Aug 16, 2012 at 9:29 PM, Steven wrote: >>>>> On Thu, Aug 16, 2012 at 1:00 PM, Max Filippov wrote: >>>>>> On Thu, Aug 16, 2012 at 8:36 PM, Steven wrote: >>>>>>> On Thu, Aug 16, 2012 at 4:02 AM, 陳韋任 (Wei-Ren Chen) >>>>>>> wrote: >>>>>>>>> I would like to is there any function that could log the register >>>>>>>>> content of the guest machine, like "info registers" in the qemu >>>>>>>>> monitor mode. >>>>>>>> >>>>>>>> Why not check how "info registes" be implemented in QEMU? ;) >>>>>>>> I guess you just have to log env->regs or something like that. >>>>>>> Thanks for pointing this out. >>>>>>> I would like to get a trace of guest memory access. So I can not use >>>>>>> "info registers". >>>>>>> What I want to do is that when tcg fetches a load instruction at >>>>>>> disas_insns(), the guest memory address should be calculated. For >>>>>> >>>>>> No, you don't want this, because the same translated code may be >>>>>> invoked multiple times with different values in registers. >>>>>> >>>>>>> example, the tb has an instruction of mov 0x4(%ebx) %eax. >>>>>>> To calculate the address of 0x4(%ebx), I need to know the value of %ebx. >>>>>>> Is this correct? Thanks. >>>>>> >>>>>> Why don't you just instrument actual memory access functions in >>>>>> softmmu_template.h ? >>>>> But this code only touches the s->pc. For registers in the load >>>>> instruction, it won't generate the memory access code. So I need to >>>>> add code to some function to get the guest memory address access. >>>> >>>> Take a close look at >>>> >>>> DATA_TYPE >>>> glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM >>>>target_ulong addr, >>>>int mmu_idx) >>>> >>>> and >>>> >>>> void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM >>>> target_ulong >>>> addr, >>>> DATA_TYPE val, >>>> int mmu_idx) >>>> >>>> At runtime they get addr, this is the virtual address of the memory access. >>>> This file is included several times to instantiate these functions for >>>> different memory access types. >>>> A set of macros manipulates access size and whether it is code or data >>>> access. >>> >>> But maybe I got you wrong and by >>> >>> What I want to do is that when tcg fetches a load instruction at >>> disas_insns(), the guest memory address should be calculated. >>> >>> you meant that you need to record code address that made an access, >>> not the accessed data address? >>> >> I want to get the guest memory address in the instruction mov >> 0x4(%ebx) %eax, whic is 0x4(%ebx). >> Since %ebx is not resolved until the execution time, the code in >> softmmu_header.h does not generate any hit or miss information. >> Do you know any place that I could resolve the memory access address? Thanks. > > I see that with the following patch > > diff --git a/softmmu_template.h b/softmmu_template.h > index b8bd700..2d02133 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), > MMUSUFFIX)(ENV_PARAM > target_phys_addr_t ioaddr; > uintptr_t retaddr; > > +fprintf(stderr, "%s: %08x\n", __func__, addr); > /* test if there is match for unaligned or IO access */ > /* XXX: could done more in memory macro in a non portable way */ > index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > > I get some memory accesses logged, but not all. That'
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Fri, Aug 17, 2012 at 9:38 AM, Steven wrote: > Hi, Max, > I appreciate your help and got some results using your patch. But I > still have two questions as blow. > >>> I see that with the following patch >>> >>> diff --git a/softmmu_template.h b/softmmu_template.h >>> index b8bd700..2d02133 100644 >>> --- a/softmmu_template.h >>> +++ b/softmmu_template.h >>> @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), >>> MMUSUFFIX)(ENV_PARAM >>> target_phys_addr_t ioaddr; >>> uintptr_t retaddr; >>> >>> +fprintf(stderr, "%s: %08x\n", __func__, addr); >>> /* test if there is match for unaligned or IO access */ >>> /* XXX: could done more in memory macro in a non portable way */ >>> index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >>> >>> I get some memory accesses logged, but not all. That's due to fast >>> path in tcg_out_qemu_ld >>> in case there's TLB hit. I guess you can play with tcg_out_qemu_ld and >>> make it produce a call >>> to a helper function, like qemu_ld_helpers, that will print addresses >>> for all memory access >>> attempts. >> >> Easier solution would be to disable fast path and always go through >> softmmu helpers, like this (specific for x86 host): >> >> diff --git a/softmmu_template.h b/softmmu_template.h >> index b8bd700..2d02133 100644 >> --- a/softmmu_template.h >> +++ b/softmmu_template.h >> @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), >> MMUSUFFIX)(ENV_PARAM >> target_phys_addr_t ioaddr; >> uintptr_t retaddr; >> >> +fprintf(stderr, "%s: %08x\n", __func__, addr); >> /* test if there is match for unaligned or IO access */ >> /* XXX: could done more in memory macro in a non portable way */ >> index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c >> index da17bba..ec68c19 100644 >> --- a/tcg/i386/tcg-target.c >> +++ b/tcg/i386/tcg-target.c >> @@ -1062,7 +1062,7 @@ static inline void tcg_out_tlb_load(TCGContext >> *s, int addrlo_idx, >> tcg_out_mov(s, type, r0, addrlo); >> >> /* jne label1 */ >> -tcg_out8(s, OPC_JCC_short + JCC_JNE); >> +tcg_out8(s, OPC_JMP_short); >> label_ptr[0] = s->code_ptr; >> s->code_ptr++; >> > > IN: > 0xc13e3a33: mov0x8(%ebp),%ebx (guest code in the tb) > __ldl_mmu: c13a9fdc > > So 0xc13a9fdc is the guest virtual memory address of 0x8(%ebp). Is this > correct? Right. > IN: > 0xc13e3a36: mov%eax,-0x10(%ebp) > However, for this instruction, no ldl_mmu is logged. > Does that mean the patch you provided does not cover this case? Yes, this is not 'ld', it is 'st'; to see it too I guess you need this: diff --git a/softmmu_template.h b/softmmu_template.h index b8bd700..b2ae078 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_phys_addr_t ioaddr; uintptr_t retaddr; +fprintf(stderr, "%s: %08x\n", __func__, addr); /* test if there is match for unaligned or IO access */ /* XXX: could done more in memory macro in a non portable way */ index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); @@ -263,6 +264,7 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM uintptr_t retaddr; int index; +fprintf(stderr, "%s: %08x\n", __func__, addr); index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); redo: tlb_addr = env->tlb_table[mmu_idx][index].addr_write; -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Fri, Aug 17, 2012 at 3:14 PM, 陳韋任 (Wei-Ren Chen) wrote: >> > On Thu, Aug 16, 2012 at 7:49 PM, Steven wrote: >> > [...] >> >> I want to get the guest memory address in the instruction mov >> >> 0x4(%ebx) %eax, whic is 0x4(%ebx). >> >> Since %ebx is not resolved until the execution time, the code in >> >> softmmu_header.h does not generate any hit or miss information. >> >> Do you know any place that I could resolve the memory access address? >> >> Thanks. >> > >> > You'll have to generate code. Look at how helpers work. >> Hi, Laurent, >> do you mean the target-i386/op_helper.c/helper.c or the tcg helper? Thanks. > > What do you mean by "resolve the memory access address"? Do you want > to get guest virtual address for each guest memory access, right? As Max > mentioned before (you can also read [1]), there are fast and slow path > in QEMU softmmu, tlb hit and tlb miss respectively. Max provided patch > for slow path. As for fast path, take a look on tcg_out_tlb_load (tcg > /i386/tcg-target.c). tcg_out_tlb_load will generate native code in the > code cache to do tlb lookup, I think you cannot use the trick Max used > since tcg_out_tlb_load will not be called when the fast path executed, That's why I've posted the following hunk that should have made all accesses go via slow path: diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index da17bba..ec68c19 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1062,7 +1062,7 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx, tcg_out_mov(s, type, r0, addrlo); /* jne label1 */ -tcg_out8(s, OPC_JCC_short + JCC_JNE); +tcg_out8(s, OPC_JMP_short); label_ptr[0] = s->code_ptr; s->code_ptr++; > it "generates" code instead. Therefore, you might have to insert your > instrument code in the code cache, perhaps modifying tcg_out_tlb_load > to log value of "addrlo" (see comments above tcg_out_tlb_load). -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Tue, Aug 21, 2012 at 9:40 AM, Steven wrote: > Hi, Max, > I wrote a small program to verify your patch could catch all the load > instructions from the guest. However, I found some problem from the > results. > > The guest OS and the emulated machine are both 32bit x86. My simple > program in the guest declares an 1048576-element integer array, > initialize the elements, and load them in a loop. It looks like this > int array[1048576]; > initialize the array; > > /* region of interests */ > int temp; > for (i=0; i < 1048576; i++) { > temp = array[i]; > } > So ideally, the path should catch the guest virtual address of in the > loop, right? > In addition, the virtual address for the beginning and end > of the array is 0xbf68b6e0 and 0xbfa8b6e0. > What i got is as follows > > __ldl_mmu, vaddr=bf68b6e0 > __ldl_mmu, vaddr=bf68b6e4 > __ldl_mmu, vaddr=bf68b6e8 > . > These should be the virtual address of the above loop. The > results look good because the gap between each vaddr is 4 bypte, which > is the length of each element. > However, after certain address, I got > > __ldl_mmu, vaddr=bf68bffc > __ldl_mmu, vaddr=bf68c000 > __ldl_mmu, vaddr=bf68d000 > __ldl_mmu, vaddr=bf68e000 > __ldl_mmu, vaddr=bf68f000 > __ldl_mmu, vaddr=bf69 > __ldl_mmu, vaddr=bf691000 > __ldl_mmu, vaddr=bf692000 > __ldl_mmu, vaddr=bf693000 > __ldl_mmu, vaddr=bf694000 > ... > __ldl_mmu, vaddr=bf727000 > __ldl_mmu, vaddr=bf728000 > __ldl_mmu, vaddr=bfa89000 > __ldl_mmu, vaddr=bfa8a000 > So the rest of the vaddr I got has a different of 4096 bytes, instead > of 4. I repeated the experiment for several times and got the same > results. Is there anything wrong? or could you explain this? Thanks. I see two possibilities here: - maybe there are more fast path shortcuts in the QEMU code? in that case output of qemu -d op,out_asm would help. - maybe your compiler had optimized that sample code? could you try to declare array in your sample as 'volatile int'? -- Thanks. -- Max
Re: [Qemu-devel] Dump guest page table inside QEMU makes system hang
On Tue, Aug 21, 2012 at 10:19 PM, Blue Swirl wrote: > On Tue, Aug 21, 2012 at 7:21 AM, 陳韋任 (Wei-Ren Chen) > wrote: >> Hi all, >> >> I want to dump guest page table when guest writes to cr3, >> the code snipt below, >> >> --- >> uint32_t pgd[1024][1024]; // guest page table >> static void dump_guest_pgtable(target_ulong cr3) >> { >> int i, j; >> uint32_t phyaddr = cr3; >> uint32_t val; >> >> for (i = 0; i < NUM_ENTRY; ++i) >> { >> phyaddr += i * 4; >> for (j = 0; j < NUM_ENTRY; ++j) >> { >> cpu_physical_memory_read(phyaddr, &val, 4); >> pgd[i][j] = val; >> } >> } >> } >> >> void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3) >> { >> env->cr[3] = new_cr3; // guest cr3 >> >> if (env->cr[0] & CR0_PG_MASK) { >> tlb_flush(env, 0); >> >> // dump guest page table by using guest cr3 >> dump_guest_pgtable(new_cr3); >> } >> } >> --- >> >> The system will hang while booting. However, if I comment >> cpu_physical_memory_read in function dump_guest_pgtable, there >> is no problem. What I am missing here? Thanks. > > cpu_physical_memory_read() can cause faults or other side effects like > MMIO. Using cpu_get_phys_page_debug() may help. > Maybe you just need to avoid accessing unsuitable physical addresses? Or maybe 'if (env->cr[0] & CR0_PG_MASK)' is not strong enough, may (CR0_PG_MASK | CR0_PE_MASK) be better? At what stage does it hang? What CR3 value changes are observed before the hang? -- Thanks. -- Max
[Qemu-devel] [PATCH for-1.2] target-xtensa: return EINVAL for unimplemented simcalls
This prevents guest from proceeding with uninitialised garbage returned from unimplemented simcalls. Signed-off-by: Max Filippov --- target-xtensa/xtensa-semi.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c index 1c8a19e..9afa99d 100644 --- a/target-xtensa/xtensa-semi.c +++ b/target-xtensa/xtensa-semi.c @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) default: qemu_log("%s(%d): not implemented\n", __func__, regs[2]); +regs[2] = -1; +regs[3] = EINVAL; break; } } -- 1.7.7.6
Re: [Qemu-devel] [PATCH for-1.2] target-xtensa: return EINVAL for unimplemented simcalls
On Wed, Aug 22, 2012 at 9:15 PM, Max Filippov wrote: > This prevents guest from proceeding with uninitialised garbage returned > from unimplemented simcalls. Oops, looks like ENOSYS is the right errno for that. -- Thanks. -- Max
[Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls
This prevents guest from proceeding with uninitialised garbage returned from unimplemented simcalls. Signed-off-by: Max Filippov --- target-xtensa/xtensa-semi.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c index 1c8a19e..6d001c2 100644 --- a/target-xtensa/xtensa-semi.c +++ b/target-xtensa/xtensa-semi.c @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) default: qemu_log("%s(%d): not implemented\n", __func__, regs[2]); +regs[2] = -1; +regs[3] = ENOSYS; break; } } -- 1.7.7.6
Re: [Qemu-devel] [PATCH] monitor: move json init from OPEN event to init
On Thu, Aug 23, 2012 at 5:22 PM, Anthony Liguori wrote: > At some point in the past, the OPEN event was changed to be issued from a > bottom half. This creates a small window whereas a data callback registered > in > init may be invoked before the OPEN event has been issued. > > This is reproducible with: > > echo "{'execute': 'qmp_capabilities'}" | qemu-system-x86_64 -M none -qmp > stdio > > We can fix this for the monitor by moving the parser initialization to init. > > The remaining state that is set in OPEN appears harmless. > > Reported-by: Daniel Berrange > Signed-off-by: Anthony Liguori > --- > monitor.c |4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 480f583..b188582 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -4832,7 +4832,6 @@ static void monitor_control_event(void *opaque, int > event) > switch (event) { > case CHR_EVENT_OPENED: > mon->mc->command_mode = 0; > -json_message_parser_init(&mon->mc->parser, handle_qmp_command); > data = get_qmp_greeting(); > monitor_json_emitter(mon, data); > qobject_decref(data); > @@ -4840,6 +4839,7 @@ static void monitor_control_event(void *opaque, int > event) > break; > case CHR_EVENT_CLOSED: > json_message_parser_destroy(&mon->mc->parser); > +json_message_parser_init(&mon->mc->parser, handle_qmp_command); > mon_refcount--; > monitor_fdsets_cleanup(); > break; > @@ -4951,6 +4951,8 @@ void monitor_init(CharDriverState *chr, int flags) >monitor_event, mon); > } > > +json_message_parser_init(&mon->mc->parser, handle_qmp_command); > + This hunk causes SIGSEGV on qemu-system-xtensa with the following trace: Program received signal SIGSEGV, Segmentation fault. json_message_parser_init (parser=0x8, func=0x556b4db0 ) at qemu/json-streamer.c:98 98 parser->emit = func; (gdb) bt #0 json_message_parser_init (parser=0x8, func=0x556b4db0 ) at qemu/json-streamer.c:98 #1 0x556ba5c7 in monitor_init (chr=0x56228fe0, flags=2) at qemu/monitor.c:4954 #2 0x5564f83d in qemu_chr_new (label=, filename=, init=0) at qemu/qemu-char.c:2828 #3 0x55626525 in serial_parse (devname=0x556f4152 "mon:stdio") at qemu/vl.c:2068 #4 serial_parse (devname=) at qemu/vl.c:2056 #5 0x55625009 in foreach_device_config (type=2, func=0x556264b0 ) at qemu/vl.c:2048 #6 0x555973f5 in main (argc=, argv=, envp=) at qemu/vl.c:3588 The command line is the following: qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -kernel ./test_b.tst > QLIST_INSERT_HEAD(&mon_list, mon, entry); > if (!default_mon || (flags & MONITOR_IS_DEFAULT)) > default_mon = mon; > -- > 1.7.5.4 > > -- Thanks. -- Max
Re: [Qemu-devel] [PATCH] monitor: move json init from OPEN event to init
On Fri, Aug 24, 2012 at 2:06 AM, Max Filippov wrote: > On Thu, Aug 23, 2012 at 5:22 PM, Anthony Liguori wrote: >> At some point in the past, the OPEN event was changed to be issued from a >> bottom half. This creates a small window whereas a data callback registered >> in >> init may be invoked before the OPEN event has been issued. >> >> This is reproducible with: >> >> echo "{'execute': 'qmp_capabilities'}" | qemu-system-x86_64 -M none -qmp >> stdio >> >> We can fix this for the monitor by moving the parser initialization to init. >> >> The remaining state that is set in OPEN appears harmless. >> >> Reported-by: Daniel Berrange >> Signed-off-by: Anthony Liguori >> --- >> monitor.c |4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index 480f583..b188582 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -4832,7 +4832,6 @@ static void monitor_control_event(void *opaque, int >> event) >> switch (event) { >> case CHR_EVENT_OPENED: >> mon->mc->command_mode = 0; >> -json_message_parser_init(&mon->mc->parser, handle_qmp_command); >> data = get_qmp_greeting(); >> monitor_json_emitter(mon, data); >> qobject_decref(data); >> @@ -4840,6 +4839,7 @@ static void monitor_control_event(void *opaque, int >> event) >> break; >> case CHR_EVENT_CLOSED: >> json_message_parser_destroy(&mon->mc->parser); >> +json_message_parser_init(&mon->mc->parser, handle_qmp_command); >> mon_refcount--; >> monitor_fdsets_cleanup(); >> break; >> @@ -4951,6 +4951,8 @@ void monitor_init(CharDriverState *chr, int flags) >> monitor_event, mon); >> } >> >> +json_message_parser_init(&mon->mc->parser, handle_qmp_command); >> + > > This hunk causes SIGSEGV on qemu-system-xtensa with the following trace: I see that '[PATCH] monitor: don't try to initialize json parser when monitor is HMP' is meant to fix it. -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Sat, Aug 25, 2012 at 9:20 PM, Steven wrote: > On Tue, Aug 21, 2012 at 3:18 AM, Max Filippov wrote: >> On Tue, Aug 21, 2012 at 9:40 AM, Steven wrote: >>> Hi, Max, >>> I wrote a small program to verify your patch could catch all the load >>> instructions from the guest. However, I found some problem from the >>> results. >>> >>> The guest OS and the emulated machine are both 32bit x86. My simple >>> program in the guest declares an 1048576-element integer array, >>> initialize the elements, and load them in a loop. It looks like this >>> int array[1048576]; >>> initialize the array; >>> >>> /* region of interests */ >>> int temp; >>> for (i=0; i < 1048576; i++) { >>> temp = array[i]; >>> } >>> So ideally, the path should catch the guest virtual address of in the >>> loop, right? >>> In addition, the virtual address for the beginning and end >>> of the array is 0xbf68b6e0 and 0xbfa8b6e0. >>> What i got is as follows >>> >>> __ldl_mmu, vaddr=bf68b6e0 >>> __ldl_mmu, vaddr=bf68b6e4 >>> __ldl_mmu, vaddr=bf68b6e8 >>> . >>> These should be the virtual address of the above loop. The >>> results look good because the gap between each vaddr is 4 bypte, which >>> is the length of each element. >>> However, after certain address, I got >>> >>> __ldl_mmu, vaddr=bf68bffc >>> __ldl_mmu, vaddr=bf68c000 >>> __ldl_mmu, vaddr=bf68d000 >>> __ldl_mmu, vaddr=bf68e000 >>> __ldl_mmu, vaddr=bf68f000 >>> __ldl_mmu, vaddr=bf69 >>> __ldl_mmu, vaddr=bf691000 >>> __ldl_mmu, vaddr=bf692000 >>> __ldl_mmu, vaddr=bf693000 >>> __ldl_mmu, vaddr=bf694000 >>> ... >>> __ldl_mmu, vaddr=bf727000 >>> __ldl_mmu, vaddr=bf728000 >>> __ldl_mmu, vaddr=bfa89000 >>> __ldl_mmu, vaddr=bfa8a000 >>> So the rest of the vaddr I got has a different of 4096 bytes, instead >>> of 4. I repeated the experiment for several times and got the same >>> results. Is there anything wrong? or could you explain this? Thanks. >> >> I see two possibilities here: >> - maybe there are more fast path shortcuts in the QEMU code? >> in that case output of qemu -d op,out_asm would help. >> - maybe your compiler had optimized that sample code? >> could you try to declare array in your sample as 'volatile int'? > After adding the "volatile" qualifier, the results are correct now. > So your patch can trap all the guest memory data load access, no > matter slow path or fast path. > > However, I found some problem when I try understanding the instruction > access. So I run the VM with "-d in_asm" to see program counter of > each guest code. I got > > __ldl_cmmu,8102ff91 > __ldl_cmmu,8102ff9a > > IN: > 0x8102ff8a: mov0x8(%rbx),%rax > 0x8102ff8e: add0x790(%rbx),%rax > 0x8102ff95: xor%edx,%edx > 0x8102ff97: mov0x858(%rbx),%rcx > 0x8102ff9e: cmp%rcx,%rax > 0x8102ffa1: je 0x8102ffb0 > . > > __ldl_cmmu,004005a1 > __ldl_cmmu,004005a6 > > IN: > 0x00400594: push %rbp > 0x00400595: mov%rsp,%rbp > 0x00400598: sub$0x20,%rsp > 0x0040059c: mov%rdi,-0x18(%rbp) > 0x004005a0: mov$0x1,%edi > 0x004005a5: callq 0x4004a0 > > From the results, I see that the guest virtual address of the pc is > slightly different between the __ldl_cmmu and the tb's pc(below IN:). > Could you help to understand this? Which one is the true pc memory > access? Thanks. Guest code is accessed at the translation time by C functions and I guess there are other layers of address translation caching. I wouldn't try to interpret these _cmmu printouts and would instead instrument [cpu_]ld{{u,s}{b,w},l,q}_code macros. -- Thanks. -- Max
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Mon, Aug 27, 2012 at 8:15 PM, Steven wrote: >> Guest code is accessed at the translation time by C functions and >> I guess there are other layers of address translation caching. I wouldn't >> try to interpret these _cmmu printouts and would instead instrument >> [cpu_]ld{{u,s}{b,w},l,q}_code macros. > yes, you are right. > Some ldub_code in x86 guest does not call __ldq_cmmu when the tlb hits. > By the way, when I use your patch, I saw too many log event for the > kernel data _mmu, ie., the addrs is > 0x7fff . There are too many such mmu event that the user mode > data can not be executed. So I have to setup a condition like > if (addr < 0x8000 ) > fprintf(stderr, "%s: %08x\n", __func__, addr); > Then my simple array access program can be finished. You can also try to differentiate kernel/userspace by mmu_idx passed to helpers. > I am wondering whether you have met the similar problem or you have > any suggestion on this. I used simple samples (tests/tcg/xtensa testsuite), their memory access pattern didn't deviate from what I expected. > My final goal is to obtain the memory access trace for a particular > process in the guest, so your patch really helps, except for too many > kernel _mmu events. Wouldn't it be easier to use qemu-user for that? -- Thanks. -- Max
Re: [Qemu-devel] TCG register allocator
> I have a bug, it segfaults when executing a translation blocks. when i > disable block chaining, the bug disappears. However, with block > chaining, i do not know which translation block jumps to the code > which caused the segfault. I want to reserve a register and use it to > record the last translation block executed. So at entry, i assign the > translation blocks address to the register and when the segfault > happens, I can get the last translation block executed. You could inject a helper call at the beginning of each TB that would record e.g. current program counter into a global variable. -- Thanks. -- Max
[Qemu-devel] [PATCH] target-xtensa: fetch 3rd opcode byte only when needed
According to ISA, 3.5.4, third opcode byte should not be fetched for 2-byte instructions. Signed-off-by: Max Filippov --- target-xtensa/translate.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index c81450d..6a0177f 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -749,7 +749,7 @@ static void disas_xtensa_insn(DisasContext *dc) uint8_t b0 = ldub_code(dc->pc); uint8_t b1 = ldub_code(dc->pc + 1); -uint8_t b2 = ldub_code(dc->pc + 2); +uint8_t b2 = 0; static const uint32_t B4CONST[] = { 0x, 1, 2, 3, 4, 5, 6, 7, 8, 10, 12, 16, 32, 64, 128, 256 @@ -764,6 +764,7 @@ static void disas_xtensa_insn(DisasContext *dc) HAS_OPTION(XTENSA_OPTION_CODE_DENSITY); } else { dc->next_pc = dc->pc + 3; +b2 = ldub_code(dc->pc + 2); } switch (OP0) { -- 1.7.7.6
[Qemu-devel] [RFC 0/9] target-xtensa: implement debug option
This patch series implements Xtensa debug option: debug interrupt, breakpoint opcodes, instruction breakpoint SRs, instruction counting SR, data breakpoint SRs. This option enables native debugging, e.g. now guest linux GDB is functional. Three exec.c patches fixe watchpoints bits that nobody seems to use currently. There are still unresolved issue with data breakpoints on unmapped memory: Xtensa ISA says that data breakpoint exception has higher priority than TLB exception and thus accessing unmapped address that hits data breakpoint will raise debug exception. Watchpoint mapping is performed in the tlb_set_page, but it is not called in case of guest TLB miss. Any idea on how to resolve it is welcome. Max Filippov (9): target-xtensa: add DEBUGCAUSE SR and configuration target-xtensa: implement instruction breakpoints target-xtensa: add ICOUNT SR and debug exception exec: add missing breaks to the watch_mem_write exec: fix check_watchpoint exiting cpu_loop exec: let cpu_watchpoint_insert accept larger watchpoints target-xtensa: add DBREAK data breakpoints target-xtensa: add DEBUG_SECTION to overlay tool target-xtensa: add breakpoint tests exec.c| 18 +++- target-xtensa/core-dc232b.c |1 + target-xtensa/core-fsf.c |1 + target-xtensa/cpu.h | 42 target-xtensa/helper.c| 43 target-xtensa/helpers.h |7 ++ target-xtensa/op_helper.c | 100 ++ target-xtensa/overlay_tool.h |5 + target-xtensa/translate.c | 153 +++- tests/tcg/xtensa/Makefile |1 + tests/tcg/xtensa/test_break.S | 223 + 11 files changed, 585 insertions(+), 9 deletions(-) create mode 100644 tests/tcg/xtensa/test_break.S -- 1.7.7.6
[Qemu-devel] [RFC 4/9] exec: add missing breaks to the watch_mem_write
Signed-off-by: Max Filippov --- exec.c | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index 5b9eb9a..0e93e0e 100644 --- a/exec.c +++ b/exec.c @@ -3279,9 +3279,15 @@ static void watch_mem_write(void *opaque, target_phys_addr_t addr, { check_watchpoint(addr & ~TARGET_PAGE_MASK, ~(size - 1), BP_MEM_WRITE); switch (size) { -case 1: stb_phys(addr, val); -case 2: stw_phys(addr, val); -case 4: stl_phys(addr, val); +case 1: +stb_phys(addr, val); +break; +case 2: +stw_phys(addr, val); +break; +case 4: +stl_phys(addr, val); +break; default: abort(); } } -- 1.7.7.6
[Qemu-devel] [RFC 9/9] target-xtensa: add breakpoint tests
Signed-off-by: Max Filippov --- tests/tcg/xtensa/Makefile |1 + tests/tcg/xtensa/test_break.S | 223 + 2 files changed, 224 insertions(+), 0 deletions(-) create mode 100644 tests/tcg/xtensa/test_break.S diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile index 8713af1..7e1e619 100644 --- a/tests/tcg/xtensa/Makefile +++ b/tests/tcg/xtensa/Makefile @@ -23,6 +23,7 @@ CRT= crt.o vectors.o TESTCASES += test_b.tst TESTCASES += test_bi.tst #TESTCASES += test_boolean.tst +TESTCASES += test_break.tst TESTCASES += test_bz.tst TESTCASES += test_clamps.tst TESTCASES += test_fail.tst diff --git a/tests/tcg/xtensa/test_break.S b/tests/tcg/xtensa/test_break.S new file mode 100644 index 000..8a8db80 --- /dev/null +++ b/tests/tcg/xtensa/test_break.S @@ -0,0 +1,223 @@ +.include "macros.inc" + +#define debug_level 6 +#define debug_vector level6 + +test_suite break + +test break +set_vector debug_vector, 0 +rsila2, debug_level +_break 0, 0 + +set_vector debug_vector, 2f +rsila2, debug_level - 1 +1: +_break 0, 0 +test_fail +2: +rsr a2, ps +movia3, 0x1f +and a2, a2, a3 +movia3, 0x10 | debug_level +assert eq, a2, a3 +rsr a2, epc6 +movia3, 1b +assert eq, a2, a3 +rsr a2, debugcause +movia3, 0x8 +assert eq, a2, a3 +test_end + +test breakn +set_vector debug_vector, 0 +rsila2, debug_level +_break.n 0 + +set_vector debug_vector, 2f +rsila2, debug_level - 1 +1: +_break.n 0 +test_fail +2: +rsr a2, ps +movia3, 0x1f +and a2, a2, a3 +movia3, 0x10 | debug_level +assert eq, a2, a3 +rsr a2, epc6 +movia3, 1b +assert eq, a2, a3 +rsr a2, debugcause +movia3, 0x10 +assert eq, a2, a3 +test_end + +test ibreak +set_vector debug_vector, 0 +rsila2, debug_level +movia2, 1f +wsr a2, ibreaka0 +movia2, 1 +wsr a2, ibreakenable +isync +1: +rsila2, debug_level - 1 +movia2, 1f +wsr a2, ibreaka0 +movia2, 0 +wsr a2, ibreakenable +isync +1: +set_vector debug_vector, 2f +movia2, 1f +wsr a2, ibreaka0 +movia2, 1 +wsr a2, ibreakenable +isync +1: +test_fail +2: +rsr a2, ps +movia3, 0x1f +and a2, a2, a3 +movia3, 0x10 | debug_level +assert eq, a2, a3 +rsr a2, epc6 +movia3, 1b +assert eq, a2, a3 +rsr a2, debugcause +movia3, 0x2 +assert eq, a2, a3 +test_end + +test ibreak_priority +set_vector debug_vector, 2f +rsila2, debug_level - 1 +movia2, 1f +wsr a2, ibreaka0 +movia2, 1 +wsr a2, ibreakenable +isync +1: +break 0, 0 +test_fail +2: +rsr a2, debugcause +movia3, 0x2 +assert eq, a2, a3 +test_end + +test icount +set_vector debug_vector, 2f +rsila2, debug_level - 1 +movia2, -2 +wsr a2, icount +movia2, 1 +wsr a2, icountlevel +isync +rsila2, 0 +nop +1: +break 0, 0 +test_fail +2: +movia2, 0 +wsr a2, icountlevel +rsr a2, epc6 +movia3, 1b +assert eq, a2, a3 +rsr a2, debugcause +movia3, 0x1 +assert eq, a2, a3 +test_end + +.macro check_dbreak dr +rsr a2, epc6 +movia3, 1b +assert eq, a2, a3 +rsr a2, debugcause +movia3, 0x4 | (\dr << 8) +assert eq, a2, a3 +movia2, 0 +wsr a2, dbreakc\dr +.endm + +.macro dbreak_test dr, ctl, break, access, op +set_vector debug_vector, 2f +rsila2, debug_level - 1 +movia2, \ctl +wsr a2, dbreakc\dr +movia2, \break +wsr a2, dbreaka\dr +movia2, \access +isync +1: +\op a3, a2, 0 +test_fail +2: +check_dbreak \dr +reset_ps +.endm + +test dbreak_exact +dbreak_test 0, 0x403f, 0xd07f, 0xd07f, l8ui +dbreak_test 1, 0x403e, 0xd07e, 0xd07e, l16ui +dbreak_test 0, 0x403c, 0xd07c, 0xd07c, l32i + +dbreak_test 1, 0x803f, 0xd07f, 0xd07f, s8i +dbreak_test 0, 0x803e, 0xd07e, 0xd07e, s16i +dbreak_test 1, 0x803c, 0xd07c, 0xd07c, s32i +test_end + +test dbreak_overlap +dbreak_test 0, 0x403f, 0xd07d, 0xd07c, l16ui +dbreak_test 1, 0x403f, 0xd07d, 0xd07c, l32i + +dbreak_test 0, 0x403e, 0xd07e, 0xd07f, l8ui +dbreak_test 1, 0x403e, 0xd07e, 0xd07c, l32i + +dbreak_test 0, 0x403c, 0xd07c, 0xd07d, l8ui +dbreak_test 1, 0x403c, 0xd07c, 0xd07c, l16ui + +dbreak_test 0, 0x4038, 0xd078, 0xd07b, l8ui +dbreak_test 1, 0x4038, 0xd078, 0xd07a, l16ui +dbreak_test 0, 0x4038, 0xd078,
[Qemu-devel] [RFC 3/9] target-xtensa: add ICOUNT SR and debug exception
ICOUNT SR gets incremented on every instruction completion provided that CINTLEVEL at the beginning of the instruction execution is lower than ICOUNTLEVEL. When ICOUNT would increment to 0 a debug exception is raised if CINTLEVEL is lower than DEBUGLEVEL. See ISA, 4.7.7.5 for more details. Signed-off-by: Max Filippov --- target-xtensa/cpu.h |6 + target-xtensa/translate.c | 49 - 2 files changed, 54 insertions(+), 1 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index 2875197..fbe5d15 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -142,6 +142,8 @@ enum { DEBUGCAUSE = 233, CCOUNT = 234, PRID = 235, +ICOUNT = 236, +ICOUNTLEVEL = 237, EXCVADDR = 238, CCOMPARE = 240, }; @@ -428,6 +430,7 @@ static inline int cpu_mmu_index(CPUState *env) #define XTENSA_TBFLAG_EXCM 0x4 #define XTENSA_TBFLAG_LITBASE 0x8 #define XTENSA_TBFLAG_DEBUG 0x10 +#define XTENSA_TBFLAG_ICOUNT 0x20 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc, target_ulong *cs_base, int *flags) @@ -447,6 +450,9 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc, if (xtensa_get_cintlevel(env) < env->config->debug_level) { *flags |= XTENSA_TBFLAG_DEBUG; } +if (xtensa_get_cintlevel(env) < env->sregs[ICOUNTLEVEL]) { +*flags |= XTENSA_TBFLAG_ICOUNT; +} } } diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 71fbf34..85f41a7 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -63,6 +63,8 @@ typedef struct DisasContext { unsigned used_window; bool debug; +bool icount; +TCGv_i32 next_icount; } DisasContext; static TCGv_ptr cpu_env; @@ -127,6 +129,8 @@ static const char * const sregnames[256] = { [DEBUGCAUSE] = "DEBUGCAUSE", [CCOUNT] = "CCOUNT", [PRID] = "PRID", +[ICOUNT] = "ICOUNT", +[ICOUNTLEVEL] = "ICOUNTLEVEL", [EXCVADDR] = "EXCVADDR", [CCOMPARE] = "CCOMPARE0", [CCOMPARE + 1] = "CCOMPARE1", @@ -313,10 +317,13 @@ static void gen_check_privilege(DisasContext *dc) static void gen_jump_slot(DisasContext *dc, TCGv dest, int slot) { tcg_gen_mov_i32(cpu_pc, dest); +gen_advance_ccount(dc); +if (dc->icount) { +tcg_gen_mov_i32(cpu_SR[ICOUNT], dc->next_icount); +} if (dc->singlestep_enabled) { gen_exception(dc, EXCP_DEBUG); } else { -gen_advance_ccount(dc); if (slot >= 0) { tcg_gen_goto_tb(slot); tcg_gen_exit_tb((tcg_target_long)dc->tb + slot); @@ -580,6 +587,22 @@ static void gen_wsr_prid(DisasContext *dc, uint32_t sr, TCGv_i32 v) { } +static void gen_wsr_icount(DisasContext *dc, uint32_t sr, TCGv_i32 v) +{ +if (dc->icount) { +tcg_gen_mov_i32(dc->next_icount, v); +} else { +tcg_gen_mov_i32(cpu_SR[sr], v); +} +} + +static void gen_wsr_icountlevel(DisasContext *dc, uint32_t sr, TCGv_i32 v) +{ +tcg_gen_andi_i32(cpu_SR[sr], v, 0xf); +/* This can change tb->flags, so exit tb */ +gen_jumpi_check_loop_end(dc, -1); +} + static void gen_wsr_ccompare(DisasContext *dc, uint32_t sr, TCGv_i32 v) { uint32_t id = sr - CCOMPARE; @@ -617,6 +640,8 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 s) [PS] = gen_wsr_ps, [DEBUGCAUSE] = gen_wsr_debugcause, [PRID] = gen_wsr_prid, +[ICOUNT] = gen_wsr_icount, +[ICOUNTLEVEL] = gen_wsr_icountlevel, [CCOMPARE] = gen_wsr_ccompare, [CCOMPARE + 1] = gen_wsr_ccompare, [CCOMPARE + 2] = gen_wsr_ccompare, @@ -2492,10 +2517,14 @@ static void gen_intermediate_code_internal( dc.is_jmp = DISAS_NEXT; dc.ccount_delta = 0; dc.debug = tb->flags & XTENSA_TBFLAG_DEBUG; +dc.icount = tb->flags & XTENSA_TBFLAG_ICOUNT; init_litbase(&dc); init_sar_tracker(&dc); reset_used_window(&dc); +if (dc.icount) { +dc.next_icount = tcg_temp_local_new_i32(); +} gen_icount_start(); @@ -2531,12 +2560,27 @@ static void gen_intermediate_code_internal( gen_io_start(); } +if (dc.icount) { +int label = gen_new_label(); + +tcg_gen_addi_i32(dc.next_icount, cpu_SR[ICOUNT], 1); +tcg_gen_brcondi_i32(TCG_COND_NE, dc.next_icount, 0, label); +tcg_gen_mov_i32(dc.next_icount, cpu_SR[ICOUNT]); +if (dc.debug) { +gen_debug_exception(&dc, DEBUGCAUSE_IC); +} +gen_set_label(label); +} + if (dc.debug) { gen_ibreak_check(env, &dc); } disas_xtensa_insn(&dc); ++insn_count; +if (dc.icount) {
[Qemu-devel] [RFC 1/9] target-xtensa: add DEBUGCAUSE SR and configuration
DEBUGCAUSE SR holds information about the most recent debug exception. See ISA, 4.7.7 for more details. Signed-off-by: Max Filippov --- target-xtensa/cpu.h | 15 +++ target-xtensa/translate.c |6 ++ 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index 0db83a6..25245d8 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -137,6 +137,7 @@ enum { PS = 230, VECBASE = 231, EXCCAUSE = 232, +DEBUGCAUSE = 233, CCOUNT = 234, PRID = 235, EXCVADDR = 238, @@ -161,6 +162,15 @@ enum { #define PS_WOE 0x4 +#define DEBUGCAUSE_IC 0x1 +#define DEBUGCAUSE_IB 0x2 +#define DEBUGCAUSE_DB 0x4 +#define DEBUGCAUSE_BI 0x8 +#define DEBUGCAUSE_BN 0x10 +#define DEBUGCAUSE_DI 0x20 +#define DEBUGCAUSE_DBNUM 0xf00 +#define DEBUGCAUSE_DBNUM_SHIFT 8 + #define MAX_NAREG 64 #define MAX_NINTERRUPT 32 #define MAX_NLEVEL 6 @@ -279,6 +289,11 @@ typedef struct XtensaConfig { uint32_t timerint[MAX_NCCOMPARE]; unsigned nextint; unsigned extint[MAX_NINTERRUPT]; + +unsigned debug_level; +unsigned nibreak; +unsigned ndbreak; + uint32_t clock_freq_khz; xtensa_tlb itlb; diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index c81450d..0786dd1 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -119,6 +119,7 @@ static const char * const sregnames[256] = { [PS] = "PS", [VECBASE] = "VECBASE", [EXCCAUSE] = "EXCCAUSE", +[DEBUGCAUSE] = "DEBUGCAUSE", [CCOUNT] = "CCOUNT", [PRID] = "PRID", [EXCVADDR] = "EXCVADDR", @@ -535,6 +536,10 @@ static void gen_wsr_ps(DisasContext *dc, uint32_t sr, TCGv_i32 v) gen_jumpi_check_loop_end(dc, -1); } +static void gen_wsr_debugcause(DisasContext *dc, uint32_t sr, TCGv_i32 v) +{ +} + static void gen_wsr_prid(DisasContext *dc, uint32_t sr, TCGv_i32 v) { } @@ -571,6 +576,7 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 s) [INTCLEAR] = gen_wsr_intclear, [INTENABLE] = gen_wsr_intenable, [PS] = gen_wsr_ps, +[DEBUGCAUSE] = gen_wsr_debugcause, [PRID] = gen_wsr_prid, [CCOMPARE] = gen_wsr_ccompare, [CCOMPARE + 1] = gen_wsr_ccompare, -- 1.7.7.6
[Qemu-devel] [RFC 6/9] exec: let cpu_watchpoint_insert accept larger watchpoints
Make cpu_watchpoint_insert accept watchpoints of any power-of-two size up to the target page size. Signed-off-by: Max Filippov --- exec.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/exec.c b/exec.c index bc6c185..39a5497 100644 --- a/exec.c +++ b/exec.c @@ -1443,7 +1443,8 @@ int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len, CPUWatchpoint *wp; /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */ -if ((len != 1 && len != 2 && len != 4 && len != 8) || (addr & ~len_mask)) { +if ((len & (len - 1)) || (addr & ~len_mask) || +len == 0 || len > TARGET_PAGE_SIZE) { fprintf(stderr, "qemu: tried to set invalid watchpoint at " TARGET_FMT_lx ", len=" TARGET_FMT_lu "\n", addr, len); return -EINVAL; -- 1.7.7.6
[Qemu-devel] [RFC 8/9] target-xtensa: add DEBUG_SECTION to overlay tool
Fill debug configuration from overlay definitions in the DEBUG_SECTION. Add DEBUG_SECTION to DC232B and FSF cores. Signed-off-by: Max Filippov --- target-xtensa/core-dc232b.c |1 + target-xtensa/core-fsf.c |1 + target-xtensa/overlay_tool.h |5 + 3 files changed, 7 insertions(+), 0 deletions(-) diff --git a/target-xtensa/core-dc232b.c b/target-xtensa/core-dc232b.c index 4d9bd55..b723c4c 100644 --- a/target-xtensa/core-dc232b.c +++ b/target-xtensa/core-dc232b.c @@ -22,6 +22,7 @@ static const XtensaConfig dc232b = { EXCEPTIONS_SECTION, INTERRUPTS_SECTION, TLB_SECTION, +DEBUG_SECTION, .clock_freq_khz = 1, }; diff --git a/target-xtensa/core-fsf.c b/target-xtensa/core-fsf.c index 7650462..88de4dd 100644 --- a/target-xtensa/core-fsf.c +++ b/target-xtensa/core-fsf.c @@ -16,6 +16,7 @@ static const XtensaConfig fsf = { EXCEPTIONS_SECTION, INTERRUPTS_SECTION, TLB_SECTION, +DEBUG_SECTION, .clock_freq_khz = 1, }; diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h index df19cc9..cbb4aa6 100644 --- a/target-xtensa/overlay_tool.h +++ b/target-xtensa/overlay_tool.h @@ -114,6 +114,7 @@ [EXC_KERNEL] = XCHAL_KERNEL_VECTOR_VADDR, \ [EXC_USER] = XCHAL_USER_VECTOR_VADDR, \ [EXC_DOUBLE] = XCHAL_DOUBLEEXC_VECTOR_VADDR, \ +[EXC_DEBUG] = XCHAL_DEBUG_VECTOR_VADDR, \ } #define INTERRUPT_VECTORS { \ @@ -288,6 +289,10 @@ #define REGISTER_CORE(core) #endif +#define DEBUG_SECTION \ +.debug_level = XCHAL_DEBUGLEVEL, \ +.nibreak = XCHAL_NUM_IBREAK, \ +.ndbreak = XCHAL_NUM_DBREAK #if XCHAL_NUM_INTLEVELS + XCHAL_HAVE_NMI + 1 <= 2 #define XCHAL_INTLEVEL2_VECTOR_VADDR 0 -- 1.7.7.6
[Qemu-devel] [RFC 7/9] target-xtensa: add DBREAK data breakpoints
Add DBREAKA/DBREAKC SRs and implement DBREAK breakpoints as debug watchpoints. This implementation is not fully compliant to ISA: when a breakpoint is set to an unmapped/inaccessible memory address it generates TLB/memory protection exception instead of debug exception. See ISA, 4.7.7.3, 4.7.7.6 for more details. Signed-off-by: Max Filippov --- target-xtensa/cpu.h | 12 target-xtensa/helper.c| 41 + target-xtensa/helpers.h |2 + target-xtensa/op_helper.c | 62 + target-xtensa/translate.c | 30 + 5 files changed, 147 insertions(+), 0 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index fbe5d15..4dbd7b7 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -128,6 +128,8 @@ enum { DTLBCFG = 92, IBREAKENABLE = 96, IBREAKA = 128, +DBREAKA = 144, +DBREAKC = 160, EPC1 = 177, DEPC = 192, EPS2 = 194, @@ -175,12 +177,18 @@ enum { #define DEBUGCAUSE_DBNUM 0xf00 #define DEBUGCAUSE_DBNUM_SHIFT 8 +#define DBREAKC_SB 0x8000 +#define DBREAKC_LB 0x4000 +#define DBREAKC_SB_LB (DBREAKC_SB | DBREAKC_LB) +#define DBREAKC_MASK 0x3f + #define MAX_NAREG 64 #define MAX_NINTERRUPT 32 #define MAX_NLEVEL 6 #define MAX_NNMI 1 #define MAX_NCCOMPARE 3 #define MAX_TLB_WAY_SIZE 8 +#define MAX_NDBREAK 2 #define REGION_PAGE_MASK 0xe000 @@ -330,6 +338,9 @@ typedef struct CPUXtensaState { int exception_taken; +/* Watchpoints for DBREAK registers */ +CPUWatchpoint *cpu_watchpoint[MAX_NDBREAK]; + CPU_COMMON } CPUXtensaState; @@ -364,6 +375,7 @@ void xtensa_tlb_set_entry(CPUState *env, bool dtlb, int xtensa_get_physical_addr(CPUState *env, uint32_t vaddr, int is_write, int mmu_idx, uint32_t *paddr, uint32_t *page_size, unsigned *access); +void debug_exception_env(CPUState *new_env, uint32_t cause); #define XTENSA_OPTION_BIT(opt) (((uint64_t)1) << (opt)) diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index b7e67b7..a0bf697 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -58,9 +58,44 @@ void xtensa_register_core(XtensaConfigList *node) xtensa_cores = node; } +static uint32_t check_hw_breakpoints(CPUState *env) +{ +unsigned i; + +for (i = 0; i < env->config->ndbreak; ++i) { +if (env->cpu_watchpoint[i] && +env->cpu_watchpoint[i]->flags & BP_WATCHPOINT_HIT) { +return DEBUGCAUSE_DB | (i << DEBUGCAUSE_DBNUM_SHIFT); +} +} +return 0; +} + +static CPUDebugExcpHandler *prev_debug_excp_handler; + +static void breakpoint_handler(CPUState *env) +{ +if (env->watchpoint_hit) { +if (env->watchpoint_hit->flags & BP_CPU) { +uint32_t cause; + +env->watchpoint_hit = NULL; +cause = check_hw_breakpoints(env); +if (cause) { +debug_exception_env(env, cause); +} +cpu_resume_from_signal(env, NULL); +} +} +if (prev_debug_excp_handler) { +prev_debug_excp_handler(env); +} +} + CPUXtensaState *cpu_xtensa_init(const char *cpu_model) { static int tcg_inited; +static int debug_handler_inited; CPUXtensaState *env; const XtensaConfig *config = NULL; XtensaConfigList *core = xtensa_cores; @@ -84,6 +119,12 @@ CPUXtensaState *cpu_xtensa_init(const char *cpu_model) xtensa_translate_init(); } +if (!debug_handler_inited && tcg_enabled()) { +debug_handler_inited = 1; +prev_debug_excp_handler = +cpu_set_debug_excp_handler(breakpoint_handler); +} + xtensa_irq_init(env); qemu_init_vcpu(env); return env; diff --git a/target-xtensa/helpers.h b/target-xtensa/helpers.h index afe39d4..48a741e 100644 --- a/target-xtensa/helpers.h +++ b/target-xtensa/helpers.h @@ -33,5 +33,7 @@ DEF_HELPER_3(wtlb, void, i32, i32, i32) DEF_HELPER_1(wsr_ibreakenable, void, i32) DEF_HELPER_2(wsr_ibreaka, void, i32, i32) +DEF_HELPER_2(wsr_dbreaka, void, i32, i32) +DEF_HELPER_2(wsr_dbreakc, void, i32, i32) #include "def-helper.h" diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c index 1feaaee..e184cf6 100644 --- a/target-xtensa/op_helper.c +++ b/target-xtensa/op_helper.c @@ -134,6 +134,14 @@ void HELPER(exception_cause_vaddr)(uint32_t pc, uint32_t cause, uint32_t vaddr) HELPER(exception_cause)(pc, cause); } +void debug_exception_env(CPUState *new_env, uint32_t cause) +{ +if (xtensa_get_cintlevel(new_env) < new_env->config->debug_level) { +env = new_env; +HELPER(debug_exception)(env->pc, cause); +} +} + void HELPER(debug_exception)(uint32_t pc, uint32_t cause) { unsigned level = env->config->debug_level; @@ -700,3 +708,57 @@ void HELPER(wsr_ibreaka)(uint32_t i, uint32_t v)
[Qemu-devel] [RFC 2/9] target-xtensa: implement instruction breakpoints
Add IBREAKA/IBREAKENABLE SRs and implement debug exception, BREAK and BREAK.N instructions and IBREAK breakpoints. IBREAK breakpoint address is considered constant for TB lifetime. On IBREAKA/IBREAKENABLE change corresponding TBs are invalidated. Signed-off-by: Max Filippov --- target-xtensa/cpu.h |9 ++ target-xtensa/helper.c|2 + target-xtensa/helpers.h |5 +++ target-xtensa/op_helper.c | 38 + target-xtensa/translate.c | 68 +++-- 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index 25245d8..2875197 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -126,6 +126,8 @@ enum { RASID = 90, ITLBCFG = 91, DTLBCFG = 92, +IBREAKENABLE = 96, +IBREAKA = 128, EPC1 = 177, DEPC = 192, EPS2 = 194, @@ -196,6 +198,7 @@ enum { EXC_KERNEL, EXC_USER, EXC_DOUBLE, +EXC_DEBUG, EXC_MAX }; @@ -424,6 +427,7 @@ static inline int cpu_mmu_index(CPUState *env) #define XTENSA_TBFLAG_RING_MASK 0x3 #define XTENSA_TBFLAG_EXCM 0x4 #define XTENSA_TBFLAG_LITBASE 0x8 +#define XTENSA_TBFLAG_DEBUG 0x10 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc, target_ulong *cs_base, int *flags) @@ -439,6 +443,11 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc, (env->sregs[LITBASE] & 1)) { *flags |= XTENSA_TBFLAG_LITBASE; } +if (xtensa_option_enabled(env->config, XTENSA_OPTION_DEBUG)) { +if (xtensa_get_cintlevel(env) < env->config->debug_level) { +*flags |= XTENSA_TBFLAG_DEBUG; +} +} } #include "cpu-all.h" diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index 2a0cb1a..b7e67b7 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -44,6 +44,7 @@ void cpu_reset(CPUXtensaState *env) env->sregs[PS] = xtensa_option_enabled(env->config, XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10; env->sregs[VECBASE] = env->config->vecbase; +env->sregs[IBREAKENABLE] = 0; env->pending_irq_level = 0; reset_mmu(env); @@ -193,6 +194,7 @@ void do_interrupt(CPUState *env) case EXC_KERNEL: case EXC_USER: case EXC_DOUBLE: +case EXC_DEBUG: qemu_log_mask(CPU_LOG_INT, "%s(%d) " "pc = %08x, a0 = %08x, ps = %08x, ccount = %08x\n", __func__, env->exception_index, diff --git a/target-xtensa/helpers.h b/target-xtensa/helpers.h index 09ab332..afe39d4 100644 --- a/target-xtensa/helpers.h +++ b/target-xtensa/helpers.h @@ -3,6 +3,8 @@ DEF_HELPER_1(exception, void, i32) DEF_HELPER_2(exception_cause, void, i32, i32) DEF_HELPER_3(exception_cause_vaddr, void, i32, i32, i32) +DEF_HELPER_2(debug_exception, void, i32, i32) + DEF_HELPER_1(nsa, i32, i32) DEF_HELPER_1(nsau, i32, i32) DEF_HELPER_1(wsr_windowbase, void, i32) @@ -29,4 +31,7 @@ DEF_HELPER_2(itlb, void, i32, i32) DEF_HELPER_2(ptlb, i32, i32, i32) DEF_HELPER_3(wtlb, void, i32, i32, i32) +DEF_HELPER_1(wsr_ibreakenable, void, i32) +DEF_HELPER_2(wsr_ibreaka, void, i32, i32) + #include "def-helper.h" diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c index 0605611..1feaaee 100644 --- a/target-xtensa/op_helper.c +++ b/target-xtensa/op_helper.c @@ -134,6 +134,19 @@ void HELPER(exception_cause_vaddr)(uint32_t pc, uint32_t cause, uint32_t vaddr) HELPER(exception_cause)(pc, cause); } +void HELPER(debug_exception)(uint32_t pc, uint32_t cause) +{ +unsigned level = env->config->debug_level; + +env->pc = pc; +env->sregs[DEBUGCAUSE] = cause; +env->sregs[EPC1 + level - 1] = pc; +env->sregs[EPS2 + level - 2] = env->sregs[PS]; +env->sregs[PS] = (env->sregs[PS] & ~PS_INTLEVEL) | PS_EXCM | +(level << PS_INTLEVEL_SHIFT); +HELPER(exception)(EXC_DEBUG); +} + uint32_t HELPER(nsa)(uint32_t v) { if (v & 0x8000) { @@ -662,3 +675,28 @@ void HELPER(wtlb)(uint32_t p, uint32_t v, uint32_t dtlb) split_tlb_entry_spec(v, dtlb, &vpn, &wi, &ei); xtensa_tlb_set_entry(env, dtlb, wi, ei, vpn, p); } + + +void HELPER(wsr_ibreakenable)(uint32_t v) +{ +uint32_t change = v ^ env->sregs[IBREAKENABLE]; +unsigned i; + +for (i = 0; i < env->config->nibreak; ++i) { +if (change & (1 << i)) { +tb_invalidate_phys_page_range( +env->sregs[IBREAKA + i], env->sregs[IBREAKA + i] + 1, 0); +} +} +env->sregs[IBREAKENABLE] = v & ((1 << env->config->nibreak) - 1); +} + +void HELPER(wsr_ibreaka)(uint32_t i, uint32_t v) +{ +if (env->sregs[IBREAKENABLE] & (1 << i) && env->sregs[IBREAKA + i] != v) { +tb_invalidate_phys_page_range( +en
[Qemu-devel] [RFC 5/9] exec: fix check_watchpoint exiting cpu_loop
In case of BP_STOP_BEFORE_ACCESS watchpoint check_watchpoint intends to signal EXCP_DEBUG exception on exit from cpu loop, but later overwrites exception code by the cpu_resume_from_signal call. Use cpu_loop_exit with BP_STOP_BEFORE_ACCESS watchpoints. Signed-off-by: Max Filippov --- exec.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/exec.c b/exec.c index 0e93e0e..bc6c185 100644 --- a/exec.c +++ b/exec.c @@ -3247,11 +3247,12 @@ static void check_watchpoint(int offset, int len_mask, int flags) tb_phys_invalidate(tb, -1); if (wp->flags & BP_STOP_BEFORE_ACCESS) { env->exception_index = EXCP_DEBUG; +cpu_loop_exit(env); } else { cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags); tb_gen_code(env, pc, cs_base, cpu_flags, 1); +cpu_resume_from_signal(env, NULL); } -cpu_resume_from_signal(env, NULL); } } else { wp->flags &= ~BP_WATCHPOINT_HIT; -- 1.7.7.6
Re: [Qemu-devel] [PATCH 14/15] target-xtensa: Clean includes
> Remove some include statements which are not needed. > > Cc: Max Filippov > Signed-off-by: Stefan Weil Acked-by: Max Filippov Thanks. -- Max
Re: [Qemu-devel] How to follow a child process created in the guest OS?
> The question is not so related to QEMU itself, but I want to give it a try. > I am running a tiny OS on QEMU and debugging it with gdbstub. The tiny OS will > fork process 1, 2, ... and so on. I want to follow the child process, but the > GDB command `set follow-fork-mode child` doesn't work. This seems to be a bug > or missing feature in GDB remote protocol. [1] QEMU gdbstub has no idea of the guest OS, its fork, its processes or threads. All that it has is a number of VCPUs that it treats like threads, and they execute continuous stream of instructions. There's no special instruction for fork and the guest OS doesn't notify gdbstub of it either. The natural way to debug processes would be to use OS's native debugger/gdbstub. It is still possible to use QEMU gdbstub, but you'll need to analyze guest OS state to determine if you e.g. hit the breakpoint in the desired process context. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH qom-next 42/59] xtensa_pic: Pass XtensaCPU to xtensa_ccompare_cb()
On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber wrote: > Am 23.05.2012 05:08, schrieb Andreas Färber: >> Needed for cpu_has_work(). >> >> Signed-off-by: Andreas Färber > > Max, could you ack this trivial patch please? It still applies. Well, it does but why do you want to add a level of indirection here? Does that mean that cpu->env may change during cpu lifetime? Commit message is not very helpful here. > I notice that you were originally not cc'ed: Probably this file was/is > not yet documented in MAINTAINERS. > > Thanks, > Andreas > >> --- >> hw/xtensa_pic.c |7 +-- >> 1 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/hw/xtensa_pic.c b/hw/xtensa_pic.c >> index 653ded6..8b9c051 100644 >> --- a/hw/xtensa_pic.c >> +++ b/hw/xtensa_pic.c >> @@ -125,7 +125,8 @@ void xtensa_rearm_ccompare_timer(CPUXtensaState *env) >> >> static void xtensa_ccompare_cb(void *opaque) >> { >> -CPUXtensaState *env = opaque; >> +XtensaCPU *cpu = opaque; >> +CPUXtensaState *env = &cpu->env; >> >> if (env->halted) { >> env->halt_clock = qemu_get_clock_ns(vm_clock); >> @@ -139,12 +140,14 @@ static void xtensa_ccompare_cb(void *opaque) >> >> void xtensa_irq_init(CPUXtensaState *env) >> { >> +XtensaCPU *cpu = xtensa_env_get_cpu(env); >> + >> env->irq_inputs = (void **)qemu_allocate_irqs( >> xtensa_set_irq, env, env->config->ninterrupt); >> if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT) && >> env->config->nccompare > 0) { >> env->ccompare_timer = >> -qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env); >> +qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, cpu); >> } >> } >> > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > -- Thanks. -- Max
Re: [Qemu-devel] [PATCH qom-next 42/59] xtensa_pic: Pass XtensaCPU to xtensa_ccompare_cb()
On Wed, Oct 10, 2012 at 8:33 PM, Andreas Färber wrote: > Am 10.10.2012 17:35, schrieb Max Filippov: >> On Wed, Oct 10, 2012 at 7:15 PM, Andreas Färber wrote: >>> Am 23.05.2012 05:08, schrieb Andreas Färber: >>>> Needed for cpu_has_work(). >>>> >>>> Signed-off-by: Andreas Färber >>> >>> Max, could you ack this trivial patch please? It still applies. >> >> Well, it does but why do you want to add a level of indirection here? >> Does that mean that cpu->env may change during cpu lifetime? >> Commit message is not very helpful here. > > Patch 43/59 in that series updates cpu_has_work() argument to CPUState*, > thus this patch prepares xtensa_pic as one of the callers. > > This is the only xtensa patch I have in my queue, so I wanted to avoid > hitting you with a large resend. > > I see now that xtensa_set_irq could get an XtensaCPU opaque as well, but > so far there was no need apparently, so that can be changed later. > > For target-specific code my general rule of thumb is, use FooCPU rather > than CPUFooState arguments and opaques wherever possible since the need > is growing. Also, any new fields that are not accessed by TCG should be > placed into FooCPU rather than CPUFooState. Ok, Acked-by: Max Filippov -- Thanks. -- Max
[Qemu-devel] [PATCH 1/2] hw/xtensa_lx60: don't prematurely explode QEMUMachineInitArgs
Don't explode QEMUMachineInitArgs before passing it to lx_init. Signed-off-by: Max Filippov --- hw/xtensa_lx60.c | 25 ++--- 1 files changed, 6 insertions(+), 19 deletions(-) diff --git a/hw/xtensa_lx60.c b/hw/xtensa_lx60.c index 5dd2e08..b4d3b8e 100644 --- a/hw/xtensa_lx60.c +++ b/hw/xtensa_lx60.c @@ -156,9 +156,7 @@ static void lx60_reset(void *opaque) } static void lx_init(const LxBoardDesc *board, -ram_addr_t ram_size, const char *boot_device, -const char *kernel_filename, const char *kernel_cmdline, -const char *initrd_filename, const char *cpu_model) +ram_addr_t ram_size, QEMUMachineInitArgs *args) { #ifdef TARGET_WORDS_BIGENDIAN int be = 1; @@ -171,6 +169,9 @@ static void lx_init(const LxBoardDesc *board, MemoryRegion *ram, *rom, *system_io; DriveInfo *dinfo; pflash_t *flash = NULL; +const char *cpu_model = args->cpu_model; +const char *kernel_filename = args->kernel_filename; +const char *kernel_cmdline = args->kernel_cmdline; int n; if (!cpu_model) { @@ -272,37 +273,23 @@ static void lx_init(const LxBoardDesc *board, static void xtensa_lx60_init(QEMUMachineInitArgs *args) { ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; static const LxBoardDesc lx60_board = { .flash_size = 0x40, .flash_sector_size = 0x1, .sram_size = 0x2, }; -lx_init(&lx60_board, ram_size, boot_device, -kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); +lx_init(&lx60_board, ram_size, args); } static void xtensa_lx200_init(QEMUMachineInitArgs *args) { ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; static const LxBoardDesc lx200_board = { .flash_size = 0x100, .flash_sector_size = 0x2, .sram_size = 0x200, }; -lx_init(&lx200_board, ram_size, boot_device, -kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); +lx_init(&lx200_board, ram_size, args); } static QEMUMachine xtensa_lx60_machine = { -- 1.7.7.6
[Qemu-devel] [PATCH 0/2] xtensa boards: don't prematurely explode QEMUMachineInitArgs
Max Filippov (2): hw/xtensa_lx60: don't prematurely explode QEMUMachineInitArgs hw/xtensa_sim: get rid of intermediate xtensa_sim_init hw/xtensa_lx60.c | 25 ++--- hw/xtensa_sim.c | 27 --- 2 files changed, 14 insertions(+), 38 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH 2/2] hw/xtensa_sim: get rid of intermediate xtensa_sim_init
Remove xtensa_sim_init that only explodes machine init args, rename sim_init to xtensa_sim_init. Signed-off-by: Max Filippov --- hw/xtensa_sim.c | 27 --- 1 files changed, 8 insertions(+), 19 deletions(-) diff --git a/hw/xtensa_sim.c b/hw/xtensa_sim.c index 2e846d8..0d633e4 100644 --- a/hw/xtensa_sim.c +++ b/hw/xtensa_sim.c @@ -44,16 +44,20 @@ static void sim_reset(void *opaque) cpu_reset(CPU(cpu)); } -static void sim_init(ram_addr_t ram_size, -const char *boot_device, -const char *kernel_filename, const char *kernel_cmdline, -const char *initrd_filename, const char *cpu_model) +static void xtensa_sim_init(QEMUMachineInitArgs *args) { XtensaCPU *cpu = NULL; CPUXtensaState *env = NULL; MemoryRegion *ram, *rom; +ram_addr_t ram_size = args->ram_size; +const char *cpu_model = args->cpu_model; +const char *kernel_filename = args->kernel_filename; int n; +if (!cpu_model) { +cpu_model = XTENSA_DEFAULT_CPU_MODEL; +} + for (n = 0; n < smp_cpus; n++) { cpu = cpu_xtensa_init(cpu_model); if (cpu == NULL) { @@ -96,21 +100,6 @@ static void sim_init(ram_addr_t ram_size, } } -static void xtensa_sim_init(QEMUMachineInitArgs *args) -{ -ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; -if (!cpu_model) { -cpu_model = XTENSA_DEFAULT_CPU_MODEL; -} -sim_init(ram_size, boot_device, kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); -} - static QEMUMachine xtensa_sim_machine = { .name = "sim", .desc = "sim machine (" XTENSA_DEFAULT_CPU_MODEL ")", -- 1.7.7.6
Re: [Qemu-devel] [PATCH 1/2] hw/xtensa_lx60: don't prematurely explode QEMUMachineInitArgs
On Thu, Oct 25, 2012 at 2:04 PM, Peter Maydell wrote: > On 25 October 2012 09:47, Max Filippov wrote: >> @@ -272,37 +273,23 @@ static void lx_init(const LxBoardDesc *board, >> static void xtensa_lx60_init(QEMUMachineInitArgs *args) >> { >> ram_addr_t ram_size = args->ram_size; >> -const char *cpu_model = args->cpu_model; >> -const char *kernel_filename = args->kernel_filename; >> -const char *kernel_cmdline = args->kernel_cmdline; >> -const char *initrd_filename = args->initrd_filename; >> -const char *boot_device = args->boot_device; >> static const LxBoardDesc lx60_board = { >> .flash_size = 0x40, >> .flash_sector_size = 0x1, >> .sram_size = 0x2, >> }; >> -lx_init(&lx60_board, ram_size, boot_device, >> -kernel_filename, kernel_cmdline, >> -initrd_filename, cpu_model); >> +lx_init(&lx60_board, ram_size, args); >> } >> >> static void xtensa_lx200_init(QEMUMachineInitArgs *args) >> { >> ram_addr_t ram_size = args->ram_size; >> -const char *cpu_model = args->cpu_model; >> -const char *kernel_filename = args->kernel_filename; >> -const char *kernel_cmdline = args->kernel_cmdline; >> -const char *initrd_filename = args->initrd_filename; >> -const char *boot_device = args->boot_device; >> static const LxBoardDesc lx200_board = { >> .flash_size = 0x100, >> .flash_sector_size = 0x2, >> .sram_size = 0x200, >> }; >> -lx_init(&lx200_board, ram_size, boot_device, >> -kernel_filename, kernel_cmdline, >> - initrd_filename, cpu_model); >> +lx_init(&lx200_board, ram_size, args); >> } > > Why not let lx_init() pull the ram_size out of args->ram_size > as well? Completely thoughtless refactoring: it didn't match the pattern. Will resend. -- Thanks. -- Max
[Qemu-devel] [PATCH v2 2/2] hw/xtensa_sim: get rid of intermediate xtensa_sim_init
Remove xtensa_sim_init that only explodes machine init args, rename sim_init to xtensa_sim_init. Signed-off-by: Max Filippov --- hw/xtensa_sim.c | 27 --- 1 files changed, 8 insertions(+), 19 deletions(-) diff --git a/hw/xtensa_sim.c b/hw/xtensa_sim.c index 2e846d8..0d633e4 100644 --- a/hw/xtensa_sim.c +++ b/hw/xtensa_sim.c @@ -44,16 +44,20 @@ static void sim_reset(void *opaque) cpu_reset(CPU(cpu)); } -static void sim_init(ram_addr_t ram_size, -const char *boot_device, -const char *kernel_filename, const char *kernel_cmdline, -const char *initrd_filename, const char *cpu_model) +static void xtensa_sim_init(QEMUMachineInitArgs *args) { XtensaCPU *cpu = NULL; CPUXtensaState *env = NULL; MemoryRegion *ram, *rom; +ram_addr_t ram_size = args->ram_size; +const char *cpu_model = args->cpu_model; +const char *kernel_filename = args->kernel_filename; int n; +if (!cpu_model) { +cpu_model = XTENSA_DEFAULT_CPU_MODEL; +} + for (n = 0; n < smp_cpus; n++) { cpu = cpu_xtensa_init(cpu_model); if (cpu == NULL) { @@ -96,21 +100,6 @@ static void sim_init(ram_addr_t ram_size, } } -static void xtensa_sim_init(QEMUMachineInitArgs *args) -{ -ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; -if (!cpu_model) { -cpu_model = XTENSA_DEFAULT_CPU_MODEL; -} -sim_init(ram_size, boot_device, kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); -} - static QEMUMachine xtensa_sim_machine = { .name = "sim", .desc = "sim machine (" XTENSA_DEFAULT_CPU_MODEL ")", -- 1.7.7.6
[Qemu-devel] [PATCH v2 1/2] hw/xtensa_lx60: don't prematurely explode QEMUMachineInitArgs
Don't explode QEMUMachineInitArgs before passing it to lx_init. Signed-off-by: Max Filippov --- hw/xtensa_lx60.c | 30 +++--- 1 files changed, 7 insertions(+), 23 deletions(-) diff --git a/hw/xtensa_lx60.c b/hw/xtensa_lx60.c index 5dd2e08..4c42edc 100644 --- a/hw/xtensa_lx60.c +++ b/hw/xtensa_lx60.c @@ -155,10 +155,7 @@ static void lx60_reset(void *opaque) cpu_reset(CPU(cpu)); } -static void lx_init(const LxBoardDesc *board, -ram_addr_t ram_size, const char *boot_device, -const char *kernel_filename, const char *kernel_cmdline, -const char *initrd_filename, const char *cpu_model) +static void lx_init(const LxBoardDesc *board, QEMUMachineInitArgs *args) { #ifdef TARGET_WORDS_BIGENDIAN int be = 1; @@ -171,6 +168,9 @@ static void lx_init(const LxBoardDesc *board, MemoryRegion *ram, *rom, *system_io; DriveInfo *dinfo; pflash_t *flash = NULL; +const char *cpu_model = args->cpu_model; +const char *kernel_filename = args->kernel_filename; +const char *kernel_cmdline = args->kernel_cmdline; int n; if (!cpu_model) { @@ -194,7 +194,7 @@ static void lx_init(const LxBoardDesc *board, } ram = g_malloc(sizeof(*ram)); -memory_region_init_ram(ram, "lx60.dram", ram_size); +memory_region_init_ram(ram, "lx60.dram", args->ram_size); vmstate_register_ram_global(ram); memory_region_add_subregion(system_memory, 0, ram); @@ -271,38 +271,22 @@ static void lx_init(const LxBoardDesc *board, static void xtensa_lx60_init(QEMUMachineInitArgs *args) { -ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; static const LxBoardDesc lx60_board = { .flash_size = 0x40, .flash_sector_size = 0x1, .sram_size = 0x2, }; -lx_init(&lx60_board, ram_size, boot_device, -kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); +lx_init(&lx60_board, args); } static void xtensa_lx200_init(QEMUMachineInitArgs *args) { -ram_addr_t ram_size = args->ram_size; -const char *cpu_model = args->cpu_model; -const char *kernel_filename = args->kernel_filename; -const char *kernel_cmdline = args->kernel_cmdline; -const char *initrd_filename = args->initrd_filename; -const char *boot_device = args->boot_device; static const LxBoardDesc lx200_board = { .flash_size = 0x100, .flash_sector_size = 0x2, .sram_size = 0x200, }; -lx_init(&lx200_board, ram_size, boot_device, -kernel_filename, kernel_cmdline, -initrd_filename, cpu_model); +lx_init(&lx200_board, args); } static QEMUMachine xtensa_lx60_machine = { -- 1.7.7.6
[Qemu-devel] [PATCH v2 0/2] xtensa boards: don't prematurely explode QEMUMachineInitArgs
Changes v1 -> v2: - remove ram_size in xtensa_lx60 as well Max Filippov (2): hw/xtensa_lx60: don't prematurely explode QEMUMachineInitArgs hw/xtensa_sim: get rid of intermediate xtensa_sim_init hw/xtensa_lx60.c | 30 +++--- hw/xtensa_sim.c | 27 --- 2 files changed, 15 insertions(+), 42 deletions(-) -- 1.7.7.6
Re: [Qemu-devel] [PATCH 4/5] target-xtensa: avoid using cpu_single_env
On Sun, Oct 28, 2012 at 7:03 PM, Blue Swirl wrote: > Pass around CPUState instead of using global cpu_single_env. > > Signed-off-by: Blue Swirl > --- > target-xtensa/translate.c | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) Acked-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v4 00/16] QEMU OpenRISC support
On 06/11/2012 10:31 AM, Jia Liu wrote: This is the OpenCores OpenRISC 1200 support for QEMU. Full implementation of the system-model and linux-user-model support. Hi Jia. When I configure the tree with openrisc patches for the debug build: $ configure --target-list='or32-softmmu,or32-linux-user' --enable-debug the build fails due to multiple helper function argument type violations: ... qemu/target-openrisc/translate.c: In function ‘dec_calc’: qemu/target-openrisc/translate.c:441:17: error: incompatible type for argument 1 of ‘tcg_temp_free_i32’ qemu/tcg/tcg.h:443:6: note: expected ‘TCGv_i32’ but argument is of type ‘TCGv_i64’ qemu/target-openrisc/translate.c:442:17: error: incompatible type for argument 1 of ‘tcg_temp_free_i32’ qemu/tcg/tcg.h:443:6: note: expected ‘TCGv_i32’ but argument is of type ‘TCGv_i64’ qemu/target-openrisc/translate.c:443:17: error: incompatible type for argument 1 of ‘tcg_gen_shri_i64’ qemu/tcg/tcg-op.h:1183:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:452:17: error: incompatible type for argument 1 of ‘tcg_temp_free_i32’ qemu/tcg/tcg.h:443:6: note: expected ‘TCGv_i32’ but argument is of type ‘TCGv_i64’ qemu/target-openrisc/translate.c: In function ‘dec_misc’: qemu/target-openrisc/translate.c:662:13: error: incompatible type for argument 1 of ‘tcg_gen_ext_i32_i64’ qemu/tcg/tcg-op.h:1597:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:663:13: error: incompatible type for argument 1 of ‘tcg_gen_concat_i32_i64’ qemu/tcg/tcg-op.h:1749:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:664:13: error: incompatible type for argument 1 of ‘tcg_gen_add_i64’ qemu/tcg/tcg-op.h:1104:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:664:13: error: incompatible type for argument 2 of ‘tcg_gen_add_i64’ qemu/tcg/tcg-op.h:1104:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:664:13: error: incompatible type for argument 3 of ‘tcg_gen_add_i64’ qemu/tcg/tcg-op.h:1104:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ qemu/target-openrisc/translate.c:665:13: error: incompatible type for argument 2 of ‘tcg_gen_trunc_i64_i32’ qemu/tcg/tcg-op.h:1583:20: note: expected ‘TCGv_i64’ but argument is of type ‘TCGv_i32’ ... -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v5 11/16] target-or32: Add a IIS dummy board
On 06/18/2012 05:02 AM, Jia Liu wrote: > Add a dummy board for IIS. > > Signed-off-by: Jia Liu [...] > +if (nd_table[0].vlan) { > +isa_ne2000_init(isa_bus, 0x9200, 4,&nd_table[0]); > +} I have noticed that the kernel you provided expects OpenCores ethernet device. We have a model for it (: You can look at lx60_net_init() in the hw/xtensa_lx60.c to see how it may be connected. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v5 11/16] target-or32: Add a IIS dummy board
On Wed, Jun 20, 2012 at 1:42 PM, Jia Liu wrote: > Hi Max, > > On Wed, Jun 20, 2012 at 2:29 PM, Max Filippov wrote: >> On 06/18/2012 05:02 AM, Jia Liu wrote: >>> Add a dummy board for IIS. >>> >>> Signed-off-by: Jia Liu >> >> [...] >> >> >>> + if (nd_table[0].vlan) { >>> + isa_ne2000_init(isa_bus, 0x9200, 4,&nd_table[0]); >>> + } >> >> I have noticed that the kernel you provided expects OpenCores ethernet >> device. >> We have a model for it (: You can look at lx60_net_init() in the >> hw/xtensa_lx60.c >> to see how it may be connected. >> > > Thank you very much for remind me! > > Is this code OK? > > static void or1200_net_init(MemoryRegion *address_space, > target_phys_addr_t base, > target_phys_addr_t buffers, > qemu_irq irq, NICInfo *nd) > { > DeviceState *dev; > SysBusDevice *s; > MemoryRegion *ram; > > dev = qdev_create(NULL, "open_eth"); > qdev_set_nic_properties(dev, nd); > qdev_init_nofail(dev); > > s = sysbus_from_qdev(dev); > sysbus_connect_irq(s, 0, irq); > memory_region_add_subregion(get_system_memory(), base, > sysbus_mmio_get_region(s, 0)); > > ram = g_malloc(sizeof(*ram)); > memory_region_init_ram(ram, "open_eth.ram", 0x100); > vmstate_register_ram_global(ram); > memory_region_add_subregion(address_space, buffers, ram); > } You haven't mapped descriptors window. Seems to me it should look like this: static void or1200_net_init(MemoryRegion *address_space, target_phys_addr_t base, target_phys_addr_t descriptors, qemu_irq irq, NICInfo *nd) { DeviceState *dev; SysBusDevice *s; dev = qdev_create(NULL, "open_eth"); qdev_set_nic_properties(dev, nd); qdev_init_nofail(dev); s = sysbus_from_qdev(dev); sysbus_connect_irq(s, 0, irq); memory_region_add_subregion(address_space, base, sysbus_mmio_get_region(s, 0)); memory_region_add_subregion(address_space, descriptors, sysbus_mmio_get_region(s, 1)); } > > if (nd_table[0].vlan) { > or1200_net_init(get_system_memory(), 0x9200, > 0x9210, env->irq[4], nd_table); > } > Also I haven't found where 0x9210 comes from. Is there a memory map documentation for this machine? -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v5 11/16] target-or32: Add a IIS dummy board
On Wed, Jun 20, 2012 at 8:41 PM, Jia Liu wrote: > Hi Max, > > On Wed, Jun 20, 2012 at 8:57 PM, Max Filippov wrote: >> On Wed, Jun 20, 2012 at 1:42 PM, Jia Liu wrote: >>> Hi Max, >>> >>> On Wed, Jun 20, 2012 at 2:29 PM, Max Filippov wrote: >>>> On 06/18/2012 05:02 AM, Jia Liu wrote: >>>>> Add a dummy board for IIS. >>>>> >>>>> Signed-off-by: Jia Liu >>>> >>>> [...] >>>> >>>> >>>>> + if (nd_table[0].vlan) { >>>>> + isa_ne2000_init(isa_bus, 0x9200, 4,&nd_table[0]); >>>>> + } >>>> >>>> I have noticed that the kernel you provided expects OpenCores ethernet >>>> device. >>>> We have a model for it (: You can look at lx60_net_init() in the >>>> hw/xtensa_lx60.c >>>> to see how it may be connected. >>>> >>> >>> Thank you very much for remind me! >>> >>> Is this code OK? >>> >>> static void or1200_net_init(MemoryRegion *address_space, >>> target_phys_addr_t base, >>> target_phys_addr_t buffers, >>> qemu_irq irq, NICInfo *nd) >>> { >>> DeviceState *dev; >>> SysBusDevice *s; >>> MemoryRegion *ram; >>> >>> dev = qdev_create(NULL, "open_eth"); >>> qdev_set_nic_properties(dev, nd); >>> qdev_init_nofail(dev); >>> >>> s = sysbus_from_qdev(dev); >>> sysbus_connect_irq(s, 0, irq); >>> memory_region_add_subregion(get_system_memory(), base, >>> sysbus_mmio_get_region(s, 0)); >>> >>> ram = g_malloc(sizeof(*ram)); >>> memory_region_init_ram(ram, "open_eth.ram", 0x100); >>> vmstate_register_ram_global(ram); >>> memory_region_add_subregion(address_space, buffers, ram); >>> } >> >> You haven't mapped descriptors window. Seems to me it should look like this: >> >> static void or1200_net_init(MemoryRegion *address_space, >> target_phys_addr_t base, >> target_phys_addr_t descriptors, >> qemu_irq irq, NICInfo *nd) >> { >> DeviceState *dev; >> SysBusDevice *s; >> >> dev = qdev_create(NULL, "open_eth"); >> qdev_set_nic_properties(dev, nd); >> qdev_init_nofail(dev); >> >> s = sysbus_from_qdev(dev); >> sysbus_connect_irq(s, 0, irq); >> memory_region_add_subregion(address_space, base, >> sysbus_mmio_get_region(s, 0)); >> memory_region_add_subregion(address_space, descriptors, >> sysbus_mmio_get_region(s, 1)); >> } >> > > Thank you very much for the code. > >>> >>> if (nd_table[0].vlan) { >>> or1200_net_init(get_system_memory(), 0x9200, >>> 0x9210, env->irq[4], nd_table); >>> } >>> >> >> Also I haven't found where 0x9210 comes from. >> Is there a memory map documentation for this machine? >> > > I'm confused about descriptors, I'm not sure whether 0x9210 is suitable. > > I find the code in linux/arch/openrisc/boot/dts/or1ksim.dts > enet0: ethoc@9200 { > compatible = "opencores,ethmac-rtlsvn338"; > reg = <0x9200 0x100>; > interrupts = <4>; > }; > > but I'm not sure what value should a pass to target_phys_addr_t > descriptors, that is, I don't know how can I get the address of > descriptors. Ok, with if (nd_table[0].vlan) { -isa_ne2000_init(isa_bus, 0x9200, 4, &nd_table[0]); +openrisc_sim_net_init(get_system_memory(), 0x9200, +0x92000400, env->irq[4], nd_table); } I was able to ping the host from the guest. I guess that from OpenCores ethernet perspective descriptors window is a logical continuation of the registers window, always starting at offset 0x400. I will post a patch that you can squish into this ISS dummy board patch. -- Thanks. -- Max
[Qemu-devel] [PATCH] target-or32: replace NE2000 with OpenCores 10/100 ethernet adapter
Signed-off-by: Max Filippov --- default-configs/or32-softmmu.mak |2 +- hw/openrisc_sim.c| 24 +++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/default-configs/or32-softmmu.mak b/default-configs/or32-softmmu.mak index 7590eed..d85b82b 100644 --- a/default-configs/or32-softmmu.mak +++ b/default-configs/or32-softmmu.mak @@ -2,5 +2,5 @@ include pci.mak CONFIG_SERIAL=y -CONFIG_NE2000_ISA=y +CONFIG_OPENCORES_ETH=y CONFIG_I8259=y diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c index 2fe27f5..7e4cbac 100644 --- a/hw/openrisc_sim.c +++ b/hw/openrisc_sim.c @@ -28,6 +28,7 @@ #include "sysemu.h" #include "isa.h" #include "qtest.h" +#include "sysbus.h" #define KERNEL_LOAD_ADDR 0x100 @@ -80,6 +81,26 @@ static uint64_t openrisc_load_kernel(void) return entry; } +static void openrisc_sim_net_init(MemoryRegion *address_space, +target_phys_addr_t base, +target_phys_addr_t descriptors, +qemu_irq irq, NICInfo *nd) +{ +DeviceState *dev; +SysBusDevice *s; + +dev = qdev_create(NULL, "open_eth"); +qdev_set_nic_properties(dev, nd); +qdev_init_nofail(dev); + +s = sysbus_from_qdev(dev); +sysbus_connect_irq(s, 0, irq); +memory_region_add_subregion(address_space, base, +sysbus_mmio_get_region(s, 0)); +memory_region_add_subregion(address_space, descriptors, +sysbus_mmio_get_region(s, 1)); +} + static void openrisc_sim_init(ram_addr_t ram_size, const char *boot_device, const char *kernel_filename, @@ -125,7 +146,8 @@ static void openrisc_sim_init(ram_addr_t ram_size, env->irq[2], 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); if (nd_table[0].vlan) { -isa_ne2000_init(isa_bus, 0x9200, 4, &nd_table[0]); +openrisc_sim_net_init(get_system_memory(), 0x9200, +0x92000400, env->irq[4], nd_table); } } -- 1.7.7.6
Re: [Qemu-devel] [PATCH v6 11/16] target-or32: Add a IIS dummy board
On Thu, Jun 21, 2012 at 12:19 PM, 陳韋任 (Wei-Ren Chen) wrote: >> + * OpenRISC simulator for use as an ISS. > ^^^ > Shoudld be IIS? I guess it stands for Instruction Set Simulator, so rather the subject should be changed. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v6 08/16] target-or32: Add instruction tanslation
On Thu, Jun 21, 2012 at 6:58 AM, Jia Liu wrote: > Add OpenRISC instruction tanslation routines. > > Signed-off-by: Jia Liu [...] > + case 0x0009: > + switch (op1) { > + case 0x03: /*l.div*/ > + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb); > + { > + int lab0 = gen_new_label(); > + int lab1 = gen_new_label(); > + int lab2 = gen_new_label(); > + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); > + if (rb == 0) { > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab0); > + } else { > + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb], > + 0x, lab1); > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra], > + 0x, lab2); > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], > + 0x8000, lab2); > + gen_set_label(lab1); > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2); Causes host division by zero/overflow. I'd suggest to brcond to lab3 set after the final tcg_gen_div. > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab2); > + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); > + } > + tcg_temp_free_i32(sr_ove); > + } > + break; > + > + default: > + gen_illegal_exception(dc); > + break; > + } > + break; > + > + case 0x000a: > + switch (op1) { > + case 0x03: /*l.divu*/ > + LOG_DIS("l.divu r%d, r%d, r%d\n", rd, ra, rb); > + { > + int lab0 = gen_new_label(); > + int lab1 = gen_new_label(); > + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); > + if (rb == 0) { > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab0); > + } else { > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], > + 0x, lab1); > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab1); Ditto. > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab1); > + tcg_gen_divu_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); > + } > + tcg_temp_free_i32(sr_ove); > + } > + break; [...] > + case 0x000e: > + switch (op1) { > + case 0x00: /*l.cmov*/ > + LOG_DIS("l.cmov r%d, r%d, r%d\n", rd, ra, rb); > + { > + int lab = gen_new_label(); > + TCGv res = tcg_temp_new(); Need to be temp_local to survive brcond. > + TCGv sr_f = tcg_temp_new(); > + tcg_gen_andi_tl(sr_f, cpu_sr, SR_F); > + tcg_gen_mov_tl(res, cpu_R[rb]); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_f, SR_F, lab); > + tcg_gen_mov_tl(res, cpu_R[ra]); > + gen_set_label(lab); > + tcg_gen_mov_tl(cpu_R[rd], res); > + tcg_temp_free(sr_f); > + tcg_temp_free(res); > + } > + break; [...] -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v6 08/16] target-or32: Add instruction tanslation
On Mon, Jun 25, 2012 at 6:50 AM, Jia Liu wrote: > Hi Max, > > On Thu, Jun 21, 2012 at 6:24 PM, Max Filippov wrote: >> On Thu, Jun 21, 2012 at 6:58 AM, Jia Liu wrote: >>> Add OpenRISC instruction tanslation routines. >>> >>> Signed-off-by: Jia Liu >> >> [...] >> >>> + case 0x0009: >>> + switch (op1) { >>> + case 0x03: /*l.div*/ >>> + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb); >>> + { >>> + int lab0 = gen_new_label(); >>> + int lab1 = gen_new_label(); >>> + int lab2 = gen_new_label(); >>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); >>> + if (rb == 0) { >>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); >>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>> + gen_exception(dc, EXCP_RANGE); >>> + gen_set_label(lab0); >>> + } else { >>> + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb], >>> + 0x, lab1); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra], >>> + 0x, lab2); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], >>> + 0x8000, lab2); >>> + gen_set_label(lab1); >>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2); >> >> Causes host division by zero/overflow. I'd suggest to brcond to lab3 set >> after >> the final tcg_gen_div. > > Causes host division by zero/overflow? Can I handle the host code? I'm > confused about this. > May I get more comment about this? Sorry I didn't understand it. The brcondi in question jumps to the division operation although we know for sure that it's going to be a division by zero or an overflow. The host is not smarter than we are, it cannot magically divide by zero. So, if the result register value is undefined in case of division by zero/overflow than you should skip the division and brcondi to a label lab3, right after it. >>> + gen_exception(dc, EXCP_RANGE); >>> + gen_set_label(lab2); >>> + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); + gen_set_label(lab3); >>> + } >>> + tcg_temp_free_i32(sr_ove); >>> + } >>> + break; A picture worth a thousand words is the test for this specific case. I think it can look like this (because SR_OVE is cleared upon reset): int main(void) { int a, b, c; b = 0x8000; c = 0x; __asm ("l.div %0, %1, %2\n" : "=r"(a) : "r"(b), "r"(c) ); return 0; } I believe that this test with the current div implementation would crash qemu. BTW, shouldn't tests that detect wrong behavior do "return 1" or something like this to make "make check" fully automatic? -- Thanks. -- Max
Re: [Qemu-devel] [PATCH 2/4] target-xtensa: drop usage of prev_debug_excp_handler
On Mon, Jun 25, 2012 at 5:55 PM, Igor Mammedov wrote: > Chains of exception handlers are currently unused feature. Dropping it > to be consistent with target-i386 but it may simplify qom-ifying CPU > in future like for target-i386. > > Signed-off-by: Igor Mammedov > --- > target-xtensa/helper.c | 8 +--- > 1 file changed, 1 insertion(+), 7 deletions(-) Acked-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v6 08/16] target-or32: Add instruction tanslation
On Tue, Jun 26, 2012 at 5:37 AM, Jia Liu wrote: > Hi Max, > > On Mon, Jun 25, 2012 at 5:00 PM, Max Filippov wrote: >> On Mon, Jun 25, 2012 at 6:50 AM, Jia Liu wrote: >>> Hi Max, >>> >>> On Thu, Jun 21, 2012 at 6:24 PM, Max Filippov wrote: >>>> On Thu, Jun 21, 2012 at 6:58 AM, Jia Liu wrote: >>>>> Add OpenRISC instruction tanslation routines. >>>>> >>>>> Signed-off-by: Jia Liu >>>> >>>> [...] >>>> >>>>> + case 0x0009: >>>>> + switch (op1) { >>>>> + case 0x03: /*l.div*/ >>>>> + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb); >>>>> + { >>>>> + int lab0 = gen_new_label(); >>>>> + int lab1 = gen_new_label(); >>>>> + int lab2 = gen_new_label(); >>>>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); >>>>> + if (rb == 0) { >>>>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>>>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, >>>>> lab0); >>>>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>>>> + gen_exception(dc, EXCP_RANGE); >>>>> + gen_set_label(lab0); >>>>> + } else { >>>>> + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb], >>>>> + 0x, lab1); >>>>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra], >>>>> + 0x, lab2); >>>>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], >>>>> + 0x8000, lab2); >>>>> + gen_set_label(lab1); >>>>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>>>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>>>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, >>>>> lab2); >>>> >>>> Causes host division by zero/overflow. I'd suggest to brcond to lab3 set >>>> after >>>> the final tcg_gen_div. >>> >>> Causes host division by zero/overflow? Can I handle the host code? I'm >>> confused about this. >>> May I get more comment about this? Sorry I didn't understand it. >> >> The brcondi in question jumps to the division operation although we >> know for sure >> that it's going to be a division by zero or an overflow. The host is >> not smarter than >> we are, it cannot magically divide by zero. So, if the result register value >> is >> undefined in case of division by zero/overflow than you should skip the >> division >> and brcondi to a label lab3, right after it. >> >>>>> + gen_exception(dc, EXCP_RANGE); >>>>> + gen_set_label(lab2); >>>>> + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); >> >> + gen_set_label(lab3); > > If we set a lab3 here, after tcg_gen_div_tl, and jump here when > (cpr_R[ra] == 0x && cpu_R[rb] == 0x8000), it won't raise a > exception or compute. That's the whole point. Overflow and division by zero jumps here only in case SR_OVE is cleared, i.e. we don't want exception. I haven't found in openrisc documentation what result l.div produces in this case. I guess there can be two options: either the result is undefined, or it's some special value, like INF/NAN in the floating point world. > It may not be a good way. > >> >>>>> + } >>>>> + tcg_temp_free_i32(sr_ove); >>>>> + } >>>>> + break; >> >> A picture worth a thousand words is the test for this specific case. >> I think it can look like this (because SR_OVE is cleared upon reset): >> >> int main(void) >> { >> int a, b, c; >> >> b = 0x8000; >> c = 0x; >> >> __asm >> ("l.div %0, %1, %2\n" >> : "=r"(a) >> : "r"(b), "r"(c) >> ); >> return 0; >> } >> >> I believe that this test with the current div implementation would crash >> qemu. >> > > Yeah, it crashed. It looks I should find a better way to handle l.div*. > Thank you for your test case. > > 0x is -1, 0x8000 is -MAX. > -1/-MAX will raise a exception, and I've handle this. > -MAX/-1 will get a MAX, for max value of a register is -(-MAX)-1, so, > it overflowed, I didn't handle this. Actually you did this with three brcondi in the else branch. The only thing left is not to crash in case guest doesn't want range exception. > I'm working on it to find a suitable path, a little difficult to me. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v6 08/16] target-or32: Add instruction tanslation
On Tue, Jun 26, 2012 at 1:14 PM, 陳韋任 (Wei-Ren Chen) wrote: >> > 0x is -1, 0x80000000 is -MAX. >> > -1/-MAX ?will raise a exception, and I've handle this. -1 / -MAX equals 0, it's not the issue here. >> > -MAX/-1 ?will get a MAX, for max value of a register is -(-MAX)-1, so, >> > it overflowed, I didn't handle this. >> >> Actually you did this with three brcondi in the else branch. The only thing >> left is not to crash in case guest doesn't want range exception. > > So Jia need to do something like below > > /* > * if ((%ra == 0x || %rb == 0x8000) && (sr_ove == 0)) Actually the condition that needs special care is ((%rb == 0 || %ra == 0x8000 && %rb == 0x) && sr_ove == 0) > * goto lab3; > * > * lab3: > * compute %ra/%rb but do NOT cause host failed? This step is empty if the result is undefined, which I think is the case. > */ > tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_OV); > tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_CY); If we get here then the first part of condition (before && sr_ove == 0) is true. > tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab3); > gen_exception(dc, EXCP_RANGE); > gen_set_label(lab2); > tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); > gen_set_label(lab3); > > ??? ??? == just nothing. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v7 08/16] target-or32: Add instruction translation
On Wed, Jun 27, 2012 at 1:54 PM, Jia Liu wrote: > Add OpenRISC instruction tanslation routines. > > Signed-off-by: Jia Liu [...] > + case 0x0009: > + switch (op1) { > + case 0x03: /* l.div */ > + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb); > + { > + int lab0 = gen_new_label(); > + int lab1 = gen_new_label(); > + int lab2 = gen_new_label(); > + int lab3 = gen_new_label(); > + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); > + if (rb == 0) { > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab0); > + } else { > + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb], > + 0x, lab1); > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra], > + 0x8000, lab2); > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], > + 0x, lab2); > + gen_set_label(lab1); > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_EQ, sr_ove, SR_OVE, lab3); You used to have tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2) here in previous series, why do you change NE to EQ? > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab2); > + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); > + gen_set_label(lab3); > + } > + tcg_temp_free_i32(sr_ove); > + } > + break; > + > + default: > + gen_illegal_exception(dc); > + break; > + } > + break; > + > + case 0x000a: > + switch (op1) { > + case 0x03: /* l.divu */ > + LOG_DIS("l.divu r%d, r%d, r%d\n", rd, ra, rb); > + { > + int lab0 = gen_new_label(); > + int lab1 = gen_new_label(); > + int lab2 = gen_new_label(); > + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); > + if (rb == 0) { > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab0); > + } else { > + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], > + 0x, lab1); > + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); > + tcg_gen_brcondi_tl(TCG_COND_EQ, sr_ove, SR_OVE, lab2); Same here. > + gen_exception(dc, EXCP_RANGE); > + gen_set_label(lab1); > + tcg_gen_divu_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); > + gen_set_label(lab2); > + } > + tcg_temp_free_i32(sr_ove); > + } > + break; -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v7 08/16] target-or32: Add instruction translation
On Wed, Jun 27, 2012 at 4:40 PM, Jia Liu wrote: > Hi Max, > > On Wed, Jun 27, 2012 at 7:03 PM, Max Filippov wrote: >> On Wed, Jun 27, 2012 at 1:54 PM, Jia Liu wrote: >>> Add OpenRISC instruction tanslation routines. >>> >>> Signed-off-by: Jia Liu >> >> [...] >> >>> + case 0x0009: >>> + switch (op1) { >>> + case 0x03: /* l.div */ >>> + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb); >>> + { >>> + int lab0 = gen_new_label(); >>> + int lab1 = gen_new_label(); >>> + int lab2 = gen_new_label(); >>> + int lab3 = gen_new_label(); >>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); >>> + if (rb == 0) { >>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); >>> + gen_exception(dc, EXCP_RANGE); >>> + gen_set_label(lab0); >>> + } else { >>> + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb], >>> + 0x, lab1); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra], >>> + 0x8000, lab2); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb], >>> + 0x, lab2); >>> + gen_set_label(lab1); >>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>> + tcg_gen_brcondi_tl(TCG_COND_EQ, sr_ove, SR_OVE, lab3); >> >> You used to have >> >> tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2) >> >> here in previous series, why do you change NE to EQ? >> > > According to the OpenRISC Arch Manual. > OV: Overflow flag > 0 No overflow occured during last arithmetic operation > 1 Overflow occured during last arithmetic operation > > OVE:Overflow flag Exception > 0 Overflow flag does not cause an exception > 1 Overflow flag causes range exception > > It will raise a exception, when sr_ove is 1. 0, not. Right. > If there was not a exception, that is, sr_ove is 0, we should use NE > and jump to lab2, compute safety. No, sr_ove does not reflect whether there was an exception or not, it controls whether we want an exception on overflow or not. > but if there was a exception, that is, sr_ove is 1, we have to jump to > lab3 to avoid crash QEMU. > Keep it NE will run the test fine, too. I think maybe EQ is better. It's strange, with my testcase you should get an exception inside the guest if you put EQ here. Did you observe it? > How it to be in your eyes? Condition code was all correct; when we didn't want an exception on overflow, i.e. sr_ove is not equal to SR_OVE, you jumped to exit, otherwise you raised an exception. You should keep NE here. >>> + gen_exception(dc, EXCP_RANGE); >>> + gen_set_label(lab2); >>> + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]); >>> + gen_set_label(lab3); >>> + } >>> + tcg_temp_free_i32(sr_ove); >>> + } >>> + break; >>> + >>> + default: >>> + gen_illegal_exception(dc); >>> + break; >>> + } >>> + break; >>> + >>> + case 0x000a: >>> + switch (op1) { >>> + case 0x03: /* l.divu */ >>> + LOG_DIS("l.divu r%d, r%d, r%d\n", rd, ra, rb); >>> + { >>> + int lab0 = gen_new_label(); >>> + int lab1 = gen_new_label(); >>> + int lab2 = gen_new_label(); >>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32(); >>> + if (rb == 0) { >>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); >>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); >>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0); >>> + gen_exception(dc, EXCP_RANGE); >>> + gen_set_label(lab
Re: [Qemu-devel] [PATCH v13 04/13] Add cache handling functions
On Wed, Jun 27, 2012 at 8:55 PM, Eric Blake wrote: > On 06/27/2012 04:34 AM, Orit Wasserman wrote: [...] >> + >> + /* round down to the nearest power of 2 */ >> + if (!is_power_of_2(num_pages)) { >> + num_pages = 1 << ffs(num_pages); > > That's not how you round down. For example, if I passed in 0x5, you end > up giving me 1 << ffs(5) == 1 << 1 == 2, but the correct answer should be 4. > > http://graphics.stanford.edu/~seander/bithacks.html#IntegerLogObvious > and http://aggregate.org/MAGIC/#Leading%20Zero%20Count give some hints > about what you really want to be doing; offhand, I came up with this (it > works because you already rejected negative num_pages): > > if (!is_power_of_2(num_pages)) { > num_pages |= num_pages >> 1; > num_pages |= num_pages >> 2; > num_pages |= num_pages >> 4; > num_pages |= num_pages >> 8; > num_pages |= num_pages >> 16; > num_pages |= num_pages >> 32; > num_pages -= num_pages / 2; > } Or if (!is_power_of_2(num_pages)) { num_pages = 0x8000ULL >> clz64(num_pages); } [...] -- Thanks. -- Max
Re: [Qemu-devel] [PATCH] x86: Fixed incorrect segment base address addition
On Mon, Jul 2, 2012 at 2:29 PM, Vitaly Chipounov wrote: > An instruction with address and segment size override triggers the bug. > inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: > (uint32_t)(gs.base + ebx * 4 + 0x260) > instead of > gs.base + (uint32_t)(ebx * 4 + 0x260) Do I understand it right that this fixes address calculation for 64-bit mode but breaks it for compatibility mode? Quote from "Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": When in compatibility mode, FS and GS overrides operate as defined by 32-bit mode behavior regardless of the value loaded into the upper 32 linear-address bits of the hidden descriptor register base field. Compatibility mode ignores the upper 32 bits when calculating an effective address. > > Signed-off-by: Vitaly Chipounov > --- > target-i386/translate.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index a902f4a..9ca7375 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -459,10 +459,10 @@ static inline void gen_op_movl_A0_seg(int reg) > static inline void gen_op_addl_A0_seg(int reg) > { > tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base)); > - tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > #ifdef TARGET_X86_64 > tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x); > #endif > + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > } > > #ifdef TARGET_X86_64 > -- > 1.7.4.1 > > -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v2] x86: Fixed incorrect segment base address addition in 64-bits mode
On Tue, Jul 3, 2012 at 2:20 AM, Vitaly Chipounov wrote: > According to the Intel manual > "Intel® 64 and IA-32 Architectures Software Developer’s Manual > Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": > > "When in compatibility mode, FS and GS overrides operate as defined by > 32-bit mode behavior regardless of the value loaded into the upper 32 > linear-address bits of the hidden descriptor register base field. > Compatibility mode ignores the upper 32 bits when calculating an effective > address." > > However, the code misses the 64-bit mode case, where an instruction with > address and segment size override would be translated incorrectly. For > example, > inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: > > (uint32_t)(gs.base + ebx * 4 + 0x260) > instead of > gs.base + (uint32_t)(ebx * 4 + 0x260) > > Signed-off-by: Vitaly Chipounov Reviewed-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] [PATCH] target-i386: honor CR0_PG_MASK in cpu_get_phys_page_debug
On Sun, Nov 18, 2012 at 12:52 AM, Max Filippov wrote: > cpu_get_phys_page_debug is not in sync with cpu_x86_handle_mmu_fault: > the latter first checks CR0_PG_MASK and only after CR4_PAE_MASK. > > This fixes odd gdb code display with PAE enabled. > > Signed-off-by: Max Filippov > --- > target-i386/helper.c | 37 - > 1 files changed, 20 insertions(+), 17 deletions(-) Ping? -- Thanks. -- Max
[Qemu-devel] [PATCH 0/8] xtensa patch queue
Hi. This is my current patch queue for xtensa: - add support for a number of Special Registers: ATOMCTL, CACHEATTR, MISC; - raise exceptions on access to unconfigured SRs/invalid access to configured SRs; - add unit tests for SR access and for s32c1i opcode; - use movcond to re-implement some opcodes more efficiently. Please review/apply. Max Filippov (8): target-xtensa: implement ATOMCTL SR target-xtensa: implement CACHEATTR SR target-xtensa: restrict available SRs by enabled options target-xtensa: better control rsr/wsr/xsr access to SRs target-xtensa: implement MISC SR target-xtensa: add SR accessibility unit tests target-xtensa: add s32c1i unit tests target-xtensa: use movcond where possible target-xtensa/cpu.c|3 + target-xtensa/cpu.h| 14 ++ target-xtensa/helper.c | 75 +++-- target-xtensa/helper.h |1 + target-xtensa/op_helper.c | 57 ++ target-xtensa/overlay_tool.h | 12 ++- target-xtensa/translate.c | 367 ++-- tests/tcg/xtensa/Makefile |2 + tests/tcg/xtensa/macros.inc|2 +- tests/tcg/xtensa/test_s32c1i.S | 39 + tests/tcg/xtensa/test_sr.S | 90 ++ 11 files changed, 484 insertions(+), 178 deletions(-) create mode 100644 tests/tcg/xtensa/test_s32c1i.S create mode 100644 tests/tcg/xtensa/test_sr.S -- 1.7.7.6
[Qemu-devel] [PATCH 8/8] target-xtensa: use movcond where possible
Use movcond for all sorts of conditional moves, ABS, CLAMPS, MIN/MAX opcodes. Signed-off-by: Max Filippov --- target-xtensa/translate.c | 92 1 files changed, 42 insertions(+), 50 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 48a22de..7b32123 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -1403,12 +1403,14 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) case 1: /*ABS*/ { -int label = gen_new_label(); -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_T]); -tcg_gen_brcondi_i32( -TCG_COND_GE, cpu_R[RRR_R], 0, label); -tcg_gen_neg_i32(cpu_R[RRR_R], cpu_R[RRR_T]); -gen_set_label(label); +TCGv_i32 zero = tcg_const_i32(0); +TCGv_i32 neg = tcg_temp_new_i32(); + +tcg_gen_neg_i32(neg, cpu_R[RRR_T]); +tcg_gen_movcond_i32(TCG_COND_GE, cpu_R[RRR_R], +cpu_R[RRR_T], zero, cpu_R[RRR_T], neg); +tcg_temp_free(neg); +tcg_temp_free(zero); } break; @@ -1755,22 +1757,20 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) { TCGv_i32 tmp1 = tcg_temp_new_i32(); TCGv_i32 tmp2 = tcg_temp_new_i32(); -int label = gen_new_label(); +TCGv_i32 zero = tcg_const_i32(0); tcg_gen_sari_i32(tmp1, cpu_R[RRR_S], 24 - RRR_T); tcg_gen_xor_i32(tmp2, tmp1, cpu_R[RRR_S]); tcg_gen_andi_i32(tmp2, tmp2, 0x << (RRR_T + 7)); -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]); -tcg_gen_brcondi_i32(TCG_COND_EQ, tmp2, 0, label); tcg_gen_sari_i32(tmp1, cpu_R[RRR_S], 31); -tcg_gen_xori_i32(cpu_R[RRR_R], tmp1, -0x >> (25 - RRR_T)); - -gen_set_label(label); +tcg_gen_xori_i32(tmp1, tmp1, 0x >> (25 - RRR_T)); +tcg_gen_movcond_i32(TCG_COND_EQ, cpu_R[RRR_R], tmp2, zero, +cpu_R[RRR_S], tmp1); tcg_temp_free(tmp1); tcg_temp_free(tmp2); +tcg_temp_free(zero); } break; @@ -1787,19 +1787,9 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) TCG_COND_LEU, TCG_COND_GEU }; -int label = gen_new_label(); - -if (RRR_R != RRR_T) { -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]); -tcg_gen_brcond_i32(cond[OP2 - 4], -cpu_R[RRR_S], cpu_R[RRR_T], label); -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_T]); -} else { -tcg_gen_brcond_i32(cond[OP2 - 4], -cpu_R[RRR_T], cpu_R[RRR_S], label); -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]); -} -gen_set_label(label); +tcg_gen_movcond_i32(cond[OP2 - 4], cpu_R[RRR_R], +cpu_R[RRR_S], cpu_R[RRR_T], +cpu_R[RRR_S], cpu_R[RRR_T]); } break; @@ -1810,15 +1800,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) gen_window_check3(dc, RRR_R, RRR_S, RRR_T); { static const TCGCond cond[] = { -TCG_COND_NE, TCG_COND_EQ, +TCG_COND_NE, +TCG_COND_LT, TCG_COND_GE, -TCG_COND_LT }; -int label = gen_new_label(); -tcg_gen_brcondi_i32(cond[OP2 - 8], cpu_R[RRR_T], 0, label); -tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]); -gen_set_label(label); +TCGv_i32 zero = tcg_const_i32(0); + +tcg_gen_movcond_i32(cond[OP2 - 8], cpu_R[RRR_R], +cpu_R[RRR_T], zero, cpu_R[RRR_S], cpu_R[RRR_R]); +tcg_temp_free(zero); } break; @@ -1827,16 +1818,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) HAS_OPTION(XTENSA_OPTION_BOOLEAN); gen_window_check
[Qemu-devel] [PATCH 7/8] target-xtensa: add s32c1i unit tests
Signed-off-by: Max Filippov --- tests/tcg/xtensa/Makefile |1 + tests/tcg/xtensa/test_s32c1i.S | 39 +++ 2 files changed, 40 insertions(+), 0 deletions(-) create mode 100644 tests/tcg/xtensa/test_s32c1i.S diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile index 56cfe0f..002fd87 100644 --- a/tests/tcg/xtensa/Makefile +++ b/tests/tcg/xtensa/Makefile @@ -42,6 +42,7 @@ endif TESTCASES += test_quo.tst TESTCASES += test_rem.tst TESTCASES += test_rst0.tst +TESTCASES += test_s32c1i.tst TESTCASES += test_sar.tst TESTCASES += test_sext.tst TESTCASES += test_shift.tst diff --git a/tests/tcg/xtensa/test_s32c1i.S b/tests/tcg/xtensa/test_s32c1i.S new file mode 100644 index 000..4536015 --- /dev/null +++ b/tests/tcg/xtensa/test_s32c1i.S @@ -0,0 +1,39 @@ +.include "macros.inc" + +test_suite s32c1i + +test s32c1i_nowrite +movia2, 1f +movia3, 1 +wsr a3, scompare1 +movia1, 2 +s32c1i a1, a2, 0 +assert ne, a1, a3 +l32ia1, a2, 0 +assert eqi, a1, 3 + +.data +.align 4 +1: +.word 3 +.text +test_end + +test s32c1i_write +movia2, 1f +movia3, 3 +wsr a3, scompare1 +movia1, 2 +s32c1i a1, a2, 0 +assert eq, a1, a3 +l32ia1, a2, 0 +assert eqi, a1, 2 + +.data +.align 4 +1: +.word 3 +.text +test_end + +test_suite_end -- 1.7.7.6
[Qemu-devel] [PATCH 2/8] target-xtensa: implement CACHEATTR SR
In XEA1, the Options for Memory Protection and Translation and the corresponding TLB management instructions are not available. Instead, functionality similar to the Region Protection Option is available through the cache attribute register. See ISA, A.2.14 for details. Signed-off-by: Max Filippov --- target-xtensa/cpu.c |1 + target-xtensa/cpu.h |2 ++ target-xtensa/helper.c | 21 - target-xtensa/overlay_tool.h |1 + target-xtensa/translate.c|1 + 5 files changed, 25 insertions(+), 1 deletions(-) diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c index c6aa45e..035b07c 100644 --- a/target-xtensa/cpu.c +++ b/target-xtensa/cpu.c @@ -48,6 +48,7 @@ static void xtensa_cpu_reset(CPUState *s) XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10; env->sregs[VECBASE] = env->config->vecbase; env->sregs[IBREAKENABLE] = 0; +env->sregs[CACHEATTR] = 0x; env->sregs[ATOMCTL] = xtensa_option_enabled(env->config, XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15; diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index d240ab7..068ad69 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -94,6 +94,7 @@ enum { XTENSA_OPTION_REGION_PROTECTION, XTENSA_OPTION_REGION_TRANSLATION, XTENSA_OPTION_MMU, +XTENSA_OPTION_CACHEATTR, /* Other */ XTENSA_OPTION_WINDOWED_REGISTER, @@ -129,6 +130,7 @@ enum { ITLBCFG = 91, DTLBCFG = 92, IBREAKENABLE = 96, +CACHEATTR = 98, ATOMCTL = 99, IBREAKA = 128, DBREAKA = 144, diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index ecd0182..200fb43 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -438,6 +438,24 @@ static unsigned region_attr_to_access(uint32_t attr) return access[attr & 0xf]; } +/*! + * Convert cacheattr to PAGE_{READ,WRITE,EXEC} mask. + * See ISA, A.2.14 The Cache Attribute Register + */ +static unsigned cacheattr_attr_to_access(uint32_t attr) +{ +static const unsigned access[16] = { + [0] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_WT, + [1] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WT, + [2] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS, + [3] = PAGE_EXEC | PAGE_CACHE_WB, + [4] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB, +[14] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE, +}; + +return access[attr & 0xf]; +} + static bool is_access_granted(unsigned access, int is_write) { switch (is_write) { @@ -584,7 +602,8 @@ int xtensa_get_physical_addr(CPUXtensaState *env, bool update_tlb, } else { *paddr = vaddr; *page_size = TARGET_PAGE_SIZE; -*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS; +*access = cacheattr_attr_to_access( +env->sregs[CACHEATTR] >> ((vaddr & 0xe000) >> 27)); return 0; } } diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h index 50bf573..45205b8 100644 --- a/target-xtensa/overlay_tool.h +++ b/target-xtensa/overlay_tool.h @@ -91,6 +91,7 @@ XCHAL_OPTION(XCHAL_HAVE_XLT_CACHEATTR, \ XTENSA_OPTION_REGION_TRANSLATION) | \ XCHAL_OPTION(XCHAL_HAVE_PTP_MMU, XTENSA_OPTION_MMU) | \ +XCHAL_OPTION(XCHAL_HAVE_CACHEATTR, XTENSA_OPTION_CACHEATTR) | \ /* Other, TODO */ \ XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \ XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG)) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 2ba2360..c246fcb 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -99,6 +99,7 @@ static const char * const sregnames[256] = { [ITLBCFG] = "ITLBCFG", [DTLBCFG] = "DTLBCFG", [IBREAKENABLE] = "IBREAKENABLE", +[CACHEATTR] = "CACHEATTR", [ATOMCTL] = "ATOMCTL", [IBREAKA] = "IBREAKA0", [IBREAKA + 1] = "IBREAKA1", -- 1.7.7.6
[Qemu-devel] [PATCH 4/8] target-xtensa: better control rsr/wsr/xsr access to SRs
There are read-only (DEBUGCAUSE, PRID) and write-only (INTCLEAR) SRs, and INTERRUPT/INTSET SR allows rsr/wsr, but not xsr. Raise illeagal opcode exception on illegal access to these SRs. Signed-off-by: Max Filippov --- target-xtensa/translate.c | 49 +++- 1 files changed, 30 insertions(+), 19 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 5416aff..fbeac7f 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -81,16 +81,27 @@ static TCGv_i32 cpu_UR[256]; typedef struct XtensaReg { const char *name; uint64_t opt_bits; +enum { +SR_R = 1, +SR_W = 2, +SR_X = 4, +SR_RW = 3, +SR_RWX = 7, +} access; } XtensaReg; -#define XTENSA_REG(regname, opt) { \ +#define XTENSA_REG_ACCESS(regname, opt, acc) { \ .name = (regname), \ .opt_bits = XTENSA_OPTION_BIT(opt), \ +.access = (acc), \ } +#define XTENSA_REG(regname, opt) XTENSA_REG_ACCESS(regname, opt, SR_RWX) + #define XTENSA_REG_BITS(regname, opt) { \ .name = (regname), \ .opt_bits = (opt), \ +.access = SR_RWX, \ } static const XtensaReg sregnames[256] = { @@ -151,15 +162,15 @@ static const XtensaReg sregnames[256] = { [EXCSAVE1 + 6] = XTENSA_REG("EXCSAVE7", XTENSA_OPTION_HIGH_PRIORITY_INTERRUPT), [CPENABLE] = XTENSA_REG("CPENABLE", XTENSA_OPTION_COPROCESSOR), -[INTSET] = XTENSA_REG("INTSET", XTENSA_OPTION_INTERRUPT), -[INTCLEAR] = XTENSA_REG("INTCLEAR", XTENSA_OPTION_INTERRUPT), +[INTSET] = XTENSA_REG_ACCESS("INTSET", XTENSA_OPTION_INTERRUPT, SR_RW), +[INTCLEAR] = XTENSA_REG_ACCESS("INTCLEAR", XTENSA_OPTION_INTERRUPT, SR_W), [INTENABLE] = XTENSA_REG("INTENABLE", XTENSA_OPTION_INTERRUPT), [PS] = XTENSA_REG_BITS("PS", XTENSA_OPTION_ALL), [VECBASE] = XTENSA_REG("VECBASE", XTENSA_OPTION_RELOCATABLE_VECTOR), [EXCCAUSE] = XTENSA_REG("EXCCAUSE", XTENSA_OPTION_EXCEPTION), -[DEBUGCAUSE] = XTENSA_REG("DEBUGCAUSE", XTENSA_OPTION_DEBUG), +[DEBUGCAUSE] = XTENSA_REG_ACCESS("DEBUGCAUSE", XTENSA_OPTION_DEBUG, SR_R), [CCOUNT] = XTENSA_REG("CCOUNT", XTENSA_OPTION_TIMER_INTERRUPT), -[PRID] = XTENSA_REG("PRID", XTENSA_OPTION_PROCESSOR_ID), +[PRID] = XTENSA_REG_ACCESS("PRID", XTENSA_OPTION_PROCESSOR_ID, SR_R), [ICOUNT] = XTENSA_REG("ICOUNT", XTENSA_OPTION_DEBUG), [ICOUNTLEVEL] = XTENSA_REG("ICOUNTLEVEL", XTENSA_OPTION_DEBUG), [EXCVADDR] = XTENSA_REG("EXCVADDR", XTENSA_OPTION_EXCEPTION), @@ -476,7 +487,7 @@ static void gen_brcondi(DisasContext *dc, TCGCond cond, tcg_temp_free(tmp); } -static void gen_check_sr(DisasContext *dc, uint32_t sr) +static void gen_check_sr(DisasContext *dc, uint32_t sr, unsigned access) { if (!xtensa_option_bits_enabled(dc->config, sregnames[sr].opt_bits)) { if (sregnames[sr].name) { @@ -485,6 +496,16 @@ static void gen_check_sr(DisasContext *dc, uint32_t sr) qemu_log("SR %d is not implemented\n", sr); } gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE); +} else if (!(sregnames[sr].access & access)) { +static const char * const access_text[] = { +[SR_R] = "rsr", +[SR_W] = "wsr", +[SR_X] = "xsr", +}; +assert(access < ARRAY_SIZE(access_text) && access_text[access]); +qemu_log("SR %s is not available for %s\n", sregnames[sr].name, +access_text[access]); +gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE); } } @@ -679,14 +700,6 @@ static void gen_wsr_ps(DisasContext *dc, uint32_t sr, TCGv_i32 v) gen_jumpi_check_loop_end(dc, -1); } -static void gen_wsr_debugcause(DisasContext *dc, uint32_t sr, TCGv_i32 v) -{ -} - -static void gen_wsr_prid(DisasContext *dc, uint32_t sr, TCGv_i32 v) -{ -} - static void gen_wsr_icount(DisasContext *dc, uint32_t sr, TCGv_i32 v) { if (dc->icount) { @@ -744,8 +757,6 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 s) [INTCLEAR] = gen_wsr_intclear, [INTENABLE] = gen_wsr_intenable, [PS] = gen_wsr_ps, -[DEBUGCAUSE] = gen_wsr_debugcause, -[PRID] = gen_wsr_prid, [ICOUNT] = gen_wsr_icount, [ICOUNTLEVEL] = gen_wsr_icountlevel, [CCOMPARE] = gen_wsr_ccompare, @@ -1467,7 +1478,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc) case 6: /*XSR*/ { TCGv_i32 tmp = tcg_temp_new_i32(); -gen_check_sr(dc, RSR_SR); +gen_check_sr(dc, RSR_SR, SR_X); if (RSR_SR >= 64) { gen_check
[Qemu-devel] [PATCH 5/8] target-xtensa: implement MISC SR
The Miscellaneous Special Registers Option provides zero to four scratch registers within the processor readable and writable by RSR, WSR, and XSR. These registers are privileged. They may be useful for some application-specific exception and interrupt processing tasks in the kernel. The MISC registers are undefined after reset. See ISA, 4.7.3 for details. Signed-off-by: Max Filippov --- target-xtensa/cpu.h |1 + target-xtensa/overlay_tool.h |1 + target-xtensa/translate.c|4 3 files changed, 6 insertions(+), 0 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index a73d32d..08fd5bc 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -153,6 +153,7 @@ enum { ICOUNTLEVEL = 237, EXCVADDR = 238, CCOMPARE = 240, +MISC = 244, }; #define PS_INTLEVEL 0xf diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h index 0b47029..dd4f51a 100644 --- a/target-xtensa/overlay_tool.h +++ b/target-xtensa/overlay_tool.h @@ -95,6 +95,7 @@ /* Other, TODO */ \ XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \ XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG) |\ +XCHAL_OPTION(XCHAL_NUM_MISC_REGS > 0, XTENSA_OPTION_MISC_SR) | \ XCHAL_OPTION(XCHAL_HAVE_THREADPTR, XTENSA_OPTION_THREAD_POINTER) | \ XCHAL_OPTION(XCHAL_HAVE_PRID, XTENSA_OPTION_PROCESSOR_ID)) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index fbeac7f..48a22de 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -179,6 +179,10 @@ static const XtensaReg sregnames[256] = { XTENSA_OPTION_TIMER_INTERRUPT), [CCOMPARE + 2] = XTENSA_REG("CCOMPARE2", XTENSA_OPTION_TIMER_INTERRUPT), +[MISC] = XTENSA_REG("MISC0", XTENSA_OPTION_MISC_SR), +[MISC + 1] = XTENSA_REG("MISC1", XTENSA_OPTION_MISC_SR), +[MISC + 2] = XTENSA_REG("MISC2", XTENSA_OPTION_MISC_SR), +[MISC + 3] = XTENSA_REG("MISC3", XTENSA_OPTION_MISC_SR), }; static const XtensaReg uregnames[256] = { -- 1.7.7.6
[Qemu-devel] [PATCH 1/8] target-xtensa: implement ATOMCTL SR
ATOMCTL SR controls s32c1i opcode behavior depending on targeted memory type. See ISA, 4.3.12.4 for details. Signed-off-by: Max Filippov --- target-xtensa/cpu.c |2 + target-xtensa/cpu.h | 10 +++ target-xtensa/helper.c | 56 +++-- target-xtensa/helper.h |1 + target-xtensa/op_helper.c| 57 ++ target-xtensa/overlay_tool.h |6 target-xtensa/translate.c| 13 + 7 files changed, 131 insertions(+), 14 deletions(-) diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c index 9d01983..c6aa45e 100644 --- a/target-xtensa/cpu.c +++ b/target-xtensa/cpu.c @@ -48,6 +48,8 @@ static void xtensa_cpu_reset(CPUState *s) XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10; env->sregs[VECBASE] = env->config->vecbase; env->sregs[IBREAKENABLE] = 0; +env->sregs[ATOMCTL] = xtensa_option_enabled(env->config, +XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15; env->pending_irq_level = 0; reset_mmu(env); diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index 74e9888..d240ab7 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -65,6 +65,7 @@ enum { XTENSA_OPTION_FP_COPROCESSOR, XTENSA_OPTION_MP_SYNCHRO, XTENSA_OPTION_CONDITIONAL_STORE, +XTENSA_OPTION_ATOMCTL, /* Interrupts and exceptions */ XTENSA_OPTION_EXCEPTION, @@ -128,6 +129,7 @@ enum { ITLBCFG = 91, DTLBCFG = 92, IBREAKENABLE = 96, +ATOMCTL = 99, IBREAKA = 128, DBREAKA = 144, DBREAKC = 160, @@ -193,6 +195,14 @@ enum { #define REGION_PAGE_MASK 0xe000 +#define PAGE_CACHE_MASK0x700 +#define PAGE_CACHE_SHIFT 8 +#define PAGE_CACHE_INVALID 0x000 +#define PAGE_CACHE_BYPASS 0x100 +#define PAGE_CACHE_WT 0x200 +#define PAGE_CACHE_WB 0x400 +#define PAGE_CACHE_ISOLATE 0x600 + enum { /* Static vectors */ EXC_RESET, diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index d94bae2..ecd0182 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -390,6 +390,7 @@ int xtensa_tlb_lookup(const CPUXtensaState *env, uint32_t addr, bool dtlb, static unsigned mmu_attr_to_access(uint32_t attr) { unsigned access = 0; + if (attr < 12) { access |= PAGE_READ; if (attr & 0x1) { @@ -398,8 +399,22 @@ static unsigned mmu_attr_to_access(uint32_t attr) if (attr & 0x2) { access |= PAGE_WRITE; } + +switch (attr & 0xc) { +case 0: +access |= PAGE_CACHE_BYPASS; +break; + +case 4: +access |= PAGE_CACHE_WB; +break; + +case 8: +access |= PAGE_CACHE_WT; +break; +} } else if (attr == 13) { -access |= PAGE_READ | PAGE_WRITE; +access |= PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE; } return access; } @@ -410,14 +425,17 @@ static unsigned mmu_attr_to_access(uint32_t attr) */ static unsigned region_attr_to_access(uint32_t attr) { -unsigned access = 0; -if ((attr < 6 && attr != 3) || attr == 14) { -access |= PAGE_READ | PAGE_WRITE; -} -if (attr > 0 && attr < 6) { -access |= PAGE_EXEC; -} -return access; +static const unsigned access[16] = { + [0] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_WT, + [1] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WT, + [2] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS, + [3] = PAGE_EXEC | PAGE_CACHE_WB, + [4] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB, + [5] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB, +[14] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE, +}; + +return access[attr & 0xf]; } static bool is_access_granted(unsigned access, int is_write) @@ -566,7 +584,7 @@ int xtensa_get_physical_addr(CPUXtensaState *env, bool update_tlb, } else { *paddr = vaddr; *page_size = TARGET_PAGE_SIZE; -*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC; +*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS; return 0; } } @@ -599,24 +617,34 @@ static void dump_tlb(FILE *f, fprintf_function cpu_fprintf, xtensa_tlb_get_entry(env, dtlb, wi, ei); if (entry->asid) { +static const char * const cache_text[8] = { +[PAGE_CACHE_BYPASS >> PAGE_CACHE_SHIFT] = "Bypass", +[PAGE_CACHE_WT >> PAGE_CACHE_SHIFT] = "WT", +[PAGE_CACHE_WB >> PAGE_CACHE_SHIFT] = "WB", +[PAGE_CACHE_ISOLATE >> PAGE_CACHE_SHIFT] = "Isolate", +}; unsigned access = attr_to_access(entry->
[Qemu-devel] [PATCH 6/8] target-xtensa: add SR accessibility unit tests
Signed-off-by: Max Filippov --- tests/tcg/xtensa/Makefile |1 + tests/tcg/xtensa/macros.inc |2 +- tests/tcg/xtensa/test_sr.S | 90 +++ 3 files changed, 92 insertions(+), 1 deletions(-) create mode 100644 tests/tcg/xtensa/test_sr.S diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile index 0ff0ccf..56cfe0f 100644 --- a/tests/tcg/xtensa/Makefile +++ b/tests/tcg/xtensa/Makefile @@ -45,6 +45,7 @@ TESTCASES += test_rst0.tst TESTCASES += test_sar.tst TESTCASES += test_sext.tst TESTCASES += test_shift.tst +TESTCASES += test_sr.tst TESTCASES += test_timer.tst TESTCASES += test_windowed.tst diff --git a/tests/tcg/xtensa/macros.inc b/tests/tcg/xtensa/macros.inc index 23bf3e9..c9be1ce 100644 --- a/tests/tcg/xtensa/macros.inc +++ b/tests/tcg/xtensa/macros.inc @@ -1,7 +1,7 @@ .macro test_suite name .data status: .word result -result: .space 20 +result: .space 256 .text .global main .align 4 diff --git a/tests/tcg/xtensa/test_sr.S b/tests/tcg/xtensa/test_sr.S new file mode 100644 index 000..470c03d --- /dev/null +++ b/tests/tcg/xtensa/test_sr.S @@ -0,0 +1,90 @@ +.include "macros.inc" + +test_suite sr + +.macro sr_op sym, op_sym, op_byte, sr +.if \sym +\op_sym a4, \sr +.else +.byte 0x40, \sr, \op_byte +.endif +.endm + +.macro test_sr_op sym, mask, op, op_byte, sr +movia4, 0 +.if (\mask) +set_vector kernel, 0 +sr_op \sym, \op, \op_byte, \sr +.else +set_vector kernel, 2f +1: +sr_op \sym, \op, \op_byte, \sr +test_fail +2: +reset_ps +rsr a2, exccause +assert eqi, a2, 0 +rsr a2, epc1 +movia3, 1b +assert eq, a2, a3 +.endif +.endm + +.macro test_sr_mask sr, sym, mask +test \sr +test_sr_op \sym, \mask & 1, rsr, 0x03, \sr +test_sr_op \sym, \mask & 2, wsr, 0x13, \sr +test_sr_op \sym, \mask & 4, xsr, 0x61, \sr +test_end +.endm + +.macro test_sr sr, conf +test_sr_mask\sr, \conf, 7 +.endm + +test_sr acchi, 1 +test_sr acclo, 1 +test_sr_mask /*atomctl*/99, 0, 0 +test_sr_mask /*br*/4, 0, 0 +test_sr_mask /*cacheattr*/98, 0, 0 +test_sr ccompare0, 1 +test_sr ccount, 1 +test_sr cpenable, 1 +test_sr dbreaka0, 1 +test_sr dbreakc0, 1 +test_sr_mask debugcause, 1, 1 +test_sr depc, 1 +test_sr dtlbcfg, 1 +test_sr epc1, 1 +test_sr epc2, 1 +test_sr eps2, 1 +test_sr exccause, 1 +test_sr excsave1, 1 +test_sr excsave2, 1 +test_sr excvaddr, 1 +test_sr ibreaka0, 1 +test_sr ibreakenable, 1 +test_sr icount, 1 +test_sr icountlevel, 1 +test_sr_mask /*intclear*/227, 0, 2 +test_sr_mask /*interrupt*/226, 0, 3 +test_sr intenable, 1 +test_sr itlbcfg, 1 +test_sr lbeg, 1 +test_sr lcount, 1 +test_sr lend, 1 +test_sr litbase, 1 +test_sr m0, 1 +test_sr misc0, 1 +test_sr_mask /*prefctl*/40, 0, 0 +test_sr_mask /*prid*/235, 0, 1 +test_sr ps, 1 +test_sr ptevaddr, 1 +test_sr rasid, 1 +test_sr sar, 1 +test_sr scompare1, 1 +test_sr vecbase, 1 +test_sr windowbase, 1 +test_sr windowstart, 1 + +test_suite_end -- 1.7.7.6
[Qemu-devel] [PATCH 3/8] target-xtensa: restrict available SRs by enabled options
Beginning with the RA-2004.1 release, SR access instructions (rsr, wsr, xsr) are associated with their corresponding SR and raise illegal opcode exception in case the register is not configured for the core. Signed-off-by: Max Filippov --- target-xtensa/cpu.h |1 + target-xtensa/overlay_tool.h |4 +- target-xtensa/translate.c| 230 +++--- 3 files changed, 130 insertions(+), 105 deletions(-) diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h index 068ad69..a73d32d 100644 --- a/target-xtensa/cpu.h +++ b/target-xtensa/cpu.h @@ -416,6 +416,7 @@ void debug_exception_env(CPUXtensaState *new_env, uint32_t cause); #define XTENSA_OPTION_BIT(opt) (((uint64_t)1) << (opt)) +#define XTENSA_OPTION_ALL (~(uint64_t)0) static inline bool xtensa_option_bits_enabled(const XtensaConfig *config, uint64_t opt) diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h index 45205b8..0b47029 100644 --- a/target-xtensa/overlay_tool.h +++ b/target-xtensa/overlay_tool.h @@ -94,7 +94,9 @@ XCHAL_OPTION(XCHAL_HAVE_CACHEATTR, XTENSA_OPTION_CACHEATTR) | \ /* Other, TODO */ \ XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \ -XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG)) +XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG) |\ +XCHAL_OPTION(XCHAL_HAVE_THREADPTR, XTENSA_OPTION_THREAD_POINTER) | \ +XCHAL_OPTION(XCHAL_HAVE_PRID, XTENSA_OPTION_PROCESSOR_ID)) #ifndef XCHAL_WINDOW_OF4_VECOFS #define XCHAL_WINDOW_OF4_VECOFS 0x diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index c246fcb..5416aff 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -78,78 +78,102 @@ static TCGv_i32 cpu_UR[256]; #include "gen-icount.h" -static const char * const sregnames[256] = { -[LBEG] = "LBEG", -[LEND] = "LEND", -[LCOUNT] = "LCOUNT", -[SAR] = "SAR", -[BR] = "BR", -[LITBASE] = "LITBASE", -[SCOMPARE1] = "SCOMPARE1", -[ACCLO] = "ACCLO", -[ACCHI] = "ACCHI", -[MR] = "MR0", -[MR + 1] = "MR1", -[MR + 2] = "MR2", -[MR + 3] = "MR3", -[WINDOW_BASE] = "WINDOW_BASE", -[WINDOW_START] = "WINDOW_START", -[PTEVADDR] = "PTEVADDR", -[RASID] = "RASID", -[ITLBCFG] = "ITLBCFG", -[DTLBCFG] = "DTLBCFG", -[IBREAKENABLE] = "IBREAKENABLE", -[CACHEATTR] = "CACHEATTR", -[ATOMCTL] = "ATOMCTL", -[IBREAKA] = "IBREAKA0", -[IBREAKA + 1] = "IBREAKA1", -[DBREAKA] = "DBREAKA0", -[DBREAKA + 1] = "DBREAKA1", -[DBREAKC] = "DBREAKC0", -[DBREAKC + 1] = "DBREAKC1", -[EPC1] = "EPC1", -[EPC1 + 1] = "EPC2", -[EPC1 + 2] = "EPC3", -[EPC1 + 3] = "EPC4", -[EPC1 + 4] = "EPC5", -[EPC1 + 5] = "EPC6", -[EPC1 + 6] = "EPC7", -[DEPC] = "DEPC", -[EPS2] = "EPS2", -[EPS2 + 1] = "EPS3", -[EPS2 + 2] = "EPS4", -[EPS2 + 3] = "EPS5", -[EPS2 + 4] = "EPS6", -[EPS2 + 5] = "EPS7", -[EXCSAVE1] = "EXCSAVE1", -[EXCSAVE1 + 1] = "EXCSAVE2", -[EXCSAVE1 + 2] = "EXCSAVE3", -[EXCSAVE1 + 3] = "EXCSAVE4", -[EXCSAVE1 + 4] = "EXCSAVE5", -[EXCSAVE1 + 5] = "EXCSAVE6", -[EXCSAVE1 + 6] = "EXCSAVE7", -[CPENABLE] = "CPENABLE", -[INTSET] = "INTSET", -[INTCLEAR] = "INTCLEAR", -[INTENABLE] = "INTENABLE", -[PS] = "PS", -[VECBASE] = "VECBASE", -[EXCCAUSE] = "EXCCAUSE", -[DEBUGCAUSE] = "DEBUGCAUSE", -[CCOUNT] = "CCOUNT", -[PRID] = "PRID", -[ICOUNT] = "ICOUNT", -[ICOUNTLEVEL] = "ICOUNTLEVEL", -[EXCVADDR] = "EXCVADDR", -[CCOMPARE] = "CCOMPARE0", -[CCOMPARE + 1] = "CCOMPARE1", -[CCOMPARE + 2] = "CCOMPARE2", +typedef struct XtensaReg { +const char *name; +uint64_t opt_bits; +} XtensaReg; + +#define XTENSA_REG(regname, opt) { \ +.name = (regname), \ +.opt_bits = XTENSA_OPTION_BIT(opt), \ +} + +#define XTENSA_REG_BITS(regname, opt) { \ +.name = (regname), \ +.opt_bits = (opt), \ +} + +static const XtensaReg sregnames[256] = { +[LBEG] = XTENSA_REG("LBEG", XTENSA_OPTION_LOOP), +[LEND] = XTENSA_REG("LEND", XTENSA_OPTION_LOOP), +[LCOUNT] = XTENSA_REG("LCOUNT", XTENSA_OPTION_LOOP), +[SAR] = XTENSA_
Re: [Qemu-devel] [PATCH] target-i386: honor CR0_PG_MASK in cpu_get_phys_page_debug
On Wed, Dec 5, 2012 at 3:15 PM, Andreas Färber wrote: > Am 17.11.2012 21:52, schrieb Max Filippov: >> cpu_get_phys_page_debug is not in sync with cpu_x86_handle_mmu_fault: >> the latter first checks CR0_PG_MASK and only after CR4_PAE_MASK. >> >> This fixes odd gdb code display with PAE enabled. >> >> Signed-off-by: Max Filippov > > You write, they are "not in sync". Would it be possible to share code to > assure this, e.g., by calling a helper function from both? I'd say yes though that'd be a bigger change. I'll try to do it. -- Thanks. -- Max
[Qemu-devel] [PATCH] target-xtensa: fix ITLB/DTLB page protection flags
With MMU option xtensa architecture has two TLBs: ITLB and DTLB. ITLB is only used for code access, DTLB is only for data. However TLB entries in both TLBs have attribute field controlling write and exec access. These bits need to be properly masked off depending on TLB type before being used as tlb_set_page prot argument. Otherwise the following happens: (1) ITLB entry for some PFN gets invalidated (2) DTLB entry for the same PFN gets updated, attributes allow code execution (3) code at the page with that PFN is executed (possible due to step 2), entry for the TB is written into the jump cache (4) QEMU TLB entry for the PFN gets replaced with an entry for some other PFN (5) code in the TB from step 3 is executed (possible due to jump cache) and it accesses data, for which there's no DTLB entry, causing DTLB miss exception (6) re-translation of the TB from step 5 is attempted, but there's no QEMU TLB entry nor xtensa ITLB entry for that PFN, which causes ITLB miss exception at the TB start address (7) ITLB miss exception is handled by the guest, but execution is resumed from the beginning of the faulting TB (the point where ITLB miss occured), not from the point where DTLB miss occured, which is wrong. With that fix the above scenario causes ITLB miss exception (that used to be step 7) at step 3, right at the beginning of the TB. Signed-off-by: Max Filippov Cc: qemu-sta...@nongnu.org --- target-xtensa/helper.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index 200fb43..bf05575 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -522,7 +522,8 @@ static int get_physical_addr_mmu(CPUXtensaState *env, bool update_tlb, INST_FETCH_PRIVILEGE_CAUSE; } -*access = mmu_attr_to_access(entry->attr); +*access = mmu_attr_to_access(entry->attr) & +~(dtlb ? PAGE_EXEC : PAGE_READ | PAGE_WRITE); if (!is_access_granted(*access, is_write)) { return dtlb ? (is_write ? -- 1.7.7.6
[Qemu-devel] [PATCH] target-xtensa: fix search_pc for the last TB opcode
Zero out tcg_ctx.gen_opc_instr_start for instructions representing the last guest opcode in the TB. Cc: qemu-sta...@nongnu.org Signed-off-by: Max Filippov --- target-xtensa/translate.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 5d8762c..d109a08 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -3005,7 +3005,13 @@ static void gen_intermediate_code_internal( gen_icount_end(tb, insn_count); *tcg_ctx.gen_opc_ptr = INDEX_op_end; -if (!search_pc) { +if (search_pc) { +j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf; +lj++; +while (lj <= j) { +tcg_ctx.gen_opc_instr_start[lj++] = 0; +} +} else { tb->size = dc.pc - pc_start; tb->icount = insn_count; } -- 1.7.7.6
Re: [Qemu-devel] [PATCH] target-xtensa: fix search_pc for the last TB opcode
On Wed, Dec 19, 2012 at 11:45 PM, Blue Swirl wrote: > On Tue, Dec 18, 2012 at 7:21 AM, Max Filippov wrote: >> Zero out tcg_ctx.gen_opc_instr_start for instructions representing the >> last guest opcode in the TB. >> >> Cc: qemu-sta...@nongnu.org >> Signed-off-by: Max Filippov >> --- >> target-xtensa/translate.c |8 +++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c >> index 5d8762c..d109a08 100644 >> --- a/target-xtensa/translate.c >> +++ b/target-xtensa/translate.c >> @@ -3005,7 +3005,13 @@ static void gen_intermediate_code_internal( >> gen_icount_end(tb, insn_count); >> *tcg_ctx.gen_opc_ptr = INDEX_op_end; >> >> -if (!search_pc) { >> +if (search_pc) { >> +j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf; >> +lj++; >> +while (lj <= j) { >> +tcg_ctx.gen_opc_instr_start[lj++] = 0; >> +} > > Instead of the loop, how about something like > memset(&tcg_ctx.gen_opc_instr_start[lj], 0, j - lj)? Off by one, j - lj + 1. Wanted to keep it looking as other instances, but can do it this way too. >> +} else { >> tb->size = dc.pc - pc_start; >> tb->icount = insn_count; >> } >> -- >> 1.7.7.6 >> -- Thanks. -- Max
[Qemu-devel] [PATCH v2] target-xtensa: fix search_pc for the last TB opcode
Zero out tcg_ctx.gen_opc_instr_start for instructions representing the last guest opcode in the TB. Cc: qemu-sta...@nongnu.org Signed-off-by: Max Filippov --- Changes v1 -> v2: - replace while loop with memset target-xtensa/translate.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 5d8762c..2931d00 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -3005,7 +3005,11 @@ static void gen_intermediate_code_internal( gen_icount_end(tb, insn_count); *tcg_ctx.gen_opc_ptr = INDEX_op_end; -if (!search_pc) { +if (search_pc) { +j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf; +memset(tcg_ctx.gen_opc_instr_start + lj + 1, 0, +(j - lj) * sizeof(tcg_ctx.gen_opc_instr_start[0])); +} else { tb->size = dc.pc - pc_start; tb->icount = insn_count; } -- 1.7.7.6
Re: [Qemu-devel] [PATCH moxie 5/5] Add top level changes for moxie port
On Thu, Feb 14, 2013 at 1:56 PM, Peter Maydell wrote: On 13 February 2013 22:28, Anthony Green wrote: > Signed-off-by: Anthony Green [...] > There's something weird going on with whatever you're > using to create these patch emails. Can you redo your next > series in the standard '0/0 cover letter + each patch mail > as a followup to the cover letter' format, please? git There's also a lot of coding standard violations, can you run your patches through scripts/checkpatch.pl and fix its findings? -- Thanks. -- Max
[Qemu-devel] CPU scheduling with TCG in SMP system emulation mode
Hello. Do I understand it right that there's no dedicated mechanism other than icount that would switch current CPU in emulated SMP system and that in the absence of icount such scheduling is a side effect of interrupt delivery to the current CPU? -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v3 08/10] target-xtensa: implement FP0 conversions
On Thu, Sep 20, 2012 at 1:59 AM, Richard Henderson wrote: > On 09/18/2012 05:23 PM, Max Filippov wrote: >> +uint32_t HELPER(ftoi)(float32 v, uint32_t rounding_mode, uint32_t scale) >> +{ >> +float_status fp_status = {0}; >> + >> +set_float_rounding_mode(rounding_mode, &fp_status); >> +return float32_to_int32( >> +float32_scalbn(v, scale, &fp_status), &fp_status); >> +} >> + >> +uint32_t HELPER(ftoui)(float32 v, uint32_t rounding_mode, uint32_t scale) >> +{ >> +float_status fp_status = {0}; >> +float32 res; >> + >> +set_float_rounding_mode(rounding_mode, &fp_status); >> + >> +res = float32_scalbn(v, scale, &fp_status); >> + >> +if (float32_is_neg(v) && !float32_is_any_nan(v)) { >> +return float32_to_int32(res, &fp_status); >> +} else { >> +return float32_to_uint32(res, &fp_status); >> +} >> +} > > Are you really intending to discard any exceptions raised here? Yes, as specified in the ISA and as unit tests on Tensilica ISS show. -- Thanks. -- Max
Re: [Qemu-devel] Shifts, ppc[64], xtensa
On Wed, Sep 19, 2012 at 11:53 PM, Richard Henderson wrote: > On 09/19/2012 11:30 AM, Peter Maydell wrote: >> ...but on the other hand that ought to work for PPC too, so >> presumably my analysis is wrong somewhere. > > It isn't. It's a target-xtensa bug. > >> OP: >> 0xd0079cff >> movi_i32 tmp0,$0xd0079cff >> movi_i32 tmp1,$0x2 >> movi_i32 tmp2,$0x1 >> movi_i32 tmp3,$advance_ccount >> call tmp3,$0x0,$0,env,tmp2 >> movi_i32 tmp2,$window_check >> call tmp2,$0x0,$0,env,tmp0,tmp1 >> and_i32 tmp0,ar9,ar8 >> movi_i32 tmp1,$0x0 >> brcond_i32 tmp0,tmp1,eq,$0x0 >> movi_i32 tmp2,$0x0 >> brcond_i32 LCOUNT,tmp2,eq,$0x1 >> movi_i32 tmp2,$0x1 >> sub_i32 LCOUNT,LCOUNT,tmp2 >> movi_i32 tmp2,$0xd0079cf2 >> mov_i32 pc,tmp2 >> goto_tb $0x0 >> exit_tb $0x4a116558 >> set_label $0x1 >> movi_i32 tmp2,$0xd0079d02 >> mov_i32 pc,tmp2 >> exit_tb $0x0 >> set_label $0x0 >> movi_i32 tmp2,$0xd0079d1a >> mov_i32 pc,tmp2 >> goto_tb $0x1 >> exit_tb $0x4a116559 >> movi_i32 tmp0,$0x0 >> brcond_i32 LCOUNT,tmp0,eq,$0x2 >> movi_i32 tmp0,$0x1 >> sub_i32 LCOUNT,LCOUNT,tmp0 >> movi_i32 tmp0,$0xd0079cf2 >> mov_i32 pc,tmp0 >> goto_tb $0x0 >> exit_tb $0x4a116558 >> set_label $0x2 >> movi_i32 tmp0,$0xd0079d02 >> mov_i32 pc,tmp0 >> exit_tb $0x0 > > There are two instances of goto_tb $0 in here. > And, amusingly, two checks for LCOUNT. > > Since there's no disassembler for xtensa, I'll > leave it to the maintainer to track down from > whence this mistake stems. This code is generated when TB ends on LEND (zero-overhead loop ending) with branching instruction. So, in addition to 3 way branch there's extra looping code generated by unconditional gen_check_loop_end(dc, 0); at the end of disas_xtensa_insn. I was pretty sure that this dead code would make no harm. --enable-debug-tcg says nothing on x86_64 host. The following should fix that: -- >8 -- From: Max Filippov Date: Thu, 20 Sep 2012 04:07:20 +0400 Subject: [PATCH] target-xtensa: don't emit extra gen_check_loop_end Unconditional gen_check_loop_end at the end of disas_xtensa_insn can emit tcg_gen_goto_tb with slot id already used in the TB (e.g. when TB ends at LEND with a branch). Not all tcg backends can handle that. Signed-off-by: Max Filippov --- target-xtensa/translate.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 63b37b3..57a2b6f 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -2502,7 +2502,9 @@ static void disas_xtensa_insn(DisasContext *dc) break; } -gen_check_loop_end(dc, 0); +if (dc->is_jmp == DISAS_NEXT) { +gen_check_loop_end(dc, 0); +} dc->pc = dc->next_pc; return; -- 1.7.7.6
Re: [Qemu-devel] Shifts, ppc[64], xtensa
On Thu, Sep 20, 2012 at 6:03 PM, Richard Henderson wrote: > On 09/19/2012 05:29 PM, Max Filippov wrote: >> Not all tcg backends can handle that. > > *No* tcg backends can handle that. > > If we fix the bug wherein i386 clobbers the goto_tb target > during re-translation you'll see the crash there too. Ok, I will document it in the places that I know. > And we were just talking about enhancing --enable-debug-tcg > to detect this case... I can try to do it too as my homework (: -- Thanks. -- Max
[Qemu-devel] [PATCH 0/2] target-xtensa: fix extui and gen_check_loop_end
Max Filippov (2): target-xtensa: fix extui shift amount target-xtensa: don't emit extra tcg_gen_goto_tb target-xtensa/translate.c | 28 1 files changed, 24 insertions(+), 4 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH 1/2] target-xtensa: fix extui shift amount
extui opcode only uses lowermost op1 bit for sa4. Reported-by: malc Signed-off-by: Max Filippov Cc: qemu-stable --- target-xtensa/translate.c | 24 +--- 1 files changed, 21 insertions(+), 3 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 1900bd5..7a1c528 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -1778,12 +1778,30 @@ static void disas_xtensa_insn(DisasContext *dc) case 5: gen_window_check2(dc, RRR_R, RRR_T); { -int shiftimm = RRR_S | (OP1 << 4); +int shiftimm = RRR_S | ((OP1 & 1) << 4); int maskimm = (1 << (OP2 + 1)) - 1; TCGv_i32 tmp = tcg_temp_new_i32(); -tcg_gen_shri_i32(tmp, cpu_R[RRR_T], shiftimm); -tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); + +if (shiftimm) { +tcg_gen_shri_i32(tmp, cpu_R[RRR_T], shiftimm); +} else { +tcg_gen_mov_i32(tmp, cpu_R[RRR_T]); +} + +switch (maskimm) { +case 0xff: +tcg_gen_ext8u_i32(cpu_R[RRR_R], tmp); +break; + +case 0x: +tcg_gen_ext16u_i32(cpu_R[RRR_R], tmp); +break; + +default: +tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); +break; +} tcg_temp_free(tmp); } break; -- 1.7.7.6
[Qemu-devel] [PATCH 2/2] target-xtensa: don't emit extra tcg_gen_goto_tb
Unconditional gen_check_loop_end at the end of disas_xtensa_insn can emit tcg_gen_goto_tb with slot id already used in the TB (e.g. when TB ends at LEND with a branch). Signed-off-by: Max Filippov Cc: qemu-stable --- target-xtensa/translate.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 7a1c528..b6643eb 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -2520,7 +2520,9 @@ static void disas_xtensa_insn(DisasContext *dc) break; } -gen_check_loop_end(dc, 0); +if (dc->is_jmp == DISAS_NEXT) { +gen_check_loop_end(dc, 0); +} dc->pc = dc->next_pc; return; -- 1.7.7.6
[Qemu-devel] [PATCH 0/2] Add TCG sanity checks (goto_tb related)
Does this look sane or should it better be merged with e.g. tcg_dump_ops? Max Filippov (2): tcg/README: document tcg_gen_goto_tb restrictions tcg: add TB sanity checking tcg/README |3 +- tcg/tcg.c | 69 2 files changed, 71 insertions(+), 1 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH 2/2] tcg: add TB sanity checking
Do a sanity checking pass on the intermediate code. Check that goto_tb indices are either 0 or 1 and used at most once per TB. Signed-off-by: Max Filippov --- tcg/tcg.c | 69 + 1 files changed, 69 insertions(+), 0 deletions(-) diff --git a/tcg/tcg.c b/tcg/tcg.c index b8a1bec..cdd1975 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1454,6 +1454,71 @@ static void check_regs(TCGContext *s) } #endif +#ifdef CONFIG_DEBUG_TCG +static void tcg_sanity_check(TCGContext *s) +{ +const uint16_t *opc_ptr; +const TCGArg *args; +TCGArg arg; +TCGOpcode c; +int nb_oargs, nb_iargs, nb_cargs, error_count = 0; +const TCGOpDef *def; +unsigned goto_tb_slots[2] = {0}; + +opc_ptr = gen_opc_buf; +args = gen_opparam_buf; +while (opc_ptr < gen_opc_ptr) { +c = *opc_ptr++; +def = &tcg_op_defs[c]; +if (c == INDEX_op_call) { +TCGArg arg; + +/* variable number of arguments */ +arg = *args++; +nb_oargs = arg >> 16; +nb_iargs = arg & 0x; +nb_cargs = def->nb_cargs; +} else { +if (c == INDEX_op_nopn) { +/* variable number of arguments */ +nb_cargs = *args; +nb_oargs = 0; +nb_iargs = 0; +} else { +nb_oargs = def->nb_oargs; +nb_iargs = def->nb_iargs; +nb_cargs = def->nb_cargs; +} +} + +switch (c) { +case INDEX_op_goto_tb: +arg = args[0]; +if (arg != 0 && arg != 1) { +qemu_log("TB ERROR: wrong goto_tb slot index: %"TCG_PRIlx"\n", +arg); +++error_count; +} else { +++goto_tb_slots[arg]; +if (goto_tb_slots[arg] > 1) { +qemu_log("TB ERROR: multiple goto_tb(%"TCG_PRIlx")\n", arg); +++error_count; +} +} +break; + +default: +break; +} + +args += nb_iargs + nb_oargs + nb_cargs; +} +if (error_count) { +qemu_log("\n"); +} +} +#endif + static void temp_allocate_frame(TCGContext *s, int temp) { TCGTemp *ts; @@ -2082,6 +2147,10 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf, } #endif +#ifdef CONFIG_DEBUG_TCG +tcg_sanity_check(s); +#endif + tcg_reg_alloc_start(s); s->code_buf = gen_code_buf; -- 1.7.7.6
[Qemu-devel] [PATCH 1/2] tcg/README: document tcg_gen_goto_tb restrictions
See http://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg03196.html for the whole story. Signed-off-by: Max Filippov --- tcg/README |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tcg/README b/tcg/README index cfdfd96..86b43f1 100644 --- a/tcg/README +++ b/tcg/README @@ -386,7 +386,8 @@ Exit the current TB and return the value t0 (word type). Exit the current TB and jump to the TB index 'index' (constant) if the current TB was linked to this TB. Otherwise execute the next -instructions. +instructions. Only indices 0 and 1 are valid and tcg_gen_goto_tb may be issued +at most once with each slot index per TB. * qemu_ld8u t0, t1, flags qemu_ld8s t0, t1, flags -- 1.7.7.6
Re: [Qemu-devel] [PATCH 8/8] tcg: Sanity check goto_tb input
On 09/22/2012 04:18 AM, Richard Henderson wrote: > Checking that we don't try for idx != [01] is trivial. Checking > that we don't issue more than one of any index requires a tad > more data and some ifdefs protecting that new variable. > > Signed-off-by: Richard Henderson > Cc: Max Filippov > --- > tcg/tcg-op.h | 11 +-- > tcg/tcg.c| 4 > tcg/tcg.h| 1 + > 3 files changed, 14 insertions(+), 2 deletions(-) Cool. Tested-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] [PATCH] target-xtensa: de-optimize EXTUI
On Wed, Sep 26, 2012 at 2:57 AM, Aurelien Jarno wrote: > Now that and with 0xff, 0x and 0x is optimized in > tcg/tcg-op.h, there is no need to do it in target-xtensa/translate.c. > > Cc: Max Filippov > Signed-off-by: Aurelien Jarno > --- > target-xtensa/translate.c | 15 +-- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c > index ba3ffcb..c1358ee 100644 > --- a/target-xtensa/translate.c > +++ b/target-xtensa/translate.c > @@ -1835,20 +1835,7 @@ static void disas_xtensa_insn(DisasContext *dc) > } else { > tcg_gen_mov_i32(tmp, cpu_R[RRR_T]); > } I guess shri above may be de-optimized as well. In any case Acked-by: Max Filippov > - > -switch (maskimm) { > -case 0xff: > -tcg_gen_ext8u_i32(cpu_R[RRR_R], tmp); > -break; > - > -case 0x: > -tcg_gen_ext16u_i32(cpu_R[RRR_R], tmp); > -break; > - > -default: > -tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); > -break; > -} > +tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); > tcg_temp_free(tmp); > } > break; > -- > 1.7.10.4 > > -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v2] target-xtensa: de-optimize EXTUI
On Mon, Oct 1, 2012 at 10:54 PM, Aurelien Jarno wrote: > Now that "and" with 0xff, 0x and 0x and "shr" with 0 shift > are optimized in tcg/tcg-op.h there is no need to do it in > target-xtensa/translate.c. > > Cc: Max Filippov > Signed-off-by: Aurelien Jarno > --- > target-xtensa/translate.c | 22 ++ > 1 file changed, 2 insertions(+), 20 deletions(-) > > v1 -> v2: also remove the test on shiftimm to select either a shift or > a move. Acked-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
On Thu, Oct 4, 2012 at 2:36 PM, Avi Kivity wrote: > The hassle and compile time overhead of maintaining both 32-bit and 64-bit > capable source isn't worth the tiny performance advantage which is seen on > a minority of configurations. Switch to compiling libhw only once, with > target_phys_addr_t unconditionally typedefed to uint64_t. > > Signed-off-by: Avi Kivity > --- > > v2: no changes, but copied the maintainers of architectures that will > see their target_phys_addr_t changed as a result. Please view and/or > test. Xtensa on x86_64 host: OK. -- Thanks. -- Max
Re: [Qemu-devel] [QEMU PATCH] create struct for machine initialization arguments (v2)
On Sat, Oct 6, 2012 at 12:22 AM, Eduardo Habkost wrote: > This should help us to: > - More easily add or remove machine initialization arguments without > having to change every single machine init function; > - More easily make mechanical changes involving the machine init > functions in the future; > - Let machine initialization forward the init arguments to other > functions more easily. > > This change was half-mechanical process: first the struct was added with > the local ram_size, boot_device, kernel_*, initrd_*, and cpu_model local > variable initialization to all functions. Then the compiler helped me > locate the local variables that are unused, so they could be removed. > > Changes v1 -> v2: > - Fix mistake on the conversion of pc_xen_hvm_init() and xen_init_pv() > > Signed-off-by: Eduardo Habkost [...] > diff --git a/vl.c b/vl.c > index 8d305ca..f663e7c 100644 > --- a/vl.c > +++ b/vl.c > @@ -3624,8 +3624,13 @@ int main(int argc, char **argv, char **envp) > > qdev_machine_init(); > > -machine->init(ram_size, boot_devices, > - kernel_filename, kernel_cmdline, initrd_filename, > cpu_model); > +QEMUMachineInitArgs args = { .ram_size = ram_size, > + .boot_device = boot_devices, > + .kernel_filename = kernel_filename, > + .kernel_cmdline = kernel_cmdline, > + initrd_filename = initrd_filename, Missing dot? > + .cpu_model = cpu_model }; > +machine->init(&args); > > cpu_synchronize_all_post_init(); -- Thanks. -- Max
Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls
On Wed, Aug 22, 2012 at 10:03 PM, Max Filippov wrote: > This prevents guest from proceeding with uninitialised garbage returned > from unimplemented simcalls. > > Signed-off-by: Max Filippov > --- > target-xtensa/xtensa-semi.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) ping? -- Thanks. -- Max