[Qemu-devel] [PATCH] arm: allow machine IDs > 0xffff
From: Jeremy Kerr
Signed-off-by: Jeremy Kerr
Cc: Paul Brook
Cc: Peter Maydell
Cc: Rob Herring
Signed-off-by: Grant Likely
---
hw/arm_boot.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index de8f1a4..f2b7332 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -26,10 +26,10 @@
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
static uint32_t bootloader[] = {
0xe3a0, /* mov r0, #0 */
- 0xe3a01000, /* mov r1, #0x?? */
- 0xe3811c00, /* orr r1, r1, #0x??00 */
- 0xe59f2000, /* ldr r2, [pc, #0] */
- 0xe59ff000, /* ldr pc, [pc, #0] */
+ 0xe59f1004, /* ldr r1, [pc, #4] */
+ 0xe59f2004, /* ldr r2, [pc, #4] */
+ 0xe59ff004, /* ldr pc, [pc, #4] */
+ 0, /* Machine ID */
0, /* Address of kernel args. Set by integratorcp_init. */
0 /* Kernel entry point. Set by integratorcp_init. */
};
@@ -327,8 +327,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
} else {
initrd_size = 0;
}
-bootloader[1] |= info->board_id & 0xff;
-bootloader[2] |= (info->board_id >> 8) & 0xff;
+bootloader[4] = info->board_id;
/* for device tree boot, we pass the DTB directly in r2. Otherwise
* we point to the kernel args */
if (info->dtb_filename)
--
1.7.5.4
Re: [Qemu-devel] [PATCH v12 4/4] arm: SoC model for Calxeda Highbank
On Fri, Jan 20, 2012 at 07:48:09AM -0600, Rob Herring wrote: > On 01/20/2012 02:47 AM, Peter Maydell wrote: > > On 19 January 2012 23:17, Rob Herring wrote: > >> On 01/19/2012 03:44 PM, Peter Maydell wrote: > >>> On 19 January 2012 21:31, Mark Langsdorf > >>> wrote: > +highbank_binfo.board_id = 0xEC10100f; /* provided by deviceTree */ > >>> > >>> Where does this number come from? It's not in > >>> http://www.arm.linux.org.uk/developer/machines/ > >>> > >>> Is 3027 (==0xbd3) you? > >>> http://www.arm.linux.org.uk/developer/machines/list.php?id=3027 > >>> > >> > >> Much of the data there is wrong as none of it is used. 0 or -1 is the > >> right value as those are obviously meaningless. A highbank kernel will > >> never be booted without devicetree and in that case this number is > >> irrelevant. This is the legacy boot interface and qemu really needs to > >> learn to boot with a separate dtb. > > > > Yeah, but the documentation even for DTB boot says we should pass > > in a machine number. If 0 or -1 are right then there should be > > some documentation that says so. I'll accept "mailing list post > > from some authoritative person [eg Grant Likely]" if necessary. > > Kernel DT co-maintainer is not authoritative enough for you? > > The documentation needs some clarification. > > > But this is an ABI between boot loaders and the kernel so I don't > > want to just have something random that happens to work. (And in > > particular if -1 is the officially sanctioned number then we need > > to fix arm_boot to be able to pass values >16 bits wide.) > > I've got a patch that fixes arm_boot. I'll send it separately. How about this patch for the kernel: g. --- diff --git a/Documentation/arm/Booting b/Documentation/arm/Booting index a341d87..72b064d 100644 --- a/Documentation/arm/Booting +++ b/Documentation/arm/Booting @@ -148,7 +148,9 @@ In either case, the following conditions must be met: - CPU register settings r0 = 0, - r1 = machine type number discovered in (3) above. + r1 = machine type number discovered in (3) above, or if booting with + a dtb then this may be set to ~0 if a valid MACH_TYPE_xxx value + does not exist for the machine. r2 = physical address of tagged list in system RAM, or physical address of device tree block (dtb) in system RAM diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt index 7c1329d..33e2b51 100644 --- a/Documentation/devicetree/booting-without-of.txt +++ b/Documentation/devicetree/booting-without-of.txt @@ -176,7 +176,8 @@ it with special cases. r1 : Valid machine type number. When using a device tree, a single machine type number will often be assigned to -represent a class or family of SoCs. +represent a class or family of SoCs. If a valid machine +type number is not assigned, then use ~0. r2 : physical pointer to the device-tree block (defined in chapter II) in RAM. Device tree can be located
Re: [Qemu-devel] [PATCH v12 4/4] arm: SoC model for Calxeda Highbank
On Fri, Jan 20, 2012 at 01:57:29PM +, Peter Maydell wrote:
> On 20 January 2012 13:48, Rob Herring wrote:
> > Kernel DT co-maintainer is not authoritative enough for you?
>
> Only if I recognise their name :-) [ie, sorry.]
>
> > The documentation needs some clarification.
> >
> >> But this is an ABI between boot loaders and the kernel so I don't
> >> want to just have something random that happens to work. (And in
> >> particular if -1 is the officially sanctioned number then we need
> >> to fix arm_boot to be able to pass values >16 bits wide.)
> >>
> >
> > Here's were the kernel sets the mach #. nr is from the database for
> > non-DT and ~0 for DT machines.
> >
> > #define MACHINE_START(_type,_name) \
> > static const struct machine_desc __mach_desc_##_type \
> > __used \
> > __attribute__((__section__(".arch.info.init"))) = { \
> > .nr = MACH_TYPE_##_type, \
> > .name = _name,
> >
> > #define MACHINE_END \
> > };
> >
> > #define DT_MACHINE_START(_name, _namestr) \
> > static const struct machine_desc __mach_desc_##_name \
> > __used \
> > __attribute__((__section__(".arch.info.init"))) = { \
> > .nr = ~0, \
> > .name = _namestr,
> >
> > In any case, the kernel ignores the value passed in if a valid dtb is
> > passed in.
>
> I wonder if we should be passing in anything-except-minus-1,
> since if you pass -1 and no DT then the kernel will fail
> silently, whereas if you pass something else and no DT the
> kernel will complain about the mismatch.
Alternately, we can make the kernel always complain about machine type
~0, which is probably safer anyway.
g.
Re: [Qemu-devel] [PATCH] arm: allow machine IDs > 0xffff
On Fri, Jan 20, 2012 at 11:43 AM, Grant Likely
wrote:
> From: Jeremy Kerr
>
> Signed-off-by: Jeremy Kerr
> Cc: Paul Brook
> Cc: Peter Maydell
> Cc: Rob Herring
> Signed-off-by: Grant Likely
Oops, this one won't apply directly since it's build on top of another
patch. I'll repost a clean version soon.
g.
> ---
> hw/arm_boot.c | 11 +--
> 1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index de8f1a4..f2b7332 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -26,10 +26,10 @@
> /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel.
> */
> static uint32_t bootloader[] = {
> 0xe3a0, /* mov r0, #0 */
> - 0xe3a01000, /* mov r1, #0x?? */
> - 0xe3811c00, /* orr r1, r1, #0x??00 */
> - 0xe59f2000, /* ldr r2, [pc, #0] */
> - 0xe59ff000, /* ldr pc, [pc, #0] */
> + 0xe59f1004, /* ldr r1, [pc, #4] */
> + 0xe59f2004, /* ldr r2, [pc, #4] */
> + 0xe59ff004, /* ldr pc, [pc, #4] */
> + 0, /* Machine ID */
> 0, /* Address of kernel args. Set by integratorcp_init. */
> 0 /* Kernel entry point. Set by integratorcp_init. */
> };
> @@ -327,8 +327,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
> *info)
> } else {
> initrd_size = 0;
> }
> - bootloader[1] |= info->board_id & 0xff;
> - bootloader[2] |= (info->board_id >> 8) & 0xff;
> + bootloader[4] = info->board_id;
> /* for device tree boot, we pass the DTB directly in r2. Otherwise
> * we point to the kernel args */
> if (info->dtb_filename)
> --
> 1.7.5.4
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[Qemu-devel] [PATCH] Add configuration variables for iscsi
Kevin, List This is version 2 of the patch to add configuration variables for iSCSI. This version adds the feature to specify configuration blocks that apply to a specific target name, allowing qemu to use different settings if/when connecting one guest to multiple different iscsi targets. This patch adds configuration variables for iSCSI to set initiator-name to use when logging in to the target, which type of header-digest to negotiate with the target and username and password for CHAP authentication. This allows specifying a initiator-name either from the command line -iscsi initiator-name=iqn.2004-01.com.example:test or from a configuration file included with -readconfig [iscsi] initiator-name = iqn.2004-01.com.example:test header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE user = CHAP username password = CHAP password In the configuration file it is also possible to set a target specific configuratyion block using the header [iscsi "iqn.target.name"] When a iscsi session is initialized, it will first try to use a configuration section that matches the target name. If no such block is found, it will fall-back to try the default [iscsi] section instead. regards ronnie sahlberg
[Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI
This patch adds configuration variables for iSCSI to set
initiator-name to use when logging in to the target,
which type of header-digest to negotiate with the target
and username and password for CHAP authentication.
This allows specifying a initiator-name either from the command line
-iscsi initiator-name=iqn.2004-01.com.example:test
or from a configuration file included with -readconfig
[iscsi]
initiator-name = iqn.2004-01.com.example:test
header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
user = CHAP username
password = CHAP password
If you use several different targets, you can also configure this on a per
target basis by using a group name:
[iscsi "iqn.target.name"]
...
Signed-off-by: Ronnie Sahlberg
---
block/iscsi.c | 139 +++
qemu-config.c | 27 +++
qemu-doc.texi | 38 +++-
qemu-options.hx | 16 +--
vl.c|8 +++
5 files changed, 213 insertions(+), 15 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 938c568..bd3ca11 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -455,6 +455,109 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status,
void *command_data,
}
}
+static int parse_chap(struct iscsi_context *iscsi, const char *target)
+{
+QemuOptsList *list;
+QemuOpts *opts;
+const char *user = NULL;
+const char *password = NULL;
+
+list = qemu_find_opts("iscsi");
+if (!list) {
+return 0;
+}
+
+opts = qemu_opts_find(list, target);
+if (opts == NULL) {
+opts = QTAILQ_FIRST(&list->head);
+if (!opts) {
+return 0;
+}
+}
+
+user = qemu_opt_get(opts, "user");
+if (!user) {
+return 0;
+}
+
+password = qemu_opt_get(opts, "password");
+if (!password) {
+error_report("CHAP username specified but no password was given");
+return -1;
+}
+
+if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
+error_report("Failed to set initiator username and password");
+return -1;
+}
+
+return 0;
+}
+
+static void parse_header_digest(struct iscsi_context *iscsi, const char
*target)
+{
+QemuOptsList *list;
+QemuOpts *opts;
+const char *digest = NULL;
+
+list = qemu_find_opts("iscsi");
+if (!list) {
+return;
+}
+
+opts = qemu_opts_find(list, target);
+if (opts == NULL) {
+opts = QTAILQ_FIRST(&list->head);
+if (!opts) {
+return;
+}
+}
+
+digest = qemu_opt_get(opts, "header-digest");
+if (!digest) {
+return;
+}
+
+if (!strcmp(digest, "CRC32C")) {
+iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
+} else if (!strcmp(digest, "NONE")) {
+iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
+} else if (!strcmp(digest, "CRC32C-NONE")) {
+iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
+} else if (!strcmp(digest, "NONE-CRC32C")) {
+iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+} else {
+error_report("Invalid header-digest setting : %s", digest);
+}
+}
+
+static char *parse_initiator_name(const char *target)
+{
+QemuOptsList *list;
+QemuOpts *opts;
+const char *name = NULL;
+
+list = qemu_find_opts("iscsi");
+if (!list) {
+return g_strdup("iqn.2008-11.org.linux-kvm");
+}
+
+opts = qemu_opts_find(list, target);
+if (opts == NULL) {
+opts = QTAILQ_FIRST(&list->head);
+if (!opts) {
+return g_strdup("iqn.2008-11.org.linux-kvm");
+}
+}
+
+name = qemu_opt_get(opts, "initiator-name");
+if (!name) {
+return g_strdup("iqn.2008-11.org.linux-kvm");
+}
+
+return g_strdup(name);
+}
+
/*
* We support iscsi url's on the form
* iscsi://[%@][:]//
@@ -465,6 +568,7 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
struct iscsi_context *iscsi = NULL;
struct iscsi_url *iscsi_url = NULL;
struct IscsiTask task;
+char *initiator_name = NULL;
int ret;
if ((BDRV_SECTOR_SIZE % 512) != 0) {
@@ -474,16 +578,6 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
return -EINVAL;
}
-memset(iscsilun, 0, sizeof(IscsiLun));
-
-/* Should really append the KVM name after the ':' here */
-iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
-if (iscsi == NULL) {
-error_report("iSCSI: Failed to create iSCSI context.");
-ret = -ENOMEM;
-goto failed;
-}
-
iscsi_url = iscsi_parse_full_url(iscsi, filename);
if (iscsi_url == NULL) {
error_report("Failed to parse URL : %s %s", filename,
@@ -492,6 +586,17 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
goto failed;
}
+memset(iscsilun, 0, sizeof(IscsiLun
Re: [Qemu-devel] vhost broken?
On 21.01.2012 02:13, Lutz Vieweg wrote: > On 01/05/2012 04:00 PM, Michael S. Tsirkin wrote: >> Just chmod /dev/vhost-net to allow access > > I wonder whether it's considered a security risk to allow non-root > users access to /dev/vhost-net? > > (Or is there a way to have root prepare limited use of vhost for only > some users or some network devices?) Usual idiom is to create a dedicated group, chgrp /dev/vhost-net to this group and add g+rw permission. That lets you to add users to that group to grant them access. Initially it was believed that /dev/kvm poses no security implications whatsoever and can be mode 0666. But later on several security bugs has been discovered in this subsystem, so it is a good idea to not grant extra privileges by default. /mjt
[Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)
The new block was filled with zero when it was allocated by g_malloc0,
but when it was reused later and only partially used, data from the
previously allocated block were still present and written to the new
block.
This caused the problems reported by bug #919242
(https://bugs.launchpad.net/qemu/+bug/919242).
Now the unused parts of the new block which are before and after the data
are always filled with zero, so it is no longer necessary to zero the whole
block with g_malloc0.
I also updated the copyright comment.
Signed-off-by: Stefan Weil
---
block/vdi.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 31cdfab..6a0011f 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -1,7 +1,7 @@
/*
* Block driver for the Virtual Disk Image (VDI) format
*
- * Copyright (c) 2009 Stefan Weil
+ * Copyright (c) 2009, 2012 Stefan Weil
*
* 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
@@ -756,15 +756,19 @@ static void vdi_aio_write_cb(void *opaque, int ret)
(uint64_t)bmap_entry * s->block_sectors;
block = acb->block_buffer;
if (block == NULL) {
-block = g_malloc0(s->block_size);
+block = g_malloc(s->block_size);
acb->block_buffer = block;
acb->bmap_first = block_index;
assert(!acb->header_modified);
acb->header_modified = 1;
}
acb->bmap_last = block_index;
+/* Copy data to be written to new block and zero unused parts. */
+memset(block, 0, sector_in_block * SECTOR_SIZE);
memcpy(block + sector_in_block * SECTOR_SIZE,
acb->buf, n_sectors * SECTOR_SIZE);
+memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
+ (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
acb->hd_iov.iov_base = (void *)block;
acb->hd_iov.iov_len = s->block_size;
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
--
1.7.7.3
[Qemu-devel] [Bug 919242] Re: qemu-img convert to VDI corrupts image
There is a bug in the VDI code of all current QEMU versions:
when two new blocks (1 MiB clusters) were allocated in sequence and the
2nd new block was only partially filled with data, the unused parts of the
new block still contained data of the 1st block instead of 0.
http://patchwork.ozlabs.org/patch/137186/ fixes this bug.
** Changed in: qemu
Status: New => Fix Committed
** Also affects: qemu-kvm (Ubuntu)
Importance: Undecided
Status: New
** Changed in: qemu-kvm (Ubuntu)
Status: New => Fix Committed
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/919242
Title:
qemu-img convert to VDI corrupts image
Status in QEMU:
Fix Committed
Status in “qemu-kvm” package in Ubuntu:
Fix Committed
Bug description:
Hello, thanks to all for the great work on qemu, an excellent
technology.
There appears to be a serious bug in qemu-img 1.0, yielding silent
corruption when converting an image to VDI format. After conversion
to VDI, an image with WinNT4sp6 (NTFS) yields a boot failure (details
below) -- presumably due to some corruption, since the image works
fine as the source .vhd (from virtualPC6), and also when converted to
QCOW2 or VMDK format.
TEST CASE:
OS X 10.6.8 on Intel i5
Qemu 1.0 from mac "ports" (macports.org)
The source BaseDrive.vhd image is from VirtualPC6 (Mac)
$ qemu-img info BaseDrive.vhd
image: BaseDrive.vhd
file format: vpc
virtual size: 2.0G (2096898048 bytes)
disk size: 190M
The image has a fresh Windows NT4sp6 NTFS installation. It's from VirtualPC6
(Connectix) inside a .vhdp package directory on OS X. Convert via:
qemu-img convert -f vpc -O vdi BaseDrive.vhd BaseDrive.vdi
Now run the resulting vdi file with:
qemu-system-i386 -cpu pentium BaseDrive.vdi
On boot, NT4 crashes with
STOP: c26c {Unable to Load Device Driver}
\??\C:\WINNT\system32\win32k.sys device driver could not be loaded.
Error Status was 0xc221
Both qemu 1.0, and VirtualBox 4.1.8 yield the same error on this VDI.
Conversion of the exact same image to QCOW2 or VMDK format yields a working
image (ie. qemu and VirtualBox boot fine):
qemu-img convert -f vpc -O qcow2 BaseDrive.vhd BaseDrive.qcow2
OR
qemu-img convert -f vpc -O vmdk BaseDrive.vhd BaseDrive.vmdk
Furthermore, I tested converting from raw, qcow2, and vmdk to vdi,
and in all these cases the original format boots, but the converted
VDI fails to boot as above.
Along the way, I think I also tested a VDI natively created and
installed from VirtualBox, which did boot fine in qemu. Thus the
problem appears to be not in qemu-system-i386 reading the VDI, rather
in the qemu-img conversion to VDI.
SEVERITY: CRITICAL
The severity of this bug is critical as it appears to produce a silently
corrupted VDI image. (which is presumably the cause of the boot failure;
though I have not explicitly check-disked the resulting VDI image). It also
impedes easy inter-use between qemu and VirtualBox.
WORKAROUND:
The workaround is to use the VMDK format instead of VDI.
VMDK is supported by both qemu and VirtualBox (and vmWare).
I can supply a test VHD/QCOW2/VMDK image if desired to reproduce the bug.
(but it's large, 190M)
-- jbthiel
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/919242/+subscriptions
[Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter
By using strncasecmp, we allow for arbitrary characters after the
"on"/"off" string. Fix this by switching to strcasecmp.
Signed-off-by: Jan Kiszka
---
hw/qdev-properties.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 02f0dae..ea3b2df 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property *props,
void *src)
/* Bit */
static int parse_bit(DeviceState *dev, Property *prop, const char *str)
{
-if (!strncasecmp(str, "on", 2))
+if (!strcasecmp(str, "on"))
bit_prop_set(dev, prop, true);
-else if (!strncasecmp(str, "off", 3))
+else if (!strcasecmp(str, "off"))
bit_prop_set(dev, prop, false);
else
return -EINVAL;
--
1.7.3.4
Re: [Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter
Am 21.01.2012 14:43, schrieb Jan Kiszka:
> By using strncasecmp, we allow for arbitrary characters after the
> "on"/"off" string. Fix this by switching to strcasecmp.
>
> Signed-off-by: Jan Kiszka
Reviewed-by: Andreas Färber
An alternative might be to increase the char count by one. For a const
char* parameter I see the responsibility for nul-terminating at the
caller though, so this seems right. Did you check all callers?
Andreas
> ---
> hw/qdev-properties.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 02f0dae..ea3b2df 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property
> *props, void *src)
> /* Bit */
> static int parse_bit(DeviceState *dev, Property *prop, const char *str)
> {
> -if (!strncasecmp(str, "on", 2))
> +if (!strcasecmp(str, "on"))
> bit_prop_set(dev, prop, true);
> -else if (!strncasecmp(str, "off", 3))
> +else if (!strcasecmp(str, "off"))
> bit_prop_set(dev, prop, false);
> else
> return -EINVAL;
--
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] qdev-property: Make bit property parsing stricter
On 2012-01-21 17:36, Andreas Färber wrote: > Am 21.01.2012 14:43, schrieb Jan Kiszka: >> By using strncasecmp, we allow for arbitrary characters after the >> "on"/"off" string. Fix this by switching to strcasecmp. >> >> Signed-off-by: Jan Kiszka > > Reviewed-by: Andreas Färber > > An alternative might be to increase the char count by one. For a const > char* parameter I see the responsibility for nul-terminating at the > caller though, so this seems right. Did you check all callers? For any normal function that takes a char * string without a maximum length, the responsibility for terminating is on caller side. Also because we checks against patterns of different length. So we don't need to jump through hoops here. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] main-loop: For tools, initialize timers as part of qemu_init_main_loop()
In some cases initializing the alarm timers can lead to non-negligable
overhead from programs that link against qemu-tool.o. At least,
setting a max-resolution WinMM alarm timer via mm_start_timer() (the
current default for Windows) can increase the "tick rate" on Windows
OSs and affect frequency scaling, and in the case of tools that run
in guest OSs such has qemu-ga, the impact can be fairly dramatic
(+20%/20% user/sys time on a core 2 processor was observed from an idle
Windows XP guest).
This patch doesn't address the issue directly (not sure what a good
solution would be for Windows, or what other situations it might be
noticeable), but it at least limits the scope of the issue to programs
that "opt-in" to using the main-loop.c functions by only enabling alarm
timers when qemu_init_main_loop() is called, which is already required
to make use of those facilities, so existing users shouldn't be
affected.
Signed-off-by: Michael Roth
---
main-loop.c |2 +-
main-loop.h | 12
qemu-tool.c |3 ++-
vl.c|5 +
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 62d95b9..db23de0 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -199,7 +199,7 @@ static int qemu_signal_init(void)
}
#endif
-int qemu_init_main_loop(void)
+int main_loop_init(void)
{
int ret;
diff --git a/main-loop.h b/main-loop.h
index f971013..4987041 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -41,10 +41,22 @@
* SIGUSR2, thread signals (SIGFPE, SIGILL, SIGSEGV, SIGBUS) and real-time
* signals if available. Remember that Windows in practice does not have
* signals, though.
+ *
+ * In the case of QEMU tools, this will also start/initialize timers.
*/
int qemu_init_main_loop(void);
/**
+ * main_loop_init: Initializes main loop
+ *
+ * Internal (but shared for compatibility reasons) initialization routine
+ * for the main loop. This should not be used by applications directly,
+ * use qemu_init_main_loop() instead.
+ *
+ */
+int main_loop_init(void);
+
+/**
* main_loop_wait: Run one iteration of the main loop.
*
* If @nonblocking is true, poll for events, otherwise suspend until
diff --git a/qemu-tool.c b/qemu-tool.c
index 6b69668..183a583 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -83,11 +83,12 @@ void qemu_clock_warp(QEMUClock *clock)
{
}
-static void __attribute__((constructor)) init_main_loop(void)
+int qemu_init_main_loop(void)
{
init_clocks();
init_timer_alarm();
qemu_clock_enable(vm_clock, false);
+return main_loop_init();
}
void slirp_select_fill(int *pnfds, fd_set *readfds,
diff --git a/vl.c b/vl.c
index ba55b35..74a47e6 100644
--- a/vl.c
+++ b/vl.c
@@ -2136,6 +2136,11 @@ static void free_and_trace(gpointer mem)
free(mem);
}
+int qemu_init_main_loop(void)
+{
+return main_loop_init();
+}
+
int main(int argc, char **argv, char **envp)
{
const char *gdbstub_dev = NULL;
--
1.7.4.1
Re: [Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)
Am 21.01.2012 13:54, schrieb Stefan Weil:
The new block was filled with zero when it was allocated by g_malloc0,
but when it was reused later and only partially used, data from the
previously allocated block were still present and written to the new
block.
This caused the problems reported by bug #919242
(https://bugs.launchpad.net/qemu/+bug/919242).
Now the unused parts of the new block which are before and after the data
are always filled with zero, so it is no longer necessary to zero the whole
block with g_malloc0.
I also updated the copyright comment.
Signed-off-by: Stefan Weil
---
block/vdi.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 31cdfab..6a0011f 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -1,7 +1,7 @@
/*
* Block driver for the Virtual Disk Image (VDI) format
*
- * Copyright (c) 2009 Stefan Weil
+ * Copyright (c) 2009, 2012 Stefan Weil
*
* 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
@@ -756,15 +756,19 @@ static void vdi_aio_write_cb(void *opaque, int ret)
(uint64_t)bmap_entry * s->block_sectors;
block = acb->block_buffer;
if (block == NULL) {
-block = g_malloc0(s->block_size);
+block = g_malloc(s->block_size);
acb->block_buffer = block;
acb->bmap_first = block_index;
assert(!acb->header_modified);
acb->header_modified = 1;
}
acb->bmap_last = block_index;
+/* Copy data to be written to new block and zero unused parts. */
+memset(block, 0, sector_in_block * SECTOR_SIZE);
memcpy(block + sector_in_block * SECTOR_SIZE,
acb->buf, n_sectors * SECTOR_SIZE);
+memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
+ (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
acb->hd_iov.iov_base = (void *)block;
acb->hd_iov.iov_len = s->block_size;
qemu_iovec_init_external(&acb->hd_qiov,&acb->hd_iov, 1);
Hi,
this patch should also be applied to the stable branches of QEMU,
at least to stable-1.0 (after the review, of course).
Regards,
Stefan Weil
[Qemu-devel] tb lock in qemu cpu-exec.c
There is a TB lock in the cpu-exec.c. It is held before a vcpu is trying to find the next tb it is trying to execute. I am wondering what this lock is for. Currently, qemu is time multiplexing on a host cpu to emulate smp. No other vcpu can be running while the tb lookup/translation is performed. Thanks Xin
Re: [Qemu-devel] tb lock in qemu cpu-exec.c
On 21 January 2012 19:39, Xin Tong wrote: > There is a TB lock in the cpu-exec.c. It is held before a vcpu is > trying to find the next tb it is trying to execute. I am wondering > what this lock is for. Currently, qemu is time multiplexing on a host > cpu to emulate smp. No other vcpu can be running while the tb > lookup/translation is performed. The thing that needs to be locked is (a) cpu thread is in this cpu-exec.c code doing a lookup for a TB (b) another thread (io thread) or signal handler (linux-user case) calls cpu_exit(), which manipulates the TB links. Unfortunately the tb_lock doesn't actually achieve this purpose; it is just broken and there are race conditions here. -- PMM
Re: [Qemu-devel] [PATCH] main-loop: For tools, initialize timers as part of qemu_init_main_loop()
Michael Roth wrote: > In some cases initializing the alarm timers can lead to non-negligable > overhead from programs that link against qemu-tool.o. At least, > setting a max-resolution WinMM alarm timer via mm_start_timer() (the > current default for Windows) can increase the "tick rate" on Windows > OSs and affect frequency scaling, and in the case of tools that run > in guest OSs such has qemu-ga, the impact can be fairly dramatic > (+20%/20% user/sys time on a core 2 processor was observed from an idle > Windows XP guest). > > This patch doesn't address the issue directly (not sure what a good > solution would be for Windows, or what other situations it might be > noticeable), Is this a timer that need to fire soon after setting, every time? I wonder if a different kind of Windows timer, lower-resolution, could be used if the timeout is longer. If it has insufficient resolution, it could be set to trigger a little early, then set a high-resolution timer at that point. Maybe that could help for Linux CONFIG_NOHZ guests? -- Jamie
Re: [Qemu-devel] tb lock in qemu cpu-exec.c
If the unlinking does not happen. ( i.e., interrupt checking in every TB is used instead of unlinking and force an exit). is the lock still needed ? Thanks Xin On Sat, Jan 21, 2012 at 2:55 PM, Peter Maydell wrote: > On 21 January 2012 19:39, Xin Tong wrote: >> There is a TB lock in the cpu-exec.c. It is held before a vcpu is >> trying to find the next tb it is trying to execute. I am wondering >> what this lock is for. Currently, qemu is time multiplexing on a host >> cpu to emulate smp. No other vcpu can be running while the tb >> lookup/translation is performed. > > The thing that needs to be locked is (a) cpu thread is in this > cpu-exec.c code doing a lookup for a TB (b) another thread (io > thread) or signal handler (linux-user case) calls cpu_exit(), > which manipulates the TB links. > > Unfortunately the tb_lock doesn't actually achieve this purpose; > it is just broken and there are race conditions here. > > -- PMM
Re: [Qemu-devel] tb lock in qemu cpu-exec.c
On 21 January 2012 20:46, Xin Tong wrote: > If the unlinking does not happen. ( i.e., interrupt checking in every > TB is used instead of unlinking and force an exit). is the lock still > needed ? I don't think so. -- PMM
Re: [Qemu-devel] [PATCH] Init win32 CRITICAL_SECTION before starting thread; crash when attaching disks
Am 29.12.2011 18:29, schrieb Bogdan Harjoc:
Git commit 8d3bc51 crashes on win32 on startup because
qemu_tcg_init_vcpu calls:
qemu_thread_create(th, qemu_tcg_cpu_thread_fn, ...
...
qemu_thread_get_handle(th)
which locks th->data->cs, a CRITICAL_SECTION which is initialized only
in the thread_fn, so it finds garbage.
Attached patch initializes it before calling _beginthreadex.
GDB/windbg probably start newly created threads sooner, because this
doesn't happen under a debugger.
With the patch below it boots until it crashes somewhere while
attaching disks (-hda raw_img).
"bt" in gdb only returns "#0 0x in ??" and generate-core-file
didn't work.
Cheers,
diff -du qemu-8d3bc51\qemu-thread-win32.c
qemu-8d3bc51-new\qemu-thread-win32.c
--- qemu-8d3bc51\qemu-thread-win32.cTue Dec 27 17:28:58 2011
+++ qemu-8d3bc51-new\qemu-thread-win32.cThu Dec 29 18:59:50 2011
@@ -215,8 +215,6 @@
if (data->mode == QEMU_THREAD_DETACHED) {
g_free(data);
data = NULL;
-} else {
-InitializeCriticalSection(&data->cs);
}
TlsSetValue(qemu_thread_tls_index, data);
qemu_thread_exit(start_routine(thread_arg));
@@ -287,6 +285,10 @@
data->arg = arg;
data->mode = mode;
data->exited = false;
+
+if (data->mode != QEMU_THREAD_DETACHED) {
+InitializeCriticalSection(&data->cs);
+}
hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
data, 0, &thread->tid);
Hi,
could you please resend your patch with a Signed-by line?
And you should use "git format-patch" to create the patch.
See http://wiki.qemu.org/Contribute/SubmitAPatch for more information.
Thanks,
Stefan Weil
Re: [Qemu-devel] [PATCH] Init win32 CRITICAL_SECTION before starting thread; crash when attaching disks
Am 21.01.2012 23:08, schrieb Stefan Weil: Hi, could you please resend your patch with a Signed-by line? Signed-off-by, of course. Sorry, it's too late for writing mails :-) And you should use "git format-patch" to create the patch. See http://wiki.qemu.org/Contribute/SubmitAPatch for more information. Thanks, Stefan Weil
Re: [Qemu-devel] [PATCH 3/4] vga: make Cirrus ISA device optional
On Fri, Jan 13, 2012 at 22:16, Andreas Färber wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > Am 13.01.2012 21:09, schrieb Jan Kiszka: >> This actually also converts it to a proper ISADevice - a value of >> its own. > > Could this by any chance help with eliminating the global > isa_mem_base? Don't see that in this patch. Not really. For that, memory API should be used to instead of a global variable. > Andreas > > $ grep -r isa_mem_base . > ./hw/ppc_prep.c: isa_mem_base = 0xc000; > ./hw/bonito.c: isa_mem_base = s->bonito_pciio_start; > ./hw/vga.c: base += isa_mem_base; > ./hw/vga.c: isa_mem_base + > 0x000a, > ./hw/isa-bus.c:target_phys_addr_t isa_mem_base = 0; > ./hw/cirrus_vga.c: isa_mem_base > + 0x000a, > ./hw/isa.h:extern target_phys_addr_t isa_mem_base; > ./hw/mips_jazz.c: isa_mem_base = 0x1100; > ./hw/mips_r4k.c: isa_mem_base = 0x1000; > ./hw/vga-isa.c: isa_mem_base + > 0x000a, > ./hw/gt64xxx.c: isa_mem_base = s->PCI0IO_start; > ./hw/gt64xxx.c: isa_mem_base = 0x1000; > > - -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > -BEGIN PGP SIGNATURE- > Version: GnuPG v2.0.18 (GNU/Linux) > > iQIcBAEBAgAGBQJPEK1HAAoJEPou0S0+fgE/9bYQAIMXlirZrps7KCArjbTiZHea > azvsPvLcNDPUbhN7GL4F26ahJV5cRnkHUD/pc6yQdil5sEK2TsPzncfPOQxhx0O0 > FIgaaQhgYSYo9vDLJBQ5yKHmy2bv1sYTtUdvVszVKcyMzOPo8rLPwNr57PO8WmR8 > fUzAjoL/bhA7w+j/N09phSE1xOTdxwye4fPmrJ7+1Ii+8muKa6mpvtHYM6stFNa4 > DIM2x9lRx9qrdPJdRmBUkwTd5CkKk37jisdNUDQjFl6zRGBlHr3P/78m7MrB9I2Q > fBzcSjr6tcUS7TZWpkwkFlmVctwNHU8RFb9a6bMtiWs3+L6r5mDYNIQ0fV0oY7J7 > UNNtoEo7B46WKx4xxG8bGa+Bs4V8U+v487XTvZpWZ1Xuap2y/fALE1SXLtdWVBGE > oE8mfCwtxh6c1c7w+l18LYCaSliGaZGj37cwdCsF0yy5LRYC6ypcMR6+INJwCGDV > CFbsxZdg08Ht1XGjQsNbwtT5LAUF4N1gGwjDScFB7I0Mx3nCER5wJCvd2GdKo9Km > nRzl4yyTdTAWT3GTyccc+XZ7I/TJdGT4GOE6vnjL5wn9hl5O+DHC5eCrCleKvZ97 > AdJWFtocudZzlnEKtMsNV8EfrKWckejLoRcSRFgXVsUZkAkOOPSuWyQ4sAxcTK96 > XqidOb4932mcJZZjjyXx > =kcX1 > -END PGP SIGNATURE-
Re: [Qemu-devel] [PULL 00/26] ppc patch queue 2012-01-21
On Sat, Jan 21, 2012 at 04:18, Alexander Graf wrote:
> Hi Anthony / Aurelien / Blue,
>
> This is my current patch queue for ppc. Please pull.
Thanks, pulled.
> Alex
>
>
> The following changes since commit 515689235c4c3d9c3f0406ddcdd21ed8da77062b:
> Anthony Liguori (1):
> Merge remote-tracking branch 'spice/spice.v47' into staging
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/agraf.git for-upstream
>
> Alexander Graf (14):
> PPC: 440EP: Initialize timer
> PPC: Bamboo: Register CPU reset
> PPC: Bamboo: Set initial TLB entry
> PPC: 440: Ignore invalid PCI IRQs
> PPC: Bamboo: recompile device tree
> PPC: 440: Default to 440EP CPU
> PPC: Enable 440EP CPU target
> PPC: bamboo: remove old machine descriptions
> PPC: bamboo: fix whitespace
> PPC: 4xx: Qdevify the 440 PCI host controller
> PPC: Bamboo: fold ppc440.c and ppc440_bamboo.c into a single file
> PPC: Bamboo: Integrate SoC instatiation, use qdev for PCI
> virtio: change memcpy to guest reads
> PPC: Pseries: Check for PCI boundaries
>
> Andreas Färber (4):
> MAINTAINERS: Add qemu-ppc to all ppc target stuff
> MAINTAINERS: Add PCI host bridge files to CHRP machines
> MAINTAINERS: Add PCI-PCI bridge to New World Mac machine
> grackle_pci: Clean up qdev names
>
> Benjamin Herrenschmidt (5):
> virtio-pci: Fix endianness of virtio config
> load_image_targphys() should enforce the max size
> Fix dirty logging with 32-bit qemu & 64-bit guests
> pseries: Support PCI extended config space in RTAS calls
> pseries: SLOF PCI flag day
>
> David Gibson (3):
> Update gitignore file
> Correct types in bmdma_addr_{read,write}
> pseries: Use correct dispatcher for PCI config space accesses
>
> .gitignore | 4 +
> MAINTAINERS | 8 ++
> Makefile.target | 2 +-
> exec.c | 14
> hw/grackle_pci.c | 17 +++-
> hw/ide/pci.c | 4 +-
> hw/loader.c | 6 +-
> hw/ppc440.c | 106 --
> hw/ppc440.h | 21 -
> hw/ppc440_bamboo.c | 154 ++-
> hw/ppc4xx_pci.c | 129 +---
> hw/spapr.c | 135 ++---
> hw/spapr_pci.c | 173
> +++
> hw/virtex_ml507.c | 1 -
> hw/virtio-pci.c | 28 +++-
> hw/virtio.c | 12 ++--
> kvm-all.c | 3 +-
> pc-bios/README | 2 +-
> pc-bios/bamboo.dtb | Bin 3179 -> 3211 bytes
> pc-bios/bamboo.dts | 128
> pc-bios/slof.bin | Bin 738744 -> 869584 bytes
> roms/SLOF | 2 +-
> target-ppc/translate_init.c | 16 +---
> 23 files changed, 458 insertions(+), 507 deletions(-)
> delete mode 100644 hw/ppc440.c
> delete mode 100644 hw/ppc440.h
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
On Mon, Jan 16, 2012 at 10:18, Kevin Wolf wrote: > Am 15.01.2012 08:51, schrieb Hervé Poussineau: >> Floppies must be read at a specific transfer rate, depending of its own >> format. >> Update floppy description table to include required transfer rate. >> >> Signed-off-by: Hervé Poussineau >> --- >> block.c | 74 >> - >> block.h | 10 +++- >> hw/fdc.c | 3 +- >> hw/pc.c | 3 +- > > Meh. Having any floppy-specific logic in the block layer is wrong. We > need to finally get this moved into fdc.c. Well, actually this code was moved recently from fdc.c to block.c (5bbdbb4676d17e782ae83055bac58e0751b25e4b). The other geometry guessing functions (ATA CHS) are also there. If we supported native floppy (or ATA) pass trough, the geometry would have to be read from the host device, so I think it's logical to keep that in block level instead of all devices. Maybe we could also split block.c into block-fdc.c, block-ata.c etc.
Re: [Qemu-devel] [PATCH 0/6] AREG0 patches v4
On Mon, Jan 9, 2012 at 23:04, Aurelien Jarno wrote: > On Sun, Jan 08, 2012 at 12:27:39PM +, Blue Swirl wrote: >> On Sun, Jan 8, 2012 at 00:52, Aurelien Jarno wrote: >> > On Sat, Jan 07, 2012 at 10:24:09PM +, Blue Swirl wrote: >> >> In this version, I made basic AREG0 free load/store implementations >> >> for all targets. Only x86-64 is tested, others have probably problems, >> >> especially 64 bit guest (Sparc64 in this case) on 32 bit hosts. >> >> >> >> I think this should be committed as a starting point if there are no >> >> major objections. >> >> >> > >> > If it is known to be broken on 32-bit hosts, I am not sure it's really >> > a good idea to commit these patches. It's just going to prevent all >> > people developing on x86 to continue testing or developing QEMU. >> >> No, the brokenness is limited to 32 bit hosts with 64 bit AREG0 free >> targets, which currently means Sparc64 on i386, Sparc64 on ARM etc. >> Sparc32 should run unless there are other problems. Other targets >> which are not AREG0 free (like x86) should be unchanged on any host. >> >> I'm not familiar with various host ABIs, fixing all of the targets is too >> much. > > Yes, but I guess your goal is to have more qemu targets than only > sparc, so it's going to become important at some point. I think it > should not be committed until we have working x86 and x86-64 host > support. OK. The problem with 64 bit guest on i386 host is that arguments need to be pushed to stack since the registers will not be enough. This happens even before my patches in some cases, but the code that handles this is not so clean. > BTW, right now I have just got a quick overview of the patch series, I > have mostly tested them. I am planning to review them more in details in > the next days. > > >> > Also I have to say I still don't get the goal of these patch series. I >> > have just tried it on an x86-64 guest with a 32-bit sparc guest. It >> > works well, but the generated TB are slightly bigger. I have done a >> > quick and dirty performance test by compiling some code inside the >> > guests, and I get a slow down of about 0.6%, though for this kind of >> > performance changes, it should probably done with other methods. >> >> Thank you for testing. I also made some tests earlier which were near >> that figure but there performance increased. But my test setup adds a >> lot of noise (CPU throttling etc.) so I didn't trust the figures much. >> >> The slow down could come from changed compiler flags. IIRC looking at >> disassembled functions, stack protector kicks in. > > Given all the translation blocks are bigger (there is one more mov > instruction per memory load/store and also for some helper), I don't > think it really work it at compiler flags or things like that. The code > to execute is longer, which also has side effects, like not playing very > well wrt instructions cache, or even qemu TB cache. > > That said 0.8% is not that big, we can probably accept that, if we have > a good reason for that. And here come the next point... > > >> > Is there another goal behind this patch series beside performance? >> >> Yes, we eliminate the global register variable, which has given us a >> lot of trouble. Those are not supported by for example LLVM or sparse. >> The semantics of calling code which has not been compiled with >> register variables from code which has been compiled so is not >> defined, but it happens to work most of the time by saving the >> variable by hand. On Sparc hosts, global register handling is a mess. > > Even if I personally don't really have this use case, I understand it's > something that might be important for others. > > My main concern there is how to reach this goal for all hosts and all > targets in a reasonable time frame. I remember the difficult moments > with both dyngen and TCG, with both softfloat-native and softfloat or > even now with qdev and non-qdev devices. > > That's why I think we should start with at least x86 and x86-64 on the > host side. On the target side, we should have at least two targets > converted, and if possible with different word size and different > endianness. That way maintainers of the TCG host code can tests all the > possible cases before committing it (experience has shown that it's > difficult to have all cases right at the first time). MIPS can be a good > candidate, as it can do both 32 and 64 bits and both big and little > endian. > > Ideally at least all hosts should be converted before the next release, > so we don't ship QEMU with support for half of the hosts only. > > Do you have any comments or plans about that? I guess it's actually the > critical part in the changes you would like to see. I think the MIPS part should work except for 64 on 32, though it is by no means optimal. If one day all QEMU targets are AREG0 free, the TCG target maintainers probably want to optimize. It should be easy to avoid most if not all extra register moves in the TLB slow path. >> A
[Qemu-devel] Commit 5632ae46 broke the network on mips.
commit 5632ae46: "mips_malta: move i8259 initialization after piix4 initialization" broke the network on mips. It still comes up, but doesn't pass packets. Try this: wget http://landley.net/aboriginal/bin/system-image-mips.tar.bz2 tar xvjf system-image-mips.tar.bz2 cd system-image-mips ./run-emulator.sh [ wait for shell prompt ] wget http://google.com Under 0.15.1 it works, under 1.0 it doesn't. Identical binary. I bisected it to the commit in question, it's still broken in -master. Rob
Re: [Qemu-devel] [PATCH 02/14] target-sparc: Fix mixup of uint64 and uint64_t
On Mon, Jan 16, 2012 at 00:46, Andreas Färber wrote:
> Commit 793a137a41ad4125011c7022cf16a1baa40a5ab6 (target-sparc:
> Implement BMASK/BSHUFFLE.) introduced a stray usage of softfloat uint64
> type.
>
> Use uint64_t instead.
>
> Signed-off-by: Andreas Färber
> Cc: Richard Henderson
> Cc: Blue Swirl
Good catch,
Reviewed-by: Blue Swirl
> ---
> target-sparc/vis_helper.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-sparc/vis_helper.c b/target-sparc/vis_helper.c
> index a992c29..9d2edb0 100644
> --- a/target-sparc/vis_helper.c
> +++ b/target-sparc/vis_helper.c
> @@ -459,7 +459,7 @@ uint32_t helper_fpackfix(uint64_t gsr, uint64_t rs2)
> return ret;
> }
>
> -uint64 helper_bshuffle(uint64_t gsr, uint64_t src1, uint64_t src2)
> +uint64_t helper_bshuffle(uint64_t gsr, uint64_t src1, uint64_t src2)
> {
> union {
> uint64_t ll[2];
> --
> 1.7.7
>
Re: [Qemu-devel] [PATCH 8/8] PPC: booke206: Implement tlbilx
On Sat, Jan 21, 2012 at 04:15, Alexander Graf wrote:
> The PowerPC 2.06 BookE ISA defines an opcode called "tlbilx" which is used
> to flush TLB entries. It's the recommended way of flushing in virtualized
> environments.
>
> So far we got away without implementing it, but Linux for e500mc uses this
> instruction, so we better add it :).
>
> Signed-off-by: Alexander Graf
>
> ---
>
> v1 -> v2:
>
> - remove sas/ts check
> - isize is only valid for mav 2.0
> ---
> target-ppc/helper.h | 1 +
> target-ppc/op_helper.c | 64
>
> target-ppc/translate.c | 25 ++
> 3 files changed, 90 insertions(+), 0 deletions(-)
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 470e42f..1635767 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -336,6 +336,7 @@ DEF_HELPER_0(booke206_tlbre, void)
> DEF_HELPER_0(booke206_tlbwe, void)
> DEF_HELPER_1(booke206_tlbsx, void, tl)
> DEF_HELPER_1(booke206_tlbivax, void, tl)
> +DEF_HELPER_2(booke206_tlbilx, void, tl, i32)
> DEF_HELPER_1(booke206_tlbflush, void, i32)
> DEF_HELPER_2(booke_setpid, void, i32, tl)
> DEF_HELPER_1(6xx_tlbd, void, tl)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 2c8a96f..29c3870 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -4406,6 +4406,70 @@ void helper_booke206_tlbivax(target_ulong address)
> }
> }
>
> +void helper_booke206_tlbilx(target_ulong address, uint32_t t)
> +{
> + int tlb_size;
> + int i, j;
> + ppcmas_tlb_t *tlb = env->tlb.tlbm;
> + int tid = (env->spr[SPR_BOOKE_MAS6] & MAS6_SPID);
> + int pid = tid >> MAS6_SPID_SHIFT;
> + int sgs = env->spr[SPR_BOOKE_MAS5] & MAS5_SGS;
> + int ind = (env->spr[SPR_BOOKE_MAS6] & MAS6_SIND) ? MAS1_IND : 0;
> + /* XXX check for unsupported isize and raise an invalid opcode then */
> + int size = env->spr[SPR_BOOKE_MAS6] & MAS6_ISIZE_MASK;
> + /* XXX implement MAV2 handling */
> + bool mav2 = false;
> +
> + /* XXX missing LPID handling */
> + switch (t) {
For better performance, this switch could be pushed to translation
time and helpers introduced for each case.
> + case 0:
> + /* flush all */
> + booke206_flush_tlb(env, -1, 1);
> + break;
> + case 1:
> + /* flush by pid */
> + for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
> + tlb_size = booke206_tlb_size(env, i);
> + for (j = 0; j < tlb_size; j++) {
> + if (!(tlb[j].mas1 & MAS1_IPROT) &&
> + ((tlb[j].mas1 & MAS1_TID_MASK) == tid)) {
> + tlb[j].mas1 &= ~MAS1_VALID;
> + }
> + }
> + tlb += booke206_tlb_size(env, i);
> + }
> + tlb_flush(env, 1);
> + break;
> + case 3:
> + /* flush by pid and ea */
> + for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
> + int ways = booke206_tlb_ways(env, i);
> +
> + for (j = 0; j < ways; j++) {
> + tlb = booke206_get_tlbm(env, i, address, j);
> + if ((ppcmas_tlb_check(env, tlb, NULL, address, pid) != 0) ||
> + (tlb->mas1 & MAS1_IPROT) ||
> + ((tlb->mas1 & MAS1_IND) != ind) ||
> + ((tlb->mas8 & MAS8_TGS) != sgs)) {
> + continue;
> + }
> + if (mav2 && ((tlb->mas1 & MAS1_TSIZE_MASK) != size)) {
> + /* XXX only check when MMUCFG[TWC] || TLBnCFG[HES] */
> + continue;
> + }
> + /* XXX e500mc doesn't match SAS, but other cores might */
> + tlb->mas1 &= ~MAS1_VALID;
> + }
> + }
> + tlb_flush(env, 1);
> + break;
> + default:
> + helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
> + POWERPC_EXCP_INVAL |
> + POWERPC_EXCP_INVAL_INVAL);
> + }
> +}
> +
> void helper_booke206_tlbflush(uint32_t type)
> {
> int flags = 0;
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index adde65b..7ceb210 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6110,6 +6110,29 @@ static void gen_tlbivax_booke206(DisasContext *ctx)
> #endif
> }
>
> +static void gen_tlbilx_booke206(DisasContext *ctx)
> +{
> +#if defined(CONFIG_USER_ONLY)
> + gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +#else
> + TCGv t0;
> + TCGv_i32 t1;
> + if (unlikely(!ctx->mem_idx)) {
> + gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> + return;
> + }
> +
> + t0 = tcg_temp_new();
> + t1 = tcg_const_i32((ctx->opcode >> 21) & 0x3);
> + gen_addr_reg_index(ctx, t0);
> +
> + gen_helper_booke206_tlbilx(t0, t1);
> +
> + tcg_temp_free(t0);
> + tcg_temp_free_i32(t1);
> +#endif
> +}
> +
>
> /* wrtee */
> static void gen_wrtee(DisasContext *ctx)
> @@ -8574,6 +8597,8 @@ GEN_HAND
Re: [Qemu-devel] [PATCH] sparc-linux-user: Fix missing symbols in .rel/.rela.plt sections
On Tue, Jan 10, 2012 at 17:38, Aurelien Jarno wrote:
> On Sat, Jan 07, 2012 at 10:01:09PM +0100, Aurelien Jarno wrote:
>> On Sat, Jan 07, 2012 at 08:36:12PM +, Blue Swirl wrote:
>> > On Sat, Jan 7, 2012 at 20:16, Aurelien Jarno wrote:
>> > > Fix .rel.plt sections in the output to not only include .rel.plt
>> > > sections from the input but also the .rel.iplt sections and to define
>> > > the hidden symbols __rel_iplt_start and __rel_iplt_end around
>> > > .rel.iplt as otherwise we get undefined references to these when
>> > > linking statically to a multiarch enabled libc (using STT_GNU_IFUNC).
>> > >
>> > > Blue Swirl
>> > > Signed-off-by: Aurelien Jarno
>> > > ---
>> > > sparc.ld | 16 ++--
>> > > 1 files changed, 14 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/sparc.ld b/sparc.ld
>> > > index 56efe34..e52c3d2 100644
>> > > --- a/sparc.ld
>> > > +++ b/sparc.ld
>> > > @@ -37,8 +37,20 @@ SECTIONS
>> > > .rela.fini : { *(.rela.fini) }
>> > > .rel.bss : { *(.rel.bss) }
>> > > .rela.bss : { *(.rela.bss) }
>> > > - .rel.plt : { *(.rel.plt) }
>> > > - .rela.plt : { *(.rela.plt) }
>> > > + .rel.plt :
>> > > + {
>> > > + *(.rel.plt)
>> > > + PROVIDE_HIDDEN (__rel_iplt_start = .);
>> > > + *(.rel.iplt)
>> > > + PROVIDE_HIDDEN (__rel_iplt_end = .);
>> > > + }
>> > > + .rela.plt :
>> > > + {
>> > > + *(.rela.plt)
>> > > + PROVIDE_HIDDEN (__rela_iplt_start = .);
>> > > + *(.rela.iplt)
>> > > + PROVIDE_HIDDEN (__rela_iplt_end = .);
>> > > + }
>> >
>> > PROVIDE_HIDDEN etc. were removed by
>> > 8733f6093c2b77502e7228503fc22024e51599b8 in order to support BSDs with
>> > older binutils (2.15). Maybe the ld scripts should be generated from a
>> > source file for example with CPP, so these lines could be tweaked.
>> >
>>
>> It seems this patch was in my local tree for too long, and it seems I
>> should have updated it before sending it...
>>
>> Other architectures are now using PROVIDE instead of PROVIDE_HIDDEN for
>> __rel{a,}_iplt_{start,end}, so I guess the same can be done for sparc and
>> ppc. I am going to do a test build to check that, and I'll come with a
>> new patch if it works.
>
> The new patch below also fixes the issue, and uses PROVIDE instead of
> PROVIDE_HIDDEN.
This should work, the info page for OpenBSD ld talks about PROVIDE
(but not PROVIDE_HIDDEN).
> sparc-linux-user: Fix missing symbols in .rel/.rela.plt sections
>
> Fix .rel.plt sections in the output to not only include .rel.plt
> sections from the input but also the .rel.iplt sections and to define
> the hidden symbols __rel_iplt_start and __rel_iplt_end around
> .rel.iplt as otherwise we get undefined references to these when
> linking statically to a multiarch enabled libc (using STT_GNU_IFUNC).
>
> Cc: Blue Swirl
> Signed-off-by: Aurelien Jarno
> ---
> sparc.ld | 16 ++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/sparc.ld b/sparc.ld
> index 56efe34..cec17c9 100644
> --- a/sparc.ld
> +++ b/sparc.ld
> @@ -37,8 +37,20 @@ SECTIONS
> .rela.fini : { *(.rela.fini) }
> .rel.bss : { *(.rel.bss) }
> .rela.bss : { *(.rela.bss) }
> - .rel.plt : { *(.rel.plt) }
> - .rela.plt : { *(.rela.plt) }
> + .rel.plt :
> + {
> + *(.rel.plt)
> + PROVIDE (__rel_iplt_start = .);
> + *(.rel.iplt)
> + PROVIDE (__rel_iplt_end = .);
> + }
> + .rela.plt :
> + {
> + *(.rela.plt)
> + PROVIDE (__rela_iplt_start = .);
> + *(.rela.iplt)
> + PROVIDE (__rela_iplt_end = .);
> + }
> .init : { *(.init) } =0x47ff041f
> .text :
> {
> --
> 1.7.7.3
>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurel...@aurel32.net http://www.aurel32.net
>
Re: [Qemu-devel] [PATCH 02/14] target-sparc: Fix mixup of uint64 and uint64_t
Am 21.01.2012 19:51, schrieb Blue Swirl: > On Mon, Jan 16, 2012 at 00:46, Andreas Färber wrote: >> Commit 793a137a41ad4125011c7022cf16a1baa40a5ab6 (target-sparc: >> Implement BMASK/BSHUFFLE.) introduced a stray usage of softfloat uint64 >> type. >> >> Use uint64_t instead. >> >> Signed-off-by: Andreas Färber >> Cc: Richard Henderson >> Cc: Blue Swirl > > Good catch, > Reviewed-by: Blue Swirl Feel free to apply this patch if Richard doesn't comment. The rest of the series needs some more review and benchmarking. Is there maybe a way to poison these types outside fpu/ meanwhile? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] TimersState timers_state in qemu-timer.c
>From what I understand, qemu-timer.c keeps a timer which fires periodically to unlink TBs. there is a TimersState timers_state;. can anyone tell me what it is for ? Thanks Xin
Re: [Qemu-devel] Commit 5632ae46 broke the network on mips.
Am 21.01.2012 20:04, schrieb Rob Landley: commit 5632ae46: "mips_malta: move i8259 initialization after piix4 initialization" broke the network on mips. It still comes up, but doesn't pass packets. Try this: wget http://landley.net/aboriginal/bin/system-image-mips.tar.bz2 tar xvjf system-image-mips.tar.bz2 cd system-image-mips ./run-emulator.sh [ wait for shell prompt ] wget http://google.com Under 0.15.1 it works, under 1.0 it doesn't. Identical binary. I bisected it to the commit in question, it's still broken in -master. Rob This was fixed with commits e9b40fd34ceb23461083d505a444a389c094455b and 0b23c5d40ea933cfece3b4f69427f79c8a23256d in master and stable-1.0. My latest QEMU works with MIPS Malta running from an NFS root. Regards, Stefan
Re: [Qemu-devel] [PATCH 2/4] memory: change dirty setting APIs to take a size
On Mon, Jan 9, 2012 at 12:11, Avi Kivity wrote:
> On 01/08/2012 11:10 PM, Blue Swirl wrote:
>> Instead of each target knowing or guessing the guest page size,
>> just pass the desired size of dirtied memory area. This should also
>> improve performance due to memset() optimizations.
>>
>> diff --git a/exec-obsolete.h b/exec-obsolete.h
>> index f8af27e..019c09a 100644
>> --- a/exec-obsolete.h
>> +++ b/exec-obsolete.h
>> @@ -76,6 +76,20 @@ static inline int
>> cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
>> return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
>> }
>>
>> +static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
>> + int length,
>> + int dirty_flags)
>> +{
>> + int i, len;
>> + uint8_t *p;
>> +
>> + len = length >> TARGET_PAGE_BITS;
>> + p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
>> + for (i = 0; i < len; i++) {
>> + p[i] |= dirty_flags;
>> + }
>> +}
>> +
>
> Breaks for start = 0xfff, length = 2.
Right. I copied this from cpu_physical_memory_mask_dirty_range() which
suffers from the same problem.
