Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete
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
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
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
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
Qemu-devel mailing list submissions
[Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support
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
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
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
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.
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)
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.
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
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
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
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
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
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
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
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
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
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
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
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
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