Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete

2013-10-19 Thread Kevin Wolf
Am 18.10.2013 um 19:59 hat Max Reitz geschrieben:
 Then there's still the problem that I'm not the one who introduced
 error_propagate. Someone obviously intended some purpose for it, so
 even if it doesn't make a difference now (and my RFC is unneeded),
 I'd still use it to propagate errors (instead of passing the error
 pointer). My point being that there *is* a function for propagating
 errors and I think we should therefore use it. The current qemu code
 seems to alternate between the two alternatives, although using
 error_propagate seems more common to me (at least, that was the
 result when I looked through the code trying to decide whether to
 use it or not).
 
 Generally, we need a proper discussion about whether error_propagate
 is obsolete or not. Afterwards, we can adapt the current codebase to
 the result of that discussion; but until then, I oppose changing
 existing code without actual need to do so but personal preference.
 
 Max
 
 
 PS: I wrote my error_propagate RFC in part because I was
 disappointed about how much of a no-op error_propagate actually is
 and was trying to change that. ;-)

It's not a no-op. The point is that the caller can pass NULL as errp if
it isn't interested in the error message. If you do anything else with
errp than just passing it to other functions - most commonly a
error_is_set() check - you need to make sure that you use a local Error
variable and propagate it. Otherwise, if the caller passed NULL, your
error path would never get executed.

So in this specific case, not having a local_err and error_propagate()
does work, but it's not generally safe. If in doubt, use local_err.

Regarding this patch, I'm not sure it's useful.

Kevin



Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete

2013-10-19 Thread Paolo Bonzini
Il 18/10/2013 19:59, Max Reitz ha scritto:
 Someone obviously intended some purpose for it, so even if it doesn't
 make a difference now (and my RFC is unneeded), I'd still use it to
 propagate errors (instead of passing the error pointer). My point being
 that there *is* a function for propagating errors and I think we should
 therefore use it. The current qemu code seems to alternate between the
 two alternatives, although using error_propagate seems more common to me
 (at least, that was the result when I looked through the code trying to
 decide whether to use it or not).
 
 Generally, we need a proper discussion about whether error_propagate is
 obsolete or not. Afterwards, we can adapt the current codebase to the
 result of that discussion; but until then, I oppose changing existing
 code without actual need to do so but personal preference.

error_propagate is not obsolete.  It is particularly pervasive in
generated code.

You can and should skip error_propagate if you are tail-calling another
function.  In this case, the extra if/error_propagate pair is useless,
makes the code less clear and adds 3-4 useless lines of code.

If you have an alternative way to see whether an error occurred
(typically based on the return value: 0 if it is int, NULL if it is a
pointer), it is fine to use it instead of error_propagate, because
error_propagate adds some complexity to the logic.  It is also fine to
use error_propagate; whatever makes the code simpler.

However, the converse is not true.  If you have a function that returns
void and takes an Error*, it is not okay to make it return int or bool
for the sake of avoiding error_propagate.

There are also cases where you have a return value, but you cannot use
it to ascertain whether an error occurred.  For example, NULL may be a
valid return value even when no error occurs.  In such case, you have to
use error_propagate.

In the end, I agree with Kevin: If in doubt, use local_err.  Tail
calls should be the only case where local_err is clearly unnecessary,
any other case needs to be considered specifically.

Paolo



Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support

2013-10-19 Thread Peter Maydell
On 19 October 2013 00:38, Roy Franz roy.fr...@linaro.org wrote:
Glad to see this go in.  Is your target-arm.next branch available
 in a public repo?

No, not generally.

-- PMM



Re: [Qemu-devel] Build failure with make-4.0

2013-10-19 Thread Peter Maydell
On 19 October 2013 00:36, Ken Moffat zarniwh...@ntlworld.com wrote:
  Seems I can just
 $export ARFLAGS=rv
 before I configure, and it will build and install.  Unless there is
 some reason NOT to do that, please consider this closed.

Well, the reason would be that nobody in practice will do
that. Make should be setting ARFLAGS correctly (as per
its documentation) unless you've somehow managed to
set ARFLAGS to something incorrect in your environment.
If you unset ARFLAGS does it work?

-- PMM



[Qemu-devel] Qemu-devel mailing list submissions

2013-10-19 Thread jun muzi
Qemu-devel mailing list submissions


[Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-19 Thread Jean-Christophe PLAGNIOL-VILLARD
as it's depend on current direction

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
---
 hw/sd/pl181.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 03875bf..91adbbd 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -344,7 +344,11 @@ static uint64_t pl181_read(void *opaque, hwaddr offset,
data engine.  DataCnt is decremented after each byte is
transferred between the serial engine and the card.
We don't emulate this level of detail, so both can be the same.  */
-tmp = (s-datacnt + 3)  2;
+   if (s-datactrl  PL181_DATA_DIRECTION)
+   tmp = s-fifo_len;
+   else
+   tmp = s-datacnt;
+tmp = (tmp + 3)  2;
 if (s-linux_hack) {
 s-linux_hack = 0;
 pl181_fifo_run(s);
-- 
1.8.4.rc3




Re: [Qemu-devel] [PATCH] Make -kernel command line option optional

2013-10-19 Thread Peter Maydell
On 18 October 2013 19:42, Roy Franz roy.fr...@linaro.org wrote:
 From: Grant Likely grant.lik...@linaro.org

 The kernel parameter is not used when booting using firmware
 such as UEFI.  The firmware image is supplied with the -pflash
 parameter, and the -kernel parameter should not be required since
 the kernel will be loaded by the firmware.

I already have a patch to do this queued, thanks.

-- PMM



Re: [Qemu-devel] [PATCH 2/2 v2] Set proper device-width for vexpress flash

2013-10-19 Thread Peter Maydell
On 18 October 2013 19:50, Roy Franz roy.fr...@linaro.org wrote:
 Create vexpress specific pflash registration
 function which properly configures the device-width
 of 16 bits (2 bytes) for the NOR flash on the
 vexpress platform.  This change is required for
 buffered flash writes to work properly.

 +/* Open code a private version of plfash registration since we

pflash

 + * need to set non-default device width for Vexpress platform.
 + */
 +static pflash_t *ve_pflash_cfi01_register(hwaddr base,
 +DeviceState *qdev, const char *name,
 +hwaddr size,
 +BlockDriverState *bs,
 +uint32_t sector_len, int nb_blocs,
 +int bank_width, uint16_t id0, uint16_t id1,
 +uint16_t id2, uint16_t id3, int be)
 +{
 +DeviceState *dev = qdev_create(NULL, cfi.pflash01);
 +
 +if (bs  qdev_prop_set_drive(dev, drive, bs)) {
 +abort();
 +}
 +qdev_prop_set_uint32(dev, num-blocks, nb_blocs);
 +qdev_prop_set_uint64(dev, sector-length, sector_len);
 +qdev_prop_set_uint8(dev, width, bank_width);
 +qdev_prop_set_uint8(dev, device-width, 2);
 +qdev_prop_set_uint8(dev, big-endian, !!be);
 +qdev_prop_set_uint16(dev, id0, id0);
 +qdev_prop_set_uint16(dev, id1, id1);
 +qdev_prop_set_uint16(dev, id2, id2);
 +qdev_prop_set_uint16(dev, id3, id3);
 +qdev_prop_set_string(dev, name, name);
 +qdev_init_nofail(dev);
 +
 +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 +return OBJECT_CHECK(pflash_t, (dev), cfi.pflash01);
 +}
 +
  static void vexpress_common_init(VEDBoardInfo *daughterboard,
   QEMUMachineInitArgs *args)
  {
 @@ -561,7 +594,8 @@ static void vexpress_common_init(VEDBoardInfo 
 *daughterboard,
  sysbus_create_simple(pl111, map[VE_CLCD], pic[14]);

  dinfo = drive_get_next(IF_PFLASH);
 -pflash0 = pflash_cfi01_register(map[VE_NORFLASH0], NULL, 
 vexpress.flash0,
 +pflash0 = ve_pflash_cfi01_register(map[VE_NORFLASH0], NULL,
 +vexpress.flash0,
  VEXPRESS_FLASH_SIZE, dinfo ? dinfo-bdrv : NULL,
  VEXPRESS_FLASH_SECT_SIZE,
  VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
 @@ -580,7 +614,7 @@ static void vexpress_common_init(VEDBoardInfo 
 *daughterboard,
  }

  dinfo = drive_get_next(IF_PFLASH);
 -if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, vexpress.flash1,
 +if (!ve_pflash_cfi01_register(map[VE_NORFLASH1], NULL, vexpress.flash1,
  VEXPRESS_FLASH_SIZE, dinfo ? dinfo-bdrv : NULL,
  VEXPRESS_FLASH_SECT_SIZE,
  VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,

Almost all the parameters you're passing here are the same for both calls,
so it would be better to hard code them in the utility function.

-- PMM



Re: [Qemu-devel] Build failure with make-4.0

2013-10-19 Thread Ken Moffat
On Sat, Oct 19, 2013 at 10:05:10AM +0100, Peter Maydell wrote:
 On 19 October 2013 00:36, Ken Moffat zarniwh...@ntlworld.com wrote:
   Seems I can just
  $export ARFLAGS=rv
  before I configure, and it will build and install.  Unless there is
  some reason NOT to do that, please consider this closed.
 
 Well, the reason would be that nobody in practice will do
 that. Make should be setting ARFLAGS correctly (as per
 its documentation) unless you've somehow managed to
 set ARFLAGS to something incorrect in your environment.
 If you unset ARFLAGS does it work?
 
 -- PMM
 I don't normally touch ARFLAGS.  Here's a freshly-untarred qemu
tree.

ken@ac4tv /scratch/ken/tmp/qemu-1.6.1 $echo $ARFLAGS

ken@ac4tv /scratch/ken/tmp/qemu-1.6.1 $unset ARFLAGS

./configure --prefix=/usr --sysconfdir=/etc --target-list=x86_64-softmmu
make

 AR libfdt/libfdt.a
ar: two different operation options specified
Makefile:234: recipe for target 'libfdt/libfdt.a' failed
make[1]: *** [libfdt/libfdt.a] Error 1
Makefile:153: recipe for target 'subdir-dtc' failed
make: *** [subdir-dtc] Error 2

ĸen
-- 
das eine Mal als Tragödie, dieses Mal als Farce



Re: [Qemu-devel] Patch v3 : POSIX timer implementation for linux-user.

2013-10-19 Thread Erik de Castro Lopo
mle...@mega-nerd.com wrote:

 
 Changes from original:
 
 * Call host's libc functions directly rather than _syscall*() (as suggested
   by Peter Maydell).
 * Remove un-needed #defines.
 
 Launchpad bug is here: https://bugs.launchpad.net/bugs/1042388


Bah! This version segfaults in some circumstances.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/



[Qemu-devel] [Bug 1042388] Re: qemu: Unsupported syscall: 257 (timer_create)

2013-10-19 Thread Erik de Castro Lopo
Bah, the patch in #13 segfaults in some circumstances, the previous one
doesn't.

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

Title:
  qemu: Unsupported syscall: 257 (timer_create)

Status in QEMU:
  Confirmed

Bug description:
  Running qemu-arm-static for git HEAD. When I try to install ghc from
  debian into my arm chroot I get:

  Setting up ghc (7.4.1-4) ...
  qemu: Unsupported syscall: 257
  ghc: timer_create: Function not implemented
  qemu: Unsupported syscall: 257
  ghc-pkg: timer_create: Function not implemented
  dpkg: error processing ghc (--configure):
   subprocess installed post-installation script returned error exit status 1
  Errors were encountered while processing:
   ghc
  E: Sub-process /usr/bin/dpkg returned an error code (1)

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



Re: [Qemu-devel] Patch v3 : POSIX timer implementation for linux-user.

2013-10-19 Thread Erik de Castro Lopo
Erik de Castro Lopo wrote:

 mle...@mega-nerd.com wrote:
 
  
  Changes from original:
  
  * Call host's libc functions directly rather than _syscall*() (as suggested
by Peter Maydell).
  * Remove un-needed #defines.
  
  Launchpad bug is here: https://bugs.launchpad.net/bugs/1042388
 
 
 Bah! This version segfaults in some circumstances.

Double bah! This version (Patch v3) is good. My testing was crap.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/



[Qemu-devel] [PATCH] qapi: fix documentation example

2013-10-19 Thread Eric Blake
The QMP wire format uses , not '', around strings.

* docs/qapi-code-gen.txt: Fix typo.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 docs/qapi-code-gen.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 91f44d0..0728f36 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -164,7 +164,7 @@ This example allows using both of the following example 
objects:
  { file: my_existing_block_device_id }
  { file: { driver: file,
  readonly: false,
- 'filename': /tmp/mydisk.qcow2 } }
+ filename: /tmp/mydisk.qcow2 } }


 === Commands ===
-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2 v3] block: Add device-width property to pflash_cfi01

2013-10-19 Thread Roy Franz
The width of the devices that make up the flash interface
is required to mask certain commands, in particular the
write length for buffered writes.  This length will be presented
to each device on the interface by the program writing the flash,
and the flash emulation code needs to be able to determine
the length of the write as recieved by each flash device.
The device-width defaults to the bank width which should
maintain existing behavior for platforms that don't need
this change.
This change is required to support buffered writes on the
vexpress platform that has a 32 bit flash interface with 2
16 bit devices on it.

Signed-off-by: Roy Franz roy.fr...@linaro.org
---
 hw/block/pflash_cfi01.c |   21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 018a967..cda8289 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -71,7 +71,8 @@ struct pflash_t {
 BlockDriverState *bs;
 uint32_t nb_blocs;
 uint64_t sector_len;
-uint8_t width;
+uint8_t bank_width;
+uint8_t device_width;
 uint8_t be;
 uint8_t wcycle; /* if 0, the flash is read normally */
 int ro;
@@ -126,9 +127,9 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
 ret = -1;
 boff = offset  0xFF; /* why this here ?? */
 
-if (pfl-width == 2)
+if (pfl-bank_width == 2)
 boff = boff  1;
-else if (pfl-width == 4)
+else if (pfl-bank_width == 4)
 boff = boff  2;
 
 #if 0
@@ -378,6 +379,8 @@ static void pflash_write(pflash_t *pfl, hwaddr offset,
 
 break;
 case 0xe8:
+/* Mask writeblock size based on device width */
+value = (1ULL  (pfl-device_width * 8)) - 1;
 DPRINTF(%s: block write of %x bytes\n, __func__, value);
 pfl-counter = value;
 pfl-wcycle++;
@@ -665,7 +668,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl-cfi_table[0x28] = 0x02;
 pfl-cfi_table[0x29] = 0x00;
 /* Max number of bytes in multi-bytes write */
-if (pfl-width == 1) {
+if (pfl-bank_width == 1) {
 pfl-cfi_table[0x2A] = 0x08;
 } else {
 pfl-cfi_table[0x2A] = 0x0B;
@@ -706,7 +709,8 @@ static Property pflash_cfi01_properties[] = {
 DEFINE_PROP_DRIVE(drive, struct pflash_t, bs),
 DEFINE_PROP_UINT32(num-blocks, struct pflash_t, nb_blocs, 0),
 DEFINE_PROP_UINT64(sector-length, struct pflash_t, sector_len, 0),
-DEFINE_PROP_UINT8(width, struct pflash_t, width, 0),
+DEFINE_PROP_UINT8(width, struct pflash_t, bank_width, 0),
+DEFINE_PROP_UINT8(device-width, struct pflash_t, device_width, 0),
 DEFINE_PROP_UINT8(big-endian, struct pflash_t, be, 0),
 DEFINE_PROP_UINT16(id0, struct pflash_t, ident0, 0),
 DEFINE_PROP_UINT16(id1, struct pflash_t, ident1, 0),
@@ -745,8 +749,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
 DeviceState *qdev, const char *name,
 hwaddr size,
 BlockDriverState *bs,
-uint32_t sector_len, int nb_blocs, int width,
-uint16_t id0, uint16_t id1,
+uint32_t sector_len, int nb_blocs,
+int bank_width, uint16_t id0, uint16_t id1,
 uint16_t id2, uint16_t id3, int be)
 {
 DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
@@ -756,7 +760,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
 }
 qdev_prop_set_uint32(dev, num-blocks, nb_blocs);
 qdev_prop_set_uint64(dev, sector-length, sector_len);
-qdev_prop_set_uint8(dev, width, width);
+qdev_prop_set_uint8(dev, width, bank_width);
+qdev_prop_set_uint8(dev, device-width, bank_width);
 qdev_prop_set_uint8(dev, big-endian, !!be);
 qdev_prop_set_uint16(dev, id0, id0);
 qdev_prop_set_uint16(dev, id1, id1);
-- 
1.7.10.4




[Qemu-devel] [PATCH 2/2 v3] block, arm: Set proper device-width for vexpress flash

2013-10-19 Thread Roy Franz
Create vexpress specific pflash registration
function which properly configures the device-width
of 16 bits (2 bytes) for the NOR flash on the
vexpress platform.  This change is required for
buffered flash writes to work properly.

Signed-off-by: Roy Franz roy.fr...@linaro.org
---
 hw/arm/vexpress.c |   43 +--
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index f48de00..8eae73c 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -480,6 +480,35 @@ static void vexpress_modify_dtb(const struct arm_boot_info 
*info, void *fdt)
 }
 }
 
+
+/* Open code a private version of pflash registration since we
+ * need to set non-default device width for VExpress platform.
+ */
+static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
+  BlockDriverState *bs)
+{
+DeviceState *dev = qdev_create(NULL, cfi.pflash01);
+
+if (bs  qdev_prop_set_drive(dev, drive, bs)) {
+abort();
+}
+
+qdev_prop_set_uint32(dev, num-blocks, VEXPRESS_FLASH_SIZE / 
VEXPRESS_FLASH_SECT_SIZE);
+qdev_prop_set_uint64(dev, sector-length, VEXPRESS_FLASH_SECT_SIZE);
+qdev_prop_set_uint8(dev, width, 4);
+qdev_prop_set_uint8(dev, device-width, 2);
+qdev_prop_set_uint8(dev, big-endian, 0);
+qdev_prop_set_uint16(dev, id0, 0x00);
+qdev_prop_set_uint16(dev, id1, 0x89);
+qdev_prop_set_uint16(dev, id2, 0x00);
+qdev_prop_set_uint16(dev, id3, 0x18);
+qdev_prop_set_string(dev, name, name);
+qdev_init_nofail(dev);
+
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+return OBJECT_CHECK(pflash_t, (dev), cfi.pflash01);
+}
+
 static void vexpress_common_init(VEDBoardInfo *daughterboard,
  QEMUMachineInitArgs *args)
 {
@@ -561,11 +590,8 @@ static void vexpress_common_init(VEDBoardInfo 
*daughterboard,
 sysbus_create_simple(pl111, map[VE_CLCD], pic[14]);
 
 dinfo = drive_get_next(IF_PFLASH);
-pflash0 = pflash_cfi01_register(map[VE_NORFLASH0], NULL, vexpress.flash0,
-VEXPRESS_FLASH_SIZE, dinfo ? dinfo-bdrv : NULL,
-VEXPRESS_FLASH_SECT_SIZE,
-VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
-0x00, 0x89, 0x00, 0x18, 0);
+pflash0 = ve_pflash_cfi01_register(map[VE_NORFLASH0], vexpress.flash0,
+   dinfo ? dinfo-bdrv : NULL);
 if (!pflash0) {
 fprintf(stderr, vexpress: error registering flash 0.\n);
 exit(1);
@@ -580,11 +606,8 @@ static void vexpress_common_init(VEDBoardInfo 
*daughterboard,
 }
 
 dinfo = drive_get_next(IF_PFLASH);
-if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, vexpress.flash1,
-VEXPRESS_FLASH_SIZE, dinfo ? dinfo-bdrv : NULL,
-VEXPRESS_FLASH_SECT_SIZE,
-VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
-0x00, 0x89, 0x00, 0x18, 0)) {
+if (!ve_pflash_cfi01_register(map[VE_NORFLASH1], vexpress.flash1,
+  dinfo ? dinfo-bdrv : NULL)) {
 fprintf(stderr, vexpress: error registering flash 1.\n);
 exit(1);
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH 0/2 v3] block, arm: Fix buffered flash writes on VExpress

2013-10-19 Thread Roy Franz
Here is my updated patch to fix buffered flash writes on the VExpress
platform.  Buffered writes should now work properly on platforms whose
flash interface width is different from the device width.  The default
is for the device-width to be set to the interface width, so platforms
that can benefit from this change will need to be updated.  This patchset
updates the configuration for the VExpress platform which requires it.
UEFI firmware uses buffered writes for persistent variable storage,
and this patchset enables this usage on QEMU.

Changes from v2:
(All changes in patch 2/2, 1/1 unchanged.)
* Set flash invariant properties directly in VExpress specific flash init
  routine rather than passing long argument list.
* Fix typo in comment.

Changes from v1:
* Add device-width property and use this to mask write length instead
  of devices mas write length
* Update vexpress init code to set device-width to proper value.

Roy Franz (2):
  Add device-width property to pflash_cfi01
  Set proper device-width for vexpress flash

 hw/arm/vexpress.c   |   43 +--
 hw/block/pflash_cfi01.c |   21 +
 2 files changed, 46 insertions(+), 18 deletions(-)

-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 2/2] acpi-test: basic acpi unit-test

2013-10-19 Thread Michael S. Tsirkin
On Sat, Oct 19, 2013 at 02:13:44AM +0200, Andreas Färber wrote:
 Am 17.10.2013 23:52, schrieb Michael S. Tsirkin:
  diff --git a/tests/acpi-test.c b/tests/acpi-test.c
  new file mode 100644
  index 000..42de248
  --- /dev/null
  +++ b/tests/acpi-test.c
 [...]
  +static void test_acpi_one(const char *params)
  +{
  +char *args;
  +uint8_t signature_low;
  +uint8_t signature_high;
  +uint16_t signature;
  +int i;
  +uint32_t off;
  +
  +
  +args = g_strdup_printf(-net none -display none %s %s,
  +   params ? params : , disk);
  +qtest_start(args);
  +
  +   /* Wait at most 1 minute */
  +#define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
  +#define TEST_CYCLES (60 * G_USEC_PER_SEC / TEST_DELAY)
  +
  +for (i = 0; i  TEST_CYCLES; ++i) {
  +signature_low = readb(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET);
  +signature_high = readb(BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);
  +signature = (signature_high  8) | signature_low;
  +if (signature == SIGNATURE) {
  +break;
  +}
  +g_usleep(TEST_DELAY);
  +}
  +g_assert_cmphex(signature, ==, SIGNATURE);
 
 Might be a good safety precaution to use QEMU_BUG_ON() or MIN(..., 1)
 for TEST_CYCLES to assure signature gets initialized before comparison.

You mean check that TEST_CYCLES  0?

  +
  +/* OK, now find RSDP */
  +for (off = 0xf; off  0x10; off += 0x10)
  +{
  +uint8_t sig[] = RSD PTR ;
  +int i;
  +
  +for (i = 0; i  sizeof sig - 1; ++i) {
  +sig[i] = readb(off + i);
  +}
  +
  +if (!memcmp(sig, RSD PTR , sizeof sig)) {
  +break;
  +}
  +}
  +
  +g_assert_cmphex(off, , 0x10);
  +
  +qtest_quit(global_qtest);
  +g_free(args);
  +}
  +
  +static void test_acpi_tcg(void)
  +{
  +test_acpi_one(-machine accel=tcg);
  +}
  +
  +static void test_acpi_kvm(void)
  +{
  +test_acpi_one(-enable-kvm -machine accel=kvm);
  +}
  +
  +int main(int argc, char *argv[])
  +{
  +const char *arch = qtest_get_arch();
  +FILE *f = fopen(disk, w);
  +fwrite(boot_sector, 1, sizeof boot_sector, f);
  +fclose(f);
  +
  +g_test_init(argc, argv, NULL);
  +
  +if (strcmp(arch, i386) == 0 || strcmp(arch, x86_64) == 0) {
  +qtest_add_func(acpi/tcg, test_acpi_tcg);
  +qtest_add_func(acpi/kvm, test_acpi_kvm);
 
 Sorry, while the intention is good, this is a no-go. Not only will make
 check fail on KVM-incompatible x86 hosts (including insufficient
 permissions for /dev/kvm), it will also fail on ppc or arm hosts since
 we are testing the target architecture here.
 
 Regards,
 Andreas

I'll limit this to tcg for now.

  +}
  +return g_test_run();
  +}
 [snip]
 
 -- 
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete

2013-10-19 Thread Max Reitz

On 2013-10-19 10:05, Kevin Wolf wrote:

Am 18.10.2013 um 19:59 hat Max Reitz geschrieben:

Then there's still the problem that I'm not the one who introduced
error_propagate. Someone obviously intended some purpose for it, so
even if it doesn't make a difference now (and my RFC is unneeded),
I'd still use it to propagate errors (instead of passing the error
pointer). My point being that there *is* a function for propagating
errors and I think we should therefore use it. The current qemu code
seems to alternate between the two alternatives, although using
error_propagate seems more common to me (at least, that was the
result when I looked through the code trying to decide whether to
use it or not).

Generally, we need a proper discussion about whether error_propagate
is obsolete or not. Afterwards, we can adapt the current codebase to
the result of that discussion; but until then, I oppose changing
existing code without actual need to do so but personal preference.

Max


PS: I wrote my error_propagate RFC in part because I was
disappointed about how much of a no-op error_propagate actually is
and was trying to change that. ;-)

It's not a no-op.


That's why I wrote “how much of a” instead of “that error_propagate is a 
no-op”. ;-)



The point is that the caller can pass NULL as errp if
it isn't interested in the error message. If you do anything else with
errp than just passing it to other functions - most commonly a
error_is_set() check - you need to make sure that you use a local Error
variable and propagate it. Otherwise, if the caller passed NULL, your
error path would never get executed.


Okay, together with Paolo's comment I believe I now can see the meaning 
of error_propagate and its intended use; thank you both for explaining.


Max



Re: [Qemu-devel] [PATCH 1/2 v3] block: Add device-width property to pflash_cfi01

2013-10-19 Thread Peter Maydell
On 19 October 2013 18:04, Roy Franz roy.fr...@linaro.org wrote:
 The width of the devices that make up the flash interface
 is required to mask certain commands, in particular the
 write length for buffered writes.  This length will be presented
 to each device on the interface by the program writing the flash,
 and the flash emulation code needs to be able to determine
 the length of the write as recieved by each flash device.
 The device-width defaults to the bank width which should
 maintain existing behavior for platforms that don't need
 this change.
 This change is required to support buffered writes on the
 vexpress platform that has a 32 bit flash interface with 2
 16 bit devices on it.

 Signed-off-by: Roy Franz roy.fr...@linaro.org
 ---
  hw/block/pflash_cfi01.c |   21 +
  1 file changed, 13 insertions(+), 8 deletions(-)

 diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
 index 018a967..cda8289 100644
 --- a/hw/block/pflash_cfi01.c
 +++ b/hw/block/pflash_cfi01.c
 @@ -71,7 +71,8 @@ struct pflash_t {
  BlockDriverState *bs;
  uint32_t nb_blocs;
  uint64_t sector_len;
 -uint8_t width;
 +uint8_t bank_width;

If you want to rename this struct field can you put that in its own patch,
please? Otherwise it's hard to see the actual functional changes.

 +uint8_t device_width;
  uint8_t be;
  uint8_t wcycle; /* if 0, the flash is read normally */
  int ro;
 @@ -126,9 +127,9 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
  ret = -1;
  boff = offset  0xFF; /* why this here ?? */

 -if (pfl-width == 2)
 +if (pfl-bank_width == 2)
  boff = boff  1;
 -else if (pfl-width == 4)
 +else if (pfl-bank_width == 4)
  boff = boff  2;

  #if 0
 @@ -378,6 +379,8 @@ static void pflash_write(pflash_t *pfl, hwaddr offset,

  break;
  case 0xe8:
 +/* Mask writeblock size based on device width */
 +value = (1ULL  (pfl-device_width * 8)) - 1;

Is this really the only guest visible difference for banked flash devices?

  DPRINTF(%s: block write of %x bytes\n, __func__, value);
  pfl-counter = value;
  pfl-wcycle++;

thanks
-- PMM



[Qemu-devel] [PATCH] openrisc-timer: Reduce overhead, Separate clock update functions

2013-10-19 Thread Sebastian Macke

The clock value is only evaluated when really necessary reducing
the overhead of the timer handling.

This also solves a problem in the way the Linux kernel
handles the timer and the expected accuracy.
The old version could lead to inaccurate timings.

Signed-off-by: Sebastian Macke sebast...@macke.de
---
 hw/openrisc/cputimer.c   |   29 +++--
 target-openrisc/cpu.h|1 +
 target-openrisc/sys_helper.c |   38 ++
 3 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index 988ca20..9c54945 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -30,19 +30,28 @@ static int is_counting;
  void cpu_openrisc_count_update(OpenRISCCPU *cpu)
 {
-uint64_t now, next;
-uint32_t wait;
+uint64_t now;
 -now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 if (!is_counting) {
-timer_del(cpu-env.timer);
-last_clk = now;
 return;
 }
-
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 cpu-env.ttcr += (uint32_t)muldiv64(now - last_clk, TIMER_FREQ,
 get_ticks_per_sec());
 last_clk = now;
+}
+
+void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
+{
+uint32_t wait;
+uint64_t now, next;
+
+if (!is_counting) {
+return;
+}
+
+cpu_openrisc_count_update(cpu);
+now = last_clk;
  if ((cpu-env.ttmr  TTMR_TP) = (cpu-env.ttcr  TTMR_TP)) {
 wait = TTMR_TP - (cpu-env.ttcr  TTMR_TP) + 1;
@@ -50,7 +59,6 @@ void cpu_openrisc_count_update(OpenRISCCPU *cpu)
 } else {
 wait = (cpu-env.ttmr  TTMR_TP) - (cpu-env.ttcr  TTMR_TP);
 }
-
 next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
 timer_mod(cpu-env.timer, next);
 }
@@ -63,8 +71,9 @@ void cpu_openrisc_count_start(OpenRISCCPU *cpu)
  void cpu_openrisc_count_stop(OpenRISCCPU *cpu)
 {
-is_counting = 0;
+timer_del(cpu-env.timer);
 cpu_openrisc_count_update(cpu);
+is_counting = 0;
 }
  static void openrisc_timer_cb(void *opaque)
@@ -84,15 +93,15 @@ static void openrisc_timer_cb(void *opaque)
 break;
 case TIMER_INTR:
 cpu-env.ttcr = 0;
-cpu_openrisc_count_start(cpu);
 break;
 case TIMER_SHOT:
 cpu_openrisc_count_stop(cpu);
 break;
 case TIMER_CONT:
-cpu_openrisc_count_start(cpu);
 break;
 }
+
+cpu_openrisc_timer_update(cpu);
 }
  void cpu_openrisc_clock_init(OpenRISCCPU *cpu)
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index 8fd0bc0..0f9efdf 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -373,6 +373,7 @@ void cpu_openrisc_pic_init(OpenRISCCPU *cpu);
 /* hw/openrisc_timer.c */
 void cpu_openrisc_clock_init(OpenRISCCPU *cpu);
 void cpu_openrisc_count_update(OpenRISCCPU *cpu);
+void cpu_openrisc_timer_update(OpenRISCCPU *cpu);
 void cpu_openrisc_count_start(OpenRISCCPU *cpu);
 void cpu_openrisc_count_stop(OpenRISCCPU *cpu);
 diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
index cccbc0e..43fd93f 100644
--- a/target-openrisc/sys_helper.c
+++ b/target-openrisc/sys_helper.c
@@ -127,33 +127,31 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 break;
 case TO_SPR(10, 0): /* TTMR */
 {
+if ((env-ttmr  TTMR_M) ^ (rb  TTMR_M)) {
+switch (rb  TTMR_M) {
+case TIMER_NONE:
+cpu_openrisc_count_stop(cpu);
+break;
+case TIMER_INTR:
+case TIMER_SHOT:
+case TIMER_CONT:
+cpu_openrisc_count_start(cpu);
+break;
+default:
+break;
+}
+   }
+
 int ip = env-ttmr  TTMR_IP;
  if (rb  TTMR_IP) {/* Keep IP bit.  */
-env-ttmr = (rb  ~TTMR_IP) + ip;
+env-ttmr = (rb  ~TTMR_IP) | ip;
 } else {/* Clear IP bit.  */
 env-ttmr = rb  ~TTMR_IP;
 cs-interrupt_request = ~CPU_INTERRUPT_TIMER;
 }
 -cpu_openrisc_count_update(cpu);
-
-switch (env-ttmr  TTMR_M) {
-case TIMER_NONE:
-cpu_openrisc_count_stop(cpu);
-break;
-case TIMER_INTR:
-cpu_openrisc_count_start(cpu);
-break;
-case TIMER_SHOT:
-cpu_openrisc_count_start(cpu);
-break;
-case TIMER_CONT:
-cpu_openrisc_count_start(cpu);
-break;
-default:
-break;
-}
+   cpu_openrisc_timer_update(cpu);
 }
 break;
 @@ -162,7 +160,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 if (env-ttmr  TIMER_NONE) {
 return;
 }
-cpu_openrisc_count_start(cpu);
+cpu_openrisc_timer_update(cpu);
 

[Qemu-devel] [PATCH] target-openrisc: Correct memory bounds checking for the tlb buffers

2013-10-19 Thread Sebastian Macke

The mtspr and mfspr routines didn't check for the correct memory boundaries.
This fixes a segmentation fault while booting Linux.

Signed-off-by: Sebastian Macke sebast...@macke.de
---
 target-openrisc/sys_helper.c |   16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
index 43fd93f..cad1b96 100644
--- a/target-openrisc/sys_helper.c
+++ b/target-openrisc/sys_helper.c
@@ -81,7 +81,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 case TO_SPR(0, 64): /* ESR */
 env-esr = rb;
 break;
-case TO_SPR(1, 512) ... TO_SPR(1, 639): /* DTLBW0MR 0-127 */
+case TO_SPR(1, 512) ... TO_SPR(1, 512+DTLB_SIZE-1): /* DTLBW0MR 
0-127 */

 idx = spr - TO_SPR(1, 512);
 if (!(rb  1)) {
 tlb_flush_page(env, env-tlb-dtlb[0][idx].mr  
TARGET_PAGE_MASK);

@@ -89,7 +89,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 env-tlb-dtlb[0][idx].mr = rb;
 break;
 -case TO_SPR(1, 640) ... TO_SPR(1, 767): /* DTLBW0TR 0-127 */
+case TO_SPR(1, 640) ... TO_SPR(1, 640+DTLB_SIZE-1): /* DTLBW0TR 
0-127 */

 idx = spr - TO_SPR(1, 640);
 env-tlb-dtlb[0][idx].tr = rb;
 break;
@@ -100,7 +100,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 case TO_SPR(1, 1280) ... TO_SPR(1, 1407): /* DTLBW3MR 0-127 */
 case TO_SPR(1, 1408) ... TO_SPR(1, 1535): /* DTLBW3TR 0-127 */
 break;
-case TO_SPR(2, 512) ... TO_SPR(2, 639):   /* ITLBW0MR 0-127 */
+case TO_SPR(2, 512) ... TO_SPR(2, 512+ITLB_SIZE-1):   /* ITLBW0MR 
0-127 */

 idx = spr - TO_SPR(2, 512);
 if (!(rb  1)) {
 tlb_flush_page(env, env-tlb-itlb[0][idx].mr  
TARGET_PAGE_MASK);

@@ -108,7 +108,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
 env-tlb-itlb[0][idx].mr = rb;
 break;
 -case TO_SPR(2, 640) ... TO_SPR(2, 767): /* ITLBW0TR 0-127 */
+case TO_SPR(2, 640) ... TO_SPR(2, 640+ITLB_SIZE-1): /* ITLBW0TR 
0-127 */

 idx = spr - TO_SPR(2, 640);
 env-tlb-itlb[0][idx].tr = rb;
 break;
@@ -212,11 +212,11 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
 case TO_SPR(0, 64): /* ESR */
 return env-esr;
 -case TO_SPR(1, 512) ... TO_SPR(1, 639): /* DTLBW0MR 0-127 */
+case TO_SPR(1, 512) ... TO_SPR(1, 512+DTLB_SIZE-1): /* DTLBW0MR 
0-127 */

 idx = spr - TO_SPR(1, 512);
 return env-tlb-dtlb[0][idx].mr;
 -case TO_SPR(1, 640) ... TO_SPR(1, 767): /* DTLBW0TR 0-127 */
+case TO_SPR(1, 640) ... TO_SPR(1, 640+DTLB_SIZE-1): /* DTLBW0TR 
0-127 */

 idx = spr - TO_SPR(1, 640);
 return env-tlb-dtlb[0][idx].tr;
 @@ -228,11 +228,11 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
 case TO_SPR(1, 1408) ... TO_SPR(1, 1535): /* DTLBW3TR 0-127 */
 break;
 -case TO_SPR(2, 512) ... TO_SPR(2, 639): /* ITLBW0MR 0-127 */
+case TO_SPR(2, 512) ... TO_SPR(2, 512+ITLB_SIZE-1): /* ITLBW0MR 
0-127 */

 idx = spr - TO_SPR(2, 512);
 return env-tlb-itlb[0][idx].mr;
 -case TO_SPR(2, 640) ... TO_SPR(2, 767): /* ITLBW0TR 0-127 */
+case TO_SPR(2, 640) ... TO_SPR(2, 640+ITLB_SIZE-1): /* ITLBW0TR 
0-127 */

 idx = spr - TO_SPR(2, 640);
 return env-tlb-itlb[0][idx].tr;
 -- 1.7.9.5




Re: [Qemu-devel] [RFC v5 2/5] hw/arm/digic: prepare DIGIC-based boards support

2013-10-19 Thread Georg Hofstetter
Am 17.10.2013 21:17, schrieb Peter Maydell:
 
  - make sure the flash emulation supports reflashing (properties)
  - change qemu memory subsystem to support execution from a flash that
can be reprogrammed (properties are rewritten during startup)
(maybe this is already possible, but it wasn't so 6 months ago)
 
 I agree that these are probably missing features in our flash
 emulation, but aren't they orthogonal to the question of how
 we handle CPU reset and what the starting PC should be?
 

Hi Peter,

absolutely - this was just the whole list of behavior to be implemented
and/or emulated to get the emulator close to real hardware.
Its just something that would prevent a clean firmware boot and came to
my mind while writing about system startup.

So yeah, its a bit off topic :)

Regards,
Georg





[Qemu-devel] [PATCH] target-xtensa: add missing DEBUG section to dc233c config

2013-10-19 Thread Max Filippov
This fixes missing debug feature opcodes of dc233c core variant.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Filippov jcmvb...@gmail.com
---
 target-xtensa/core-dc233c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-xtensa/core-dc233c.c b/target-xtensa/core-dc233c.c
index 11acbf3..738d543 100644
--- a/target-xtensa/core-dc233c.c
+++ b/target-xtensa/core-dc233c.c
@@ -49,6 +49,7 @@ static const XtensaConfig dc233c = {
 EXCEPTIONS_SECTION,
 INTERRUPTS_SECTION,
 TLB_SECTION,
+DEBUG_SECTION,
 .clock_freq_khz = 1,
 };
 
-- 
1.8.1.4




[Qemu-devel] [PATCH] target-openrisc: Separate branch flag from Supervision register

2013-10-19 Thread Sebastian Macke

The branch flag is very often used. To increase the speed
the flag is separated. This patch removes several
ands and ors and branches from the generated code.
The additional flag btaken is no longer necessary.

Signed-off-by: Sebastian Macke sebast...@macke.de
---
 target-openrisc/cpu.c  |1 +
 target-openrisc/cpu.h  |   13 +++--
 target-openrisc/gdbstub.c  |4 +-
 target-openrisc/interrupt.c|2 +-
 target-openrisc/interrupt_helper.c |2 +-
 target-openrisc/machine.c  |1 +
 target-openrisc/sys_helper.c   |6 +--
 target-openrisc/translate.c|   97 
++--

 8 files changed, 56 insertions(+), 70 deletions(-)

diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 8137943..09ba728 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -42,6 +42,7 @@ static void openrisc_cpu_reset(CPUState *s)
  cpu-env.pc = 0x100;
 cpu-env.sr = SR_FO | SR_SM;
+cpu-env.srf = 0;
 cpu-env.exception_index = -1;
  cpu-env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index 0f9efdf..5cacc0d 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -272,6 +272,14 @@ typedef struct CPUOpenRISCTLBContext {
 } CPUOpenRISCTLBContext;
 #endif
 +/* Helper for the supervision register */
+#define ENV_GET_SR(env) (((env)-sr~SR_F) | ((env)-srf?SR_F:0))
+
+#define ENV_SET_SR(env, srtemp)   do {\
+  (env)-sr = ((srtemp)  ~SR_F) | 
SR_FO;\

+  (env)-srf = (srtemp)  SR_F;\
+  } while (0)
+
 typedef struct CPUOpenRISCState {
 target_ulong gpr[32]; /* General registers */
 target_ulong pc;  /* Program counter */
@@ -288,7 +296,8 @@ typedef struct CPUOpenRISCState {
 target_ulong epcr;/* Exception PC register */
 target_ulong eear;/* Exception EA register */
 -uint32_t sr;  /* Supervisor register */
+uint32_t sr;  /* Supervision register */
+uint32_t srf; /* separated branch flag of Supervision 
register*/

 uint32_t vr;  /* Version register */
 uint32_t upr; /* Unit presence register */
 uint32_t cpucfgr; /* CPU configure register */
@@ -300,8 +309,6 @@ typedef struct CPUOpenRISCState {
  uint32_t flags;   /* cpu_flags, we only use it for exception
  in solt so far.  */
-uint32_t btaken;  /* the SR_F bit */
-
 CPU_COMMON
  #ifndef CONFIG_USER_ONLY
diff --git a/target-openrisc/gdbstub.c b/target-openrisc/gdbstub.c
index 18bcc46..81acf2d 100644
--- a/target-openrisc/gdbstub.c
+++ b/target-openrisc/gdbstub.c
@@ -37,7 +37,7 @@ int openrisc_cpu_gdb_read_register(CPUState *cs, 
uint8_t *mem_buf, int n)

 return gdb_get_reg32(mem_buf, env-npc);
  case 34:/* SR */
-return gdb_get_reg32(mem_buf, env-sr);
+return gdb_get_reg32(mem_buf, ENV_GET_SR(env));
  default:
 break;
@@ -72,7 +72,7 @@ int openrisc_cpu_gdb_write_register(CPUState *cs, 
uint8_t *mem_buf, int n)

 break;
  case 34: /* SR */
-env-sr = tmp;
+ENV_SET_SR(env, tmp);
 break;
  default:
diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
index 2153e7e..d1d6ae2 100644
--- a/target-openrisc/interrupt.c
+++ b/target-openrisc/interrupt.c
@@ -45,7 +45,7 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
we need flush TLB when we enterexit EXCP.  */
 tlb_flush(env, 1);
 -env-esr = env-sr;
+env-esr = ENV_GET_SR(env);
 env-sr = ~SR_DME;
 env-sr = ~SR_IME;
 env-sr |= SR_SM;
diff --git a/target-openrisc/interrupt_helper.c 
b/target-openrisc/interrupt_helper.c

index 844648f..8a07b09 100644
--- a/target-openrisc/interrupt_helper.c
+++ b/target-openrisc/interrupt_helper.c
@@ -31,7 +31,7 @@ void HELPER(rfe)(CPUOpenRISCState *env)
 #endif
 cpu-env.pc = cpu-env.epcr;
 cpu-env.npc = cpu-env.epcr;
-cpu-env.sr = cpu-env.esr;
+ENV_SET_SR((cpu-env), cpu-env.esr);
  #ifndef CONFIG_USER_ONLY
 if (cpu-env.sr  SR_DME) {
diff --git a/target-openrisc/machine.c b/target-openrisc/machine.c
index 6f864fe..2bdd40f 100644
--- a/target-openrisc/machine.c
+++ b/target-openrisc/machine.c
@@ -28,6 +28,7 @@ static const VMStateDescription vmstate_env = {
 .fields = (VMStateField[]) {
 VMSTATE_UINT32_ARRAY(gpr, CPUOpenRISCState, 32),
 VMSTATE_UINT32(sr, CPUOpenRISCState),
+VMSTATE_UINT32(srf, CPUOpenRISCState),
 VMSTATE_UINT32(epcr, CPUOpenRISCState),
 VMSTATE_UINT32(eear, CPUOpenRISCState),
 VMSTATE_UINT32(esr, CPUOpenRISCState),
diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
index cad1b96..687f899 100644
--- a/target-openrisc/sys_helper.c