[Qemu-devel] [Bug 1185311] Re: could not hot-remove disabled NIC from Win2012 guest by 'devel_del id1'

2018-03-24 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1185311

Title:
  could not hot-remove disabled NIC from Win2012 guest by 'devel_del
  id1'

Status in QEMU:
  Expired

Bug description:
  # qemu-latest-upstream -mon chardev=qmp,mode=control,pretty=on \
   -chardev socket,id=qmp,host=localhost,port=1234,server,nowait -vnc :0 \
   -monitor stdio /images/win2012-64-virtio.qcow2 \
   -device virtio-net-pci,netdev=ndev1,id=id1 -netdev tap,id=ndev1 \
   -device e1000,netdev=ndev2,id=id2 -netdev tap,id=ndev2 \
   -device rtl8139,netdev=ndev3,id=id3 -netdev tap,id=ndev3 \
   -smp 4 -m 3000 -usbdevice tablet

  If disable nic in guest's "Network Connections" panel, nic could not
  be hot-removed through qemu monitor.

  1) if disable nic in guest
  (qemu) devel_del id1 (nic still in "Network Connections". if enable nic, nic 
can work)
  (qemu) devel_del id1
  (qemu) devel_del id1

  2) if enable nic in guest
  (qemu) devel_del id1 (nic will be removed, disappear from "Network 
Connections")
  (qemu) devel_del id1
  Device 'id1' not found

  Could not reproduced this problem with all linux guests & other Windows guests
  Problem exists with virtio-nic/e1000/rtl8139, it seems the problem of 
pci-hotplug in piix4.

  Could not reproduce this problem with Vmware + win2012 guest.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1185311/+subscriptions



[Qemu-devel] [Bug 1176366] Re: TCPIP not working on qemu 1.4.50 (master)

2018-03-24 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1176366

Title:
  TCPIP not working on qemu 1.4.50 (master)

Status in QEMU:
  Expired

Bug description:
  whenever I try, in the guest OS, in this case it's NT 3.1, to enable
  TCP/IP, it crashes the whole emulator. With either the ne2000 isa,
  ne2000 pci or PCnet, still crashes

  below is attached a screenshot.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1176366/+subscriptions



Re: [Qemu-devel] [PATCH] i386/kvm: add support for KVM_CAP_X86_DISABLE_EXITS

2018-03-24 Thread Wanpeng Li
2018-03-24 4:18 GMT+08:00 Eduardo Habkost :
> On Fri, Mar 16, 2018 at 07:36:42AM -0700, Wanpeng Li wrote:
>> From: Wanpeng Li 
>>
>> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace 
>> with
>> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> in order that to improve latency in some workloads.
>>
>> Cc: Paolo Bonzini 
>> Cc: Radim Krčmář 
>> Cc: Eduardo Habkost 
>> Signed-off-by: Wanpeng Li 
>
>
> Thanks.
>
> Patch looks good (except for comment below), but I would like to
> see QEMU documentation mentioning what exactly are the practical
> consequences of setting "+kvm-hint-dedicated" (especially what
> could happen if people enable the flag without properly
> configuring vCPU pinning).
>
>
> [...]
>> +if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> +int disable_exits = kvm_check_extension(cs->kvm_state, 
>> KVM_CAP_X86_DISABLE_EXITS);
>> +if (disable_exits) {
>> +disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> +  KVM_X86_DISABLE_EXITS_HLT |
>> +  KVM_X86_DISABLE_EXITS_PAUSE);
>> +}
>
> Documentation/virtual/kvm/api.txt says that KVM_FEATURE_PV_UNHALT
> shouldn't be enabled if disabling HLT exits.  This needs to be
> handled by QEMU.

This is handled by KVM(in kvm_update_cpuid()) currently to avoid kvm
userspace doing something crazy.
https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=caa057a2cad647fb368a12c8e6c410ac4c28e063

>
> Probably the simplest solution is to not allow kvm-hint-dedicated
> to be enabled if kvm-pv-unhalt is.  This should be mentioned in
> QEMU documentation, also, especially considering that we might
> enable kvm-pv-unhalt by default in future QEMU versions.

As Locking guy Waiman mentioned before:
> Generally speaking, unfair lock performs well for VMs with a small number of 
> vCPUs. Native qspinlock may perform better than pvqspinlock if there is vCPU 
> pinning and there is no vCPU over-commitment.
I think +kvm-hint-dedicated, -kvm-pv-unhalt is more suitable for vCPU
pinning and there is no vCPU over-commitment, on the contrary,
-kvm-hint-dedicated, +kvm-pv-unhalt is more prefer.

Regards,
Wanpeng Li



Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 2:23 PM, Peter Maydell 
wrote:

> On 24 March 2018 at 18:13, Michael Clark  wrote:
> > The sifive_u machine already marks its ROM readonly. This fixes
> > the remaining boards.
> >
> > Cc: Sagar Karandikar 
> > Cc: Bastian Koppelmann 
> > Signed-off-by: Michael Clark 
> > Signed-off-by: Palmer Dabbelt 
> > ---
> >  hw/riscv/sifive_u.c  |  9 +
> >  hw/riscv/spike.c | 18 ++
> >  hw/riscv/virt.c  |  7 ---
> >  include/hw/riscv/spike.h |  8 
> >  4 files changed, 19 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 6116c38..25df16c 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState
> *machine)
> >  SiFiveUState *s = g_new0(SiFiveUState, 1);
> >  MemoryRegion *sys_memory = get_system_memory();
> >  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> > +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> >
> >  /* Initialize SOC */
> >  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> > @@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState
> *machine)
> >  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
> >
> >  /* boot rom */
> > -memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
> > +memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
> > memmap[SIFIVE_U_MROM].base, &error_fatal);
> > -memory_region_set_readonly(boot_rom, true);
> > -memory_region_add_subregion(sys_memory, 0x0, boot_rom);
> > +memory_region_set_readonly(mask_rom, true);
> > +memory_region_add_subregion(sys_memory, 0x0, mask_rom);
>
> memory_region_init_ram + memory_region_set_readonly is
> equivalent to memory_region_init_rom.
>
> >  if (machine->kernel_filename) {
> >  load_kernel(machine->kernel_filename);
> > @@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState
> *machine)
> >  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
> >  cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
> >  sizeof(reset_vec), s->fdt, s->fdt_size);
> > +memory_region_set_readonly(mask_rom, true);
>
> Rather than doing this, you should use
> rom_add_blob_fixed(). That works even on ROMs which
> means you can just create them as read-only from the
> start rather than waiting til you've written to them
> and then marking them read-only. It also means that
> you get the contents correctly reset on reset, even
> if the user has been messing with their contents
> via the debugger or something.
>
> hw/arm/boot.c has code which (among a lot of other
> things) loads initial kernels and dtb images into
> guest memory. You can also find ppc code doing
> similar things.
>

I don't mind to make this change, however it is a case of good vs perfect.
Currently we have some machines with writable ROM sections, this change
fixes it and has been tested.

I can address this early next week. Change in itself introduces risk. The
current set of changes against the series have been pretty well tested and
the most recent changes have been very small.

In any case I'll try to address this in spin 7 early next week as I noticed
some issues myself after switching from writing and submitting patches mode
to reviewing my own patches mode, so we're going to need to do a respin.
Below is the v7 changelog. Only one of the changes is logic and it is a
very tiny change.

v7

* fix typo in mstatus.FS workaround comment
* remove privilege mode from mstatus.mxr page protection check
* shift class initialization boilerplate patch hunk to correct patch


Re: [Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 2:25 PM, Peter Maydell 
wrote:

> On 24 March 2018 at 18:13, Michael Clark  wrote:
> > Remove a potential buffer overflow (not seen in practice).
> > Perhaps cpu_physical_memory_write already has bound checks.
>
> cpu_physical_memory_write() writes to the guest address
> space, so it won't overflow. If you ask it to write
> off the end of a ROM then it will correctly write into
> an unassigned part of the guest memory space (which does
> nothing) or into whatever device or other ram is there.
> You probably don't want to do that, but it is not a buffer
> overflow.
>

I assumed that was the case but it is still probably good discipline to
have the bounds check.

We have also expanded the ROM regions to account for the default FDT size
which was larger than the previous ROM region sizes. I discovered this
while debugging another issue, where I had a debug statement to print the
fdt_size and noticed it was larger than the ROM region reserved for it.

It's belts and braces change. I'd prefer we at least make sure our ROM
regions are large enough for the default FDT size. It could be overflowed
on the virt board eventually if we enable many CPUs and add more devices.
The error message is a nice to have, as we'll know if the FDT size is too
large rather than have a subtle failure due to the boot loader parsing
truncated device tree.

This problem is not seen in practice... yet... but I still think it is
worth fixing.


Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

2018-03-24 Thread no-reply
Hi,

This series failed docker-build@min-glib build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180324192455.12254-1-mdavidsa...@gmail.com
Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cd5d44142f tests: drop ds1338-test
c568d03629 timer: ds-rtc model ds1375
360e76e958 timer: ds-rtc handle CENTURY bit
8e600ad679 timer: generalize ds1338
43f1db5002 timer: rename file ds1338.c -> ds-rtc.c
5ec505be2e timer: rename ds1338 -> dsrtc
628ed92468 tests: ds-rtc test wday offset
2582baf810 timer: ds1338 fix wday_offset handling
793b89687b tests: ds-rtc test 12 hour mode
3f9b2eb07b timer: ds1338 change write handling
45c4df6895 timer: ds1338 clarify HOUR handling
bb5d6cd534 timer: ds1338 persist 12-hour mode selection
8291a6b22d timer: ds1338 use registerfields.h
c785175358 tests: more thorough tests of ds1338

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-8pwvp3st/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
  GEN 
/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: 
/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723/qemu.tar:
 Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPYRUNNER
RUN test-build in qemu:min-glib 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Environment variables:
HOSTNAME=2ad891f2b17f
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install

ERROR: DTC (libfdt) version >= 1.4.2 not present.
   Please install the DTC (libfdt) devel package

Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in 
sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 
'com.qemu.instance.uuid=c219b4d82faf11e8b99852540069c830', '-u', '0', 
'--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 
'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 
'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-8pwvp3st/src/docker-src.2018-03-24-18.07.16.7723:/var/tmp/qemu:z,ro',
 'qemu:min-glib', '/var/tmp/qemu/run', 'test-build']' returned non-zero exit 
status 1
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-8pwvp3st/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-build@min-glib] 
Error 2

real0m54.355s
user0m8.995s
sys 0m6.886s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patc

Re: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

2018-03-24 Thread no-reply
Hi,

This series failed docker-quick@centos6 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180324192455.12254-1-mdavidsa...@gmail.com
Subject: [Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20180323202344.70640-1-dgilb...@redhat.com 
-> patchew/20180323202344.70640-1-dgilb...@redhat.com
Switched to a new branch 'test'
cd5d44142f tests: drop ds1338-test
c568d03629 timer: ds-rtc model ds1375
360e76e958 timer: ds-rtc handle CENTURY bit
8e600ad679 timer: generalize ds1338
43f1db5002 timer: rename file ds1338.c -> ds-rtc.c
5ec505be2e timer: rename ds1338 -> dsrtc
628ed92468 tests: ds-rtc test wday offset
2582baf810 timer: ds1338 fix wday_offset handling
793b89687b tests: ds-rtc test 12 hour mode
3f9b2eb07b timer: ds1338 change write handling
45c4df6895 timer: ds1338 clarify HOUR handling
bb5d6cd534 timer: ds1338 persist 12-hour mode selection
8291a6b22d timer: ds1338 use registerfields.h
c785175358 tests: more thorough tests of ds1338

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-9orj1if7/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-9orj1if7/src'
  GEN 
/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: 
/var/tmp/patchew-tester-tmp-9orj1if7/src/docker-src.2018-03-24-18.04.07.5505/qemu.tar:
 Wrote only 2048 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPYRUNNER
RUN test-quick in qemu:centos6 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc gettext git glib2-devel libepoxy-devel libfdt-devel
 librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=53429828786d
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install

ERROR: DTC (libfdt) version >= 1.4.2 not present.
   Please install the DTC (libfdt) devel package

Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in 
sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
ret

[Qemu-devel] [PATCH v6 21/26] RISC-V: Remove support for adhoc X_COP interrupt

2018-03-24 Thread Michael Clark
This is essentially dead-code elimination. Support for more
local interrupts will be added in a future revision, as they
will be defined in a future version of the Privileged ISA
specification.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/cpu_bits.h  | 1 -
 target/riscv/op_helper.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 12b4757..133e070 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -346,7 +346,6 @@
 #define IRQ_S_EXT   9
 #define IRQ_H_EXT   10 /* until: priv-1.9.1 */
 #define IRQ_M_EXT   11 /* until: priv-1.9.1 */
-#define IRQ_X_COP   12 /* non-standard */
 
 /* Default addresses */
 #define DEFAULT_RSTVEC 0x1000
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index ba3639d..1fdde90 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -90,7 +90,7 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 target_ulong csrno)
 {
 #ifndef CONFIG_USER_ONLY
-uint64_t delegable_ints = MIP_SSIP | MIP_STIP | MIP_SEIP | (1 << 
IRQ_X_COP);
+uint64_t delegable_ints = MIP_SSIP | MIP_STIP | MIP_SEIP;
 uint64_t all_ints = delegable_ints | MIP_MSIP | MIP_MTIP;
 #endif
 
-- 
2.7.0




Re: [Qemu-devel] [PULL 0/5] migration queue

2018-03-24 Thread Peter Maydell
On 23 March 2018 at 20:23, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> The following changes since commit 4c2c1015905fa1d616750dfe024b4c0b35875950:
>
>   Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20180323' into 
> staging (2018-03-23 10:20:54 +)
>
> are available in the Git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20180323a
>
> for you to fetch changes up to 09576e74dbe697c5f0a7bc2ad7b59601457a2ff4:
>
>   migration: Fix block migration flag case (2018-03-23 18:24:11 +)
>
> 
> Migration fixes for 2.12
>
> All small fixes.  Dan's is a missing piece
> of a cleanup that finally completes something,
> and between Paolo, Dan and myself we recon it's
> still on the edge of being a bug fix.
>
> 

Applied, thanks.

-- PMM



[Qemu-devel] [PATCH 11/14] timer: generalize ds1338

2018-03-24 Thread Michael Davidsaver
Add class level handling for address space size
and control register.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds-rtc.c | 63 ---
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 28f788dd8e..2df1bce3f8 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -21,10 +21,10 @@
  */
 #define NVRAM_SIZE 64
 
-#define CTRL_OSF   0x20
-
-#define TYPE_DSRTC "ds1338"
+#define TYPE_DSRTC "dsrtc"
 #define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
+#define DSRTC_CLASS(klass) OBJECT_CLASS_CHECK(DSRTCClass, (klass), TYPE_DSRTC)
+#define DSRTC_GET_CLASS(obj) OBJECT_GET_CLASS(DSRTCClass, (obj), TYPE_DSRTC)
 
 /* values stored in BCD */
 /* 00-59 */
@@ -40,7 +40,7 @@
 /* 0-99 */
 #define R_YEAR  (0x6)
 
-#define R_CTRL  (0x7)
+#define R_DS1338_CTRL (0x7)
 
 /* use 12 hour mode when set */
 FIELD(HOUR, SET12, 6, 1)
@@ -67,6 +67,15 @@ typedef struct DSRTCState {
 bool addr_byte;
 } DSRTCState;
 
+typedef struct DSRTCClass {
+I2CSlaveClass parent_obj;
+
+/* actual address space size must be <= NVRAM_SIZE */
+unsigned addr_size;
+unsigned ctrl_offset;
+void (*ctrl_write)(DSRTCState *s, uint8_t);
+} DSRTCClass;
+
 static const VMStateDescription vmstate_dsrtc = {
 .name = "ds1338",
 .version_id = 2,
@@ -119,11 +128,12 @@ static void capture_current_time(DSRTCState *s)
 
 static void inc_regptr(DSRTCState *s)
 {
-/* The register pointer wraps around after 0x3F; wraparound
+DSRTCClass *k = DSRTC_GET_CLASS(s);
+/* The register pointer wraps around after k->addr_size-1; wraparound
  * causes the current time/date to be retransferred into
  * the secondary registers.
  */
-s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
+s->ptr = (s->ptr + 1) % k->addr_size;
 if (!s->ptr) {
 capture_current_time(s);
 }
@@ -206,22 +216,15 @@ static void dsrtc_update(DSRTCState *s)
 static int dsrtc_send(I2CSlave *i2c, uint8_t data)
 {
 DSRTCState *s = DSRTC(i2c);
+DSRTCClass *k = DSRTC_GET_CLASS(s);
 
 if (s->addr_byte) {
-s->ptr = data & (NVRAM_SIZE - 1);
+s->ptr = data % k->addr_size;
 s->addr_byte = false;
 return 0;
 }
-if (s->ptr == R_CTRL) {
-/* Control register. */
-
-/* Ensure bits 2, 3 and 6 will read back as zero. */
-data &= 0xB3;
-
-/* Attempting to write the OSF flag to logic 1 leaves the
-   value unchanged. */
-data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF);
-
+if (s->ptr == k->ctrl_offset) {
+(k->ctrl_write)(s, data);
 }
 s->nvram[s->ptr] = data;
 if (s->ptr <= R_YEAR) {
@@ -256,15 +259,41 @@ static void dsrtc_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo dsrtc_info = {
+.abstract  = true,
 .name  = TYPE_DSRTC,
 .parent= TYPE_I2C_SLAVE,
 .instance_size = sizeof(DSRTCState),
 .class_init= dsrtc_class_init,
 };
 
+static void ds1338_control_write(DSRTCState *s, uint8_t data)
+{
+/* Control register. */
+
+/* allow guest to set no-op controls for clock out pin */
+s->nvram[R_DS1338_CTRL] = data & 0x93;
+}
+
+static void ds1338_class_init(ObjectClass *klass, void *data)
+{
+DSRTCClass *k = DSRTC_CLASS(klass);
+
+k->addr_size = 0x40;
+k->ctrl_offset = R_DS1338_CTRL;
+k->ctrl_write = ds1338_control_write;
+}
+
+static const TypeInfo ds1338_info = {
+.name  = "ds1338",
+.parent= TYPE_DSRTC,
+.class_size= sizeof(DSRTCClass),
+.class_init= ds1338_class_init,
+};
+
 static void dsrtc_register_types(void)
 {
 type_register_static(&dsrtc_info);
+type_register_static(&ds1338_info);
 }
 
 type_init(dsrtc_register_types)
-- 
2.11.0




Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code

2018-03-24 Thread Peter Maydell
On 24 March 2018 at 18:13, Michael Clark  wrote:
> The sifive_u machine already marks its ROM readonly. This fixes
> the remaining boards.
>
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Signed-off-by: Michael Clark 
> Signed-off-by: Palmer Dabbelt 
> ---
>  hw/riscv/sifive_u.c  |  9 +
>  hw/riscv/spike.c | 18 ++
>  hw/riscv/virt.c  |  7 ---
>  include/hw/riscv/spike.h |  8 
>  4 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 6116c38..25df16c 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState *machine)
>  SiFiveUState *s = g_new0(SiFiveUState, 1);
>  MemoryRegion *sys_memory = get_system_memory();
>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>
>  /* Initialize SOC */
>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState *machine)
>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>
>  /* boot rom */
> -memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
> +memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
> memmap[SIFIVE_U_MROM].base, &error_fatal);
> -memory_region_set_readonly(boot_rom, true);
> -memory_region_add_subregion(sys_memory, 0x0, boot_rom);
> +memory_region_set_readonly(mask_rom, true);
> +memory_region_add_subregion(sys_memory, 0x0, mask_rom);

memory_region_init_ram + memory_region_set_readonly is
equivalent to memory_region_init_rom.

>  if (machine->kernel_filename) {
>  load_kernel(machine->kernel_filename);
> @@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState *machine)
>  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
>  cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
>  sizeof(reset_vec), s->fdt, s->fdt_size);
> +memory_region_set_readonly(mask_rom, true);

Rather than doing this, you should use
rom_add_blob_fixed(). That works even on ROMs which
means you can just create them as read-only from the
start rather than waiting til you've written to them
and then marking them read-only. It also means that
you get the contents correctly reset on reset, even
if the user has been messing with their contents
via the debugger or something.

hw/arm/boot.c has code which (among a lot of other
things) loads initial kernels and dtb images into
guest memory. You can also find ppc code doing
similar things.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt

2018-03-24 Thread Peter Maydell
On 24 March 2018 at 18:13, Michael Clark  wrote:
> Remove a potential buffer overflow (not seen in practice).
> Perhaps cpu_physical_memory_write already has bound checks.

cpu_physical_memory_write() writes to the guest address
space, so it won't overflow. If you ask it to write
off the end of a ROM then it will correctly write into
an unassigned part of the guest memory space (which does
nothing) or into whatever device or other ram is there.
You probably don't want to do that, but it is not a buffer
overflow.

thanks
-- PMM



[Qemu-devel] [PATCH v1] RISC-V: RISC-V TCG backend work in progress

2018-03-24 Thread Michael Clark
This patch adds an experimental RISC-V TCG backend.

We have been dogfooding the RISC-V QEMU front-end with Fedora
to develop a RISC-V TCG backend. The RISC-V TCG backend can
be built inside of the QEMU RISC-V 'virt' machine using
the Fedora stage 4 disk image:

- https://fedoraproject.org/wiki/Architectures/RISC-V

Below are brief instructions on building riscv64-linux-user
and x86_64-linux-user QEMU inside a Fedora RISC-V environment
using either QEMU RISC-V or SiFive's HiFive Unleashed board:

```
sudo dnf install git python flex bison \
zlib-devel glib2-devel pixman-devel
git clone --recursive https://github.com/michaeljclark/riscv-qemu.git
cd riscv-qemu
git checkout wip-riscv-tcg-backend
./configure \
--prefix=/opt/riscv/qemu \
--disable-capstone \
--target-list=riscv64-linux-user,x86_64-linux-user
make -j$(nproc)
```

Testing

There is a user-mode version of riscv-tests that can
be used for testing RISC-V QEMU linux-user.

- https://github.com/arsv/riscv-qemu-tests

These tests can also be used to test the RISC-V TCG
back-end via the RISC-V front-end. e.g.

```
for ext in i m a f d; do
for i in $(find rv64${ext} -type f -a -executable); do
echo $i
../riscv-qemu/riscv64-linux-user/qemu-riscv64 $i
done
done
```

At present the Base ISA tests pass except for mulhu and mulhsu
when compiled using the riscv newlib toolchain. When running
with --singlestep, all tests pass. Note: TCG performs constant
folding so in some cases the tests can be eliminated in the
TCG optimizer because constants are constructed as immediate
operands instead of being loaded from the data section.

```
$ sh run-tests.sh
rv64i/sub
rv64i/srai
rv64i/slti
rv64i/sltu
rv64i/lwu
rv64i/ori
rv64i/addw
rv64i/lw
rv64i/subw
rv64i/xori
rv64i/jal
rv64i/sb
rv64i/blt
rv64i/slt
rv64i/bgeu
rv64i/bne
rv64i/add
rv64i/and
rv64i/lui
rv64i/sll
rv64i/slliw
rv64i/aiupc
rv64i/bge
rv64i/sltiu
rv64i/jalr
rv64i/srli
rv64i/beq
rv64i/lb
rv64i/sw
rv64i/sra
rv64i/lhu
rv64i/andi
rv64i/addi
rv64i/sraiw
rv64i/srliw
rv64i/srlw
rv64i/xor
rv64i/sllw
rv64i/slli
rv64i/or
rv64i/lbu
rv64i/bltu
rv64i/srl
rv64i/ld
rv64i/sd
rv64i/sraw
rv64m/mulhu
FAIL
rv64m/divuw
rv64m/mulhsu
FAIL
rv64m/mulh
rv64m/divw
rv64m/divu
rv64m/remw
rv64m/remu
rv64m/rem
rv64m/remuw
rv64m/mul
rv64m/div
rv64m/mulw
rv64a/amoswap_w
rv64a/amoor_w
rv64a/amoadd_d
rv64a/amoand_w
rv64a/amomax_w
rv64a/amoor_d
rv64a/amominu_d
rv64a/lrsc_d
rv64a/amomin_w
rv64a/zero
rv64a/amomaxu_d
rv64a/amoxor_w
rv64a/amoxor_d
rv64a/amomaxu_w
rv64a/amoadd_w
rv64a/amominu_w
rv64a/amoand_d
rv64a/amomin_d
rv64a/amoswap_d
rv64a/amomax_d
rv64a/lrsc_w
rv64f/movex
rv64f/ldst
rv64f/fsgnj
rv64f/fadd
rv64f/fcvt
rv64f/move
rv64f/recoding
rv64f/fdiv
rv64f/fcvt_w
rv64f/fmin
rv64f/fclass
rv64f/fcmp
rv64f/fmadd
rv64d/ldst
rv64d/fsgnj
rv64d/fadd
rv64d/fcvt
rv64d/move
rv64d/recoding
rv64d/fdiv
rv64d/fcvt_w
rv64d/fmin
rv64d/fclass
rv64d/fmadd
```

Many of the rv8-bench tests compiled for riscv64 and x86_64
will run (using musl-libc via the musl-riscv-toolchain):

- https://github.com/rv8-io/musl-riscv-toolchain/
- https://github.com/rv8-io/rv8-bench/
- https://rv8.io/bench

Running with `-d in_asm,op,op_opt,out_asm` is very helpful
for debugging. Note: due to a limitation in QEMU, the backend
disassembler is not compiled, unless the backend matches
the front-end, so `scripts/disas-objdump.pl` is required
to decode the emmitted RISC-V assembly when using the x86_64
front-end. When using the RISC-V front-end, the back-end
disassembly can be seen without any special decoding, so
the RISC-V front-end is a little easier for debugging.

Caveats:

- 64-bit on 32-bit hosts is not yet supported
  (tcg_out_brcond2 and tcg_out_setcond2 are not implemented)
- softmmu is not yet supported
  (tcg_out_tlb_load, tcg_out_qemu_ld_slow_path and
  tcg_out_qemu_st_slow_path are not implemented)
- big endian is not yet supported
  (tcg_out_qemu_ld_direct and tcg_out_qemu_st_direct
  do not support MO_BSWAP)
- subtle bugs still exist. i.e. glibc dynamic executables
  do not run but many static musl libc exectables do run.
---
 accel/tcg/user-exec.c |   12 +
 configure |   10 +-
 disas.c   |   10 +-
 include/elf.h |   55 ++
 include/exec/poison.h |1 +
 linux-user/host/riscv32/hostdep.h |   15 +
 linux-user/host/riscv64/hostdep.h |   15 +
 tcg/riscv/tcg-target.h|  170 +
 tcg/riscv/tcg-target.inc.c| 1466 +
 9 files changed, 1751 insertions(+), 3 deletions(-)
 c

[Qemu-devel] [PATCH 03/14] timer: ds1338 persist 12-hour mode selection

2018-03-24 Thread Michael Davidsaver
Need to save HOUR[HOUR12] bit to keep
track of guest selection of 12-hour mode.
Write through current time registers to
achieve this.  Will be overwritten
by the next read/latch.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index b5630e214a..72a4692d60 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -224,10 +224,8 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
value unchanged. */
 data = (data & ~CTRL_OSF) | (data & s->nvram[s->ptr] & CTRL_OSF);
 
-s->nvram[s->ptr] = data;
-} else {
-s->nvram[s->ptr] = data;
 }
+s->nvram[s->ptr] = data;
 inc_regptr(s);
 return 0;
 }
-- 
2.11.0




[Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug

2018-03-24 Thread Michael Clark
This change is a workaround for a bug where mstatus.FS
is not correctly reporting dirty when MTTCG and SMP are
enabled which results in the floating point register file
not being saved during context switches. This a critical
bug for RISC-V in QEMU as it results in floating point
register file corruption when running SMP Linux in the
RISC-V 'virt' machine.

This workaround will return dirty if mstatus.FS is
switched from off to initial or clean. We have checked
the specification and it is legal for an implementation
to return either off, or dirty, if set to initial or clean.

This workaround will result in unnecessary floating point
save restore. When mstatus.FS is off, floating point
instruction trap to indicate the process is using the FPU.
The OS can then save floating-point state of the previous
process using the FPU and set mstatus.FS to initial or
clean. With this workaround, mstatus.FS will always return
dirty if set to a non-zero value, indicating floating point
save restore is necessary, versus misreporting mstatus.FS
resulting in floating point register file corruption.

Cc: Palmer Dabbelt 
Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Cc: Richard W.M. Jones 
Cc: Peter Maydell 
Signed-off-by: Michael Clark 
---
 target/riscv/op_helper.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 1fdde90..d345688 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -144,8 +144,23 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 }
 
 mstatus = (mstatus & ~mask) | (val_to_write & mask);
-int dirty = (mstatus & MSTATUS_FS) == MSTATUS_FS;
-dirty |= (mstatus & MSTATUS_XS) == MSTATUS_XS;
+
+/* Note: this is a workaround for an issue where mstatus.FS
+   does not report dirty when SMP and MTTCG is enabled. This
+   workaround is technically compliant with the RISC-V Privileged
+   specification as it is legal to return only off, or dirty,
+   however this may cause unnecessary saves of floating point state.
+   Without this workaround, floating point state is not saved and
+   restored coorectly when SMP and MTTCG is enabled, */
+if (qemu_tcg_mttcg_enabled()) {
+/* FP is always dirty or off */
+if (mstatus & MSTATUS_FS) {
+mstatus |= MSTATUS_FS;
+}
+}
+
+int dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
+((mstatus & MSTATUS_XS) == MSTATUS_XS);
 mstatus = set_field(mstatus, MSTATUS_SD, dirty);
 env->mstatus = mstatus;
 break;
-- 
2.7.0




[Qemu-devel] [PATCH v6 20/26] RISC-V: No traps on writes to misa, minstret, mcycle

2018-03-24 Thread Michael Clark
These fields are marked WARL in the specification so illegal
writes are silently dropped.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/op_helper.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 36b9e8e..ba3639d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -200,17 +200,19 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 break;
 }
 case CSR_MINSTRET:
-qemu_log_mask(LOG_UNIMP, "CSR_MINSTRET: write not implemented");
-goto do_illegal;
+/* minstret is WARL so unsupported writes are ignored */
+break;
 case CSR_MCYCLE:
-qemu_log_mask(LOG_UNIMP, "CSR_MCYCLE: write not implemented");
-goto do_illegal;
+/* mcycle is WARL so unsupported writes are ignored */
+break;
+#if defined(TARGET_RISCV32)
 case CSR_MINSTRETH:
-qemu_log_mask(LOG_UNIMP, "CSR_MINSTRETH: write not implemented");
-goto do_illegal;
+/* minstreth is WARL so unsupported writes are ignored */
+break;
 case CSR_MCYCLEH:
-qemu_log_mask(LOG_UNIMP, "CSR_MCYCLEH: write not implemented");
-goto do_illegal;
+/* mcycleh is WARL so unsupported writes are ignored */
+break;
+#endif
 case CSR_MUCOUNTEREN:
 env->mucounteren = val_to_write;
 break;
@@ -300,10 +302,9 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 case CSR_MBADADDR:
 env->mbadaddr = val_to_write;
 break;
-case CSR_MISA: {
-qemu_log_mask(LOG_UNIMP, "CSR_MISA: misa writes not supported");
-goto do_illegal;
-}
+case CSR_MISA:
+/* misa is WARL so unsupported writes are ignored */
+break;
 case CSR_PMPCFG0:
 case CSR_PMPCFG1:
 case CSR_PMPCFG2:
@@ -328,7 +329,6 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 case CSR_PMPADDR15:
pmpaddr_csr_write(env, csrno - CSR_PMPADDR0, val_to_write);
break;
-do_illegal:
 #endif
 default:
 do_raise_exception_err(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
-- 
2.7.0




Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 12:45 PM, Michael Clark  wrote:

>
>
> On Sat, Mar 24, 2018 at 11:13 AM, Michael Clark  wrote:
>
>> The sifive_u machine already marks its ROM readonly. This fixes
>> the remaining boards.
>>
>> Cc: Sagar Karandikar 
>> Cc: Bastian Koppelmann 
>> Signed-off-by: Michael Clark 
>> Signed-off-by: Palmer Dabbelt 
>> ---
>>  hw/riscv/sifive_u.c  |  9 +
>>  hw/riscv/spike.c | 18 ++
>>  hw/riscv/virt.c  |  7 ---
>>  include/hw/riscv/spike.h |  8 
>>  4 files changed, 19 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> index 6116c38..25df16c 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState
>> *machine)
>>  SiFiveUState *s = g_new0(SiFiveUState, 1);
>>  MemoryRegion *sys_memory = get_system_memory();
>>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
>> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>
>>  /* Initialize SOC */
>>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
>> @@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState
>> *machine)
>>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>>
>>  /* boot rom */
>> -memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
>> +memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
>> memmap[SIFIVE_U_MROM].base, &error_fatal);
>> -memory_region_set_readonly(boot_rom, true);
>> -memory_region_add_subregion(sys_memory, 0x0, boot_rom);
>> +memory_region_set_readonly(mask_rom, true);
>> +memory_region_add_subregion(sys_memory, 0x0, mask_rom);
>>
>>  if (machine->kernel_filename) {
>>  load_kernel(machine->kernel_filename);
>> @@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState
>> *machine)
>>  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
>>  cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
>>  sizeof(reset_vec), s->fdt, s->fdt_size);
>> +memory_region_set_readonly(mask_rom, true);
>>
>>  /* MMIO */
>>  s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
>> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
>> index 7710333..74edf33 100644
>> --- a/hw/riscv/spike.c
>> +++ b/hw/riscv/spike.c
>> @@ -173,7 +173,7 @@ static void spike_v1_10_0_board_init(MachineState
>> *machine)
>>  SpikeState *s = g_new0(SpikeState, 1);
>>  MemoryRegion *system_memory = get_system_memory();
>>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
>> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>
>>  /* Initialize SOC */
>>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
>> @@ -196,9 +196,9 @@ static void spike_v1_10_0_board_init(MachineState
>> *machine)
>>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>>
>>  /* boot rom */
>> -memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
>> +memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
>> s->fdt_size + 0x2000, &error_fatal);
>> -memory_region_add_subregion(system_memory, 0x0, boot_rom);
>> +memory_region_add_subregion(system_memory, 0x0, mask_rom);
>>
>>  if (machine->kernel_filename) {
>>  load_kernel(machine->kernel_filename);
>> @@ -228,9 +228,10 @@ static void spike_v1_10_0_board_init(MachineState
>> *machine)
>>  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
>>  cpu_physical_memory_write(memmap[SPIKE_MROM].base +
>> sizeof(reset_vec),
>>  s->fdt, s->fdt_size);
>> +memory_region_set_readonly(mask_rom, true);
>>
>>  /* initialize HTIF using symbols found in load_kernel */
>> -htif_mm_init(system_memory, boot_rom, &s->soc.harts[0].env,
>> serial_hds[0]);
>> +htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env,
>> serial_hds[0]);
>>
>>  /* Core Local Interruptor (timer and IPI) */
>>  sifive_clint_create(memmap[SPIKE_CLINT].base,
>> memmap[SPIKE_CLINT].size,
>> @@ -244,7 +245,7 @@ static void spike_v1_09_1_board_init(MachineState
>> *machine)
>>  SpikeState *s = g_new0(SpikeState, 1);
>>  MemoryRegion *system_memory = get_system_memory();
>>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
>> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>
>>  /* Initialize SOC */
>>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
>> @@ -264,9 +265,9 @@ static void spike_v1_09_1_board_init(MachineState
>> *machine)
>>  main_mem);
>>
>>  /* boot rom */
>> -memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
>> +memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
>> 0x4, &er

[Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375

2018-03-24 Thread Michael Davidsaver
differences from ds1338

* Has alarms (not modeled)
* different control register (not modeled)
* smaller address space (0x20 vs. 0x40)

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds-rtc.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 5a4df1b115..e5da36cae8 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -1,8 +1,9 @@
 /*
- * MAXIM DSRTC I2C RTC+NVRAM
+ * MAXIM/Dallas DS1338 and DS1375 I2C RTC+NVRAM
  *
+ * Copyright (c) 2018 Michael Davidsaver
  * Copyright (c) 2009 CodeSourcery.
- * Written by Paul Brook
+ * Written by Paul Brook, Michael Davidsaver
  *
  * This code is licensed under the GNU GPL v2.
  *
@@ -41,6 +42,7 @@
 #define R_YEAR  (0x6)
 
 #define R_DS1338_CTRL (0x7)
+#define R_DS1375_CTRL (0xe)
 
 /* use 12 hour mode when set */
 FIELD(HOUR, SET12, 6, 1)
@@ -296,10 +298,34 @@ static const TypeInfo ds1338_info = {
 .class_init= ds1338_class_init,
 };
 
+static void ds1375_control_write(DSRTCState *s, uint8_t data)
+{
+/* just store it, we don't model any features */
+s->nvram[R_DS1375_CTRL] = data;
+}
+
+static void ds1375_class_init(ObjectClass *klass, void *data)
+{
+DSRTCClass *k = DSRTC_CLASS(klass);
+
+k->has_century = true;
+k->addr_size = 0x20;
+k->ctrl_offset = R_DS1375_CTRL;
+k->ctrl_write = ds1375_control_write;
+}
+
+static const TypeInfo ds1375_info = {
+.name  = "ds1375",
+.parent= TYPE_DSRTC,
+.class_size= sizeof(DSRTCClass),
+.class_init= ds1375_class_init,
+};
+
 static void dsrtc_register_types(void)
 {
 type_register_static(&dsrtc_info);
 type_register_static(&ds1338_info);
+type_register_static(&ds1375_info);
 }
 
 type_init(dsrtc_register_types)
-- 
2.11.0




[Qemu-devel] [PATCH 07/14] timer: ds1338 fix wday_offset handling

2018-03-24 Thread Michael Davidsaver
Correctly handle different real weekday in
guest and host timezones.
Allow guest to use any day as start of week
(day 1).  eg. Monday instead of Sunday.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 071c031563..a969b5c8ba 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -108,7 +108,10 @@ static void capture_current_time(DS1338State *s)
 } else {
 ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
 }
-s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
+s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7;
+if (s->nvram[R_WDAY] == 0) {
+s->nvram[R_WDAY] = 7;
+}
 s->nvram[R_DATE] = to_bcd(now.tm_mday);
 s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
 s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
@@ -182,17 +185,22 @@ static void ds1338_update(DS1338State *s)
 } else {
 now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
 }
-{
-/* The day field is supposed to contain a value in
-   the range 1-7. Otherwise behavior is undefined.
- */
-int user_wday = (s->nvram[R_WDAY] & 7) - 1;
-s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
-}
+now.tm_wday = from_bcd(s->nvram[R_WDAY]) - 1u;
 now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);
 now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
 now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
 s->offset = qemu_timedate_diff(&now);
+
+{
+/* Round trip to get real wday_offset based on time delta and
+ * ref. timezone.
+ * Race if midnight (in ref. timezone) happens here.
+ */
+int user_wday = now.tm_wday;
+qemu_get_timedate(&now, s->offset);
+
+s->wday_offset = (user_wday - now.tm_wday) % 7 + 1;
+}
 }
 
 static int ds1338_send(I2CSlave *i2c, uint8_t data)
-- 
2.11.0




[Qemu-devel] [PATCH 01/14] tests: more thorough tests of ds1338

2018-03-24 Thread Michael Davidsaver
Test current time and set+get round trip.
Separate current time test from set/get
tests to avoid test order issues.

Signed-off-by: Michael Davidsaver 
---
 tests/Makefile.include  |   4 ++
 tests/ds-rtc-common.h   |  67 +
 tests/ds-rtc-current-test.c |  59 ++
 tests/ds-rtc-set-test.c | 117 
 4 files changed, 247 insertions(+)
 create mode 100644 tests/ds-rtc-common.h
 create mode 100644 tests/ds-rtc-current-test.c
 create mode 100644 tests/ds-rtc-set-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index eb218a9539..d256095c88 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -372,6 +372,8 @@ check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
+check-qtest-arm-y += tests/ds-rtc-current-test$(EXESUF)
+check-qtest-arm-y += tests/ds-rtc-set-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
 check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
@@ -771,6 +773,8 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
+tests/ds-rtc-current-test$(EXESUF): tests/ds-rtc-current-test.o 
$(libqos-imx-obj-y)
+tests/ds-rtc-set-test$(EXESUF): tests/ds-rtc-set-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
new file mode 100644
index 00..c8e6c2bc5b
--- /dev/null
+++ b/tests/ds-rtc-common.h
@@ -0,0 +1,67 @@
+/* Common code for testing of Dallas/Maxim I2C bus RTC devices
+ *
+ * Copyright (c) 2018 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+#ifndef DSRTCCOMMON_H
+#define DSRTCCOMMON_H
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "libqos/i2c.h"
+
+#define IMX25_I2C_0_BASE 0x43F8
+#define DS1338_ADDR 0x68
+
+static I2CAdapter *i2c;
+static uint8_t addr;
+static bool use_century;
+
+/* input buffer must have at least 7 elements */
+static inline time_t rtc_parse(const uint8_t *buf)
+{
+struct tm parts;
+
+parts.tm_sec = from_bcd(buf[0]);
+parts.tm_min = from_bcd(buf[1]);
+if (buf[2] & 0x40) {
+/* 12 hour */
+/* HOUR register is 1-12. */
+parts.tm_hour = from_bcd(buf[2] & 0x1f);
+g_assert_cmpuint(parts.tm_hour, >=, 1);
+g_assert_cmpuint(parts.tm_hour, <=, 12);
+parts.tm_hour %= 12u; /* wrap 12 -> 0 */
+if (buf[2] & 0x20) {
+parts.tm_hour += 12u;
+}
+} else {
+/* 24 hour */
+parts.tm_hour = from_bcd(buf[2] & 0x3f);
+}
+parts.tm_wday = from_bcd(buf[3]);
+parts.tm_mday = from_bcd(buf[4]);
+parts.tm_mon =  from_bcd((buf[5] & 0x1f) - 1u);
+parts.tm_year = from_bcd(buf[6]);
+if (!use_century || (buf[5] & 0x80)) {
+parts.tm_year += 100u;
+}
+
+return mktimegm(&parts);
+}
+
+static time_t rtc_gettime(void)
+{
+uint8_t buf[7];
+
+/* zero address pointer */
+buf[0] = 0;
+i2c_send(i2c, addr, buf, 1);
+/* read back current time registers */
+i2c_recv(i2c, addr, buf, 7);
+
+return rtc_parse(buf);
+}
+
+#endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
new file mode 100644
index 00..6acbbed9a6
--- /dev/null
+++ b/tests/ds-rtc-current-test.c
@@ -0,0 +1,59 @@
+/* Testing of Dallas/Maxim I2C bus RTC devices
+ *
+ * Copyright (c) 2018 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+#include 
+
+#include "qemu/osdep.h"
+#include "qemu/bcd.h"
+#include "qemu/cutils.h"
+#include "libqtest.h"
+#include "libqos/libqos.h"
+#include "libqos/i2c.h"
+
+#include "ds-rtc-common.h"
+
+/* read back and compare with current system time */
+static
+void test_rtc_current(void)
+{
+time_t expected, actual;
+/* relax test to limit false positives when host may be overloaded.
+ * Allow larger delta when running "-m quick"
+ */
+time_t max_delta = g_test_slow() ? 1 : 30;
+
+actual = time(NULL);
+/* new second may start here */
+expected = rtc_gettime();
+g_assert_cmpuint(expected, <=, actual + max_delta);
+g_assert_cmpuint(expected, >=, actual);
+}
+
+int main(int argc, char *argv[])
+{
+int ret;
+const char *arch = qtest_get_arch();
+QTestState *s = NULL;
+
+g_test_init(&argc, &argv, NULL);
+
+if (strcmp(arch, "arm"

Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 11:13 AM, Michael Clark  wrote:

> The sifive_u machine already marks its ROM readonly. This fixes
> the remaining boards.
>
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Signed-off-by: Michael Clark 
> Signed-off-by: Palmer Dabbelt 
> ---
>  hw/riscv/sifive_u.c  |  9 +
>  hw/riscv/spike.c | 18 ++
>  hw/riscv/virt.c  |  7 ---
>  include/hw/riscv/spike.h |  8 
>  4 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 6116c38..25df16c 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState *machine)
>  SiFiveUState *s = g_new0(SiFiveUState, 1);
>  MemoryRegion *sys_memory = get_system_memory();
>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>
>  /* Initialize SOC */
>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState
> *machine)
>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>
>  /* boot rom */
> -memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
> +memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
> memmap[SIFIVE_U_MROM].base, &error_fatal);
> -memory_region_set_readonly(boot_rom, true);
> -memory_region_add_subregion(sys_memory, 0x0, boot_rom);
> +memory_region_set_readonly(mask_rom, true);
> +memory_region_add_subregion(sys_memory, 0x0, mask_rom);
>
>  if (machine->kernel_filename) {
>  load_kernel(machine->kernel_filename);
> @@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState *machine)
>  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
>  cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
>  sizeof(reset_vec), s->fdt, s->fdt_size);
> +memory_region_set_readonly(mask_rom, true);
>
>  /* MMIO */
>  s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 7710333..74edf33 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -173,7 +173,7 @@ static void spike_v1_10_0_board_init(MachineState
> *machine)
>  SpikeState *s = g_new0(SpikeState, 1);
>  MemoryRegion *system_memory = get_system_memory();
>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>
>  /* Initialize SOC */
>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -196,9 +196,9 @@ static void spike_v1_10_0_board_init(MachineState
> *machine)
>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>
>  /* boot rom */
> -memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
> +memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
> s->fdt_size + 0x2000, &error_fatal);
> -memory_region_add_subregion(system_memory, 0x0, boot_rom);
> +memory_region_add_subregion(system_memory, 0x0, mask_rom);
>
>  if (machine->kernel_filename) {
>  load_kernel(machine->kernel_filename);
> @@ -228,9 +228,10 @@ static void spike_v1_10_0_board_init(MachineState
> *machine)
>  qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
>  cpu_physical_memory_write(memmap[SPIKE_MROM].base +
> sizeof(reset_vec),
>  s->fdt, s->fdt_size);
> +memory_region_set_readonly(mask_rom, true);
>
>  /* initialize HTIF using symbols found in load_kernel */
> -htif_mm_init(system_memory, boot_rom, &s->soc.harts[0].env,
> serial_hds[0]);
> +htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env,
> serial_hds[0]);
>
>  /* Core Local Interruptor (timer and IPI) */
>  sifive_clint_create(memmap[SPIKE_CLINT].base,
> memmap[SPIKE_CLINT].size,
> @@ -244,7 +245,7 @@ static void spike_v1_09_1_board_init(MachineState
> *machine)
>  SpikeState *s = g_new0(SpikeState, 1);
>  MemoryRegion *system_memory = get_system_memory();
>  MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> -MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> +MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>
>  /* Initialize SOC */
>  object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -264,9 +265,9 @@ static void spike_v1_09_1_board_init(MachineState
> *machine)
>  main_mem);
>
>  /* boot rom */
> -memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
> +memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
> 0x4, &error_fatal);
> -memory_region_add_subregion(system_memory, 0x0, boot_rom);
> +memory_region_add_subregion(system_memory, 0x0, mask_rom);
>
>  if (machine->kernel_filename)

Re: [Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug

2018-03-24 Thread Richard W.M. Jones
On Sat, Mar 24, 2018 at 11:13:40AM -0700, Michael Clark wrote:
> This change is a workaround for a bug where mstatus.FS
> is not correctly reporting dirty when MTTCG and SMP are
> enabled which results in the floating point register file
> not being saved during context switches. This a critical
> bug for RISC-V in QEMU as it results in floating point
> register file corruption when running SMP Linux in the
> RISC-V 'virt' machine.
> 
> This workaround will return dirty if mstatus.FS is
> switched from off to initial or clean. We have checked
> the specification and it is legal for an implementation
> to return either off, or dirty, if set to initial or clean.
> 
> This workaround will result in unnecessary floating point
> save restore. When mstatus.FS is off, floating point
> instruction trap to indicate the process is using the FPU.
> The OS can then save floating-point state of the previous
> process using the FPU and set mstatus.FS to initial or
> clean. With this workaround, mstatus.FS will always return
> dirty if set to a non-zero value, indicating floating point
> save restore is necessary, versus misreporting mstatus.FS
> resulting in floating point register file corruption.
> 
> Cc: Palmer Dabbelt 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: Richard W.M. Jones 
> Cc: Peter Maydell 
> Signed-off-by: Michael Clark 

I tested this by running qemu from git with and without this patch,
both times compiling and running the “sched.c” test program:

http://oirase.annexia.org/tmp/sched.c

In my tests it fixes the problem, and therefore:

Tested-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v



Re: [Qemu-devel] [PATCH 2/5] tests: more thorough test of ds1338

2018-03-24 Thread Michael Davidsaver
> On 02/20/2018 09:44 AM, Michael Davidsaver wrote:
>> On 02/18/2018 11:39 PM, Thomas Huth wrote:
...
>> That magic (together with patch 1/5) is IMHO a little bit ugly. I've hit
>> the same problem with the m48t59 test recently, and I solved it by
>> moving the qtest_start() and qtest_end() calls from the main() function
>> into the single tests instead, so that each test starts with a clean state:
>>
>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=9c29830c90d82f27f
>>
>> Could you maybe try whether that approach works for your test cases
>> here, too? Then you could do this without the "0xff" hack here...
> 
> Your right, this looks clearer.  I'll try this approach.

I ultimately decided not to go with this approach as test failures
didn't call qtest_quit(), and the process under test is left running
after gtester exits.

Instead I split the current time test off into a second executable.
This avoids all of the magic.


FYI.  I also looked at using g_test_add(), keeping the QTestState* in a
Fixture struct, and using setup and teardown functions to call
qtest_start()/qtest_quit().  This works, but seemed to me like too much
effort in this case.



Re: [Qemu-devel] [PATCH v6 10/26] RISC-V: Improve page table walker spec compliance

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 11:13 AM, Michael Clark  wrote:

> - Inline PTE_TABLE check for better readability
> - Improve readibility of User page U mode and SUM test
> - Disallow non U mode from fetching from User pages
> - Add reserved PTE flag check: W or W|X
> - Add misaligned PPN check
> - Set READ flag for PTE X flag if mstatus.mxr is in effect
> - Change access checks from ternary operator to if statements
> - Improves page walker comments
> - No measurable performance impact on dd test
>
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Signed-off-by: Michael Clark 
> Signed-off-by: Palmer Dabbelt 
> ---
>  target/riscv/cpu_bits.h |  2 --
>  target/riscv/helper.c   | 59 ++
> ---
>  2 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 64aa097..12b4757 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -407,5 +407,3 @@
>  #define PTE_SOFT  0x300 /* Reserved for Software */
>
>  #define PTE_PPN_SHIFT 10
> -
> -#define PTE_TABLE(PTE) (((PTE) & (PTE_V | PTE_R | PTE_W | PTE_X)) ==
> PTE_V)
> diff --git a/target/riscv/helper.c b/target/riscv/helper.c
> index 02cbcea..9010620 100644
> --- a/target/riscv/helper.c
> +++ b/target/riscv/helper.c
> @@ -185,16 +185,36 @@ restart:
>  #endif
>  target_ulong ppn = pte >> PTE_PPN_SHIFT;
>
> -if (PTE_TABLE(pte)) { /* next level of page table */
> +if (!(pte & PTE_V)) {
> +/* Invalid PTE */
> +return TRANSLATE_FAIL;
> +} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> +/* Inner PTE, continue walking */
>  base = ppn << PGSHIFT;
> -} else if ((pte & PTE_U) ? (mode == PRV_S) && !sum : !(mode ==
> PRV_S)) {
> -break;
> -} else if (!(pte & PTE_V) || (!(pte & PTE_R) && (pte & PTE_W))) {
> -break;
> -} else if (access_type == MMU_INST_FETCH ? !(pte & PTE_X) :
> -  access_type == MMU_DATA_LOAD ?  !(pte & PTE_R) &&
> -  !(mxr && (pte & PTE_X)) : !((pte & PTE_R) && (pte &
> PTE_W))) {
> -break;
> +} else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
> +/* Reserved leaf PTE flags: PTE_W */
> +return TRANSLATE_FAIL;
> +} else if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
> +/* Reserved leaf PTE flags: PTE_W + PTE_X */
> +return TRANSLATE_FAIL;
> +} else if ((pte & PTE_U) && ((mode != PRV_U) &&
> +   (!sum || access_type == MMU_INST_FETCH))) {
> +/* User PTE flags when not U mode and mstatus.SUM is not set,
> +   or the access type is an instruction fetch */
> +return TRANSLATE_FAIL;
> +} else if (ppn & ((1ULL << ptshift) - 1)) {
> +/* Misasligned PPN */
> +return TRANSLATE_FAIL;
> +} else if (access_type == MMU_DATA_LOAD && !((pte & PTE_R) ||
> +   ((pte & PTE_X) && mxr))) {
> +/* Read access check failed */
> +return TRANSLATE_FAIL;
> +} else if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
> +/* Write access check failed */
> +return TRANSLATE_FAIL;
> +} else if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
> +/* Fetch access check failed */
> +return TRANSLATE_FAIL;
>  } else {
>  /* if necessary, set accessed and dirty bits. */
>  target_ulong updated_pte = pte | PTE_A |
> @@ -202,11 +222,14 @@ restart:
>
>  /* Page table updates need to be atomic with MTTCG enabled */
>  if (updated_pte != pte) {
> -/* if accessed or dirty bits need updating, and the PTE is
> - * in RAM, then we do so atomically with a compare and
> swap.
> - * if the PTE is in IO space, then it can't be updated.
> - * if the PTE changed, then we must re-walk the page table
> -   as the PTE is no longer valid */
> +/*
> + * - if accessed or dirty bits need updating, and the PTE
> is
> + *   in RAM, then we do so atomically with a compare and
> swap.
> + * - if the PTE is in IO space or ROM, then it can't be
> updated
> + *   and we return TRANSLATE_FAIL.
> + * - if the PTE changed by the time we went to update it,
> then
> + *   it is no longer valid and we must re-walk the page
> table.
> + */
>  MemoryRegion *mr;
>  hwaddr l = sizeof(target_ulong), addr1;
>  mr = address_space_translate(cs->as, pte_addr,
> @@ -239,15 +262,15 @@ restart:
>  target_ulong vpn = addr >> PGSHIFT;
>  *physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
>
> -if ((pte & PTE_R)) {
> +  

[Qemu-devel] [PATCH 10/14] timer: rename file ds1338.c -> ds-rtc.c

2018-03-24 Thread Michael Davidsaver
Signed-off-by: Michael Davidsaver 
---
 default-configs/arm-softmmu.mak | 2 +-
 hw/timer/Makefile.objs  | 2 +-
 hw/timer/{ds1338.c => ds-rtc.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename hw/timer/{ds1338.c => ds-rtc.c} (100%)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index dd29e741c2..0afffa2a8a 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -33,7 +33,7 @@ CONFIG_SMC91C111=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
 CONFIG_FTGMAC100=y
-CONFIG_DS1338=y
+CONFIG_DSRTC=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_PFLASH_CFI02=y
 CONFIG_MICRODRIVE=y
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8b27a4b7ef..d4c59df1d1 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -3,7 +3,7 @@ common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
 common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o
 common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
 common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
-common-obj-$(CONFIG_DS1338) += ds1338.o
+common-obj-$(CONFIG_DSRTC) += ds-rtc.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
diff --git a/hw/timer/ds1338.c b/hw/timer/ds-rtc.c
similarity index 100%
rename from hw/timer/ds1338.c
rename to hw/timer/ds-rtc.c
-- 
2.11.0




[Qemu-devel] [PATCH 04/14] timer: ds1338 clarify HOUR handling

2018-03-24 Thread Michael Davidsaver
Simplify and comment the translation between
registers and struct tm.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 72a4692d60..9bcda26e51 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -88,23 +88,23 @@ static void capture_current_time(DS1338State *s)
  * which will be actually read by the data transfer operation.
  */
 struct tm now;
+bool mode12 = ARRAY_FIELD_EX32(s->nvram, HOUR, SET12);
 qemu_get_timedate(&now, s->offset);
+
 s->nvram[R_SEC] = to_bcd(now.tm_sec);
 s->nvram[R_MIN] = to_bcd(now.tm_min);
-if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
-int tmp = now.tm_hour;
-if (tmp % 12 == 0) {
-tmp += 12;
-}
-s->nvram[R_HOUR] = 0;
+s->nvram[R_HOUR] = 0;
+if (mode12) {
+/* map 0-23 to 1-12 am/pm */
 ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
-if (tmp <= 12) {
-ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
-ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
-} else {
-ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
-ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
+ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, now.tm_hour >= 12u);
+now.tm_hour %= 12u; /* wrap 0-23 to 0-11 */
+if (now.tm_hour == 0u) {
+/* midnight/noon stored as 12 */
+now.tm_hour = 12u;
 }
+ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(now.tm_hour));
+
 } else {
 ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
 }
@@ -182,14 +182,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 break;
 case R_HOUR:
 if (FIELD_EX32(data, HOUR, SET12)) {
-int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
+/* 12 hour (1-12) */
+/* read and wrap 1-12 -> 0-11 */
+now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
 if (FIELD_EX32(data, HOUR, AMPM)) {
-tmp += 12;
+now.tm_hour += 12;
 }
-if (tmp % 12 == 0) {
-tmp -= 12;
-}
-now.tm_hour = tmp;
+
 } else {
 now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
 }
-- 
2.11.0




[Qemu-devel] [PATCH 09/14] timer: rename ds1338 -> dsrtc

2018-03-24 Thread Michael Davidsaver
Prepare to generalize with a more generic
name.

Keep device name and vmstate name "ds1338"
for compatibility.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 74 +++
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index a969b5c8ba..28f788dd8e 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -1,5 +1,5 @@
 /*
- * MAXIM DS1338 I2C RTC+NVRAM
+ * MAXIM DSRTC I2C RTC+NVRAM
  *
  * Copyright (c) 2009 CodeSourcery.
  * Written by Paul Brook
@@ -23,8 +23,8 @@
 
 #define CTRL_OSF   0x20
 
-#define TYPE_DS1338 "ds1338"
-#define DS1338(obj) OBJECT_CHECK(DS1338State, (obj), TYPE_DS1338)
+#define TYPE_DSRTC "ds1338"
+#define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
 
 /* values stored in BCD */
 /* 00-59 */
@@ -57,7 +57,7 @@ FIELD(MONTH, CENTURY, 7, 1)
 
 FIELD(CTRL, OSF, 5, 1)
 
-typedef struct DS1338State {
+typedef struct DSRTCState {
 I2CSlave parent_obj;
 
 int64_t offset;
@@ -65,24 +65,24 @@ typedef struct DS1338State {
 uint8_t nvram[NVRAM_SIZE];
 int32_t ptr;
 bool addr_byte;
-} DS1338State;
+} DSRTCState;
 
-static const VMStateDescription vmstate_ds1338 = {
+static const VMStateDescription vmstate_dsrtc = {
 .name = "ds1338",
 .version_id = 2,
 .minimum_version_id = 1,
 .fields = (VMStateField[]) {
-VMSTATE_I2C_SLAVE(parent_obj, DS1338State),
-VMSTATE_INT64(offset, DS1338State),
-VMSTATE_UINT8_V(wday_offset, DS1338State, 2),
-VMSTATE_UINT8_ARRAY(nvram, DS1338State, NVRAM_SIZE),
-VMSTATE_INT32(ptr, DS1338State),
-VMSTATE_BOOL(addr_byte, DS1338State),
+VMSTATE_I2C_SLAVE(parent_obj, DSRTCState),
+VMSTATE_INT64(offset, DSRTCState),
+VMSTATE_UINT8_V(wday_offset, DSRTCState, 2),
+VMSTATE_UINT8_ARRAY(nvram, DSRTCState, NVRAM_SIZE),
+VMSTATE_INT32(ptr, DSRTCState),
+VMSTATE_BOOL(addr_byte, DSRTCState),
 VMSTATE_END_OF_LIST()
 }
 };
 
-static void capture_current_time(DS1338State *s)
+static void capture_current_time(DSRTCState *s)
 {
 /* Capture the current time into the secondary registers
  * which will be actually read by the data transfer operation.
@@ -117,7 +117,7 @@ static void capture_current_time(DS1338State *s)
 s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
 }
 
-static void inc_regptr(DS1338State *s)
+static void inc_regptr(DSRTCState *s)
 {
 /* The register pointer wraps around after 0x3F; wraparound
  * causes the current time/date to be retransferred into
@@ -129,9 +129,9 @@ static void inc_regptr(DS1338State *s)
 }
 }
 
-static int ds1338_event(I2CSlave *i2c, enum i2c_event event)
+static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
 {
-DS1338State *s = DS1338(i2c);
+DSRTCState *s = DSRTC(i2c);
 
 switch (event) {
 case I2C_START_RECV:
@@ -152,9 +152,9 @@ static int ds1338_event(I2CSlave *i2c, enum i2c_event event)
 return 0;
 }
 
-static int ds1338_recv(I2CSlave *i2c)
+static int dsrtc_recv(I2CSlave *i2c)
 {
-DS1338State *s = DS1338(i2c);
+DSRTCState *s = DSRTC(i2c);
 uint8_t res;
 
 res  = s->nvram[s->ptr];
@@ -165,7 +165,7 @@ static int ds1338_recv(I2CSlave *i2c)
 /* call after guest writes to current time registers
  * to re-compute our offset from host time.
  */
-static void ds1338_update(DS1338State *s)
+static void dsrtc_update(DSRTCState *s)
 {
 
 struct tm now;
@@ -203,9 +203,9 @@ static void ds1338_update(DS1338State *s)
 }
 }
 
-static int ds1338_send(I2CSlave *i2c, uint8_t data)
+static int dsrtc_send(I2CSlave *i2c, uint8_t data)
 {
-DS1338State *s = DS1338(i2c);
+DSRTCState *s = DSRTC(i2c);
 
 if (s->addr_byte) {
 s->ptr = data & (NVRAM_SIZE - 1);
@@ -225,15 +225,15 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 }
 s->nvram[s->ptr] = data;
 if (s->ptr <= R_YEAR) {
-ds1338_update(s);
+dsrtc_update(s);
 }
 inc_regptr(s);
 return 0;
 }
 
-static void ds1338_reset(DeviceState *dev)
+static void dsrtc_reset(DeviceState *dev)
 {
-DS1338State *s = DS1338(dev);
+DSRTCState *s = DSRTC(dev);
 
 /* The clock is running and synchronized with the host */
 s->offset = 0;
@@ -243,28 +243,28 @@ static void ds1338_reset(DeviceState *dev)
 s->addr_byte = false;
 }
 
-static void ds1338_class_init(ObjectClass *klass, void *data)
+static void dsrtc_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
 
-k->event = ds1338_event;
-k->recv = ds1338_recv;
-k->send = ds1338_send;
-dc->reset = ds1338_reset;
-dc->vmsd = &vmstate_ds1338;
+k->event = dsrtc_event;
+k->recv = dsrtc_recv;
+k->send = dsrtc_send;
+dc->reset = dsrtc_reset;
+dc->vmsd = &vmstate_dsrtc;
 }
 
-static const TypeInfo ds1338_info = {
-.name  = TYPE_DS1338,
+static const

Re: [Qemu-devel] [PATCH for-2.12] qemu-pr-helper: Actually allow users to specify pidfile

2018-03-24 Thread Eric Blake

On 03/24/2018 12:14 AM, Michal Privoznik wrote:

Due to wrong specification of arguments to getopt_long() any
attempt to set pidfile resulted in:

1) the default to be leaked
2) the @pidfile variable to be set to NULL (because optarg is
NULL without this patch).


Broken since the introduction in commit b855f8d, in 2.11 (documentation 
has always mentioned the argument, code has never supported it).




Signed-off-by: Michal Privoznik 


CC: qemu-sta...@nongnu.org
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



[Qemu-devel] [PATCH 02/14] timer: ds1338 use registerfields.h

2018-03-24 Thread Michael Davidsaver
Use names for registers and bits except
for R_CTRL which will be dealt with later,
and isn't modeled anyway.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 84 ++-
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 3849b74a68..b5630e214a 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/i2c/i2c.h"
+#include "hw/registerfields.h"
 #include "qemu/bcd.h"
 
 /* Size of NVRAM including both the user-accessible area and the
@@ -20,15 +21,42 @@
  */
 #define NVRAM_SIZE 64
 
-/* Flags definitions */
-#define SECONDS_CH 0x80
-#define HOURS_12   0x40
-#define HOURS_PM   0x20
 #define CTRL_OSF   0x20
 
 #define TYPE_DS1338 "ds1338"
 #define DS1338(obj) OBJECT_CHECK(DS1338State, (obj), TYPE_DS1338)
 
+/* values stored in BCD */
+/* 00-59 */
+#define R_SEC   (0x0)
+/* 00-59 */
+#define R_MIN   (0x1)
+#define R_HOUR  (0x2)
+/* 1-7 */
+#define R_WDAY  (0x3)
+/* 0-31 */
+#define R_DATE  (0x4)
+#define R_MONTH (0x5)
+/* 0-99 */
+#define R_YEAR  (0x6)
+
+#define R_CTRL  (0x7)
+
+/* use 12 hour mode when set */
+FIELD(HOUR, SET12, 6, 1)
+/* 00-23 */
+FIELD(HOUR, HOUR24, 0, 6)
+/* PM when set */
+FIELD(HOUR, AMPM, 5, 1)
+/* 1-12 (not 0-11!) */
+FIELD(HOUR, HOUR12, 0, 5)
+
+/* 1-12 */
+FIELD(MONTH, MONTH, 0, 5)
+FIELD(MONTH, CENTURY, 7, 1)
+
+FIELD(CTRL, OSF, 5, 1)
+
 typedef struct DS1338State {
 I2CSlave parent_obj;
 
@@ -61,25 +89,29 @@ static void capture_current_time(DS1338State *s)
  */
 struct tm now;
 qemu_get_timedate(&now, s->offset);
-s->nvram[0] = to_bcd(now.tm_sec);
-s->nvram[1] = to_bcd(now.tm_min);
-if (s->nvram[2] & HOURS_12) {
+s->nvram[R_SEC] = to_bcd(now.tm_sec);
+s->nvram[R_MIN] = to_bcd(now.tm_min);
+if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
 int tmp = now.tm_hour;
 if (tmp % 12 == 0) {
 tmp += 12;
 }
+s->nvram[R_HOUR] = 0;
+ARRAY_FIELD_DP32(s->nvram, HOUR, SET12, 1);
 if (tmp <= 12) {
-s->nvram[2] = HOURS_12 | to_bcd(tmp);
+ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 0);
+ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp));
 } else {
-s->nvram[2] = HOURS_12 | HOURS_PM | to_bcd(tmp - 12);
+ARRAY_FIELD_DP32(s->nvram, HOUR, AMPM, 1);
+ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR12, to_bcd(tmp - 12));
 }
 } else {
-s->nvram[2] = to_bcd(now.tm_hour);
+ARRAY_FIELD_DP32(s->nvram, HOUR, HOUR24, to_bcd(now.tm_hour));
 }
-s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
-s->nvram[4] = to_bcd(now.tm_mday);
-s->nvram[5] = to_bcd(now.tm_mon + 1);
-s->nvram[6] = to_bcd(now.tm_year - 100);
+s->nvram[R_WDAY] = (now.tm_wday + s->wday_offset) % 7 + 1;
+s->nvram[R_DATE] = to_bcd(now.tm_mday);
+s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
+s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
 }
 
 static void inc_regptr(DS1338State *s)
@@ -141,17 +173,17 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 struct tm now;
 qemu_get_timedate(&now, s->offset);
 switch(s->ptr) {
-case 0:
+case R_SEC:
 /* TODO: Implement CH (stop) bit.  */
 now.tm_sec = from_bcd(data & 0x7f);
 break;
-case 1:
+case R_MIN:
 now.tm_min = from_bcd(data & 0x7f);
 break;
-case 2:
-if (data & HOURS_12) {
-int tmp = from_bcd(data & (HOURS_PM - 1));
-if (data & HOURS_PM) {
+case R_HOUR:
+if (FIELD_EX32(data, HOUR, SET12)) {
+int tmp = from_bcd(FIELD_EX32(data, HOUR, HOUR12));
+if (FIELD_EX32(data, HOUR, AMPM)) {
 tmp += 12;
 }
 if (tmp % 12 == 0) {
@@ -159,10 +191,10 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 }
 now.tm_hour = tmp;
 } else {
-now.tm_hour = from_bcd(data & (HOURS_12 - 1));
+now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
 }
 break;
-case 3:
+case R_WDAY:
 {
 /* The day field is supposed to contain a value in
the range 1-7. Otherwise behavior is undefined.
@@ -171,18 +203,18 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
 }
 break;
-case 4:
+case R_DATE:
 now.tm_mday = from_bcd(data & 0x3f);
 break;
-case 5:
+case R_MONTH:
 now.tm_mon = from_bcd(data & 0x1f) - 1;
 break;
-case 6:
+case R_YEAR:
 now.tm_year = from_bcd(data) + 100;
 bre

[Qemu-devel] [PATCH 06/14] tests: ds-rtc test 12 hour mode

2018-03-24 Thread Michael Davidsaver
Signed-off-by: Michael Davidsaver 
---
 tests/ds-rtc-set-test.c | 52 +
 1 file changed, 52 insertions(+)

diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
index 35e1a36281..c48406ee2c 100644
--- a/tests/ds-rtc-set-test.c
+++ b/tests/ds-rtc-set-test.c
@@ -29,6 +29,18 @@ static uint8_t test_time_24_12am[8] = {
 0x17,
 };
 
+static uint8_t test_time_12_12am[8] = {
+0, /* address */
+/* Wed, 22 Nov 2017 00:30:53 + */
+0x53,
+0x30,
+0x52, /* 12 AM in 12 hour mode */
+0x03, /* monday is our day 1 */
+0x22,
+0x11 | 0x80,
+0x17,
+};
+
 static uint8_t test_time_24_6am[8] = {
 0, /* address */
 /* Wed, 22 Nov 2017 06:30:53 + */
@@ -41,6 +53,18 @@ static uint8_t test_time_24_6am[8] = {
 0x17,
 };
 
+static uint8_t test_time_12_6am[8] = {
+0, /* address */
+/* Wed, 22 Nov 2017 06:30:53 + */
+0x53,
+0x30,
+0x46, /* 6 AM in 12 hour mode */
+0x03, /* monday is our day 1 */
+0x22,
+0x11 | 0x80,
+0x17,
+};
+
 static uint8_t test_time_24_12pm[8] = {
 0, /* address */
 /* Wed, 22 Nov 2017 12:30:53 + */
@@ -53,6 +77,18 @@ static uint8_t test_time_24_12pm[8] = {
 0x17,
 };
 
+static uint8_t test_time_12_12pm[8] = {
+0, /* address */
+/* Wed, 22 Nov 2017 12:30:53 + */
+0x53,
+0x30,
+0x72, /* 12 PM in 24 hour mode */
+0x03, /* monday is our day 1 */
+0x22,
+0x11 | 0x80,
+0x17,
+};
+
 static uint8_t test_time_24_6pm[8] = {
 0, /* address */
 /* Wed, 22 Nov 2017 18:30:53 + */
@@ -65,6 +101,18 @@ static uint8_t test_time_24_6pm[8] = {
 0x17,
 };
 
+static uint8_t test_time_12_6pm[8] = {
+0, /* address */
+/* Wed, 22 Nov 2017 18:30:53 + */
+0x53,
+0x30,
+0x66, /* 6 PM in 12 hour mode */
+0x03, /* monday is our day 1 */
+0x22,
+0x11 | 0x80,
+0x17,
+};
+
 /* write in and read back known time */
 static
 void test_rtc_set(const void *raw)
@@ -108,6 +156,10 @@ int main(int argc, char *argv[])
 qtest_add_data_func("/ds-rtc-i2c/set24_6am", test_time_24_6am, 
test_rtc_set);
 qtest_add_data_func("/ds-rtc-i2c/set24_12pm", test_time_24_12pm, 
test_rtc_set);
 qtest_add_data_func("/ds-rtc-i2c/set24_6pm", test_time_24_6pm, 
test_rtc_set);
+qtest_add_data_func("/ds-rtc-i2c/set12_12am", test_time_12_12am, 
test_rtc_set);
+qtest_add_data_func("/ds-rtc-i2c/set12_6am", test_time_12_6am, 
test_rtc_set);
+qtest_add_data_func("/ds-rtc-i2c/set12_12pm", test_time_12_12pm, 
test_rtc_set);
+qtest_add_data_func("/ds-rtc-i2c/set12_6pm", test_time_12_6pm, 
test_rtc_set);
 
 ret = g_test_run();
 
-- 
2.11.0




Re: [Qemu-devel] [PATCH PULL 0/8] RDMA queue

2018-03-24 Thread Peter Maydell
On 23 March 2018 at 15:52, Marcel Apfelbaum  wrote:
> The following changes since commit 4c2c1015905fa1d616750dfe024b4c0b35875950:
>
>   Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20180323' into 
> staging (2018-03-23 10:20:54 +)
>
> are available in the git repository at:
>
>   https://github.com/marcel-apf/qemu tags/rdma-pull-request
>
> for you to fetch changes up to 6f559013c86d16255991ca23e47bd161407b95c8:
>
>   hw/rdma: Fix 32-bit compilation (2018-03-23 18:38:55 +0300)
>
> 
> * fix PVRDMA compilation errors and warnings
> * implement query_qp for the PVRDMA device
> * fix make - switch from -I to -iquote
>
> 
> Marcel Apfelbaum (1):
>   hw/rdma: fix clang compilation errors
>
> Michael S. Tsirkin (2):
>   rdma: fix up include directives
>   make: switch from -I to -iquote
>
> Yuval Shaia (5):
>   hw/rdma: Add Query QP operation
>   hw/rdma: Add support for Query QP verb to pvrdma device
>   hw/rdma: Change host_virt to void *
>   hw/rdma: Use correct print format in CHK_ATTR macro
>   hw/rdma: Fix 32-bit compilation
>

Applied, thanks.

-- PMM



[Qemu-devel] [PATCH 08/14] tests: ds-rtc test wday offset

2018-03-24 Thread Michael Davidsaver
Signed-off-by: Michael Davidsaver 
---
 tests/ds-rtc-common.h   | 10 +++---
 tests/ds-rtc-current-test.c |  9 -
 tests/ds-rtc-set-test.c |  6 --
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tests/ds-rtc-common.h b/tests/ds-rtc-common.h
index c8e6c2bc5b..633131c55f 100644
--- a/tests/ds-rtc-common.h
+++ b/tests/ds-rtc-common.h
@@ -20,7 +20,7 @@ static uint8_t addr;
 static bool use_century;
 
 /* input buffer must have at least 7 elements */
-static inline time_t rtc_parse(const uint8_t *buf)
+static inline time_t rtc_parse(const uint8_t *buf, unsigned *wday)
 {
 struct tm parts;
 
@@ -48,10 +48,14 @@ static inline time_t rtc_parse(const uint8_t *buf)
 parts.tm_year += 100u;
 }
 
+if (wday) {
+*wday = parts.tm_wday;
+}
+
 return mktimegm(&parts);
 }
 
-static time_t rtc_gettime(void)
+static time_t rtc_gettime(unsigned *wday)
 {
 uint8_t buf[7];
 
@@ -61,7 +65,7 @@ static time_t rtc_gettime(void)
 /* read back current time registers */
 i2c_recv(i2c, addr, buf, 7);
 
-return rtc_parse(buf);
+return rtc_parse(buf, wday);
 }
 
 #endif /* DSRTCCOMMON_H */
diff --git a/tests/ds-rtc-current-test.c b/tests/ds-rtc-current-test.c
index 6acbbed9a6..7dc3202261 100644
--- a/tests/ds-rtc-current-test.c
+++ b/tests/ds-rtc-current-test.c
@@ -20,17 +20,24 @@
 static
 void test_rtc_current(void)
 {
+struct tm tm_actual;
 time_t expected, actual;
 /* relax test to limit false positives when host may be overloaded.
  * Allow larger delta when running "-m quick"
  */
 time_t max_delta = g_test_slow() ? 1 : 30;
 
+unsigned wday_expect;
+
 actual = time(NULL);
 /* new second may start here */
-expected = rtc_gettime();
+expected = rtc_gettime(&wday_expect);
+
+gmtime_r(&actual, &tm_actual);
+
 g_assert_cmpuint(expected, <=, actual + max_delta);
 g_assert_cmpuint(expected, >=, actual);
+g_assert_cmpuint(wday_expect, ==, tm_actual.tm_wday);
 }
 
 int main(int argc, char *argv[])
diff --git a/tests/ds-rtc-set-test.c b/tests/ds-rtc-set-test.c
index c48406ee2c..12aeb2580a 100644
--- a/tests/ds-rtc-set-test.c
+++ b/tests/ds-rtc-set-test.c
@@ -124,16 +124,18 @@ void test_rtc_set(const void *raw)
 
 const uint8_t *testtime = raw;
 time_t expected, actual;
+unsigned wday_expect, wday_actual;
 
 /* skip address pointer and parse remainder */
-expected = rtc_parse(&testtime[1]);
+expected = rtc_parse(&testtime[1], &wday_expect);
 
 i2c_send(i2c, addr, testtime, 8);
 /* host may start new second here */
-actual = rtc_gettime();
+actual = rtc_gettime(&wday_actual);
 
 g_assert_cmpuint(expected, <=, actual);
 g_assert_cmpuint(expected + max_delta, >=, actual);
+g_assert_cmpuint(wday_expect, ==, wday_actual);
 }
 
 int main(int argc, char *argv[])
-- 
2.11.0




[Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

2018-03-24 Thread Michael Davidsaver
Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds-rtc.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
index 2df1bce3f8..5a4df1b115 100644
--- a/hw/timer/ds-rtc.c
+++ b/hw/timer/ds-rtc.c
@@ -70,6 +70,7 @@ typedef struct DSRTCState {
 typedef struct DSRTCClass {
 I2CSlaveClass parent_obj;
 
+bool has_century;
 /* actual address space size must be <= NVRAM_SIZE */
 unsigned addr_size;
 unsigned ctrl_offset;
@@ -91,7 +92,7 @@ static const VMStateDescription vmstate_dsrtc = {
 }
 };
 
-static void capture_current_time(DSRTCState *s)
+static void capture_current_time(DSRTCState *s, DSRTCClass *k)
 {
 /* Capture the current time into the secondary registers
  * which will be actually read by the data transfer operation.
@@ -123,25 +124,28 @@ static void capture_current_time(DSRTCState *s)
 }
 s->nvram[R_DATE] = to_bcd(now.tm_mday);
 s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
-s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
+s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
+
+ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
+ k->has_century && now.tm_year >= 100)
 }
 
-static void inc_regptr(DSRTCState *s)
+static void inc_regptr(DSRTCState *s, DSRTCClass *k)
 {
-DSRTCClass *k = DSRTC_GET_CLASS(s);
 /* The register pointer wraps around after k->addr_size-1; wraparound
  * causes the current time/date to be retransferred into
  * the secondary registers.
  */
 s->ptr = (s->ptr + 1) % k->addr_size;
 if (!s->ptr) {
-capture_current_time(s);
+capture_current_time(s, k);
 }
 }
 
 static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
 {
 DSRTCState *s = DSRTC(i2c);
+DSRTCClass *k = DSRTC_GET_CLASS(s);
 
 switch (event) {
 case I2C_START_RECV:
@@ -150,7 +154,7 @@ static int dsrtc_event(I2CSlave *i2c, enum i2c_event event)
  * START_SEND, because the guest can't get at that data
  * without going through a START_RECV which would overwrite it.
  */
-capture_current_time(s);
+capture_current_time(s, k);
 break;
 case I2C_START_SEND:
 s->addr_byte = true;
@@ -165,10 +169,11 @@ static int dsrtc_event(I2CSlave *i2c, enum i2c_event 
event)
 static int dsrtc_recv(I2CSlave *i2c)
 {
 DSRTCState *s = DSRTC(i2c);
+DSRTCClass *k = DSRTC_GET_CLASS(s);
 uint8_t res;
 
 res  = s->nvram[s->ptr];
-inc_regptr(s);
+inc_regptr(s, k);
 return res;
 }
 
@@ -230,7 +235,7 @@ static int dsrtc_send(I2CSlave *i2c, uint8_t data)
 if (s->ptr <= R_YEAR) {
 dsrtc_update(s);
 }
-inc_regptr(s);
+inc_regptr(s, k);
 return 0;
 }
 
@@ -278,6 +283,7 @@ static void ds1338_class_init(ObjectClass *klass, void 
*data)
 {
 DSRTCClass *k = DSRTC_CLASS(klass);
 
+k->has_century = false;
 k->addr_size = 0x40;
 k->ctrl_offset = R_DS1338_CTRL;
 k->ctrl_write = ds1338_control_write;
-- 
2.11.0




[Qemu-devel] [PATCH 05/14] timer: ds1338 change write handling

2018-03-24 Thread Michael Davidsaver
instead of a read-modify-write, do direct translation
of device registers to struct tm members.

This new ds1338_update() is the reverse of
the existing capture_current_time().

Simplifies later handling of CENTURY bit in
similar Dallas RTC chips.

Signed-off-by: Michael Davidsaver 
---
 hw/timer/ds1338.c | 86 ++-
 1 file changed, 40 insertions(+), 46 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 9bcda26e51..071c031563 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -159,6 +159,42 @@ static int ds1338_recv(I2CSlave *i2c)
 return res;
 }
 
+/* call after guest writes to current time registers
+ * to re-compute our offset from host time.
+ */
+static void ds1338_update(DS1338State *s)
+{
+
+struct tm now;
+memset(&now, 0, sizeof(now));
+
+/* TODO: Implement CH (stop) bit?  */
+now.tm_sec = from_bcd(s->nvram[R_SEC] & 0x7f);
+now.tm_min = from_bcd(s->nvram[R_MIN] & 0x7f);
+if (ARRAY_FIELD_EX32(s->nvram, HOUR, SET12)) {
+/* 12 hour (1-12) */
+/* read and wrap 1-12 -> 0-11 */
+now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR12)) % 12u;
+if (ARRAY_FIELD_EX32(s->nvram, HOUR, AMPM)) {
+now.tm_hour += 12;
+}
+
+} else {
+now.tm_hour = from_bcd(ARRAY_FIELD_EX32(s->nvram, HOUR, HOUR24));
+}
+{
+/* The day field is supposed to contain a value in
+   the range 1-7. Otherwise behavior is undefined.
+ */
+int user_wday = (s->nvram[R_WDAY] & 7) - 1;
+s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
+}
+now.tm_mday = from_bcd(s->nvram[R_DATE] & 0x3f);
+now.tm_mon = from_bcd(s->nvram[R_MONTH] & 0x1f) - 1;
+now.tm_year = from_bcd(s->nvram[R_YEAR]) + 100;
+s->offset = qemu_timedate_diff(&now);
+}
+
 static int ds1338_send(I2CSlave *i2c, uint8_t data)
 {
 DS1338State *s = DS1338(i2c);
@@ -168,52 +204,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 s->addr_byte = false;
 return 0;
 }
-if (s->ptr < 7) {
-/* Time register. */
-struct tm now;
-qemu_get_timedate(&now, s->offset);
-switch(s->ptr) {
-case R_SEC:
-/* TODO: Implement CH (stop) bit.  */
-now.tm_sec = from_bcd(data & 0x7f);
-break;
-case R_MIN:
-now.tm_min = from_bcd(data & 0x7f);
-break;
-case R_HOUR:
-if (FIELD_EX32(data, HOUR, SET12)) {
-/* 12 hour (1-12) */
-/* read and wrap 1-12 -> 0-11 */
-now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR12)) % 12u;
-if (FIELD_EX32(data, HOUR, AMPM)) {
-now.tm_hour += 12;
-}
-
-} else {
-now.tm_hour = from_bcd(FIELD_EX32(data, HOUR, HOUR24));
-}
-break;
-case R_WDAY:
-{
-/* The day field is supposed to contain a value in
-   the range 1-7. Otherwise behavior is undefined.
- */
-int user_wday = (data & 7) - 1;
-s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
-}
-break;
-case R_DATE:
-now.tm_mday = from_bcd(data & 0x3f);
-break;
-case R_MONTH:
-now.tm_mon = from_bcd(data & 0x1f) - 1;
-break;
-case R_YEAR:
-now.tm_year = from_bcd(data) + 100;
-break;
-}
-s->offset = qemu_timedate_diff(&now);
-} else if (s->ptr == R_CTRL) {
+if (s->ptr == R_CTRL) {
 /* Control register. */
 
 /* Ensure bits 2, 3 and 6 will read back as zero. */
@@ -225,6 +216,9 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
 
 }
 s->nvram[s->ptr] = data;
+if (s->ptr <= R_YEAR) {
+ds1338_update(s);
+}
 inc_regptr(s);
 return 0;
 }
-- 
2.11.0




Re: [Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 11:13 AM, Michael Clark  wrote:

> This change is a workaround for a bug where mstatus.FS
> is not correctly reporting dirty when MTTCG and SMP are
> enabled which results in the floating point register file
> not being saved during context switches. This a critical
> bug for RISC-V in QEMU as it results in floating point
> register file corruption when running SMP Linux in the
> RISC-V 'virt' machine.
>
> This workaround will return dirty if mstatus.FS is
> switched from off to initial or clean. We have checked
> the specification and it is legal for an implementation
> to return either off, or dirty, if set to initial or clean.
>
> This workaround will result in unnecessary floating point
> save restore. When mstatus.FS is off, floating point
> instruction trap to indicate the process is using the FPU.
> The OS can then save floating-point state of the previous
> process using the FPU and set mstatus.FS to initial or
> clean. With this workaround, mstatus.FS will always return
> dirty if set to a non-zero value, indicating floating point
> save restore is necessary, versus misreporting mstatus.FS
> resulting in floating point register file corruption.
>
> Cc: Palmer Dabbelt 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: Richard W.M. Jones 
> Cc: Peter Maydell 
> Signed-off-by: Michael Clark 
> ---
>  target/riscv/op_helper.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 1fdde90..d345688 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -144,8 +144,23 @@ void csr_write_helper(CPURISCVState *env,
> target_ulong val_to_write,
>  }
>
>  mstatus = (mstatus & ~mask) | (val_to_write & mask);
> -int dirty = (mstatus & MSTATUS_FS) == MSTATUS_FS;
> -dirty |= (mstatus & MSTATUS_XS) == MSTATUS_XS;
> +
> +/* Note: this is a workaround for an issue where mstatus.FS
> +   does not report dirty when SMP and MTTCG is enabled. This
> +   workaround is technically compliant with the RISC-V Privileged
> +   specification as it is legal to return only off, or dirty,
> +   however this may cause unnecessary saves of floating point
> state.
> +   Without this workaround, floating point state is not saved and
> +   restored coorectly when SMP and MTTCG is enabled, */
>

typo in "correctly". I will fix this...


> +if (qemu_tcg_mttcg_enabled()) {
> +/* FP is always dirty or off */
> +if (mstatus & MSTATUS_FS) {
> +mstatus |= MSTATUS_FS;
> +}
> +}
> +
> +int dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
> +((mstatus & MSTATUS_XS) == MSTATUS_XS);
>  mstatus = set_field(mstatus, MSTATUS_SD, dirty);
>  env->mstatus = mstatus;
>  break;
> --
> 2.7.0
>
>


[Qemu-devel] [PATCH 14/14] tests: drop ds1338-test

2018-03-24 Thread Michael Davidsaver
redundant to ds-rtc-*-test.c

Signed-off-by: Michael Davidsaver 
---
 tests/Makefile.include |  2 --
 tests/ds1338-test.c| 75 --
 2 files changed, 77 deletions(-)
 delete mode 100644 tests/ds1338-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d256095c88..04a99ea6fb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -371,7 +371,6 @@ check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
-check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 check-qtest-arm-y += tests/ds-rtc-current-test$(EXESUF)
 check-qtest-arm-y += tests/ds-rtc-set-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
@@ -772,7 +771,6 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
-tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
 tests/ds-rtc-current-test$(EXESUF): tests/ds-rtc-current-test.o 
$(libqos-imx-obj-y)
 tests/ds-rtc-set-test$(EXESUF): tests/ds-rtc-set-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
diff --git a/tests/ds1338-test.c b/tests/ds1338-test.c
deleted file mode 100644
index 742dad9113..00
--- a/tests/ds1338-test.c
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * QTest testcase for the DS1338 RTC
- *
- * Copyright (c) 2013 Jean-Christophe Dubois
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under the terms of the GNU General Public License as published by the
- *  Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful, but WITHOUT
- *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- *  FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- *  for more details.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, see .
- */
-
-#include "qemu/osdep.h"
-#include "libqtest.h"
-#include "libqos/i2c.h"
-
-#define IMX25_I2C_0_BASE 0x43F8
-
-#define DS1338_ADDR 0x68
-
-static I2CAdapter *i2c;
-static uint8_t addr;
-
-static inline uint8_t bcd2bin(uint8_t x)
-{
-return ((x) & 0x0f) + ((x) >> 4) * 10;
-}
-
-static void send_and_receive(void)
-{
-uint8_t cmd[1];
-uint8_t resp[7];
-time_t now = time(NULL);
-struct tm *tm_ptr = gmtime(&now);
-
-/* reset the index in the RTC memory */
-cmd[0] = 0;
-i2c_send(i2c, addr, cmd, 1);
-
-/* retrieve the date */
-i2c_recv(i2c, addr, resp, 7);
-
-/* check retrieved time againt local time */
-g_assert_cmpuint(bcd2bin(resp[4]), == , tm_ptr->tm_mday);
-g_assert_cmpuint(bcd2bin(resp[5]), == , 1 + tm_ptr->tm_mon);
-g_assert_cmpuint(2000 + bcd2bin(resp[6]), == , 1900 + tm_ptr->tm_year);
-}
-
-int main(int argc, char **argv)
-{
-QTestState *s = NULL;
-int ret;
-
-g_test_init(&argc, &argv, NULL);
-
-s = qtest_start("-display none -machine imx25-pdk");
-i2c = imx_i2c_create(s, IMX25_I2C_0_BASE);
-addr = DS1338_ADDR;
-
-qtest_add_func("/ds1338/tx-rx", send_and_receive);
-
-ret = g_test_run();
-
-qtest_quit(s);
-g_free(i2c);
-
-return ret;
-}
-- 
2.11.0




[Qemu-devel] [PATCH 00/14] Generalize Dallas/Maxim I2C RTC devices v2

2018-03-24 Thread Michael Davidsaver
This series generalizes the ds1338 model to also support the ds1375.
As previously, only the time of day registers are modeled.  This
series is largely a do-over wrt. my previous series.  This time I
started with incremental changes from the existing ds1338 model, and only
add support for the ds1375 (which I care about).

I've added a more thorough test of the time of day function, covering
reading and setting in both 12 and 24 hour mode.  This corrects two
(practically inconsequential) bugs with the handling of 12 hour mode,
and day of the week.

In an attempt to address concerns about false positive test failures
in CI builds, instead of comparing the parts of 'struct tm' seperately
I've changed the logic of the tests to compare the difference between
the expected and actual time in seconds.  The threshold is 30 seconds
when run with 'gtester -m quick', and 1 second otherwise.

Comparision of day of the week is still exact, so there is a chance of
a false positive if the test is running across midnight UTC.

Michael Davidsaver (14):
  tests: more thorough tests of ds1338
  timer: ds1338 use registerfields.h
  timer: ds1338 persist 12-hour mode selection
  timer: ds1338 clarify HOUR handling
  timer: ds1338 change write handling
  tests: ds-rtc test 12 hour mode
  timer: ds1338 fix wday_offset handling
  tests: ds-rtc test wday offset
  timer: rename ds1338 -> dsrtc
  timer: rename file ds1338.c -> ds-rtc.c
  timer: generalize ds1338
  timer: ds-rtc handle CENTURY bit
  timer: ds-rtc model ds1375
  tests: drop ds1338-test

 default-configs/arm-softmmu.mak |   2 +-
 hw/timer/Makefile.objs  |   2 +-
 hw/timer/ds-rtc.c   | 331 
 hw/timer/ds1338.c   | 239 -
 tests/Makefile.include  |   6 +-
 tests/ds-rtc-common.h   |  71 +
 tests/ds-rtc-current-test.c |  66 
 tests/ds-rtc-set-test.c | 171 +
 tests/ds1338-test.c |  75 -
 9 files changed, 645 insertions(+), 318 deletions(-)
 create mode 100644 hw/timer/ds-rtc.c
 delete mode 100644 hw/timer/ds1338.c
 create mode 100644 tests/ds-rtc-common.h
 create mode 100644 tests/ds-rtc-current-test.c
 create mode 100644 tests/ds-rtc-set-test.c
 delete mode 100644 tests/ds1338-test.c

-- 
2.11.0




Re: [Qemu-devel] [PULL 00/24] RISC-V: Post-merge spec conformance and cleanup v5

2018-03-24 Thread Michael Clark
On Sat, Mar 24, 2018 at 11:54 AM, Michael Clark  wrote:

> Hi Peter,
>
> I did actually have the full `riscv-qemu-2.12-fixes-v5` tag in the second
> PR. See below.
>
> It was the v4 pull request prior to this where I made the mistake of not
> including the series version in the tag. I had since dropped the
> riscv_isa_string and rcu_read_lock patches so deleted the unversioned tag.
> I'll make sure I always have the series version in the tag in future PRs.
> I'm starting to get the hang of this...
>
> I haven't yet made a riscv-qemu-2.12-fixes-v6 tag. The first 24 commits
> are the same as v5, however, there is the addition of two new bug fixes (25
> and 26). I will wait until I get some feedback and can tag and make a PR on
> Monday or Tuesday next week assuming we don't have to make changes to any
> of the other commits in the series... It seems we still have time to get
> bug fixes in before the final release, and this series has been tested
> quite extensively by folk working on the Fedora port... as I believe they
> have been using the riscv-all branch in the riscv.org repo which includes
> qemu-2.12-fixes (the branch) which will be tagged as riscv-qemu-2.12-fixes-v6
> when I make the next PR, and patch 26 is basically the Fedora patch with a
> comment, checkpatch warnings fixed, and a conditional to only enable the
> behaviour if mttcg is enabled (as uniprocessor mstatus.FS apepars to be
> okay).
>

I've just pushed a `riscv-qemu-2.12-fixes-v6` tag to the riscv.org repo,
however I think we should wait for some review. Hopefully v7 just adds
'Reviewed-by' to the patches that haven't been reviewed yet... as I don't
mind if we follow the process. I'll try to rally some reviewers. I'll also
try and reserve some time to review other patches, however admittedly I'm
best qualified to do this for target/riscv and hw/riscv as those are the
areas I am familar with. The mstatus.FS fix i've added Signed-off-by. I
think this workaround is preferrable to a corrupt floating point register
file, unless I can get a more comprehensive fix for the QEMU 2.12 release.
I've checked with Palmer, and he thinks the workaround is okay, and
preferrable to the current state. I think the new commit 26 in the series
is one of the more important bug fixes.


> Thanks,
> Michael
>
> On Wed, Mar 21, 2018 at 1:46 PM, Michael Clark  wrote:
>
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> The following changes since commit f1a63fcfcd92c88be8942b5ae71aef
>> 9749a4f135:
>>
>>   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
>>
>> are available in the git repository at:
>>
>>   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-2.12-fixes-v5
>>
>> for you to fetch changes up to 6ad43207f802c91a04e49d86d658b6696faddae0:
>>
>>   RISC-V: Remove erroneous comment from translate.c (2018-03-21 11:28:43
>> -0700)
>>
>> - 
>> Post-merge spec conformance and cleanup
>>
>> This is a series of spec conformance bug fixes and code cleanups
>> that we would like to get in before the QEMU 2.12 release.
>>
>> The focus of this series is specification conformance bug fixes.
>>
>> This series also addresses post-merge feedback such as updating
>> the cpu initialization model to conform with other architectures
>> as requested by Igor Mammedov.
>>
>> The riscv_isa_string patch has been dropped as it was merged
>> independently. The patch to hold rcu_read_lock when accessing
>> physical memory has been dropped as requested by Paolo Bonzini.
>>
>> * Implements WARL behavior for CSRs that don't support writes
>> * Improves specification conformance of the page table walker
>>   * Change access checks from ternary operator to if statements
>>   * Checks for misaligned PPNs
>>   * Disallow M-mode or S-mode from fetching from User pages
>>   * Adds reserved PTE flag check: W or W|X
>>   * Set READ flag for PTE X flag if mstatus.mxr is in effect
>>   * Improves page walker comments and general readability
>> * Several trivial code cleanups to hw/riscv
>>   * Replacing hard coded constants with reference to enums
>> or the machine memory maps.
>>   * Remove unnecessary class initialization boilerplate
>> * Adds bounds checks when writing device-tree to ROM
>> * Updates the cpu model to use a more modern interface
>> * Adds hexidecimal instruction bytes to disassembly output
>> * Sets mtval/stval to zero on exceptions without addresses
>>
>> v5
>>
>> * dropped fix for memory allocation bug in riscv_isa_string
>> * dropped Hold rcu_read_lock when accessing memory
>>
>> v4
>>
>> * added fix for memory allocation bug in riscv_isa_string
>> * trivial fix to remove erroneous comment from translate.c
>>
>> v3
>>
>> * refactor rcu_read_lock in PTE update to use single unlock
>> * mstatus.mxr is in effect regardless of privilege mode
>> * remove unnecessary class init from riscv_hart
>> * set mtval/stval to zero on exceptions without addresses
>>
>> v2
>>
>> * remove unus

Re: [Qemu-devel] [PULL 00/24] RISC-V: Post-merge spec conformance and cleanup v5

2018-03-24 Thread Michael Clark
Hi Peter,

I did actually have the full `riscv-qemu-2.12-fixes-v5` tag in the second
PR. See below.

It was the v4 pull request prior to this where I made the mistake of not
including the series version in the tag. I had since dropped the
riscv_isa_string and rcu_read_lock patches so deleted the unversioned tag.
I'll make sure I always have the series version in the tag in future PRs.
I'm starting to get the hang of this...

I haven't yet made a riscv-qemu-2.12-fixes-v6 tag. The first 24 commits are
the same as v5, however, there is the addition of two new bug fixes (25 and
26). I will wait until I get some feedback and can tag and make a PR on
Monday or Tuesday next week assuming we don't have to make changes to any
of the other commits in the series... It seems we still have time to get
bug fixes in before the final release, and this series has been tested
quite extensively by folk working on the Fedora port... as I believe they
have been using the riscv-all branch in the riscv.org repo which includes
qemu-2.12-fixes (the branch) which will be tagged as riscv-qemu-2.12-fixes-v6
when I make the next PR, and patch 26 is basically the Fedora patch with a
comment, checkpatch warnings fixed, and a conditional to only enable the
behaviour if mttcg is enabled (as uniprocessor mstatus.FS apepars to be
okay).

Thanks,
Michael

On Wed, Mar 21, 2018 at 1:46 PM, Michael Clark  wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> The following changes since commit f1a63fcfcd92c88be8942b5ae71aef
> 9749a4f135:
>
>   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
>
> are available in the git repository at:
>
>   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-2.12-fixes-v5
>
> for you to fetch changes up to 6ad43207f802c91a04e49d86d658b6696faddae0:
>
>   RISC-V: Remove erroneous comment from translate.c (2018-03-21 11:28:43
> -0700)
>
> - 
> Post-merge spec conformance and cleanup
>
> This is a series of spec conformance bug fixes and code cleanups
> that we would like to get in before the QEMU 2.12 release.
>
> The focus of this series is specification conformance bug fixes.
>
> This series also addresses post-merge feedback such as updating
> the cpu initialization model to conform with other architectures
> as requested by Igor Mammedov.
>
> The riscv_isa_string patch has been dropped as it was merged
> independently. The patch to hold rcu_read_lock when accessing
> physical memory has been dropped as requested by Paolo Bonzini.
>
> * Implements WARL behavior for CSRs that don't support writes
> * Improves specification conformance of the page table walker
>   * Change access checks from ternary operator to if statements
>   * Checks for misaligned PPNs
>   * Disallow M-mode or S-mode from fetching from User pages
>   * Adds reserved PTE flag check: W or W|X
>   * Set READ flag for PTE X flag if mstatus.mxr is in effect
>   * Improves page walker comments and general readability
> * Several trivial code cleanups to hw/riscv
>   * Replacing hard coded constants with reference to enums
> or the machine memory maps.
>   * Remove unnecessary class initialization boilerplate
> * Adds bounds checks when writing device-tree to ROM
> * Updates the cpu model to use a more modern interface
> * Adds hexidecimal instruction bytes to disassembly output
> * Sets mtval/stval to zero on exceptions without addresses
>
> v5
>
> * dropped fix for memory allocation bug in riscv_isa_string
> * dropped Hold rcu_read_lock when accessing memory
>
> v4
>
> * added fix for memory allocation bug in riscv_isa_string
> * trivial fix to remove erroneous comment from translate.c
>
> v3
>
> * refactor rcu_read_lock in PTE update to use single unlock
> * mstatus.mxr is in effect regardless of privilege mode
> * remove unnecessary class init from riscv_hart
> * set mtval/stval to zero on exceptions without addresses
>
> v2
>
> * remove unused class boilerplate retains qom parent_obj
> * convert cpu definition towards future model
> * honor mstatus.mxr flag in page table walker
>
> v1
>
> * initial post merge cleanup patch series
>
> - 
> Michael Clark (24):
>   RISC-V: Make virt create_fdt interface consistent
>   RISC-V: Replace hardcoded constants with enum values
>   RISC-V: Make virt board description match spike
>   RISC-V: Use ROM base address and size from memmap
>   RISC-V: Remove identity_translate from load_elf
>   RISC-V: Mark ROM read-only after copying in code
>   RISC-V: Remove unused class definitions
>   RISC-V: Make sure rom has space for fdt
>   RISC-V: Include intruction hex in disassembly
>   RISC-V: Improve page table walker spec compliance
>   RISC-V: Update E order and I extension order
>   RISC-V: Make some header guards more specific
>   RISC-V: Make virt header comment title consistent
>   RISC-V: Use me

Re: [Qemu-devel] [PULL 00/25] RISC-V Post-merge spec conformance and cleanup

2018-03-24 Thread Michael Clark
On Fri, Mar 23, 2018 at 3:20 AM, Peter Maydell 
wrote:

> On 20 March 2018 at 22:25, Michael Clark  wrote:
> > -BEGIN PGP SIGNED MESSAGE-
> > Hash: SHA1
> >
> > The following changes since commit f1a63fcfcd92c88be8942b5ae71aef
> 9749a4f135:
> >
> >   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
> >
> > are available in the git repository at:
> >
> >   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-2.12-fixes
> >
> > for you to fetch changes up to e1a247183ac0816284dfda61423f7030b6686679:
> >
> >   RISC-V: Remove erroneous comment from translate.c (2018-03-20 14:34:13
> -0700)
>
> I know this pull request needs a respin, but I gave it a test build
> run anyway to see if there were any issues thrown up by that (there
> weren't, which is great).
>
> I did notice that the tag name you quote here,
> "tags/riscv-qemu-2.12-fixes",
> doesn't seem to be a tag pushed to your git repo. I tested with
> "tags/riscv-qemu-2.12-fixes-v5" which I'm guessing is what you meant.
> You probably want to have a look into the workflow which generates
> these pullreq emails though to ensure it's creating them with the
> tag names that you intend, though.
>

I'll make sure I have the version number on the tag in future PRs.

I just sent a v6 series which includes two new bug fixes, and all of the
unreviewed patches as requested by Phil.

It's interesting to note that the reviewed patches are the cleanups which
are not critical and the unreviewed patches are mostly the actual bug
fixes, and likely require a reading of the specification. The bug fixes are
of course harder to review. I might ask around to see if we can find a
reviewer familar with RISC-V. The CSR return 0 vs trap behaviour is
something I discussed with Andrew Waterman and there were issues raised on
the riscv-isa-manual github repo as we are trying to clarify the behaviour
or MMU, no-MMU; S-mode vs no S-mode, etc. I have more fixes planned as we
try to more accurately emulate the MCU class cores with no-MMU but I don't
have the bandwidth at present to make all the required changes. i.e. cores
that don't report misa.S should trap on all s* CSRs but cores with misa.S
with no-MMU should just return 0 for satp (Supervisor Address Translation
Pointer) and ignore writes. The page walker changes are also made after a
closer reading of the specification i.e. handling mstatus.MXR permissions
and various other subtleties. There are also bounds checks for device-tree,
which don't actually show up in practice likely because the physical memory
write routines appear to silently handle bounds overruns. I added some
printfs and discovered the device tree size was larger than the ROM, so the
ROM space has been increased and bounds checks have been added.

And of course we have added the critical workaround for the mstatus.FS
problem, after checking that the workaround behaviour is legal, which it is.

26/26 in the series is what I would refer to as a critical fix. Of course
we'd like to get all of the changes in, as I believe the Fedora folk have
been using QEMU with this series applied. Indeed 26/26 is a derivative of
Richard W.M. Jones fix for the fedora rpm, however I've made it checkpatch
compliant and added comments describing the workaround.

I've cc'd Jim, while he works on GCC, he has helped with some spec
conformance bug fixes in QEMU, although not to do with the page walker. I
had added Palmer's sign-off to some of the patches after speaking with him,
as I sit near to him, so I guess he can't review them, but he could review
the latest fixes which don't have his sign-off. I'll try and find somebody
next week who doesn't mind looking at the Privileged Spec and
riscv-isa-manual issue tracker discussions to review the spec related bug
fixes... It would be nice to get this series in to QEMU 2.12...

Thanks,
Michael.


Re: [Qemu-devel] [PATCH] ccid-card: include libcacard.h only

2018-03-24 Thread Michal Privoznik
On 03/24/2018 12:01 PM, Marc-André Lureau wrote:
> Hi
> 
> On Sat, Mar 24, 2018 at 6:13 AM, Michal Privoznik  wrote:
>> When trying to build with latest libcacard-2.5.1, I hit the
>> following error:
>>
>> In file included from hw/usb/ccid-card-passthru.c:12:0:
>> /usr/include/cacard/vscard_common.h:26:2: error: #warning "Only 
>>  can be included directly" [-Werror=cpp]
>>  #warning "Only  can be included directly"
>>
> 
> The warning was promptly removed in 2.5.2:
> https://cgit.freedesktop.org/spice/libcacard/commit/?id=998db1e88eb8219264476c022d1446f3cb4330e8

Cool. But We can still include just top level header file instead of
individual files, can't we?

Michal



[Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt

2018-03-24 Thread Michael Clark
Remove a potential buffer overflow (not seen in practice).
Perhaps cpu_physical_memory_write already has bound checks.
This change however makes space for the maximum device tree
size and adds an explicit bounds check and error message.
It doesn't trigger, but it may help in the future if the
device-tree size is exceeded. e.g. large bootargs.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 hw/riscv/sifive_u.c | 20 
 hw/riscv/spike.c| 16 +++-
 hw/riscv/virt.c | 13 +
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 083043a..57b4f4f 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -52,7 +52,7 @@ static const struct MemmapEntry {
 hwaddr size;
 } sifive_u_memmap[] = {
 [SIFIVE_U_DEBUG] ={0x0,  0x100 },
-[SIFIVE_U_MROM] = { 0x1000, 0x2000 },
+[SIFIVE_U_MROM] = { 0x1000,0x11000 },
 [SIFIVE_U_CLINT] ={  0x200,0x1 },
 [SIFIVE_U_PLIC] = {  0xc00,  0x400 },
 [SIFIVE_U_UART0] ={ 0x10013000, 0x1000 },
@@ -221,7 +221,7 @@ static void riscv_sifive_u_init(MachineState *machine)
 const struct MemmapEntry *memmap = sifive_u_memmap;
 
 SiFiveUState *s = g_new0(SiFiveUState, 1);
-MemoryRegion *sys_memory = get_system_memory();
+MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
 MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
 
@@ -239,7 +239,7 @@ static void riscv_sifive_u_init(MachineState *machine)
 /* register RAM */
 memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram",
machine->ram_size, &error_fatal);
-memory_region_add_subregion(sys_memory, memmap[SIFIVE_U_DRAM].base,
+memory_region_add_subregion(system_memory, memmap[SIFIVE_U_DRAM].base,
 main_mem);
 
 /* create device tree */
@@ -247,9 +247,9 @@ static void riscv_sifive_u_init(MachineState *machine)
 
 /* boot rom */
 memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
-   memmap[SIFIVE_U_MROM].base, &error_fatal);
-memory_region_set_readonly(mask_rom, true);
-memory_region_add_subregion(sys_memory, 0x0, mask_rom);
+   memmap[SIFIVE_U_MROM].size, &error_fatal);
+memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
+mask_rom);
 
 if (machine->kernel_filename) {
 load_kernel(machine->kernel_filename);
@@ -276,6 +276,10 @@ static void riscv_sifive_u_init(MachineState *machine)
 copy_le32_to_phys(memmap[SIFIVE_U_MROM].base, reset_vec, 
sizeof(reset_vec));
 
 /* copy in the device tree */
+if (s->fdt_size >= memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) {
+error_report("qemu: not enough space to store device-tree");
+exit(1);
+}
 qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
 cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
 sizeof(reset_vec), s->fdt, s->fdt_size);
@@ -293,9 +297,9 @@ static void riscv_sifive_u_init(MachineState *machine)
 SIFIVE_U_PLIC_CONTEXT_BASE,
 SIFIVE_U_PLIC_CONTEXT_STRIDE,
 memmap[SIFIVE_U_PLIC].size);
-sifive_uart_create(sys_memory, memmap[SIFIVE_U_UART0].base,
+sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base,
 serial_hds[0], SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]);
-/* sifive_uart_create(sys_memory, memmap[SIFIVE_U_UART1].base,
+/* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base,
 serial_hds[1], SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */
 sifive_clint_create(memmap[SIFIVE_U_CLINT].base,
 memmap[SIFIVE_U_CLINT].size, smp_cpus,
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 64e585e..c7d937b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -46,7 +46,7 @@ static const struct MemmapEntry {
 hwaddr base;
 hwaddr size;
 } spike_memmap[] = {
-[SPIKE_MROM] = { 0x1000, 0x2000 },
+[SPIKE_MROM] = { 0x1000,0x11000 },
 [SPIKE_CLINT] ={  0x200,0x1 },
 [SPIKE_DRAM] = { 0x8000,0x0 },
 };
@@ -197,8 +197,9 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 
 /* boot rom */
 memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
-   s->fdt_size + 0x2000, &error_fatal);
-memory_region_add_subregion(system_memory, 0x0, mask_rom);
+   memmap[SPIKE_MROM].size, &error_fatal);
+memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
+mask_rom);
 
 if (machine->kernel_filename) {
 load_kernel(machine->kernel_filename);
@@ -225,6 +226,10 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 copy_le32_to_p

[Qemu-devel] [PATCH v6 10/26] RISC-V: Improve page table walker spec compliance

2018-03-24 Thread Michael Clark
- Inline PTE_TABLE check for better readability
- Improve readibility of User page U mode and SUM test
- Disallow non U mode from fetching from User pages
- Add reserved PTE flag check: W or W|X
- Add misaligned PPN check
- Set READ flag for PTE X flag if mstatus.mxr is in effect
- Change access checks from ternary operator to if statements
- Improves page walker comments
- No measurable performance impact on dd test

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/cpu_bits.h |  2 --
 target/riscv/helper.c   | 59 ++---
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 64aa097..12b4757 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -407,5 +407,3 @@
 #define PTE_SOFT  0x300 /* Reserved for Software */
 
 #define PTE_PPN_SHIFT 10
-
-#define PTE_TABLE(PTE) (((PTE) & (PTE_V | PTE_R | PTE_W | PTE_X)) == PTE_V)
diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 02cbcea..9010620 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -185,16 +185,36 @@ restart:
 #endif
 target_ulong ppn = pte >> PTE_PPN_SHIFT;
 
-if (PTE_TABLE(pte)) { /* next level of page table */
+if (!(pte & PTE_V)) {
+/* Invalid PTE */
+return TRANSLATE_FAIL;
+} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
+/* Inner PTE, continue walking */
 base = ppn << PGSHIFT;
-} else if ((pte & PTE_U) ? (mode == PRV_S) && !sum : !(mode == PRV_S)) 
{
-break;
-} else if (!(pte & PTE_V) || (!(pte & PTE_R) && (pte & PTE_W))) {
-break;
-} else if (access_type == MMU_INST_FETCH ? !(pte & PTE_X) :
-  access_type == MMU_DATA_LOAD ?  !(pte & PTE_R) &&
-  !(mxr && (pte & PTE_X)) : !((pte & PTE_R) && (pte & PTE_W))) 
{
-break;
+} else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
+/* Reserved leaf PTE flags: PTE_W */
+return TRANSLATE_FAIL;
+} else if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
+/* Reserved leaf PTE flags: PTE_W + PTE_X */
+return TRANSLATE_FAIL;
+} else if ((pte & PTE_U) && ((mode != PRV_U) &&
+   (!sum || access_type == MMU_INST_FETCH))) {
+/* User PTE flags when not U mode and mstatus.SUM is not set,
+   or the access type is an instruction fetch */
+return TRANSLATE_FAIL;
+} else if (ppn & ((1ULL << ptshift) - 1)) {
+/* Misasligned PPN */
+return TRANSLATE_FAIL;
+} else if (access_type == MMU_DATA_LOAD && !((pte & PTE_R) ||
+   ((pte & PTE_X) && mxr))) {
+/* Read access check failed */
+return TRANSLATE_FAIL;
+} else if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
+/* Write access check failed */
+return TRANSLATE_FAIL;
+} else if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
+/* Fetch access check failed */
+return TRANSLATE_FAIL;
 } else {
 /* if necessary, set accessed and dirty bits. */
 target_ulong updated_pte = pte | PTE_A |
@@ -202,11 +222,14 @@ restart:
 
 /* Page table updates need to be atomic with MTTCG enabled */
 if (updated_pte != pte) {
-/* if accessed or dirty bits need updating, and the PTE is
- * in RAM, then we do so atomically with a compare and swap.
- * if the PTE is in IO space, then it can't be updated.
- * if the PTE changed, then we must re-walk the page table
-   as the PTE is no longer valid */
+/*
+ * - if accessed or dirty bits need updating, and the PTE is
+ *   in RAM, then we do so atomically with a compare and swap.
+ * - if the PTE is in IO space or ROM, then it can't be updated
+ *   and we return TRANSLATE_FAIL.
+ * - if the PTE changed by the time we went to update it, then
+ *   it is no longer valid and we must re-walk the page table.
+ */
 MemoryRegion *mr;
 hwaddr l = sizeof(target_ulong), addr1;
 mr = address_space_translate(cs->as, pte_addr,
@@ -239,15 +262,15 @@ restart:
 target_ulong vpn = addr >> PGSHIFT;
 *physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
 
-if ((pte & PTE_R)) {
+/* set permissions on the TLB entry */
+if ((pte & PTE_R) || (mode != PRV_U && (pte & PTE_X) && mxr)) {
 *prot |= PAGE_READ;
 }
 if ((pte & PTE_X)) {
 *prot |= PAGE_EXEC;
 }
-   

[Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code

2018-03-24 Thread Michael Clark
The sifive_u machine already marks its ROM readonly. This fixes
the remaining boards.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 hw/riscv/sifive_u.c  |  9 +
 hw/riscv/spike.c | 18 ++
 hw/riscv/virt.c  |  7 ---
 include/hw/riscv/spike.h |  8 
 4 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 6116c38..25df16c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState *machine)
 SiFiveUState *s = g_new0(SiFiveUState, 1);
 MemoryRegion *sys_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
 
 /* Initialize SOC */
 object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState *machine)
 create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
 /* boot rom */
-memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
+memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
memmap[SIFIVE_U_MROM].base, &error_fatal);
-memory_region_set_readonly(boot_rom, true);
-memory_region_add_subregion(sys_memory, 0x0, boot_rom);
+memory_region_set_readonly(mask_rom, true);
+memory_region_add_subregion(sys_memory, 0x0, mask_rom);
 
 if (machine->kernel_filename) {
 load_kernel(machine->kernel_filename);
@@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState *machine)
 qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
 cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
 sizeof(reset_vec), s->fdt, s->fdt_size);
+memory_region_set_readonly(mask_rom, true);
 
 /* MMIO */
 s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 7710333..74edf33 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -173,7 +173,7 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 SpikeState *s = g_new0(SpikeState, 1);
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
 
 /* Initialize SOC */
 object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -196,9 +196,9 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
 /* boot rom */
-memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
+memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
s->fdt_size + 0x2000, &error_fatal);
-memory_region_add_subregion(system_memory, 0x0, boot_rom);
+memory_region_add_subregion(system_memory, 0x0, mask_rom);
 
 if (machine->kernel_filename) {
 load_kernel(machine->kernel_filename);
@@ -228,9 +228,10 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
 cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec),
 s->fdt, s->fdt_size);
+memory_region_set_readonly(mask_rom, true);
 
 /* initialize HTIF using symbols found in load_kernel */
-htif_mm_init(system_memory, boot_rom, &s->soc.harts[0].env, serial_hds[0]);
+htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hds[0]);
 
 /* Core Local Interruptor (timer and IPI) */
 sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size,
@@ -244,7 +245,7 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 SpikeState *s = g_new0(SpikeState, 1);
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
 
 /* Initialize SOC */
 object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -264,9 +265,9 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 main_mem);
 
 /* boot rom */
-memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
+memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
0x4, &error_fatal);
-memory_region_add_subregion(system_memory, 0x0, boot_rom);
+memory_region_add_subregion(system_memory, 0x0, mask_rom);
 
 if (machine->kernel_filename) {
 load_kernel(machine->kernel_filename);
@@ -325,9 +326,10 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 /* copy in the config string */
 cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec),
 config_st

[Qemu-devel] [PATCH v6 19/26] RISC-V: vectored traps are optional

2018-03-24 Thread Michael Clark
Vectored traps for asynchrounous interrupts are optional.
The mtvec/stvec mode field is WARL and hence does not trap
if an illegal value is written. Illegal values are ignored.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/op_helper.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f79716a..36b9e8e 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -262,11 +262,10 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 env->sepc = val_to_write;
 break;
 case CSR_STVEC:
-if (val_to_write & 1) {
-qemu_log_mask(LOG_UNIMP, "CSR_STVEC: vectored traps not 
supported");
-goto do_illegal;
+/* we do not support vectored traps for asynchrounous interrupts */
+if ((val_to_write & 3) == 0) {
+env->stvec = val_to_write >> 2 << 2;
 }
-env->stvec = val_to_write >> 2 << 2;
 break;
 case CSR_SCOUNTEREN:
 env->scounteren = val_to_write;
@@ -284,11 +283,10 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 env->mepc = val_to_write;
 break;
 case CSR_MTVEC:
-if (val_to_write & 1) {
-qemu_log_mask(LOG_UNIMP, "CSR_MTVEC: vectored traps not 
supported");
-goto do_illegal;
+/* we do not support vectored traps for asynchrounous interrupts */
+if ((val_to_write & 3) == 0) {
+env->mtvec = val_to_write >> 2 << 2;
 }
-env->mtvec = val_to_write >> 2 << 2;
 break;
 case CSR_MCOUNTEREN:
 env->mcounteren = val_to_write;
-- 
2.7.0




[Qemu-devel] [PATCH v6 23/26] RISC-V: Clear mtval/stval on exceptions without info

2018-03-24 Thread Michael Clark
mtval/stval must be set on all exceptions but zero is
a legal value if there is no exception specific info.
Placing the instruction bytes for illegal instruction
exceptions in mtval/stval is an optional feature and
is currently not supported by QEMU RISC-V.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Palmer Dabbelt 
Signed-off-by: Michael Clark 
---
 target/riscv/helper.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index b2e3f45..0d802a8 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -489,6 +489,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 ": badaddr 0x" TARGET_FMT_lx, env->mhartid, env->badaddr);
 }
 env->sbadaddr = env->badaddr;
+} else {
+/* otherwise we must clear sbadaddr/stval
+ * todo: support populating stval on illegal instructions */
+env->sbadaddr = 0;
 }
 
 target_ulong s = env->mstatus;
@@ -510,6 +514,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 ": badaddr 0x" TARGET_FMT_lx, env->mhartid, env->badaddr);
 }
 env->mbadaddr = env->badaddr;
+} else {
+/* otherwise we must clear mbadaddr/mtval
+ * todo: support populating mtval on illegal instructions */
+env->mbadaddr = 0;
 }
 
 target_ulong s = env->mstatus;
-- 
2.7.0




[Qemu-devel] [PATCH v6 11/26] RISC-V: Update E order and I extension order

2018-03-24 Thread Michael Clark
Section 22.8 Subset Naming Convention of the RISC-V ISA Specification
defines the canonical order for extensions in the ISA string. It is
silent on the position of the E extension however E is a substitute
for I so it must come early in the extension list order. A comment
is added to state E and I are mutually exclusive, as the E extension
will be added to the RISC-V port in the future.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/cpu.c | 2 +-
 target/riscv/cpu.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9de34d7..ad65b39 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -26,7 +26,7 @@
 
 /* RISC-V CPU definitions */
 
-static const char riscv_exts[26] = "IMAFDQECLBJTPVNSUHKORWXYZG";
+static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
 
 const char * const riscv_int_regnames[] = {
   "zero", "ra  ", "sp  ", "gp  ", "tp  ", "t0  ", "t1  ", "t2  ",
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 41e06ac..1fdcd75 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -72,6 +72,7 @@
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 #define RVI RV('I')
+#define RVE RV('E') /* E and I are mutually exclusive */
 #define RVM RV('M')
 #define RVA RV('A')
 #define RVF RV('F')
-- 
2.7.0




[Qemu-devel] [PATCH v6 14/26] RISC-V: Use memory_region_is_ram in pte update

2018-03-24 Thread Michael Clark
After reading cpu_physical_memory_write and friends, it seems
that memory_region_is_ram is a more appropriate interface,
and matches the intent of the code that is calling it.

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 9010620..b2e3f45 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -234,7 +234,7 @@ restart:
 hwaddr l = sizeof(target_ulong), addr1;
 mr = address_space_translate(cs->as, pte_addr,
 &addr1, &l, false);
-if (memory_access_is_direct(mr, true)) {
+if (memory_region_is_ram(mr)) {
 target_ulong *pte_pa =
 qemu_map_ram_ptr(mr->ram_block, addr1);
 #if TCG_OVERSIZED_GUEST
-- 
2.7.0




[Qemu-devel] [PATCH v6 00/26] RISC-V: Fixes and cleanups for QEMU 2.12

2018-03-24 Thread Michael Clark
This is a series of bug fixes and code cleanups that we would
like to get in before the QEMU 2.12 release. We are respinning
v6 of this series to include two new bug fixes. These changes 
are present in the downstream riscv.org riscv-all branch:

- https://github.com/riscv/riscv-qemu/commits/riscv-all

This series also addresses post-merge feedback such as updating
the cpu initialization model to conform with other architectures
as requested by Igor Mammedov.

The riscv_isa_string patch has been dropped as it was merged
independently. The patch to hold rcu_read_lock when accessing
physical memory has been dropped as requested by Paolo Bonzini.

* Implements WARL behavior for CSRs that don't support writes
* Improves specification conformance of the page table walker
  * Change access checks from ternary operator to if statements
  * Checks for misaligned PPNs
  * Disallow M-mode or S-mode from fetching from User pages
  * Adds reserved PTE flag check: W or W|X
  * Set READ flag for PTE X flag if mstatus.mxr is in effect
  * Improves page walker comments and general readability 
* Several trivial code cleanups to hw/riscv
  * Replacing hard coded constants with reference to enums
or the machine memory maps.
  * Remove unnecessary class initialization boilerplate
* Adds bounds checks when writing device-tree to ROM
* Updates the cpu model to use a more modern interface
* Adds hexidecimal instruction bytes to disassembly output
* Sets mtval/stval to zero on exceptions without addresses
* Critical fix for an mstatus.FS bug when MTTCG is enabled
* Fix for incorrect disassembly of addiw instructions

v6

* added workaround for critical mstatus.FS MTTCG bug
* added fix for incorrect disassembly of addiw

v5

* dropped fix for memory allocation bug in riscv_isa_string
* dropped Hold rcu_read_lock when accessing memory

v4

* added fix for memory allocation bug in riscv_isa_string
* trivial fix to remove erroneous comment from translate.c

v3

* refactor rcu_read_lock in PTE update to use single unlock
* mstatus.mxr is in effect regardless of privilege mode
* remove unnecessary class init from riscv_hart
* set mtval/stval to zero on exceptions without addresses

v2

* remove unused class boilerplate retains qom parent_obj
* convert cpu definition towards future model
* honor mstatus.mxr flag in page table walker

v1

* initial post merge cleanup patch series

Michael Clark (26):
  RISC-V: Make virt create_fdt interface consistent
  RISC-V: Replace hardcoded constants with enum values
  RISC-V: Make virt board description match spike
  RISC-V: Use ROM base address and size from memmap
  RISC-V: Remove identity_translate from load_elf
  RISC-V: Mark ROM read-only after copying in code
  RISC-V: Remove unused class definitions
  RISC-V: Make sure rom has space for fdt
  RISC-V: Include intruction hex in disassembly
  RISC-V: Improve page table walker spec compliance
  RISC-V: Update E order and I extension order
  RISC-V: Make some header guards more specific
  RISC-V: Make virt header comment title consistent
  RISC-V: Use memory_region_is_ram in pte update
  RISC-V: Remove EM_RISCV ELF_MACHINE indirection
  RISC-V: Hardwire satp to 0 for no-mmu case
  RISC-V: Remove braces from satp case statement
  RISC-V: riscv-qemu port supports sv39 and sv48
  RISC-V: vectored traps are optional
  RISC-V: No traps on writes to misa,minstret,mcycle
  RISC-V: Remove support for adhoc X_COP interrupt
  RISC-V: Convert cpu definition towards future model
  RISC-V: Clear mtval/stval on exceptions without info
  RISC-V: Remove erroneous comment from translate.c
  RISC-V: Fix incorrect disassembly for addiw
  RISC-V: Workaround for critical mstatus.FS MTTCG bug

 disas/riscv.c   |  41 ++---
 hw/riscv/riscv_hart.c   |   6 --
 hw/riscv/sifive_clint.c |   9 +--
 hw/riscv/sifive_e.c |  34 +--
 hw/riscv/sifive_u.c |  65 +++--
 hw/riscv/spike.c|  65 -
 hw/riscv/virt.c |  77 +
 include/hw/riscv/sifive_clint.h |   4 ++
 include/hw/riscv/sifive_e.h |   5 --
 include/hw/riscv/sifive_u.h |   9 ++-
 include/hw/riscv/spike.h|  15 ++---
 include/hw/riscv/virt.h |  17 +++---
 target/riscv/cpu.c  | 125 ++--
 target/riscv/cpu.h  |   6 +-
 target/riscv/cpu_bits.h |   3 -
 target/riscv/helper.c   |  69 --
 target/riscv/op_helper.c|  71 ++-
 target/riscv/translate.c|   1 -
 18 files changed, 285 insertions(+), 337 deletions(-)

-- 
2.7.0




[Qemu-devel] [PATCH v6 16/26] RISC-V: Hardwire satp to 0 for no-mmu case

2018-03-24 Thread Michael Clark
satp is WARL so it should not trap on illegal writes, rather
it can be hardwired to zero and silently ignore illegal writes.

It seems the RISC-V WARL behaviour is preferred to having to
trap overhead versus simply reading back the value and checking
if the write took (saves hundreds of cycles and more complex
trap handling code).

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/op_helper.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index e34715d..dd3e417 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -242,7 +242,7 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 }
 case CSR_SATP: /* CSR_SPTBR */ {
 if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
-goto do_illegal;
+break;
 }
 if (env->priv_ver <= PRIV_VERSION_1_09_1 && (val_to_write ^ 
env->sptbr))
 {
@@ -452,7 +452,10 @@ target_ulong csr_read_helper(CPURISCVState *env, 
target_ulong csrno)
 return env->scounteren;
 case CSR_SCAUSE:
 return env->scause;
-case CSR_SPTBR:
+case CSR_SATP: /* CSR_SPTBR */
+if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
+return 0;
+}
 if (env->priv_ver >= PRIV_VERSION_1_10_0) {
 return env->satp;
 } else {
-- 
2.7.0




[Qemu-devel] [PATCH v6 24/26] RISC-V: Remove erroneous comment from translate.c

2018-03-24 Thread Michael Clark
Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Palmer Dabbelt 
Signed-off-by: Michael Clark 
---
 target/riscv/translate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 808eab7..c3a029a 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -280,7 +280,6 @@ static void gen_arith(DisasContext *ctx, uint32_t opc, int 
rd, int rs1,
 tcg_gen_andi_tl(source2, source2, 0x1F);
 tcg_gen_sar_tl(source1, source1, source2);
 break;
-/* fall through to SRA */
 #endif
 case OPC_RISC_SRA:
 tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
-- 
2.7.0




[Qemu-devel] [PATCH v6 18/26] RISC-V: riscv-qemu port supports sv39 and sv48

2018-03-24 Thread Michael Clark
Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Michael Clark 
Signed-off-by: Palmer Dabbelt 
---
 target/riscv/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1dcbdbe..cd337ab 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -24,8 +24,8 @@
 #define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
 #if defined(TARGET_RISCV64)
 #define TARGET_LONG_BITS 64
-#define TARGET_PHYS_ADDR_SPACE_BITS 50
-#define TARGET_VIRT_ADDR_SPACE_BITS 39
+#define TARGET_PHYS_ADDR_SPACE_BITS 52
+#define TARGET_VIRT_ADDR_SPACE_BITS 48
 #elif defined(TARGET_RISCV32)
 #define TARGET_LONG_BITS 32
 #define TARGET_PHYS_ADDR_SPACE_BITS 34
-- 
2.7.0




[Qemu-devel] [PATCH v6 25/26] RISC-V: Fix incorrect disassembly for addiw

2018-03-24 Thread Michael Clark
This fixes a bug in the disassembler constraints used
to lift instructions into pseudo-instructions, whereby
addiw instructions are always lifted to sext.w instead
of just lifting addiw with a zero immediate.

An associated fix has been made to the metadata used to
machine generate the disseasembler:

https://github.com/michaeljclark/riscv-meta/
commit/4a6b2f3898430768acfe201405224d2ea31e1477

Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Cc: Palmer Dabbelt 
Cc: Peter Maydell 
Signed-off-by: Michael Clark 
---
 disas/riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 4580308..2cecf0d 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -600,7 +600,7 @@ static const rvc_constraint rvcc_mv[] = { rvc_imm_eq_zero, 
rvc_end };
 static const rvc_constraint rvcc_not[] = { rvc_imm_eq_n1, rvc_end };
 static const rvc_constraint rvcc_neg[] = { rvc_rs1_eq_x0, rvc_end };
 static const rvc_constraint rvcc_negw[] = { rvc_rs1_eq_x0, rvc_end };
-static const rvc_constraint rvcc_sext_w[] = { rvc_rs2_eq_x0, rvc_end };
+static const rvc_constraint rvcc_sext_w[] = { rvc_imm_eq_zero, rvc_end };
 static const rvc_constraint rvcc_seqz[] = { rvc_imm_eq_p1, rvc_end };
 static const rvc_constraint rvcc_snez[] = { rvc_rs1_eq_x0, rvc_end };
 static const rvc_constraint rvcc_sltz[] = { rvc_rs2_eq_x0, rvc_end };
-- 
2.7.0




Re: [Qemu-devel] [PULL 0/4] QAPI patches for 2018-03-23, for 2.12-rc1

2018-03-24 Thread Peter Maydell
On 23 March 2018 at 17:37, Eric Blake  wrote:
> The following changes since commit 4c2c1015905fa1d616750dfe024b4c0b35875950:
>
>   Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20180323' into 
> staging (2018-03-23 10:20:54 +)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-qapi-2018-03-23
>
> for you to fetch changes up to 39615354fc07af34e04ab5efb5b6d478b0d24e32:
>
>   qapi: Force UTF8 encoding when parsing qapi files (2018-03-23 12:29:07 
> -0500)
>
> There may be another rc1 pull request on Monday, but let's get
> these in sooner rather than later to minimize the window where
> iotests are broken during bisects.
>
> 
> qapi patches for 2018-03-12, 2.12-rc1
>
> - Peter Xu: 0/4 Turn OOB off for 2.12-rc1, revert OOB tests
> - Eric Blake: qapi: Force UTF8 encoding when parsing qapi files
>

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH] qemu-pr-helper: Actually allow users to specify pidfile

2018-03-24 Thread Eric Blake

On 03/24/2018 12:14 AM, Michal Privoznik wrote:

Due to wrong specification of arguments to getopt_long() any
attempt to set pidfile resulted in:

1) the default to be leaked
2) the @pidfile variable to be set to NULL (because optarg is
NULL without this patch).


Broken in introduction with commit b855f8d, in 2.11.



Signed-off-by: Michal Privoznik 


CC: qemu-sta...@nongnu.org
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



Re: [Qemu-devel] [PATCH] ccid-card: include libcacard.h only

2018-03-24 Thread Marc-André Lureau
Hi

On Sat, Mar 24, 2018 at 1:40 PM, Michal Privoznik  wrote:
> On 03/24/2018 12:01 PM, Marc-André Lureau wrote:
>> Hi
>>
>> On Sat, Mar 24, 2018 at 6:13 AM, Michal Privoznik  
>> wrote:
>>> When trying to build with latest libcacard-2.5.1, I hit the
>>> following error:
>>>
>>> In file included from hw/usb/ccid-card-passthru.c:12:0:
>>> /usr/include/cacard/vscard_common.h:26:2: error: #warning "Only 
>>>  can be included directly" [-Werror=cpp]
>>>  #warning "Only  can be included directly"
>>>
>>
>> The warning was promptly removed in 2.5.2:
>> https://cgit.freedesktop.org/spice/libcacard/commit/?id=998db1e88eb8219264476c022d1446f3cb4330e8
>
> Cool. But We can still include just top level header file instead of
> individual files, can't we?

Yes, if we bump libcacard version dependency. 2.5.1 (that added
top-level libcacard.h) is from 2015-11-24.



-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH 1/2] hw/sysbus.h: New sysbus_init_child() helper function

2018-03-24 Thread Philippe Mathieu-Daudé
Hi,

On 02/20/2018 10:23 AM, Igor Mammedov wrote:
> On Tue, 20 Feb 2018 13:13:49 +0100
> Paolo Bonzini  wrote:
> 
>> On 16/02/2018 18:40, Philippe Mathieu-Daudé wrote:
>>> we can keep object_initialize() when no parent,
>>> and add object_initialize_child(, const char *childname, Object *parent)
>>> 'parent' last because all previous args are child-related.
>>>   
  
> +qdev_set_parent_bus(DEVICE(child), sysbus_get_default());  
 and then assuming we don't create sysbus devices, nor should be able to,
 which are not attached to sysbus_get_default() this one can go sysbus' 
 instance_init()  
>>> good idea.
>>>   
> [...]
> 
>> I'm not sure about moving qdev_set_parent_bus to instance_init().  It
>> would be a bit different from other buses and possibly confusing.
> That's what this series sort of does, i.e. creating a type/class
> specific helper(s). Which becomes confusing as number of helpers
> increases (frankly it's just 2 different approaches to code i.e.
> functional vs OOM).
> 
> It could be better to keep single QOM API and let SYSBUS base class
> instance_init() to do all implicit initialization that must be done
> for inherited classes.

What is the consensus on this series once 2.12 gets released?

> Yes, it will be different from other devices with buses but
> users won't really care (there is no other buss to assign to),
> for them it will look like a typical bus-less device and it
> would be less error-prone to code.
> 
>  
>> Potentially there could be a "hierarchy" of *_initialize_child
>> functions, adding or removing arguments as needed for the specific kind
>> of bus:
>>
>>  /* adds qdev_set_parent_bus */
>>  device_initialize_child(Object *parent, const char *childname,
>>  BusState *bus, void *data, size_t size,
>>  const char *typename);
>>  /* uses sysbus_get_default() */
>>  sysbus_initialize_child(Object *parent, const char *childname,
>>  void *data, size_t size,
>>  const char *typename);
> 
> 
>>  /* initializes "addr" property too */
>>  pci_initialize_child(Object *parent, const char *childname,
>>   BusState *bus, int addr,
>>   void *data, size_t size,
>>   const char *typename);
> PCI could also incorporate PCI specific bus setting/
> verification logic inside of base class.
> 
> It could allow us to drop bus assigning magic in qdev_device_add(),
> bringing it closer to simple QOM object handling, with specifics
> abstracted by respective TYPE implementations.
> Maybe we would be able to unify -device with -object in the end.

Thanks,

Phil.



Re: [Qemu-devel] [PATCH] monitor: fix expected qmp_capabilities error description regression

2018-03-24 Thread Eric Blake

On 03/23/2018 08:41 PM, Peter Xu wrote:


There have been quite a few patch ideas across multiple threads related to
OOB fallout.  Hopefully I can keep straight which patches are intended for
2.12 (anything that fixes a bug, like this one, is a good candidate,


I'll mark patches with "for-2.12" if there are.


and it
would be nice if we can undo the temporary reversion of exposing OOB if we
can solve all the issues that iotests exposed).


IMHO it'll still be risky considering what has already reported.

Here's my plan, hopefully to make everyone happy - we keep OOB turned
off for 2.12 and even later.  In 2.13, I'll post some new patches to
add a new monitor parameter to allow user to enable OOB explicitly,
otherwise we never enable it.  After all, for now the only real user
should be postcopy. Then we don't need to struggle around all these
mess.  What do you think?


If you're going to add a CLI parameter that must be specified for OOB to 
even be advertised, then it is MUCH less invasive to existing clients 
(it does mean that opting in to OOB now requires the command line 
argument AND the capability request during qmp_capabilities) - as such, 
enabling the opt-in during 2.12 is less controversial, and I see no 
reason to defer it to 2.13, especially if you want to maximize testing 
of the new feature to shake out the bugs it encounters.


If you want to be cautious, name the command-line parameter --x-oob for 
now, we can rename it later to drop the x- prefix, or remove the 
parameter altogether if we decide by opting in via merely 
qmp_capabilities is sufficient.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



Re: [Qemu-devel] [PATCH] ccid-card: include libcacard.h only

2018-03-24 Thread Marc-André Lureau
Hi

On Sat, Mar 24, 2018 at 6:13 AM, Michal Privoznik  wrote:
> When trying to build with latest libcacard-2.5.1, I hit the
> following error:
>
> In file included from hw/usb/ccid-card-passthru.c:12:0:
> /usr/include/cacard/vscard_common.h:26:2: error: #warning "Only  
> can be included directly" [-Werror=cpp]
>  #warning "Only  can be included directly"
>

The warning was promptly removed in 2.5.2:
https://cgit.freedesktop.org/spice/libcacard/commit/?id=998db1e88eb8219264476c022d1446f3cb4330e8

> Signed-off-by: Michal Privoznik 
> ---
>  hw/usb/ccid-card-emulated.c | 5 +
>  hw/usb/ccid-card-passthru.c | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index ea42e4681d..7b538ae6da 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -27,10 +27,7 @@
>   */
>
>  #include "qemu/osdep.h"
> -#include 
> -#include 
> -#include 
> -#include 
> +#include 
>
>  #include "qemu/thread.h"
>  #include "qemu/main-loop.h"
> diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
> index b7dd3602dc..982d575edd 100644
> --- a/hw/usb/ccid-card-passthru.c
> +++ b/hw/usb/ccid-card-passthru.c
> @@ -9,7 +9,7 @@
>   */
>
>  #include "qemu/osdep.h"
> -#include 
> +#include 
>  #include "chardev/char-fe.h"
>  #include "qemu/error-report.h"
>  #include "qemu/sockets.h"
> --
> 2.16.1
>
>



-- 
Marc-André Lureau



[Qemu-devel] [PATCH for-2.12] target/hppa: Include priv level in user-only iaoq

2018-03-24 Thread Richard Henderson
A recent glibc change relies on the fact that the priv level in the iaoq
must be 3, and computes an address based on that.  QEMU had been ignoring
the priv level for user-only, which produced an incorrect address.

Reported-by: John David Anglin 
Signed-off-by: Richard Henderson 
---

I have not set up everything in order to test this vs current glibc,
but since I'm forcing the value upon starting translation, I expect
it to work.  It does at least work with my current sysroot.


r~
---
 target/hppa/cpu.h   |  4 ++--
 target/hppa/translate.c | 12 
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 19dd12a93e..861bbb1f16 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -305,8 +305,8 @@ static inline void cpu_get_tb_cpu_state(CPUHPPAState *env, 
target_ulong *pc,
incomplete virtual address.  This also means that we must separate
out current cpu priviledge from the low bits of IAOQ_F.  */
 #ifdef CONFIG_USER_ONLY
-*pc = env->iaoq_f;
-*cs_base = env->iaoq_b;
+*pc = env->iaoq_f & -4;
+*cs_base = env->iaoq_b & -4;
 #else
 /* ??? E, T, H, L, B, P bits need to be here, when implemented.  */
 flags |= env->psw & (PSW_W | PSW_C | PSW_D);
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 6499b392f9..c532889b1f 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1909,9 +1909,6 @@ static DisasJumpType do_ibranch(DisasContext *ctx, 
TCGv_reg dest,
  */
 static TCGv_reg do_ibranch_priv(DisasContext *ctx, TCGv_reg offset)
 {
-#ifdef CONFIG_USER_ONLY
-return offset;
-#else
 TCGv_reg dest;
 switch (ctx->privilege) {
 case 0:
@@ -1931,7 +1928,6 @@ static TCGv_reg do_ibranch_priv(DisasContext *ctx, 
TCGv_reg offset)
 break;
 }
 return dest;
-#endif
 }
 
 #ifdef CONFIG_USER_ONLY
@@ -1967,7 +1963,7 @@ static DisasJumpType do_page_zero(DisasContext *ctx)
 goto do_sigill;
 }
 
-switch (ctx->iaoq_f) {
+switch (ctx->iaoq_f & -4) {
 case 0x00: /* Null pointer call */
 gen_excp_1(EXCP_IMP);
 return DISAS_NORETURN;
@@ -1978,7 +1974,7 @@ static DisasJumpType do_page_zero(DisasContext *ctx)
 
 case 0xe0: /* SET_THREAD_POINTER */
 tcg_gen_st_reg(cpu_gr[26], cpu_env, offsetof(CPUHPPAState, cr[27]));
-tcg_gen_mov_reg(cpu_iaoq_f, cpu_gr[31]);
+tcg_gen_ori_reg(cpu_iaoq_f, cpu_gr[31], 3);
 tcg_gen_addi_reg(cpu_iaoq_b, cpu_iaoq_f, 4);
 return DISAS_IAQ_N_UPDATED;
 
@@ -4697,8 +4693,8 @@ static int hppa_tr_init_disas_context(DisasContextBase 
*dcbase,
 #ifdef CONFIG_USER_ONLY
 ctx->privilege = MMU_USER_IDX;
 ctx->mmu_idx = MMU_USER_IDX;
-ctx->iaoq_f = ctx->base.pc_first;
-ctx->iaoq_b = ctx->base.tb->cs_base;
+ctx->iaoq_f = ctx->base.pc_first | MMU_USER_IDX;
+ctx->iaoq_b = ctx->base.tb->cs_base | MMU_USER_IDX;
 #else
 ctx->privilege = (ctx->tb_flags >> TB_FLAG_PRIV_SHIFT) & 3;
 ctx->mmu_idx = (ctx->tb_flags & PSW_D ? ctx->privilege : MMU_PHYS_IDX);
-- 
2.14.3




Re: [Qemu-devel] [PATCH v2 0/5] coccinelle: re-run scripts from scripts/coccinelle

2018-03-24 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180323143202.28879-1-lviv...@redhat.com
Subject: [Qemu-devel] [PATCH v2 0/5] coccinelle: re-run scripts from 
scripts/coccinelle

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
84a0e34b6c Remove unnecessary variables for function return value
17594118dc qdict: remove useless cast
92e0e646c2 error: Remove NULL checks on error_propagate() calls
d34ed23173 error: Strip trailing '\n' from error string arguments (again again)
a1d1581ad5 make: improve check for stale generated files in source dir

=== OUTPUT BEGIN ===
Checking PATCH 1/5: make: improve check for stale generated files in source 
dir...
Checking PATCH 2/5: error: Strip trailing '\n' from error string arguments 
(again again)...
Checking PATCH 3/5: error: Remove NULL checks on error_propagate() calls...
Checking PATCH 4/5: qdict: remove useless cast...
Checking PATCH 5/5: Remove unnecessary variables for function return value...
ERROR: return is not a function, parentheses are not required
#253: FILE: target/mips/dsp_helper.c:3281:
+return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#272: FILE: target/mips/dsp_helper.c:3308:
+return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#291: FILE: target/mips/dsp_helper.c:3341:
+return (temp[1] << 63) | (temp[0] >> 1);

total: 3 errors, 0 warnings, 817 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org