[Qemu-devel] [PATCH v2] exec: make -mem-path filenames deterministic
From: Peter Feiner
Adds ramblocks' names to their backing files when using -mem-path. Eases
introspection and debugging.
Signed-off-by: Peter Feiner
---
On Tue, Jan 8, 2013 at 2:04 PM, Anthony Liguori wrote:
>
> Yes, please submit the oneliner.
Here it is :)
The commit should probably be called "exec: add ramblocks' names to -mem-path
files" since the paths aren't deterministic.
v1 -> v2: Just add ramblock name to mkstemp template.
Thanks,
Peter
exec.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index a41bcb8..16a5452 100644
--- a/exec.c
+++ b/exec.c
@@ -865,7 +865,8 @@ static void *file_ram_alloc(RAMBlock *block,
return NULL;
}
-filename = g_strdup_printf("%s/qemu_back_mem.XX", path);
+filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path,
+ block->mr->name);
fd = mkstemp(filename);
if (fd < 0) {
--
1.7.10.4
[Qemu-devel] [PATCH v3] exec: make -mem-path filenames deterministic
From: Peter Feiner
Adds ramblocks' names to their backing files when using -mem-path. Eases
introspection and debugging.
Signed-off-by: Peter Feiner
---
The commit should probably be called "exec: add ramblocks' names to -mem-path
files" since the paths aren't deterministic.
v1 -> v2: Just add ramblock name to mkstemp template.
v2 -> v3: Safely handle ramblocks with "/" in their names
exec.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index 46a2830..f84e095 100644
--- a/exec.c
+++ b/exec.c
@@ -844,6 +844,8 @@ static void *file_ram_alloc(RAMBlock *block,
const char *path)
{
char *filename;
+char *sanitized_name;
+char *c;
void *area;
int fd;
#ifdef MAP_POPULATE
@@ -865,7 +867,16 @@ static void *file_ram_alloc(RAMBlock *block,
return NULL;
}
-filename = g_strdup_printf("%s/qemu_back_mem.XX", path);
+/* Make name safe to use with mkstemp by replacing '/' with '_'. */
+sanitized_name = g_strdup(block->mr->name);
+for (c = sanitized_name; *c != '\0'; c++) {
+if (*c == '/')
+*c = '_';
+}
+
+filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path,
+ sanitized_name);
+g_free(sanitized_name);
fd = mkstemp(filename);
if (fd < 0) {
--
1.7.10.4
Re: [Qemu-devel] [Patch] ARM: Simplify and fix imx_epit implementation.
>>>>> "Andreas" == Andreas Färber writes: >> Thanks; applied to target-arm.next. Andreas> Also if there is a reset bug, then fixing that in its own Andreas> patch would better allow backporting that to 1.6.1. The way Andreas> it is right now with no Cc: line for qemu-stable, the Andreas> released version will keep the thinko. Changing debug output Andreas> from stdout to stderr would've also been a change of its own Andreas> that is not even mentioned in the commit message. Thanks for the comments. Too late for this patch, but will try to do better next time. Peter C
Re: [Qemu-devel] [Patch] ARM: Add an L2 cache controller to KZM
>>>>> "Andreas" == Andreas Färber writes: Andreas> Am 05.08.2013 11:18, schrieb Peter Maydell: >> On 5 August 2013 02:21, Peter Chubb >> wrote: >>> Reads to unassigned memory now return non-zero (since patch >>> 9b8c69243585). This breaks guests runnong on i.MX31 that use the >>> cache controller --- they poll forever waiting for the L2CC cache >>> invalidate regsiter to be zero. >> Andreas> Peter Ch., if you know the exact differences, why don't you Andreas> just derive an imx-l2cc type (or so) derived from ARM's type, Andreas> overriding the values mentioned above? Sounds trivial to me. Because I don't know how -- can you point me at some documentation? Peter C
[Qemu-devel] [PATCH] doc: add "setup" to list of migration states
From: Peter Feiner On a slow VM (e.g., nested), you see the "setup" state when you query the migration status. Signed-off-by: Peter Feiner --- qapi-schema.json |2 +- qmp-commands.hx |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qapi-schema.json b/qapi-schema.json index 36cb964..f4ffede 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -691,7 +691,7 @@ # Information about current migration process. # # @status: #optional string describing the current migration status. -# As of 0.14.0 this can be 'active', 'completed', 'failed' or +# As of 0.14.0 this can be 'setup', 'active', 'completed', 'failed' or # 'cancelled'. If this field is not returned, no migration process # has been initiated # diff --git a/qmp-commands.hx b/qmp-commands.hx index cae890e..408ae9c 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2937,7 +2937,7 @@ block migration status. The main json-object contains the following: - "status": migration status (json-string) - - Possible values: "active", "completed", "failed", "cancelled" + - Possible values: "setup", "active", "completed", "failed", "cancelled" - "total-time": total amount of ms since migration started. If migration has ended, it returns the total migration time (json-int) -- 1.7.9.5
Re: [Qemu-devel] GSoC Intro - Intel Itanium (IA64) processor emulation
> "Prashant" == Prashant Vaibhav writes: Prashant> Sorry I should have been more precise about that. I don't Prashant> mean a standalone disassembler (objdump should handle that), Prashant> I meant a simple instruction bundle decoder to decode the op Prashant> code, arguments, predicate etc. so that we can generate the Prashant> equivalent tcg primitives for it, like what target-i386/ Prashant> translate.c does. It's basically a warmup exercise for me so Prashant> that I fully understand the ISA (completely different from Prashant> x86 which I am more familiar with). I can then use it in my Prashant> own target-ia64. It may be worth taking a look at the source to `ski' --- an open source IA64 emulator. It's in the Debian source tree. It has a high performance instruction recogniser in it. Prashant> ~Prashant Prashant> On Sat, Jun 25, 2011 at 5:28 PM, Andreas Färber Prashant> wrote: Prashant> Hello Prashant, Prashant> Am 25.06.2011 um 07:50 schrieb Prashant Vaibhav: Prashant> Currently I am writing an IA64 ISA disassembler. Prashant> Why are you writing one yourself? Is that due to GPLv2 Prashant> vs. GPLv3 licensing issues with GNU binutils? Prashant> Andreas
[Qemu-devel] [PATCH] qapi/x86: add control registers to query-cpus
From: Peter Feiner
Adds control registers that govern virtual address translation to query-cpus.
Given these registers and the guest's physical memory, which can be obtained
with dump-guest-memory, a client can perform virtual-to-physical translations.
This is useful for debugging and introspection.
Signed-off-by: Peter Feiner
---
cpus.c |8
qapi-schema.json | 10 ++
2 files changed, 18 insertions(+)
diff --git a/cpus.c b/cpus.c
index a4390c3..e9cc620 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1224,6 +1224,14 @@ CpuInfoList *qmp_query_cpus(Error **errp)
#if defined(TARGET_I386)
info->value->has_pc = true;
info->value->pc = env->eip + env->segs[R_CS].base;
+info->value->has_cr0 = true;
+info->value->cr0 = env->cr[0];
+info->value->has_cr3 = true;
+info->value->cr3 = env->cr[3];
+info->value->has_cr4 = true;
+info->value->cr4 = env->cr[4];
+info->value->has_efer = true;
+info->value->efer = env->efer;
#elif defined(TARGET_PPC)
info->value->has_nip = true;
info->value->nip = env->nip;
diff --git a/qapi-schema.json b/qapi-schema.json
index 6d7252b..80df503 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -569,6 +569,15 @@
#If the target is Sparc, this is the PC component of the
#instruction pointer.
#
+# @cr0: #optional If the target is i386 or x86_64, this is the CR1 register.
+#
+# @cr3: #optional If the target is i386 or x86_64, this is the CR3 register.
+#
+# @cr4: #optional If the target is i386 or x86_64, this is the CR4 register.
+#
+# @efer: #optional If the target is i386 or x86_64, this is the "efer"
+# (extended features) register.
+#
# @nip: #optional If the target is PPC, the instruction pointer
#
# @npc: #optional If the target is Sparc, the NPC component of the instruction
@@ -585,6 +594,7 @@
##
{ 'type': 'CpuInfo',
'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int',
+ '*cr0': 'int', '*cr3': 'int', '*cr4': 'int', '*efer', 'int',
'*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} }
##
--
1.7.10.4
Re: [Qemu-devel] [PATCH v2] i.MX: Improve EPIT timer code.
Jean-Christophe wrote: This is fine by me ... > * Unify function and type naming > * use dynamic cast whenever possible > * simplify Debug printf. > * use new style device intialization. > > Signed-off-by: Jean-Christophe DUBOIS Reviewed-by: Peter Chubb Peter C
Re: [Qemu-devel] Updated OpenBIOS images
On 4/16/07, Aurelien Jarno <[EMAIL PROTECTED]> wrote: Is it possible (even hackish) to ignore openbios an jump directly to the kernel when using -kernel? No. The kernel queries the BIOS for "hardware" configuration.
[Qemu-devel] Potential sparc32 MMU bug
While working on getting SunOS to boot under qemu, I ran into a very odd bug, and I'm not sure whose fault it is. The SunOS bootloader tries to install trap 0 by writing to the trap table. The trap table is in the .text (read-only) section of the OpenBIOS ROM. The bug is that the write to the read-only section silently fails -- it doesn't cause an interrupt in the VM. It looks like the VM believes all of the ROM is rwx (based on my examination of cpu_sparc_handle_mmu_fault). I presume the write fails because of Linux's memory protection (since the OpenBIOS ELF is mmap'd). But I'm not sure why the disallowed write doesn't cause _something_ to happen. Should qemu be catching the failed write and passing it on to the VM? Does qemu need to tell the VM's MMU which portions of the loaded ROM are read-only? Or does OpenBIOS need to inform the VM's MMU that the loaded .text section is read-only? I presume it's something OpenBIOS should be doing, but that mailing list is very very quiet, and I figured I'd make sure it wasn't an underlying qemu bug. How should qemu be handling this? What parts of qemu should I look at next? If people are interested in booting SunOS under qemu, you may want to check out my posting to the OpenBIOS mailing list, since most (if not all) of the problems are OpenBIOS's. ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] Potential sparc32 MMU bug
It definitely gets blocked by something: if I leave the the trap table in the .text section, the write silently fails. If I move the trap table to the .data section, the write succeeds. If I move the trap table over to .rodata, the write fails again. What are you looking at that suggests the whole sparc bios is loaded read/write? On 2/16/07, Paul Brook <[EMAIL PROTECTED]> wrote: On Friday 16 February 2007 16:55, Peter wrote: > While working on getting SunOS to boot under qemu, I ran into a very > odd bug, and I'm not sure whose fault it is. > > The SunOS bootloader tries to install trap 0 by writing to the trap > table. The trap table is in the .text (read-only) section of the > OpenBIOS ROM. I don't know about sparc, but it's normal for writes to ROM to be ignored. However by my reading the sparc bios is loaded into RAM anyway, so it shouldn't matter. Paul ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] Potential sparc32 MMU bug
Where is the policy of silently ignoring ROM writes implemented? It may not be the proper behavior for sparc, and I'd like to tinker with it. I'm just not sure where the write is getting suppressed (or, alternatively, where the exception is getting suppressed). On 2/16/07, Paul Brook <[EMAIL PROTECTED]> wrote: > > I don't know about sparc, but it's normal for writes to ROM to be > > ignored. However by my reading the sparc bios is loaded into RAM anyway, > > so it shouldn't matter. > > It definitely gets blocked by something: if I leave the the trap table > in the .text section, the write silently fails. If I move the trap > table to the .data section, the write succeeds. If I move the trap > table over to .rodata, the write fails again. What are you looking at > that suggests the whole sparc bios is loaded read/write? I was mistaken. There is a ROM area defined, it's just the elf loader doesn't care whether it's loading to rom or ram. My comment about rom writes being silently ignored still applies. Paul ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] running (open) solaris on sparc32
There are a number of bugs in OpenBIOS, and at least one in qemu. Your best bet is to check the OpenBIOS mailing list, where I've discussed a number of the OpenBIOS issues. On 2/27/07, Markus Schiltknecht <[EMAIL PROTECTED]> wrote: Hi, the qemu documentation states about sparc emulation: "... Please note that currently NetBSD, OpenBSD or Solaris kernels don't work." What's needed to get Solaris to word? Or any of the BSDs? Regards Markus ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] sparc32 bug/misfeature
hw/sun4m.c, line 154: m48t59_write(nvram, i++, 0x80); /* Sun4m OBP */ This is not a machine type recognized by SunOS (and probably other early versions of Solaris). According to idprom.h (some which is quoted at http://www.sunmanagers.org/archives/1993/0050.html), the only recognized sun4m IDs are: #define IDM_SUN4M_6900x71 /* SPARCsystem 600 series */ #define IDM_SUN4M_50 0x72 /* Campus 2 */ A SparcStation 10 is IDM_SUN4M_50. SunOS tries to print an error message and panic() if the machine ID is neither 0x71 or 0x72. (It fails with a data access exception because some of the devices haven't been mapped yet.) So this either needs to be changed to 0x72 or there needs to be some way to configure qemu to specify the machine ID from the command line (or config file). ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] sparc32 bug/misfeature
On 2/27/07, Blue Swirl <[EMAIL PROTECTED]> wrote: In Linux the IDs are listed in include/asm-sparc/machines.h, there are much more than just these two. There are much more than those two in the original file as well. These are the only two supported by SunOS 4.x (sun4m). So should this be added to qemu as a "new machine" or should the existing sparc target simply be made more configurable? ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [SPARC] Looking for SuperSPARC User's Guide
I've been searching high and low for the "SuperSPARC User's Guide". At one time it was published as Sun part number 801-4272-01 and TI part number 2647726-9761. A previous TI version was apparently published as SPKU005 (and possibly 2647726-9721, although that could be a typo). Sun and TI don't seem to have it any more. Has anyone here been able to track down a copy? Ironically, this message will probably simply add to the few extant references to this document on the Internet, all of which are searching in vain. (I found the SuperSPARC II Addendum, but it just documents the differences between the SuperSPARC I and II. It doesn't provide a full description of the processor.)
[Qemu-devel] [Bug 1646610] [NEW] "Assertion `!r->req.sg' failed." during live migration with VirtIO
Public bug reported:
We've hit this issue twice so far, but don't have an obvious repro yet.
It's pretty rare for us to hit it but I'm still trying so I can get a
core and backtrace. The guest was Windows running a constant workload.
We were using VirtIO SCSI drivers in both cases.
In both cases we hit the assert here:
hw/scsi/scsi-generic.c:
static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
qemu_put_sbe32s(f, &r->buflen);
if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
*** assert(!r->req.sg);
qemu_put_buffer(f, r->buf, r->req.cmd.xfer);
}
}
>From code inspection, it seems that this will always happen if
scsi_req_enqueue_internal in hw/scsi/scsi-bus.c is ever invoked.
static void scsi_req_enqueue_internal(SCSIRequest *req)
{
assert(!req->enqueued);
scsi_req_ref(req);
if (req->bus->info->get_sg_list) {
req->sg = req->bus->info->get_sg_list(req);
} else {
req->sg = NULL;
}
req->enqueued = true;
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
}
req->bus->info->get_sg_list will return &req->qsgl for a virtio-scsi bus
since it's a method stored on the SCSIBusInfo struct. I didn't see
anything that would clear the req->sg if scsi_req_enqueue_internal is
called at least once.
I think this can only happen if scsi_dma_restart_bh in hw/scsi/scsi-
bus.c is called. The only other location I see scsi_req_enqueue_internal
is on the load side for the destination of a migration.
static void scsi_dma_restart_bh(void *opaque)
{
SCSIDevice *s = opaque;
SCSIRequest *req, *next;
qemu_bh_delete(s->bh);
s->bh = NULL;
QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
scsi_req_ref(req);
if (req->retry) {
req->retry = false;
switch (req->cmd.mode) {
case SCSI_XFER_FROM_DEV:
case SCSI_XFER_TO_DEV:
scsi_req_continue(req);
break;
case SCSI_XFER_NONE:
scsi_req_dequeue(req);
scsi_req_enqueue(req); // *** this calls
scsi_req_enqueue_internal
break;
}
}
scsi_req_unref(req);
}
}
Finally when put_scsi_requests is called for migration, it seems like it
will call both virtio_scsi_save_request (from bus->info->save_request)
and scsi_generic_save_request (from req->ops->save_request) and trigger
the assert.
I searched for a bit, but didn't find anyone else reporting this. Has
anyone else seen this? It seems to me like that assert should check that
the sg list is empty instead of checking that it exists. Is this an
appropriate assessment? Assuming I find a repro, I'll try to test this
solution.
Thanks!
** Affects: qemu
Importance: Undecided
Status: New
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1646610
Title:
"Assertion `!r->req.sg' failed." during live migration with VirtIO
Status in QEMU:
New
Bug description:
We've hit this issue twice so far, but don't have an obvious repro
yet. It's pretty rare for us to hit it but I'm still trying so I can
get a core and backtrace. The guest was Windows running a constant
workload. We were using VirtIO SCSI drivers in both cases.
In both cases we hit the assert here:
hw/scsi/scsi-generic.c:
static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
qemu_put_sbe32s(f, &r->buflen);
if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
*** assert(!r->req.sg);
qemu_put_buffer(f, r->buf, r->req.cmd.xfer);
}
}
From code inspection, it seems that this will always happen if
scsi_req_enqueue_internal in hw/scsi/scsi-bus.c is ever invoked.
static void scsi_req_enqueue_internal(SCSIRequest *req)
{
assert(!req->enqueued);
scsi_req_ref(req);
if (req->bus->info->get_sg_list) {
req->sg = req->bus->info->get_sg_list(req);
} else {
req->sg = NULL;
}
req->enqueued = true;
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
}
req->bus->info->get_sg_list will return &req->qsgl for a virtio-scsi
bus since it's a method stored on the SCSIBusInfo struct. I didn't see
anything that would clear the req->sg if scsi_req_enqueue_internal is
called at least once.
I think this can only happen if scsi_dma_restart_bh in hw/scsi/scsi-
bus.c is called. The only other location I see
scsi_req_enqueue_internal is on the load side for the destination of a
migration.
static void scsi_dma_restart_bh(void *opaque)
{
SCSIDevice *s = opaque;
SCSIRequest *req, *next;
qemu_bh_delete(s->bh);
s->bh = NULL;
QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
scsi_req_ref(req);
[Qemu-devel] [Bug 1646610] Re: "Assertion `!r->req.sg' failed." during live migration with VirtIO
Hi Thomas,
Thanks for looking. We're using version 2.3.0.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1646610
Title:
"Assertion `!r->req.sg' failed." during live migration with VirtIO
Status in QEMU:
New
Bug description:
We've hit this issue twice so far, but don't have an obvious repro
yet. It's pretty rare for us to hit it but I'm still trying so I can
get a core and backtrace. The guest was Windows running a constant
workload. We were using VirtIO SCSI drivers in both cases.
In both cases we hit the assert here:
hw/scsi/scsi-generic.c:
static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
qemu_put_sbe32s(f, &r->buflen);
if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
*** assert(!r->req.sg);
qemu_put_buffer(f, r->buf, r->req.cmd.xfer);
}
}
From code inspection, it seems that this will always happen if
scsi_req_enqueue_internal in hw/scsi/scsi-bus.c is ever invoked.
static void scsi_req_enqueue_internal(SCSIRequest *req)
{
assert(!req->enqueued);
scsi_req_ref(req);
if (req->bus->info->get_sg_list) {
req->sg = req->bus->info->get_sg_list(req);
} else {
req->sg = NULL;
}
req->enqueued = true;
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
}
req->bus->info->get_sg_list will return &req->qsgl for a virtio-scsi
bus since it's a method stored on the SCSIBusInfo struct. I didn't see
anything that would clear the req->sg if scsi_req_enqueue_internal is
called at least once.
I think this can only happen if scsi_dma_restart_bh in hw/scsi/scsi-
bus.c is called. The only other location I see
scsi_req_enqueue_internal is on the load side for the destination of a
migration.
static void scsi_dma_restart_bh(void *opaque)
{
SCSIDevice *s = opaque;
SCSIRequest *req, *next;
qemu_bh_delete(s->bh);
s->bh = NULL;
QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
scsi_req_ref(req);
if (req->retry) {
req->retry = false;
switch (req->cmd.mode) {
case SCSI_XFER_FROM_DEV:
case SCSI_XFER_TO_DEV:
scsi_req_continue(req);
break;
case SCSI_XFER_NONE:
scsi_req_dequeue(req);
scsi_req_enqueue(req); // *** this calls
scsi_req_enqueue_internal
break;
}
}
scsi_req_unref(req);
}
}
Finally when put_scsi_requests is called for migration, it seems like
it will call both virtio_scsi_save_request (from
bus->info->save_request) and scsi_generic_save_request (from
req->ops->save_request) and trigger the assert.
I searched for a bit, but didn't find anyone else reporting this. Has
anyone else seen this? It seems to me like that assert should check
that the sg list is empty instead of checking that it exists. Is this
an appropriate assessment? Assuming I find a repro, I'll try to test
this solution.
Thanks!
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1646610/+subscriptions
[Qemu-devel] [Bug 1646610] Re: "Assertion `!r->req.sg' failed." during live migration with VirtIO
Thanks Thomas. Will do.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1646610
Title:
"Assertion `!r->req.sg' failed." during live migration with VirtIO
Status in QEMU:
Incomplete
Bug description:
We've hit this issue twice so far, but don't have an obvious repro
yet. It's pretty rare for us to hit it but I'm still trying so I can
get a core and backtrace. The guest was Windows running a constant
workload. We were using VirtIO SCSI drivers in both cases.
In both cases we hit the assert here:
hw/scsi/scsi-generic.c:
static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
qemu_put_sbe32s(f, &r->buflen);
if (r->buflen && r->req.cmd.mode == SCSI_XFER_TO_DEV) {
*** assert(!r->req.sg);
qemu_put_buffer(f, r->buf, r->req.cmd.xfer);
}
}
From code inspection, it seems that this will always happen if
scsi_req_enqueue_internal in hw/scsi/scsi-bus.c is ever invoked.
static void scsi_req_enqueue_internal(SCSIRequest *req)
{
assert(!req->enqueued);
scsi_req_ref(req);
if (req->bus->info->get_sg_list) {
req->sg = req->bus->info->get_sg_list(req);
} else {
req->sg = NULL;
}
req->enqueued = true;
QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
}
req->bus->info->get_sg_list will return &req->qsgl for a virtio-scsi
bus since it's a method stored on the SCSIBusInfo struct. I didn't see
anything that would clear the req->sg if scsi_req_enqueue_internal is
called at least once.
I think this can only happen if scsi_dma_restart_bh in hw/scsi/scsi-
bus.c is called. The only other location I see
scsi_req_enqueue_internal is on the load side for the destination of a
migration.
static void scsi_dma_restart_bh(void *opaque)
{
SCSIDevice *s = opaque;
SCSIRequest *req, *next;
qemu_bh_delete(s->bh);
s->bh = NULL;
QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
scsi_req_ref(req);
if (req->retry) {
req->retry = false;
switch (req->cmd.mode) {
case SCSI_XFER_FROM_DEV:
case SCSI_XFER_TO_DEV:
scsi_req_continue(req);
break;
case SCSI_XFER_NONE:
scsi_req_dequeue(req);
scsi_req_enqueue(req); // *** this calls
scsi_req_enqueue_internal
break;
}
}
scsi_req_unref(req);
}
}
Finally when put_scsi_requests is called for migration, it seems like
it will call both virtio_scsi_save_request (from
bus->info->save_request) and scsi_generic_save_request (from
req->ops->save_request) and trigger the assert.
I searched for a bit, but didn't find anyone else reporting this. Has
anyone else seen this? It seems to me like that assert should check
that the sg list is empty instead of checking that it exists. Is this
an appropriate assessment? Assuming I find a repro, I'll try to test
this solution.
Thanks!
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1646610/+subscriptions
Re: [Qemu-devel] Regarding qemu support for mrrc/mcrr
On 4 August 2012 20:27, Aakanksha Pudipeddi wrote: > I came across this while working with qemu for a project. It looks like > there is no support for mrrc/mcrr in qemu which results in the linux > arch_timer code throwing a reserved instruction exception. Could you please > let me know if anybody is working on providing this support in qemu? I'll be > more than glad to help. Also, any suggestions on how to get started on > implementing it would be greatly appreciated. Thanks. This is rather vague. The important question here is "which cp15 registers are we trying to access". I gather from your question that the answer is "the cp15 architected timers". QEMU doesn't support those currently. (We do now have infrastructure for adding 64 bit coprocessor registers, so there is no extra work required for mrrc/mcrr themselves.) We probably ought to implement the arch.timers, I agree, but it's not on my stuff-I'm-paid-to-do TODO list at the moment. If you were to contribute some patches that would be cool. How to get started is a bit tricky, but definitely use head of git master QEMU, and look at how some existing timer devices and some existing cp15 64 bit registers are implemented, I guess. You'll need a copy of the ARM architecture reference manual too so you know what you're implementing. PS: I assume you mean an UNDEF exception, there's no such thing as a reserved instruction exception. -- PMM
Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
On 6 August 2012 03:16, Peter A. G. Crosthwaite
wrote:
> Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
> SSI slave state (e.g. the CS line state).
This is more me being confused about how this should work than a
review comment, but it seems a bit odd that we have a hierarchy
Device->SSI->ADS7846[etc], where the VMState for the ADS7846
includes a field for the SSI VMState, but the SSI VMState doesn't
include a field for the Device VMState.
What you've done here matches how i2c works currently, but I've
just cc'd Anthony and Juan to check whether there's a better way
of doing it.
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> hw/ads7846.c |1 +
> hw/max111x.c |1 +
> hw/spitz.c |2 ++
> hw/ssi.c | 10 ++
> hw/ssi.h | 10 ++
> hw/stellaris.c |1 +
> hw/z2.c|1 +
> 7 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ads7846.c b/hw/ads7846.c
> index 41c7f10..6a523f6 100644
> --- a/hw/ads7846.c
> +++ b/hw/ads7846.c
> @@ -124,6 +124,7 @@ static const VMStateDescription vmstate_ads7846 = {
> .minimum_version_id_old = 0,
> .post_load = ads7856_post_load,
> .fields = (VMStateField[]) {
> +VMSTATE_SSI_SLAVE(ssidev, ADS7846State),
> VMSTATE_INT32_ARRAY(input, ADS7846State, 8),
> VMSTATE_INT32(noise, ADS7846State),
> VMSTATE_INT32(cycle, ADS7846State),
> diff --git a/hw/max111x.c b/hw/max111x.c
> index 706d89f..948fd97 100644
> --- a/hw/max111x.c
> +++ b/hw/max111x.c
> @@ -103,6 +103,7 @@ static const VMStateDescription vmstate_max111x = {
> .minimum_version_id = 0,
> .minimum_version_id_old = 0,
> .fields = (VMStateField[]) {
> +VMSTATE_SSI_SLAVE(ssidev, MAX111xState),
> VMSTATE_UINT8(tb1, MAX111xState),
> VMSTATE_UINT8(rb2, MAX111xState),
> VMSTATE_UINT8(rb3, MAX111xState),
> diff --git a/hw/spitz.c b/hw/spitz.c
> index 20e7835..f5d502d 100644
> --- a/hw/spitz.c
> +++ b/hw/spitz.c
> @@ -1087,6 +1087,7 @@ static const VMStateDescription vmstate_corgi_ssp_regs
> = {
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField []) {
> +VMSTATE_SSI_SLAVE(ssidev, CorgiSSPState),
> VMSTATE_UINT32_ARRAY(enable, CorgiSSPState, 3),
> VMSTATE_END_OF_LIST(),
> }
> @@ -1115,6 +1116,7 @@ static const VMStateDescription
> vmstate_spitz_lcdtg_regs = {
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField []) {
> +VMSTATE_SSI_SLAVE(ssidev, SpitzLCDTG),
> VMSTATE_UINT32(bl_intensity, SpitzLCDTG),
> VMSTATE_UINT32(bl_power, SpitzLCDTG),
> VMSTATE_END_OF_LIST(),
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 35d0a04..2db88fc 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -80,6 +80,16 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
> return r;
> }
>
> +const VMStateDescription vmstate_ssi_slave = {
> +.name = "SSISlave",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.minimum_version_id_old = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static void ssi_slave_register_types(void)
> {
> type_register_static(&ssi_bus_info);
> diff --git a/hw/ssi.h b/hw/ssi.h
> index 06657d7..975f9fb 100644
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -38,6 +38,16 @@ struct SSISlave {
> #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
> #define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
>
> +extern const VMStateDescription vmstate_ssi_slave;
> +
> +#define VMSTATE_SSI_SLAVE(_field, _state) { \
> +.name = (stringify(_field)), \
> +.size = sizeof(SSISlave), \
> +.vmsd = &vmstate_ssi_slave,\
> +.flags = VMS_STRUCT,\
> +.offset = vmstate_offset_value(_state, _field, SSISlave),\
> +}
> +
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>
> /* Master interface. */
> diff --git a/hw/stellaris.c b/hw/stellaris.c
> index 562fbbf..4d26857 100644
> --- a/hw/stellaris.c
> +++ b/hw/stellaris.c
> @@ -1188,6 +1188,7 @@ static const VMStateDescription
> vmstate_stellaris_ssi_bus = {
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField[]) {
> +VMSTATE_SSI_SLAVE(ssidev, stellaris_ssi_bus_state),
> VMSTATE_INT32(c
Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
On 6 August 2012 10:13, Peter Maydell wrote: > On 6 August 2012 03:16, Peter A. G. Crosthwaite > wrote: >> Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic >> SSI slave state (e.g. the CS line state). > > This is more me being confused about how this should work than a > review comment, but it seems a bit odd that we have a hierarchy > Device->SSI->ADS7846[etc], where the VMState for the ADS7846 > includes a field for the SSI VMState, but the SSI VMState doesn't > include a field for the Device VMState. > > What you've done here matches how i2c works currently, but I've > just cc'd Anthony and Juan to check whether there's a better way > of doing it. Oh, and just to mention the obvious, if we add fields to these vmstates we need to bump the version numbers. -- PMM
Re: [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour
On 6 August 2012 03:16, Peter A. G. Crosthwaite
wrote:
> Added default CS behaviour for SSI slaves. SSI devices can set a property
> to enable CS behaviour which will create a GPIO on the device which is the
> CS. Tristating of the bus on SSI transfers is implemented.
>
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> hw/ssd0323.c |6 ++
> hw/ssi-sd.c |6 ++
> hw/ssi.c | 39 ++-
> hw/ssi.h | 27 +++
> 4 files changed, 77 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ssd0323.c b/hw/ssd0323.c
> index b0b2e94..db16d20 100644
> --- a/hw/ssd0323.c
> +++ b/hw/ssd0323.c
> @@ -277,6 +277,7 @@ static void ssd0323_cd(void *opaque, int n, int level)
>
> static void ssd0323_save(QEMUFile *f, void *opaque)
> {
> +SSISlave *ss = SSI_SLAVE(opaque);
> ssd0323_state *s = (ssd0323_state *)opaque;
> int i;
>
> @@ -294,10 +295,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
> qemu_put_be32(f, s->remap);
> qemu_put_be32(f, s->mode);
> qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
> +
> +qemu_put_be32(f, ss->cs ? 1 : 0);
You don't need this ?: -- the standard bool-to-integer casting
rules will do it for you. Also, stray blank line, and you're
changing save/load format so you need to update a version number
somewhere.
> }
>
> static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
> {
> +SSISlave *ss = SSI_SLAVE(opaque);
> ssd0323_state *s = (ssd0323_state *)opaque;
> int i;
>
> @@ -319,6 +323,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int
> version_id)
> s->mode = qemu_get_be32(f);
> qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>
> +ss->cs = !!qemu_get_be32(f);
!! unnecessary here.
Same comments as here and the one above apply to the other devices
below.
> +
> return 0;
> }
>
> diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
> index b519bdb..6fd9ab9 100644
> --- a/hw/ssi-sd.c
> +++ b/hw/ssi-sd.c
> @@ -197,6 +197,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t
> val)
>
> static void ssi_sd_save(QEMUFile *f, void *opaque)
> {
> +SSISlave *ss = SSI_SLAVE(opaque);
> ssi_sd_state *s = (ssi_sd_state *)opaque;
> int i;
>
> @@ -209,10 +210,13 @@ static void ssi_sd_save(QEMUFile *f, void *opaque)
> qemu_put_be32(f, s->arglen);
> qemu_put_be32(f, s->response_pos);
> qemu_put_be32(f, s->stopping);
> +
> +qemu_put_be32(f, ss->cs ? 1 : 0);
> }
>
> static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
> {
> +SSISlave *ss = SSI_SLAVE(opaque);
> ssi_sd_state *s = (ssi_sd_state *)opaque;
> int i;
>
> @@ -229,6 +233,8 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int
> version_id)
> s->response_pos = qemu_get_be32(f);
> s->stopping = qemu_get_be32(f);
>
> +ss->cs = !!qemu_get_be32(f);
> +
> return 0;
> }
>
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 2db88fc..2e4f2fe 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -27,19 +27,55 @@ static const TypeInfo ssi_bus_info = {
> .instance_size = sizeof(SSIBus),
> };
>
> +static void ssi_cs_default(void *opaque, int n, int level)
> +{
> +SSISlave *s = SSI_SLAVE(opaque);
> +bool cs = !!level;
> +assert(n == 0);
> +if (s->cs != cs) {
> +SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
> +if (ssc->set_cs) {
> +ssc->set_cs(s, cs);
> +}
> +}
> +s->cs = cs;
> +}
> +
> +static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
> +{
> +SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(dev);
> +
> +if ((dev->cs && ssc->cs_polarity == SSI_CS_HIGH) ||
> +(!dev->cs && ssc->cs_polarity == SSI_CS_LOW) ||
> +ssc->cs_polarity == SSI_CS_NONE) {
> +return ssc->transfer(dev, val);
> +}
> +return 0;
> +}
> +
> static int ssi_slave_init(DeviceState *dev)
> {
> SSISlave *s = SSI_SLAVE(dev);
> SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
>
> +if (ssc->transfer_raw == ssi_transfer_raw_default &&
> +ssc->cs_polarity != SSI_CS_NONE) {
> +qdev_init_gpio_in(&s->qdev, ssi_cs_default, 1);
> +}
> +
> return ssc->init(s);
> }
>
> static void ssi_slave_class_init(ObjectClass *klass, void *data)
> {
> +SSISlaveClass *ssc = SSI_SLAVE_CLASS(klass);
> DeviceClass *dc = DEVICE_CLASS(klass);
> +
> dc->
Re: [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init()
On 6 August 2012 03:16, Peter A. G. Crosthwaite
wrote:
> Slave creation function that can be used to create an SSI slave without
> qdev_init() being called. This give machine models a change to set properties.
Not convinced about this one -- I think that if machine models need to
do more complicated things with the qdev device then they should just
call qdev_create/set properties/qdev_init themselves.
-- PMM
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> hw/ssi.c |9 +++--
> hw/ssi.h |1 +
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 2e4f2fe..c47419d 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -86,10 +86,15 @@ static TypeInfo ssi_slave_info = {
> .abstract = true,
> };
>
> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name)
> +{
> +return qdev_create(&bus->qbus, name);
> +}
> +
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
> {
> -DeviceState *dev;
> -dev = qdev_create(&bus->qbus, name);
> +DeviceState *dev = ssi_create_slave_no_init(bus, name);
> +
> qdev_init_nofail(dev);
> return dev;
> }
> diff --git a/hw/ssi.h b/hw/ssi.h
> index 5b69a3b..80b9664 100644
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -76,6 +76,7 @@ extern const VMStateDescription vmstate_ssi_slave;
> }
>
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>
> /* Master interface. */
> SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
> --
> 1.7.0.4
>
Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
On 6 August 2012 03:16, Peter A. G. Crosthwaite
wrote:
> Allow multiple qdev_init_gpio_in() calls for the one device. The first call
> will
> define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be
> handled
> with different handlers. Needed when two levels of the QOM class heirachy both
> define GPIO functionality, as a single GPIO handler with an index selecter is
> not possible.
I generally like this idea and I think there are a few devices in the
existing codebase that could profitably use this rather than trying
to sort things out in the handler function. (Long term we would ideally
replace the single gpio array with a set of named arrays of Pins but
this is useful in the meantime.)
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> hw/qdev.c | 16 +---
> 1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index b5b74b9..ce91a72 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -293,9 +293,19 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
>
> void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
> {
> -assert(dev->num_gpio_in == 0);
> -dev->num_gpio_in = n;
> -dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
> +qemu_irq *new_irqs = qemu_allocate_irqs(handler, dev, n);
> +
> +if (dev->num_gpio_in == 0) {
> +dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
In this case we've just called qemu_allocate_irqs() twice and leaked
the copy in new_irqs.
> +} else {
> +qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
> +memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * dev->num_gpio_in);
> +g_free(dev->gpio_in);
> +memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * n),
> +g_free(new_irqs);
I think this is rather looking into private details of what qemu_allocate_irqs
does, and it would be better to define a new qemu_reallocate_irqs() in irq.c
so we can use it here. (when doing so, consider whether g_renew() might help
in producing nice looking code)
> +dev->gpio_in = all_irqs;
> +}
> +dev->num_gpio_in += n;
> }
>
> void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
> --
> 1.7.0.4
>
-- PMM
Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
On 6 August 2012 03:16, Peter A. G. Crosthwaite wrote: > To be more consistent with the newer ways of error signalling. That and SIGABT > is easier to debug with than exit(1). > > Signed-off-by: Peter A. G. Crosthwaite Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
On 6 August 2012 03:16, Peter A. G. Crosthwaite wrote: > Added a FIFO API that can be used to create and operate byte FIFOs. I'm not asking for actual conversions, but it would be nice to see a list of some devices that could in principle be moved to using this FIFO, as an indication of its general utility. Would it make sense for the FIFO to be a QOM object, or is that a silly idea? -- PMM
Re: [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128
On 6 August 2012 03:16, Peter A. G. Crosthwaite
wrote:
> Added SPI controller to the reference design, with two n25q128 spi-flashes
> connected.
>
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> hw/petalogix_ml605_mmu.c | 28 +++-
> 1 files changed, 27 insertions(+), 1 deletions(-)
>
> diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
> index 6a7d0c0..f0ecc1f 100644
> --- a/hw/petalogix_ml605_mmu.c
> +++ b/hw/petalogix_ml605_mmu.c
> @@ -36,6 +36,7 @@
> #include "blockdev.h"
> #include "pc.h"
> #include "exec-memory.h"
> +#include "ssi.h"
>
> #include "microblaze_boot.h"
> #include "microblaze_pic_cpu.h"
> @@ -54,6 +55,8 @@
> #define AXIENET_BASEADDR 0x8278
> #define AXIDMA_BASEADDR 0x8460
>
> +#define NUM_SPI_FLASHES 2
> +
> static void machine_cpu_reset(MicroBlazeCPU *cpu)
> {
> CPUMBState *env = &cpu->env;
> @@ -78,6 +81,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
> MemoryRegion *address_space_mem = get_system_memory();
> DeviceState *dev;
> MicroBlazeCPU *cpu;
> +SysBusDevice *busdev;
> CPUMBState *env;
> DriveInfo *dinfo;
> int i;
> @@ -135,9 +139,31 @@ petalogix_ml605_init(ram_addr_t ram_size,
> irq[1], irq[0], 100 * 100);
> }
>
> +{
> +SSIBus *spi;
> +
> +dev = qdev_create(NULL, "xilinx,spi");
> +qdev_prop_set_uint8(dev, "num-cs", NUM_SPI_FLASHES);
> +qdev_init_nofail(dev);
> +busdev = sysbus_from_qdev(dev);
> +sysbus_mmio_map(busdev, 0, 0x40a0);
> +sysbus_connect_irq(busdev, 0, irq[4]);
> +
> +spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
> +
> +for (i = 0; i < NUM_SPI_FLASHES; i++) {
> +qemu_irq cs_line;
> +
> +dev = ssi_create_slave_no_init(spi, "m25p80");
> +qdev_prop_set_string(dev, "partname", (char *)"n25q128");
Why the cast?
> +qdev_init_nofail(dev);
> +cs_line = qdev_get_gpio_in(dev, 0);
> +sysbus_connect_irq(busdev, i+1, cs_line);
> +}
> +}
> +
> microblaze_load_kernel(cpu, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
>
> machine_cpu_reset);
> -
> }
>
> static QEMUMachine petalogix_ml605_machine = {
> --
> 1.7.0.4
>
-- PMM
Re: [Qemu-devel] [PATCH v6 1/4] hw: introduce standard SD host controller
On 6 August 2012 04:25, Peter A. G. Crosthwaite
wrote:
> +static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
> +{
> +bool page_aligned = false;
> +unsigned int n, begin;
> +const uint16_t block_size = s->blksize & 0x0fff;
> +uint32_t boundary_chk = 1 << (((s->blksize & 0xf000) >> 12) + 12);
> +uint32_t boundary_count = boundary_chk - (s->sdmasysad % boundary_chk);
> +
> +/* XXX: Some sd/mmc drivers (for example, u-boot-slp) do not account for
> + * possible stop at page boundary if initial address is not page aligned,
> + * allow them to work properly */
> +if ((s->sdmasysad % boundary_chk) == 0) {
> +page_aligned = true;
> +}
It's not quite clear to me what this comment is indicating. Is it
a bit of behaviour which is "not specified but behave as hardware
happens to do because software is accidentally relying on it", or
are we behaving differently from hardware here?
> +static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
> +{
> +uint32_t adma1 = 0;
> +uint64_t adma2 = 0;
> +target_phys_addr_t entry_addr = (target_phys_addr_t)s->admasysaddr;
> +
> +switch (SDHC_DMA_TYPE(s->hostctl)) {
> +case SDHC_CTRL_ADMA2_32:
> +cpu_physical_memory_read(entry_addr, (uint8_t *)&adma2,
> sizeof(adma2));
> +dscr->addr = (target_phys_addr_t)((adma2 >> 32) & 0xfffc);
> +dscr->length = (uint16_t)((adma2 >> 16) & 0x);
> +dscr->attr = (uint8_t)(adma2 & 0x3F);
Does the SDHCI spec define that these words are interpreted like
this regardless of system endianness, or is this an accidental
assumption of little-endian behaviour?
-- PMM
Re: [Qemu-devel] [PATCH v6 2/4] exynos4210: Added SD host controller model
On 6 August 2012 04:25, Peter A. G. Crosthwaite
wrote:
> +static uint64_t
> +exynos4210_sdhci_readfn(void *opaque, target_phys_addr_t offset, unsigned
> size)
> +{
> +Exynos4SDHCIState *s = (Exynos4SDHCIState *)opaque;
> +uint32_t ret;
> +
> +switch (offset & ~0x3) {
> +case SDHC_BDATA:
> +/* Buffer data port read can be disabled by CONTROL2 register */
> +if (s->control2 & EXYNOS4_SDHC_DISBUFRD) {
> +ret = 0;
> +} else {
> +ret = SDHCI_GET_CLASS(s)->mem_read(SDHCI(s), offset, size);
> +}
> +break;
> +case SDHC_ADMAERR:
> +ret = (s->admaerr >> 8 * (offset - SDHC_ADMAERR)) &
> +((1 << 8 * size) - 1);
If size == 4 you've just shifted right by 32, which is undefined behaviour
when ints are 32 bits. Try
ret = extract32(s->admaerr, (offset & 3) << 3, size * 8);
and similarly below.
> +static void exynos4210_sdhci_writefn(void *opaque, target_phys_addr_t offset,
> +uint64_t val, unsigned size)
> +{
> +Exynos4SDHCIState *s = (Exynos4SDHCIState *)opaque;
> +SDHCIState *sdhci = SDHCI(s);
> +unsigned shift;
> +
> +DPRINT_L2("write %ub: addr[0x%04x] <- %u(0x%x)\n", size,
> (uint32_t)offset,
> +(uint32_t)val, (uint32_t)val);
> +
> +switch (offset) {
> +case SDHC_CLKCON:
> +if ((val & SDHC_CLOCK_SDCLK_EN) &&
> +(sdhci->prnsts & SDHC_CARD_PRESENT)) {
> +val |= EXYNOS4_SDHC_SDCLK_STBL;
> +} else {
> +val &= ~EXYNOS4_SDHC_SDCLK_STBL;
> +}
> +/* Break out to superclass write to handle the rest of this register
> */
> +break;
> +case EXYNOS4_SDHC_CONTROL2 ... EXYNOS4_SDHC_CONTROL2 + 3:
Why do we switch (offset & 3) in the readfn but switch (offset)
and use case FOO ... FOO + 3 in the writefn? Consistency would be
nice.
> +shift = (offset - EXYNOS4_SDHC_CONTROL2) * 8;
> +s->control2 = (s->control2 & ~(((1 << 8 * size) - 1) << shift)) |
> +(val << shift);
s->control2 = deposit32(s->control2, (offset & 3) << 3, size * 8, val);
and similarly below.
> +case SDHC_ADMAERR ... SDHC_ADMAERR + 3:
> +if (size == 4 || (size == 2 && offset == SDHC_ADMAERR) ||
> +(size == 1 && offset == (SDHC_ADMAERR + 1))) {
> +uint32_t mask = 0;
> +
> +if (size == 2) {
> +mask = 0x;
> +} else if (size == 1) {
> +mask = 0x00FF;
> +val <<= 8;
> +}
> +
> +s->admaerr = (s->admaerr & (mask | EXYNOS4_SDHC_FINAL_BLOCK |
> + EXYNOS4_SDHC_IRQ_STAT)) | (val & ~(EXYNOS4_SDHC_FINAL_BLOCK |
> + EXYNOS4_SDHC_IRQ_STAT | EXYNOS4_SDHC_CONTINUE_REQ));
> +s->admaerr &= ~(val & EXYNOS4_SDHC_IRQ_STAT);
> +if ((s->stopped_adma) && (val & EXYNOS4_SDHC_CONTINUE_REQ) &&
> +(SDHC_DMA_TYPE(sdhci->hostctl) == SDHC_CTRL_ADMA2_32)) {
> +s->stopped_adma = false;
> +SDHCI_GET_CLASS(sdhci)->do_adma(sdhci);
> +}
> +} else {
> +uint32_t mask = (1 << (size * 8)) - 1;
> +shift = 8 * (offset & 0x3);
> +val <<= shift;
> +mask = ~(mask << shift);
> +s->admaerr = (s->admaerr & mask) | val;
> +}
> +return;
This case just looks odd. I think it would be clearer to first
calculate the updated value of admaerr (using deposit32) and
then act on the changes (xor of old and new value is handy
to identify which bits are changed).
-- PMM
Re: [Qemu-devel] [PATCH v6 1/4] hw: introduce standard SD host controller
On 6 August 2012 04:25, Peter A. G. Crosthwaite
wrote:
> From: Igor Mitsyanko
>
> Device model for standard SD Host Controller Interface (SDHCI) compliant with
> version 2.00 of SD association specification.
> +typedef struct ADMADescr {
> +target_phys_addr_t addr;
> +uint16_t length;
> +uint8_t attr;
> +uint8_t incr;
> +} ADMADescr;
> +
> +static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
> +{
> +uint32_t adma1 = 0;
> +uint64_t adma2 = 0;
> +target_phys_addr_t entry_addr = (target_phys_addr_t)s->admasysaddr;
> +
> +switch (SDHC_DMA_TYPE(s->hostctl)) {
> +case SDHC_CTRL_ADMA2_32:
> +cpu_physical_memory_read(entry_addr, (uint8_t *)&adma2,
> sizeof(adma2));
> +dscr->addr = (target_phys_addr_t)((adma2 >> 32) & 0xfffc);
> +dscr->length = (uint16_t)((adma2 >> 16) & 0x);
> +dscr->attr = (uint8_t)(adma2 & 0x3F);
> +dscr->incr = 8;
> +break;
> +case SDHC_CTRL_ADMA1_32:
> +cpu_physical_memory_read(entry_addr, (uint8_t *)&adma1,
> sizeof(adma1));
> +dscr->addr = (target_phys_addr_t)(adma1 & 0xF000);
> +dscr->attr = (uint8_t)(adma1 & 0x3F);
> +dscr->incr = 4;
> +if ((dscr->attr & SDHC_ADMA_ATTR_ACT_MASK) ==
> SDHC_ADMA_ATTR_SET_LEN) {
> +dscr->length = (uint16_t)((adma1 >> 12) & 0x);
> +} else {
> +dscr->length = 4096;
> +}
> +break;
> +case SDHC_CTRL_ADMA2_64:
> +cpu_physical_memory_read(entry_addr, (uint8_t *)(&dscr->attr), 1);
> +cpu_physical_memory_read(entry_addr + 2, (uint8_t *)(&dscr->length),
> 2);
> +cpu_physical_memory_read(entry_addr + 4, (uint8_t *)(&dscr->addr),
> 8);
> +dscr->attr &= 0xfff8;
> +dscr->incr = 12;
> +break;
> +}
> +}
> +
> +/* Advanced DMA data transfer */
> +static void sdhci_start_adma(SDHCIState *s)
> +{
> +unsigned int n, begin, length;
> +const uint16_t block_size = s->blksize & 0x0fff;
> +ADMADescr dscr;
> +s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;
> +
> +while (1) {
> +get_adma_description(s, &dscr);
> +DPRINT_L2("ADMA loop: addr=" TARGET_FMT_plx ", len=%d, attr=%x\n",
> +dscr.addr, dscr.length, dscr.attr);
> +
> +if ((dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) {
> +/* Indicate that error occurred in ST_FDS state */
> +s->admaerr &= ~SDHC_ADMAERR_STATE_MASK;
> +s->admaerr |= SDHC_ADMAERR_STATE_ST_FDS;
> +
> +/* Generate ADMA error interrupt */
> +if (s->errintstsen & SDHC_EISEN_ADMAERR) {
> +s->errintsts |= SDHC_EIS_ADMAERR;
> +s->norintsts |= SDHC_NIS_ERR;
> +}
> +
> +sdhci_update_irq(s);
> +break;
> +}
> +
> +length = dscr.length ? dscr.length : 65536;
> +
> +switch (dscr.attr & SDHC_ADMA_ATTR_ACT_MASK) {
> +case SDHC_ADMA_ATTR_ACT_TRAN: /* data transfer */
> +
> +if (s->trnmod & SDHC_TRNS_READ) {
> +while (length) {
> +if (s->data_count == 0) {
> +for (n = 0; n < block_size; n++) {
> +s->fifo_buffer[n] = sd_read_data(s->card);
> +}
> +}
> +begin = s->data_count;
> +if ((length + begin) < block_size) {
> +s->data_count = length + begin;
> +length = 0;
> + } else {
> +s->data_count = block_size;
> +length -= block_size - begin;
> +}
> +cpu_physical_memory_write(dscr.addr,
> &s->fifo_buffer[begin],
> +s->data_count - begin);
> +dscr.addr += s->data_count - begin;
> +if (s->data_count == block_size) {
> +s->data_count = 0;
> +if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
> +s->blkcnt--;
> +if (s->blkcnt == 0) {
> +break;
> +}
> +}
> +}
> +}
> +} else {
> +while (len
Re: [Qemu-devel] [PATCH v6 1/4] hw: introduce standard SD host controller
On 6 August 2012 12:28, Igor Mitsyanko wrote:
> On 08/06/2012 02:30 PM, Peter Maydell wrote:
>>> +static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
>>> +{
>>> +uint32_t adma1 = 0;
>>> +uint64_t adma2 = 0;
>>> +target_phys_addr_t entry_addr = (target_phys_addr_t)s->admasysaddr;
>>> +
>>> +switch (SDHC_DMA_TYPE(s->hostctl)) {
>>> +case SDHC_CTRL_ADMA2_32:
>>> +cpu_physical_memory_read(entry_addr, (uint8_t *)&adma2,
>>> sizeof(adma2));
>>> +dscr->addr = (target_phys_addr_t)((adma2 >> 32) & 0xfffc);
>>> +dscr->length = (uint16_t)((adma2 >> 16) & 0x);
>>> +dscr->attr = (uint8_t)(adma2 & 0x3F);
>>
>> Does the SDHCI spec define that these words are interpreted like
>> this regardless of system endianness, or is this an accidental
>> assumption of little-endian behaviour?
>
>
> Spec never says it explicitly, but it's quite obvious that descriptor table
> has a little endian format. There is even a comment in linux SDHCI driver
> that says:
>
> /*
> * The spec does not specify endianness of descriptor table.
> * We currently guess that it is LE.
> */
OK; we could probably use a similar comment.
-- PMM
Re: [Qemu-devel] [PATCH] linux-user: Fix SNDCTL_DSP_MAP{IN, OUT}BUF ioctl definitions
Ping? Patchwork url: http://patchwork.ozlabs.org/patch/172731/
-- PMM
On 23 July 2012 19:06, Peter Maydell wrote:
> Fix the SNDCTL_DSP_MAP{IN,OUT}BUF ioctl definitions so that they
> refer to a suitably defined target struct layout rather than hardcoding
> the ioctl number. This fixes complaints from the syscall_init()
> consistency check when running an x86_64-to-x86_64 linux-user qemu.
>
> Signed-off-by: Peter Maydell
> ---
> linux-user/ioctls.h|4 ++--
> linux-user/syscall_defs.h |4 ++--
> linux-user/syscall_types.h |3 +++
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index eb96a08..8a47767 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -186,8 +186,8 @@
>IOCTL(SNDCTL_DSP_GETISPACE, IOC_R,
> MK_PTR(MK_STRUCT(STRUCT_audio_buf_info)))
>IOCTL(SNDCTL_DSP_GETOSPACE, IOC_R,
> MK_PTR(MK_STRUCT(STRUCT_audio_buf_info)))
>IOCTL(SNDCTL_DSP_GETTRIGGER, IOC_R, MK_PTR(TYPE_INT))
> - IOCTL(SNDCTL_DSP_MAPINBUF, IOC_R, MK_PTR(TYPE_INT))
> - IOCTL(SNDCTL_DSP_MAPOUTBUF, IOC_R, MK_PTR(TYPE_INT))
> + IOCTL(SNDCTL_DSP_MAPINBUF, IOC_R, MK_PTR(MK_STRUCT(STRUCT_buffmem_desc)))
> + IOCTL(SNDCTL_DSP_MAPOUTBUF, IOC_R, MK_PTR(MK_STRUCT(STRUCT_buffmem_desc)))
>IOCTL(SNDCTL_DSP_NONBLOCK, 0, TYPE_NULL)
>IOCTL(SNDCTL_DSP_POST, 0, TYPE_NULL)
>IOCTL(SNDCTL_DSP_RESET, 0, TYPE_NULL)
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 0b239c4..ed829e9 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2163,8 +2163,8 @@ struct target_eabi_flock64 {
> #define TARGET_SNDCTL_DSP_GETTRIGGER TARGET_IOR('P',16, int)
> #define TARGET_SNDCTL_DSP_GETIPTR TARGET_IORU('P',17)
> #define TARGET_SNDCTL_DSP_GETOPTR TARGET_IORU('P',18)
> -#define TARGET_SNDCTL_DSP_MAPINBUF0x80085013
> -#define TARGET_SNDCTL_DSP_MAPOUTBUF 0x80085014
> +#define TARGET_SNDCTL_DSP_MAPINBUFTARGET_IORU('P', 19)
> +#define TARGET_SNDCTL_DSP_MAPOUTBUF TARGET_IORU('P', 20)
> #define TARGET_SNDCTL_DSP_NONBLOCK0x500e
> #define TARGET_SNDCTL_DSP_SAMPLESIZE 0xc0045005
> #define TARGET_SNDCTL_DSP_SETDUPLEX 0x5016
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index 601618d..44b6a58 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -77,6 +77,9 @@ STRUCT(audio_buf_info,
> STRUCT(count_info,
> TYPE_INT, TYPE_INT, TYPE_INT)
>
> +STRUCT(buffmem_desc,
> + TYPE_PTRVOID, TYPE_INT)
> +
> STRUCT(mixer_info,
> MK_ARRAY(TYPE_CHAR, 16), MK_ARRAY(TYPE_CHAR, 32), TYPE_INT,
> MK_ARRAY(TYPE_INT, 10))
>
> --
> 1.7.5.4
>
Re: [Qemu-devel] [PATCH] linux-user: Move target_to_host_errno_table[] setup out of ioctl loop
Ping? Patchwork url: http://patchwork.ozlabs.org/patch/172732/ -- PMM On 23 July 2012 19:07, Peter Maydell wrote: > The code to initialise the target_to_host_errno_table[] array was > accidentally inside the loop through checking and initialising all > the supported ioctls. This was harmless but meant that we reinitialised the > array several hundred times on startup. > > Signed-off-by: Peter Maydell > --- > The code seems to have been incorrectly placed like this since it was > first committed... > > linux-user/syscall.c | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 539af3f..9f9ad9a 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -4594,6 +4594,11 @@ void syscall_init(void) > #undef STRUCT > #undef STRUCT_SPECIAL > > +/* Build target_to_host_errno_table[] table from > + * host_to_target_errno_table[]. */ > +for (i=0; i < ERRNO_TABLE_SIZE; i++) > +target_to_host_errno_table[host_to_target_errno_table[i]] = i; > + > /* we patch the ioctl size if necessary. We rely on the fact that > no ioctl has all the bits at '1' in the size field */ > ie = ioctl_entries; > @@ -4613,11 +4618,6 @@ void syscall_init(void) > (size << TARGET_IOC_SIZESHIFT); > } > > -/* Build target_to_host_errno_table[] table from > - * host_to_target_errno_table[]. */ > -for (i=0; i < ERRNO_TABLE_SIZE; i++) > -target_to_host_errno_table[host_to_target_errno_table[i]] = > i; > - > /* automatic consistency check if same arch */ > #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \ > (defined(__x86_64__) && defined(TARGET_X86_64)) > -- > 1.7.5.4 > >
Re: [Qemu-devel] [PATCH] linux-user: Fix incorrect TARGET_BLKBSZGET, TARGET_BLKBSZSET
Ping? Patchwork URL: http://patchwork.ozlabs.org/patch/172730/
let me know if you want a v2 patch rather than just hand-fixing
the signed-off-by line snafu.
thanks
-- PMM
On 23 July 2012 19:05, Peter Maydell wrote:
> The definitions for the ioctl numbers TARGET_BLKBSZGET and
> TARGET_BLKBSZSET had the wrong size parameters (they are defined
> with size_t, not int, even though the ioctl implementations themselves
> read and write integers). Since commit 354a0008 we now have an
> ioctl wrapper definition for BLKBSZGET and so on an x86-64-to-x86-64
> linux-user binary we were triggering the mismatch warning in
> syscall_init().
>
> Signed-off-by: Peter Maydell
> ---
> linux-user/syscall_defs.h |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index a79b67d..0b239c4 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -832,8 +832,8 @@ struct target_pollfd {
> #define TARGET_BLKSECTGET TARGET_IO(0x12,103)/* get max sectors per request
> (ll_rw_blk.c) */
> #define TARGET_BLKSSZGET TARGET_IO(0x12,104)/* get block device sector size
> */
> /* A jump here: 108-111 have been used for various private purposes. */
> -#define TARGET_BLKBSZGET TARGET_IOR(0x12,112,int)
> -#define TARGET_BLKBSZSET TARGET_IOW(0x12,113,int)
> +#define TARGET_BLKBSZGET TARGET_IOR(0x12, 112, abi_ulong)
> +#define TARGET_BLKBSZSET TARGET_IOW(0x12, 113, abi_ulong)
> #define TARGET_BLKGETSIZE64 TARGET_IOR(0x12,114,abi_ulong)
> /* return device size in bytes
> (u64 *arg) */
> --
> 1.7.5.4
>
>
Re: [Qemu-devel] [PATCH] arm: translate: comment typo - s/middel/middle/
On 6 August 2012 08:05, Peter A. G. Crosthwaite
wrote:
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> target-arm/translate.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 29008a4..494c682 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9892,7 +9892,7 @@ static inline void
> gen_intermediate_code_internal(CPUARMState *env,
> } else {
> /* While branches must always occur at the end of an IT block,
> there are a few other things that can cause us to terminate
> - the TB in the middel of an IT block:
> + the TB in the middle of an IT block:
> - Exception generating instructions (bkpt, swi, undefined).
> - Page boundaries.
> - Hardware watchpoints.
> --
> 1.7.0.4
>
Reviewed-by: Peter Maydell
Happy for this to go via the trivial queue.
-- PMM
[Qemu-devel] [PATCH] target-arm: Fix typos in comments
Fix a variety of typos in comments in target-arm files.
Signed-off-by: Peter Maydell
---
Includes all the ones I spotted but not the 'middel' fix which
Peter C has already submitted a patch for, so the two patches
shouldn't conflict.
target-arm/arm-semi.c|2 +-
target-arm/cpu.h |2 +-
target-arm/helper.c |6 +++---
target-arm/neon_helper.c | 26 +-
target-arm/op_helper.c |2 +-
target-arm/translate.c | 10 +-
6 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 88ca9bb..2495206 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -281,7 +281,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
return len - ret;
}
case TARGET_SYS_READC:
- /* XXX: Read from debug cosole. Not implemented. */
+ /* XXX: Read from debug console. Not implemented. */
return 0;
case TARGET_SYS_ISTTY:
if (use_gdb_syscalls()) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 191895c..d7f93d9 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -79,7 +79,7 @@ struct arm_boot_info;
typedef struct CPUARMState {
/* Regs for current mode. */
uint32_t regs[16];
-/* Frequently accessed CPSR bits are stored separately for efficiently.
+/* Frequently accessed CPSR bits are stored separately for efficiency.
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5727da2..dceaa95 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -988,7 +988,7 @@ static void ttbr164_reset(CPUARMState *env, const
ARMCPRegInfo *ri)
}
static const ARMCPRegInfo lpae_cp_reginfo[] = {
-/* NOP AMAIR0/1: the override is because these clash with tha rather
+/* NOP AMAIR0/1: the override is because these clash with the rather
* broadly specified TLB_LOCKDOWN entry in the generic cp_reginfo.
*/
{ .name = "AMAIR0", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,
@@ -2899,8 +2899,8 @@ uint32_t HELPER(logicq_cc)(uint64_t val)
return (val >> 32) | (val != 0);
}
-/* VFP support. We follow the convention used for VFP instrunctions:
- Single precition routines have a "s" suffix, double precision a
+/* VFP support. We follow the convention used for VFP instructions:
+ Single precision routines have a "s" suffix, double precision a
"d" suffix. */
/* Convert host exception flags to vfp form. */
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index e0b9dbf..d9dbd91 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -530,7 +530,7 @@ NEON_VOP(rshl_s16, neon_s16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
{
int32_t dest;
@@ -547,8 +547,8 @@ uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit inputs values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -590,7 +590,7 @@ NEON_VOP(rshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
{
uint32_t dest;
@@ -608,8 +608,8 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit inputs values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
@@ -817,7 +817,7 @@ NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t val, uint32_t
shiftop)
{
uint32_t dest;
@@ -846,8 +846,8 @@ uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t
val, uint32_t shiftop
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit inputs values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon
Re: [Qemu-devel] [PATCH] target-arm: Fix typos in comments
On 6 August 2012 17:33, Peter Maydell wrote: > Fix a variety of typos in comments in target-arm files. > -/* Handling addition overflow with 64 bits inputs values is more > - * tricky than with 32 bits values. */ > +/* Handling addition overflow with 64 bit inputs values is more > + * tricky than with 32 bit values. */ In accordance with Muphry's Law (http://en.wikipedia.org/wiki/Muphry%27s_law) I've just noticed an error here: should be 'input', not 'inputs'. Will send v2 patch in a bit... -- PMM
[Qemu-devel] [PATCH v2] target-arm: Fix typos in comments
Fix a variety of typos in comments in target-arm files.
Signed-off-by: Peter Maydell
---
Changes v1->v2: s/inputs values/input values/
target-arm/arm-semi.c|2 +-
target-arm/cpu.h |2 +-
target-arm/helper.c |6 +++---
target-arm/neon_helper.c | 26 +-
target-arm/op_helper.c |2 +-
target-arm/translate.c | 10 +-
6 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 88ca9bb..2495206 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -281,7 +281,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
return len - ret;
}
case TARGET_SYS_READC:
- /* XXX: Read from debug cosole. Not implemented. */
+ /* XXX: Read from debug console. Not implemented. */
return 0;
case TARGET_SYS_ISTTY:
if (use_gdb_syscalls()) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 191895c..d7f93d9 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -79,7 +79,7 @@ struct arm_boot_info;
typedef struct CPUARMState {
/* Regs for current mode. */
uint32_t regs[16];
-/* Frequently accessed CPSR bits are stored separately for efficiently.
+/* Frequently accessed CPSR bits are stored separately for efficiency.
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5727da2..dceaa95 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -988,7 +988,7 @@ static void ttbr164_reset(CPUARMState *env, const
ARMCPRegInfo *ri)
}
static const ARMCPRegInfo lpae_cp_reginfo[] = {
-/* NOP AMAIR0/1: the override is because these clash with tha rather
+/* NOP AMAIR0/1: the override is because these clash with the rather
* broadly specified TLB_LOCKDOWN entry in the generic cp_reginfo.
*/
{ .name = "AMAIR0", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,
@@ -2899,8 +2899,8 @@ uint32_t HELPER(logicq_cc)(uint64_t val)
return (val >> 32) | (val != 0);
}
-/* VFP support. We follow the convention used for VFP instrunctions:
- Single precition routines have a "s" suffix, double precision a
+/* VFP support. We follow the convention used for VFP instructions:
+ Single precision routines have a "s" suffix, double precision a
"d" suffix. */
/* Convert host exception flags to vfp form. */
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index e0b9dbf..8bb5129 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -530,7 +530,7 @@ NEON_VOP(rshl_s16, neon_s16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
{
int32_t dest;
@@ -547,8 +547,8 @@ uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -590,7 +590,7 @@ NEON_VOP(rshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
{
uint32_t dest;
@@ -608,8 +608,8 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
@@ -817,7 +817,7 @@ NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t val, uint32_t
shiftop)
{
uint32_t dest;
@@ -846,8 +846,8 @@ uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t
val, uint32_t shiftop
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_qrshl_u64)(CPUARMState *env, uint64_t val, uint64_t
shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -914,7 +914,
[Qemu-devel] [PATCH] kvm-all.c: Move init of irqchip_inject_ioctl out of kvm_irqchip_create()
Move the init of the irqchip_inject_ioctl field of KVMState out of
kvm_irqchip_create() and into kvm_init(), so that kvm_set_irq()
can be used even when no irqchip is created (for architectures
that support async interrupt notification even without an in
kernel irqchip).
Signed-off-by: Peter Maydell
---
We can't just set irqchip_inject_ioctl in target-*/kvm.c because
the KVMState struct layout is private to kvm-all.c. Moving the
default initialisation of this field seemed the simplest approach.
It's safe because the use in kvm_set_irq() is guarded by a check of
kvm_async_interrupts_enabled().
The other approach would be to have a helper function for setting
the field, but that seems overkill when KVM_IRQ_LINE is the standard
default value. (KVM_IRQ_LINE_STATUS seems to be undocumented,
incidentally. I am going to assume it's another x86ism until somebody
does document it :-))
kvm-all.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kvm-all.c b/kvm-all.c
index 6def6c9..9a1f001 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1200,7 +1200,6 @@ static int kvm_irqchip_create(KVMState *s)
return ret;
}
-s->irqchip_inject_ioctl = KVM_IRQ_LINE;
if (kvm_check_extension(s, KVM_CAP_IRQ_INJECT_STATUS)) {
s->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
}
@@ -1321,6 +1320,8 @@ int kvm_init(void)
s->direct_msi = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 0);
#endif
+s->irqchip_inject_ioctl = KVM_IRQ_LINE;
+
ret = kvm_arch_init(s);
if (ret < 0) {
goto err;
--
1.7.9.5
Re: [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init()
On Mon, Aug 6, 2012 at 7:29 PM, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> wrote:
>> Slave creation function that can be used to create an SSI slave without
>> qdev_init() being called. This give machine models a change to set
>> properties.
>
> Not convinced about this one -- I think that if machine models need to
> do more complicated things with the qdev device then they should just
> call qdev_create/set properties/qdev_init themselves.
>
Yeh I tried that didnt work. See comment below.
> -- PMM
>
>> Signed-off-by: Peter A. G. Crosthwaite
>> ---
>> hw/ssi.c |9 +++--
>> hw/ssi.h |1 +
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ssi.c b/hw/ssi.c
>> index 2e4f2fe..c47419d 100644
>> --- a/hw/ssi.c
>> +++ b/hw/ssi.c
>> @@ -86,10 +86,15 @@ static TypeInfo ssi_slave_info = {
>> .abstract = true,
>> };
>>
>> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name)
>> +{
>> +return qdev_create(&bus->qbus, name);
bus->qbus accesses requires the definition of struct SSIBus which is
private to ssi.c. Machine models cant access the qbus field which they
need for qdev_create().
Regards,
Peter
>> +}
>> +
>> DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
>> {
>> -DeviceState *dev;
>> -dev = qdev_create(&bus->qbus, name);
>> +DeviceState *dev = ssi_create_slave_no_init(bus, name);
>> +
>> qdev_init_nofail(dev);
>> return dev;
>> }
>> diff --git a/hw/ssi.h b/hw/ssi.h
>> index 5b69a3b..80b9664 100644
>> --- a/hw/ssi.h
>> +++ b/hw/ssi.h
>> @@ -76,6 +76,7 @@ extern const VMStateDescription vmstate_ssi_slave;
>> }
>>
>> DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>>
>> /* Master interface. */
>> SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
>> --
>> 1.7.0.4
>>
Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
On Mon, Aug 6, 2012 at 7:38 PM, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> wrote:
>> Allow multiple qdev_init_gpio_in() calls for the one device. The first call
>> will
>> define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be
>> handled
>> with different handlers. Needed when two levels of the QOM class heirachy
>> both
>> define GPIO functionality, as a single GPIO handler with an index selecter is
>> not possible.
>
> I generally like this idea and I think there are a few devices in the
> existing codebase that could profitably use this rather than trying
> to sort things out in the handler function. (Long term we would ideally
> replace the single gpio array with a set of named arrays of Pins but
> this is useful in the meantime.)
>
>> Signed-off-by: Peter A. G. Crosthwaite
>> ---
>> hw/qdev.c | 16 +---
>> 1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index b5b74b9..ce91a72 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -293,9 +293,19 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
>>
>> void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
>> {
>> -assert(dev->num_gpio_in == 0);
>> -dev->num_gpio_in = n;
>> -dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
>> +qemu_irq *new_irqs = qemu_allocate_irqs(handler, dev, n);
RE comment below, this should be in the else condition, ill move it.
>> +
>> +if (dev->num_gpio_in == 0) {
>> +dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
>
> In this case we've just called qemu_allocate_irqs() twice and leaked
> the copy in new_irqs.
>
>> +} else {
>> +qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
>> +memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) *
>> dev->num_gpio_in);
>> +g_free(dev->gpio_in);
>> +memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) *
>> n),
>> +g_free(new_irqs);
>
> I think this is rather looking into private details of what qemu_allocate_irqs
> does, and it would be better to define a new qemu_reallocate_irqs() in irq.c
> so we can use it here. (when doing so, consider whether g_renew() might help
> in producing nice looking code)
My understanding is Anthony is doing major refactoring in the GPIO
area anyways. Long term, will this code in qdev.c/irq.c even going to
exist? In this patch, I took something of a path of least resistance
to just make this series work, as I suspect this patch in its current
form will be short lived due to continued QOM development.
cc Andreas and Anthony.
Regards,
Peter
>
>
>> +dev->gpio_in = all_irqs;
>> +}
>> +dev->num_gpio_in += n;
>> }
>>
>> void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
>> --
>> 1.7.0.4
>>
>
>
> -- PMM
Re: [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour
On Mon, Aug 6, 2012 at 7:25 PM, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> wrote:
>> Added default CS behaviour for SSI slaves. SSI devices can set a property
>> to enable CS behaviour which will create a GPIO on the device which is the
>> CS. Tristating of the bus on SSI transfers is implemented.
>>
>> Signed-off-by: Peter A. G. Crosthwaite
>> ---
>> hw/ssd0323.c |6 ++
>> hw/ssi-sd.c |6 ++
>> hw/ssi.c | 39 ++-
>> hw/ssi.h | 27 +++
>> 4 files changed, 77 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/ssd0323.c b/hw/ssd0323.c
>> index b0b2e94..db16d20 100644
>> --- a/hw/ssd0323.c
>> +++ b/hw/ssd0323.c
>> @@ -277,6 +277,7 @@ static void ssd0323_cd(void *opaque, int n, int level)
>>
>> static void ssd0323_save(QEMUFile *f, void *opaque)
>> {
>> +SSISlave *ss = SSI_SLAVE(opaque);
>> ssd0323_state *s = (ssd0323_state *)opaque;
>> int i;
>>
>> @@ -294,10 +295,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
>> qemu_put_be32(f, s->remap);
>> qemu_put_be32(f, s->mode);
>> qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>> +
>> +qemu_put_be32(f, ss->cs ? 1 : 0);
>
> You don't need this ?: -- the standard bool-to-integer casting
> rules will do it for you. Also, stray blank line, and you're
> changing save/load format so you need to update a version number
> somewhere.
>
Ok, any quick guidelines for where to find information on how to do this?
>> }
>>
>> static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>> {
>> +SSISlave *ss = SSI_SLAVE(opaque);
>> ssd0323_state *s = (ssd0323_state *)opaque;
>> int i;
>>
>> @@ -319,6 +323,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int
>> version_id)
>> s->mode = qemu_get_be32(f);
>> qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>>
>> +ss->cs = !!qemu_get_be32(f);
>
> !! unnecessary here.
>
> Same comments as here and the one above apply to the other devices
> below.
>
>> +
>> return 0;
>> }
>>
>> diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
>> index b519bdb..6fd9ab9 100644
>> --- a/hw/ssi-sd.c
>> +++ b/hw/ssi-sd.c
>> @@ -197,6 +197,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t
>> val)
>>
>> static void ssi_sd_save(QEMUFile *f, void *opaque)
>> {
>> +SSISlave *ss = SSI_SLAVE(opaque);
>> ssi_sd_state *s = (ssi_sd_state *)opaque;
>> int i;
>>
>> @@ -209,10 +210,13 @@ static void ssi_sd_save(QEMUFile *f, void *opaque)
>> qemu_put_be32(f, s->arglen);
>> qemu_put_be32(f, s->response_pos);
>> qemu_put_be32(f, s->stopping);
>> +
>> +qemu_put_be32(f, ss->cs ? 1 : 0);
>> }
>>
>> static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
>> {
>> +SSISlave *ss = SSI_SLAVE(opaque);
>> ssi_sd_state *s = (ssi_sd_state *)opaque;
>> int i;
>>
>> @@ -229,6 +233,8 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int
>> version_id)
>> s->response_pos = qemu_get_be32(f);
>> s->stopping = qemu_get_be32(f);
>>
>> +ss->cs = !!qemu_get_be32(f);
>> +
>> return 0;
>> }
>>
>> diff --git a/hw/ssi.c b/hw/ssi.c
>> index 2db88fc..2e4f2fe 100644
>> --- a/hw/ssi.c
>> +++ b/hw/ssi.c
>> @@ -27,19 +27,55 @@ static const TypeInfo ssi_bus_info = {
>> .instance_size = sizeof(SSIBus),
>> };
>>
>> +static void ssi_cs_default(void *opaque, int n, int level)
>> +{
>> +SSISlave *s = SSI_SLAVE(opaque);
>> +bool cs = !!level;
>> +assert(n == 0);
>> +if (s->cs != cs) {
>> +SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
>> +if (ssc->set_cs) {
>> +ssc->set_cs(s, cs);
>> +}
>> +}
>> +s->cs = cs;
>> +}
>> +
>> +static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
>> +{
>> +SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(dev);
>> +
>> +if ((dev->cs && ssc->cs_polarity == SSI_CS_HIGH) ||
>> +(!dev->cs && ssc->cs_polarity == SSI_CS_LOW) ||
>> +ssc->cs_polarity == SSI_CS_NONE) {
>> +return ssc->transfer(dev, val);
>> +}
>> +
Re: [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128
On Mon, Aug 6, 2012 at 7:50 PM, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> wrote:
>> Added SPI controller to the reference design, with two n25q128 spi-flashes
>> connected.
>>
>> Signed-off-by: Peter A. G. Crosthwaite
>> ---
>> hw/petalogix_ml605_mmu.c | 28 +++-
>> 1 files changed, 27 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
>> index 6a7d0c0..f0ecc1f 100644
>> --- a/hw/petalogix_ml605_mmu.c
>> +++ b/hw/petalogix_ml605_mmu.c
>> @@ -36,6 +36,7 @@
>> #include "blockdev.h"
>> #include "pc.h"
>> #include "exec-memory.h"
>> +#include "ssi.h"
>>
>> #include "microblaze_boot.h"
>> #include "microblaze_pic_cpu.h"
>> @@ -54,6 +55,8 @@
>> #define AXIENET_BASEADDR 0x8278
>> #define AXIDMA_BASEADDR 0x8460
>>
>> +#define NUM_SPI_FLASHES 2
>> +
>> static void machine_cpu_reset(MicroBlazeCPU *cpu)
>> {
>> CPUMBState *env = &cpu->env;
>> @@ -78,6 +81,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
>> MemoryRegion *address_space_mem = get_system_memory();
>> DeviceState *dev;
>> MicroBlazeCPU *cpu;
>> +SysBusDevice *busdev;
>> CPUMBState *env;
>> DriveInfo *dinfo;
>> int i;
>> @@ -135,9 +139,31 @@ petalogix_ml605_init(ram_addr_t ram_size,
>> irq[1], irq[0], 100 * 100);
>> }
>>
>> +{
>> +SSIBus *spi;
>> +
>> +dev = qdev_create(NULL, "xilinx,spi");
>> +qdev_prop_set_uint8(dev, "num-cs", NUM_SPI_FLASHES);
>> +qdev_init_nofail(dev);
>> +busdev = sysbus_from_qdev(dev);
>> +sysbus_mmio_map(busdev, 0, 0x40a0);
>> +sysbus_connect_irq(busdev, 0, irq[4]);
>> +
>> +spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
>> +
>> +for (i = 0; i < NUM_SPI_FLASHES; i++) {
>> +qemu_irq cs_line;
>> +
>> +dev = ssi_create_slave_no_init(spi, "m25p80");
>> +qdev_prop_set_string(dev, "partname", (char *)"n25q128");
>
> Why the cast?
>
Used to issue a warning, but just checked and this was fixed recently
(3b25597bcf7fa8c92ba2107fbdb260ce0eccd64b). Can remove it now. Will do
so in the next revision.
Regards,
Peter
>> +qdev_init_nofail(dev);
>> +cs_line = qdev_get_gpio_in(dev, 0);
>> +sysbus_connect_irq(busdev, i+1, cs_line);
>> +}
>> +}
>> +
>> microblaze_load_kernel(cpu, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
>>
>> machine_cpu_reset);
>> -
>> }
>>
>> static QEMUMachine petalogix_ml605_machine = {
>> --
>> 1.7.0.4
>>
>
>
> -- PMM
Re: [Qemu-devel] [PATCH v2] target-arm: Fix typos in comments
On Mon, 2012-08-06 at 17:42 +0100, Peter Maydell wrote:
> Fix a variety of typos in comments in target-arm files.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Peter Crosthwaite
> ---
> Changes v1->v2: s/inputs values/input values/
>
> target-arm/arm-semi.c|2 +-
> target-arm/cpu.h |2 +-
> target-arm/helper.c |6 +++---
> target-arm/neon_helper.c | 26 +-
> target-arm/op_helper.c |2 +-
> target-arm/translate.c | 10 +-
> 6 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
> index 88ca9bb..2495206 100644
> --- a/target-arm/arm-semi.c
> +++ b/target-arm/arm-semi.c
> @@ -281,7 +281,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
> return len - ret;
> }
> case TARGET_SYS_READC:
> - /* XXX: Read from debug cosole. Not implemented. */
> + /* XXX: Read from debug console. Not implemented. */
> return 0;
> case TARGET_SYS_ISTTY:
> if (use_gdb_syscalls()) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 191895c..d7f93d9 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -79,7 +79,7 @@ struct arm_boot_info;
> typedef struct CPUARMState {
> /* Regs for current mode. */
> uint32_t regs[16];
> -/* Frequently accessed CPSR bits are stored separately for efficiently.
> +/* Frequently accessed CPSR bits are stored separately for efficiency.
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5727da2..dceaa95 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -988,7 +988,7 @@ static void ttbr164_reset(CPUARMState *env, const
> ARMCPRegInfo *ri)
> }
>
> static const ARMCPRegInfo lpae_cp_reginfo[] = {
> -/* NOP AMAIR0/1: the override is because these clash with tha rather
> +/* NOP AMAIR0/1: the override is because these clash with the rather
> * broadly specified TLB_LOCKDOWN entry in the generic cp_reginfo.
> */
> { .name = "AMAIR0", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,
> @@ -2899,8 +2899,8 @@ uint32_t HELPER(logicq_cc)(uint64_t val)
> return (val >> 32) | (val != 0);
> }
>
> -/* VFP support. We follow the convention used for VFP instrunctions:
> - Single precition routines have a "s" suffix, double precision a
> +/* VFP support. We follow the convention used for VFP instructions:
> + Single precision routines have a "s" suffix, double precision a
> "d" suffix. */
>
> /* Convert host exception flags to vfp form. */
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index e0b9dbf..8bb5129 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -530,7 +530,7 @@ NEON_VOP(rshl_s16, neon_s16, 2)
> #undef NEON_FN
>
> /* The addition of the rounding constant may overflow, so we use an
> - * intermediate 64 bits accumulator. */
> + * intermediate 64 bit accumulator. */
> uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
> {
> int32_t dest;
> @@ -547,8 +547,8 @@ uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t
> shiftop)
> return dest;
> }
>
> -/* Handling addition overflow with 64 bits inputs values is more
> - * tricky than with 32 bits values. */
> +/* Handling addition overflow with 64 bit input values is more
> + * tricky than with 32 bit values. */
> uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
> {
> int8_t shift = (int8_t)shiftop;
> @@ -590,7 +590,7 @@ NEON_VOP(rshl_u16, neon_u16, 2)
> #undef NEON_FN
>
> /* The addition of the rounding constant may overflow, so we use an
> - * intermediate 64 bits accumulator. */
> + * intermediate 64 bit accumulator. */
> uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
> {
> uint32_t dest;
> @@ -608,8 +608,8 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t
> shiftop)
> return dest;
> }
>
> -/* Handling addition overflow with 64 bits inputs values is more
> - * tricky than with 32 bits values. */
> +/* Handling addition overflow with 64 bit input values is more
> + * tricky than with 32 bit values. */
> uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
> {
> int8_t shift = (uint8_t)shiftop;
> @@ -817,7 +817,7 @@ NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
> #undef NEON_FN
>
> /* The addition of the rounding constant may overflow, so we use an
>
Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
On Mon, Aug 6, 2012 at 7:48 PM, Peter Maydell wrote: > On 6 August 2012 03:16, Peter A. G. Crosthwaite > wrote: >> Added a FIFO API that can be used to create and operate byte FIFOs. > > I'm not asking for actual conversions, but it would be nice to see a > list of some devices that could in principle be moved to using this FIFO, > as an indication of its general utility. > The two device models in this series. mcf_uart.c, strongarm.c (StrongARMUARTState), xilinx_uartlite, cadence_uart.c, sh_serial.c, tsc210x.c to name the ones Ive just had a brief look over. There are more. grep -i fifo hw | grep uint8_t gives you a short list of files to look for potential clients. Dominated by uarts as they are the ones that need 8-bit fifos. Usability is greatly expanded if the fifo can be an arbitrary data type (uint32_t, uint16_t, struct foo), but that is a much harder problem to solve that id like to say is out of scope of this series. I think this is what Igor is getting at RE his PL330 comments on his continuation of this thread. Can we get the ball rolling with uint8_t then talk about generalisation strategy? > Would it make sense for the FIFO to be a QOM object, or is that a > silly idea? There could be merit in making the data entries some form of QOM object. It would hurt performance, but could solve the arbitrary data problem. Regards, Peter > > -- PMM
Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
>> +
>> +extern const VMStateDescription vmstate_fifo8;
>> +
>> +#define VMSTATE_FIFO8(_field, _state) { \
>> +.name = (stringify(_field)), \
>> +.size = sizeof(Fifo8), \
>> +.vmsd = &vmstate_fifo8,\
>> +.flags = VMS_STRUCT,\
>> +.offset = vmstate_offset_value(_state, _field, Fifo8), \
>> +}
>
>
> how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro
> instead?
This has no existing precedent in QEMU so I am unsure of what you mean?
And maybe this should go to vmstate.h header
I disagree. All other clients of VMS_STRUCT are out in their repective
device specific headers (pci.h, i2c.h) etc. Unless this is new
established policy, I dont really want to change the current adopted
approach.
Regards,
Peter
>
>
>> +
>> +#endif /* FIFO_H */
>
>
Re: [Qemu-devel] [PATCH v6 1/4] hw: introduce standard SD host controller
>>> +sdhci_update_irq(s); >>> +break; >>> +} >>> +} >>> +} >> >> So I think the guest can make this loop never terminate if it sets up >> a loop of ACT_LINK descriptors, right? I don't know how we should >> handle this but I'm pretty sure "make qemu sit there forever not >> responding >> to anything and not resettable" isn't it. >> Can I suggest that this is a general problem, that needs to be solved by the AIO layer. The AIO+block layer uses coroutines to do chunk-by-chunk non-blocking IO. I dont see how this is different. The problem is solved by setting up asynchronous DMA transactions or even better, arbitrary asynchronous hardware events. AIO DMA would have a similar api to the block layer and would solve this problem once, rather than having to put ptimers or coroutine-foo in every SGDMA capable devices to watchdog for infinite loops. >> -- PMM >> > > That could only happen if guest would do this on purpose, but I see your > point. There's no way for us to tell if SD card read/write succeeded or not, I think we are talking about corner cases here. If there is major infrastructural developments needed to do this properly (which I think there might very well be), then can we declare this issue out of scope of this series and come back to this as an incremental development. To summarise, its a hard problem to solve with minimal reward. Regards, Peter > so I think the only way to to this is to suspend transfer after some > reasonable amount of loops and resume it by qemu_timer, giving CPU time to > do something useful. > I've never seen long ADMA loops, so it wouldn't reflect on performance in > any way.
Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
On Tue, Aug 7, 2012 at 4:28 PM, Igor Mitsyanko wrote:
> On 08/07/2012 10:10 AM, Peter Crosthwaite wrote:
>>>>
>>>> +
>>>> +extern const VMStateDescription vmstate_fifo8;
>>>> +
>>>> +#define VMSTATE_FIFO8(_field, _state) { \
>>>> +.name = (stringify(_field)), \
>>>> +.size = sizeof(Fifo8), \
>>>> +.vmsd = &vmstate_fifo8,\
>>>> +.flags = VMS_STRUCT,\
>>>> +.offset = vmstate_offset_value(_state, _field, Fifo8), \
>>>> +}
>>>
>>>
>>> how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro
>>> instead?
>>
>> This has no existing precedent in QEMU so I am unsure of what you mean?
>
>
> I meant VMSTATE_TIMER_TEST() in vmstate.h as an example, which is a wrapper
> to VMSTATE_POINTER_TEST(). With this approach, fifo macro could be
>
> #define VMSTATE_FIFO8(_field, _state) \
> VMSTATE_STRUCT(_field, _state, 0, vmstate_fifo8, Fifo8)
>
Yeh just greppin around it looks like this is functionally equivalent.
VMSTATE_IDE_BUS looks like a reasonable precedent for that. Any
opinions one way or the other RE with my original approach (based on
I2C) vs Igors shortened version?
Regards,
Peter
>
>
>>
>> And maybe this should go to vmstate.h header
>>
>> I disagree. All other clients of VMS_STRUCT are out in their repective
>> device specific headers (pci.h, i2c.h) etc. Unless this is new
>> established policy, I dont really want to change the current adopted
>> approach.
>
>
> Yeah, looks like you're right.
>
>
>> Regards,
>> Peter
>>
>>>
>>>> +
>>>> +#endif /* FIFO_H */
>>>
>>>
>
Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
On 7 August 2012 01:12, Peter Crosthwaite wrote: > On Mon, Aug 6, 2012 at 7:38 PM, Peter Maydell > wrote: >> On 6 August 2012 03:16, Peter A. G. Crosthwaite >> wrote: >>> +qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in); >>> +memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * >>> dev->num_gpio_in); >>> +g_free(dev->gpio_in); >>> +memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * >>> n), >>> +g_free(new_irqs); >> >> I think this is rather looking into private details of what >> qemu_allocate_irqs >> does, and it would be better to define a new qemu_reallocate_irqs() in irq.c >> so we can use it here. (when doing so, consider whether g_renew() might help >> in producing nice looking code) > > My understanding is Anthony is doing major refactoring in the GPIO > area anyways. Long term, will this code in qdev.c/irq.c even going to > exist? In this patch, I took something of a path of least resistance > to just make this series work, as I suspect this patch in its current > form will be short lived due to continued QOM development. Yes, long term this will all disappear, but I wouldn't hold your breath (and indeed it's because I don't think the Pin reworking will hit before next release that I think this patch makes sense at all). But it will be around for a fair while, which is why the code should be in the right place. It's only adding a five or ten line function to irq.c. -- PMM
Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
On 6 August 2012 19:21, Phil Staub wrote: > On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-requ...@nongnu.org wrote: >> From: Richard Henderson >> On 2012-06-07 18:04, Maciej W. Rozycki wrote: >> > I have verified this change with system emulation running the GDB test >> > suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated), >> > there were 55 progressions and no regressions. >> > >> > Signed-off-by: Maciej W. Rozycki >> > --- >> > >> > Sent on behalf of Nathan, who's since left the company. Please apply. >> > >> > Maciej >> > >> > qemu-mips-fcr0.diff >> > Index: qemu-git-trunk/target-mips/translate.c >> > === >> > --- qemu-git-trunk.orig/target-mips/translate.c 2012-06-04 >> > 05:35:53.245610241 +0100 >> > +++ qemu-git-trunk/target-mips/translate.c 2012-06-04 05:39:26.245563823 >> > +0100 >> > @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env) >> > env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3; >> > env->CP0_SRSConf4_rw_bitmask = >> > env->cpu_model->CP0_SRSConf4_rw_bitmask; >> > env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4; >> > +env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0; >> >> Reviewed-by: Richard Henderson > What are the plans for this patch? It doesn't appear to have been > applied in any of the repository branches. Basically MIPS is currently without an active maintainer, so people submitting patches need to keep pinging them until one of the core maintainers (usually Blue Swirl) applies them. For this purpose the usual approach is to follow up to the patch mail saying "Ping" and giving a url to the patch in patchwork, like this one: http://patchwork.ozlabs.org/patch/163705/ Eventually somebody will take pity on it and apply it, but it does require a bit more persistence than for more actively maintained areas of the codebase. -- PMM
Re: [Qemu-devel] [RFC/PATCH 1/1] USB code fenced for s390
On 7 August 2012 13:19, Christian Borntraeger wrote:
> +#if defined(TARGET_HAS_USB) && (TARGET_HAS_USB == 1)
> /* init USB devices */
> if (usb_enabled) {
> if (foreach_device_config(DEV_USB, usb_parse) < 0)
> exit(1);
> }
> +#endif
Whether there is USB or not is a property of the machine model,
not the target CPU architecture, so a TARGET_HAS_USB define
is definitely the wrong approach.
-- PMM
[Qemu-devel] [PATCH 0/2] clean up cpu_dump_state flags
This is a small cleanup patchset which moves some cpu_dump_state flags from being x86 only to being generic, since the extra info they ask for is not particularly x86 specific (many of our target architectures have an fpu, and similarly several implement the TCG condition-code optimisation). This allows us to remove some ugly TARGET_I386 ifdefs from target-independent code. I've also implemented the DUMP_FPU flag for ARM, by reinstating (somewhat modified) some code which had been #if'd out for years. There should be no behaviour change for other architectures. Peter Maydell (2): cpu_dump_state: move DUMP_FPU and DUMP_CCOP flags from x86-only to generic target-arm: Reinstate display of VFP registers in cpu_dump_state cpu-all.h|3 +++ cpu-exec.c |2 +- cpus.c |6 +- exec.c | 12 ++-- monitor.c|8 +--- target-arm/translate.c | 42 -- target-i386/cpu.c|2 +- target-i386/cpu.h|4 target-i386/helper.c |4 ++-- target-i386/seg_helper.c |4 ++-- target-i386/smm_helper.c |4 ++-- 11 files changed, 31 insertions(+), 60 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH 1/2] cpu_dump_state: move DUMP_FPU and DUMP_CCOP flags from x86-only to generic
Move the DUMP_FPU and DUMP_CCOP flags for cpu_dump_state() from being
x86-specific flags to being generic ones. This allows us to drop some
TARGET_I386 ifdefs in various places, and means that we can (potentially)
be more consistent across architectures about which monitor commands or
debug abort printouts include FPU register contents and info about
QEMU's condition-code optimisations.
Signed-off-by: Peter Maydell
---
cpu-all.h|3 +++
cpu-exec.c |2 +-
cpus.c |6 +-
exec.c | 12 ++--
monitor.c|8 +---
target-i386/cpu.c|2 +-
target-i386/cpu.h|4
target-i386/helper.c |4 ++--
target-i386/seg_helper.c |4 ++--
target-i386/smm_helper.c |4 ++--
10 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index 82ba1d7..19cc34d 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -345,6 +345,9 @@ CPUArchState *cpu_copy(CPUArchState *env);
CPUArchState *qemu_get_cpu(int cpu);
#define CPU_DUMP_CODE 0x0001
+#define CPU_DUMP_FPU 0x0002 /* dump FPU register state, not just integer */
+/* dump info about TCG QEMU's condition code optimization state */
+#define CPU_DUMP_CCOP 0x0004
void cpu_dump_state(CPUArchState *env, FILE *f, fprintf_function cpu_fprintf,
int flags);
diff --git a/cpu-exec.c b/cpu-exec.c
index 543460c..58ded3e 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -554,7 +554,7 @@ int cpu_exec(CPUArchState *env)
#if defined(TARGET_I386)
env->eflags = env->eflags | cpu_cc_compute_all(env, CC_OP)
| (DF & DF_MASK);
-log_cpu_state(env, X86_DUMP_CCOP);
+log_cpu_state(env, CPU_DUMP_CCOP);
env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A |
CC_P | CC_C);
#elif defined(TARGET_M68K)
cpu_m68k_flush_flags(env, env->cc_op);
diff --git a/cpus.c b/cpus.c
index 3de2e27..1760101 100644
--- a/cpus.c
+++ b/cpus.c
@@ -394,11 +394,7 @@ void hw_error(const char *fmt, ...)
fprintf(stderr, "\n");
for(env = first_cpu; env != NULL; env = env->next_cpu) {
fprintf(stderr, "CPU #%d:\n", env->cpu_index);
-#ifdef TARGET_I386
-cpu_dump_state(env, stderr, fprintf, X86_DUMP_FPU);
-#else
-cpu_dump_state(env, stderr, fprintf, 0);
-#endif
+cpu_dump_state(env, stderr, fprintf, CPU_DUMP_FPU);
}
va_end(ap);
abort();
diff --git a/exec.c b/exec.c
index a42a0b5..bde2bbf 100644
--- a/exec.c
+++ b/exec.c
@@ -1744,20 +1744,12 @@ void cpu_abort(CPUArchState *env, const char *fmt, ...)
fprintf(stderr, "qemu: fatal: ");
vfprintf(stderr, fmt, ap);
fprintf(stderr, "\n");
-#ifdef TARGET_I386
-cpu_dump_state(env, stderr, fprintf, X86_DUMP_FPU | X86_DUMP_CCOP);
-#else
-cpu_dump_state(env, stderr, fprintf, 0);
-#endif
+cpu_dump_state(env, stderr, fprintf, CPU_DUMP_FPU | CPU_DUMP_CCOP);
if (qemu_log_enabled()) {
qemu_log("qemu: fatal: ");
qemu_log_vprintf(fmt, ap2);
qemu_log("\n");
-#ifdef TARGET_I386
-log_cpu_state(env, X86_DUMP_FPU | X86_DUMP_CCOP);
-#else
-log_cpu_state(env, 0);
-#endif
+log_cpu_state(env, CPU_DUMP_FPU | CPU_DUMP_CCOP);
qemu_log_flush();
qemu_log_close();
}
diff --git a/monitor.c b/monitor.c
index 49dccfe..2ea78c4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -899,13 +899,7 @@ static void do_info_registers(Monitor *mon)
{
CPUArchState *env;
env = mon_get_cpu();
-#ifdef TARGET_I386
-cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
- X86_DUMP_FPU);
-#else
-cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
- 0);
-#endif
+cpu_dump_state(env, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU);
}
static void do_info_jit(Monitor *mon)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 857b94e..60e152a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1618,7 +1618,7 @@ static void x86_cpu_reset(CPUState *s)
if (qemu_loglevel_mask(CPU_LOG_RESET)) {
qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
-log_cpu_state(env, X86_DUMP_FPU | X86_DUMP_CCOP);
+log_cpu_state(env, CPU_DUMP_FPU | CPU_DUMP_CCOP);
}
xcc->parent_reset(s);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2a61c81..a65 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -945,10 +945,6 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t
new_cr4);
void cpu_smm_update(CPUX86State *env);
uint64_t cpu_get_tsc(CPUX86State *env);
-/* used to debug */
-#define X86_DUMP_FPU 0x0001 /* dump FPU state too */
-#define X86_DUMP_CCOP 0x0002 /* dump qemu flag cache */
-
#define TARGET_PAGE_BITS 12
#ifdef TARGET_X86_64
diff --git a/target-i386/hel
[Qemu-devel] [PATCH 2/2] target-arm: Reinstate display of VFP registers in cpu_dump_state
Reinstate the display of VFP registers in cpu_dump_state(), if
the CPU has them (this code had been #if 0'd out a for a long time).
We drop the attempt ot display the values as floating point, since
this makes assumptions about the host 'float' and 'double' formats
and is not done by eg the i386 cpu_dump_state().
This display is gated on the CPU_DUMP_FPU flag, as for x86.
Signed-off-by: Peter Maydell
---
target-arm/translate.c | 42 --
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 29008a4..1ada5fc 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9970,19 +9970,6 @@ void cpu_dump_state(CPUARMState *env, FILE *f,
fprintf_function cpu_fprintf,
int flags)
{
int i;
-#if 0
-union {
-uint32_t i;
-float s;
-} s0, s1;
-CPU_DoubleU d;
-/* ??? This assumes float64 and double have the same layout.
- Oh well, it's only debug dumps. */
-union {
-float64 f64;
-double d;
-} d0;
-#endif
uint32_t psr;
for(i=0;i<16;i++) {
@@ -10002,20 +9989,23 @@ void cpu_dump_state(CPUARMState *env, FILE *f,
fprintf_function cpu_fprintf,
psr & CPSR_T ? 'T' : 'A',
cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);
-#if 0
-for (i = 0; i < 16; i++) {
-d.d = env->vfp.regs[i];
-s0.i = d.l.lower;
-s1.i = d.l.upper;
-d0.f64 = d.d;
-cpu_fprintf(f, "s%02d=%08x(%8g) s%02d=%08x(%8g) d%02d=%08x%08x(%8g)\n",
-i * 2, (int)s0.i, s0.s,
-i * 2 + 1, (int)s1.i, s1.s,
-i, (int)(uint32_t)d.l.upper, (int)(uint32_t)d.l.lower,
-d0.d);
+if (flags & CPU_DUMP_FPU) {
+int numvfpregs = 0;
+if (arm_feature(env, ARM_FEATURE_VFP)) {
+numvfpregs += 16;
+}
+if (arm_feature(env, ARM_FEATURE_VFP3)) {
+numvfpregs += 16;
+}
+for (i = 0; i < numvfpregs; i++) {
+uint64_t v = float64_val(env->vfp.regs[i]);
+cpu_fprintf(f, "s%02d=%08x s%02d=%08x d%02d=%016" PRIx64 "\n",
+i * 2, (uint32_t)v,
+i * 2 + 1, (uint32_t)(v >> 32),
+i, v);
+}
+cpu_fprintf(f, "FPSCR: %08x\n", (int)env->vfp.xregs[ARM_VFP_FPSCR]);
}
-cpu_fprintf(f, "FPSCR: %08x\n", (int)env->vfp.xregs[ARM_VFP_FPSCR]);
-#endif
}
void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb, int pc_pos)
--
1.7.9.5
Re: [Qemu-devel] [PATCH v2 0/7] split out uses of kvm_irqchip_in_kernel()
On 2 August 2012 10:14, Jan Kiszka wrote: > On 2012-07-26 16:35, Peter Maydell wrote: >> This patch series removes all uses of kvm_irqchip_in_kernel() >> from architecture-independent code, by creating a set of more >> specific functions instead to test for the particular aspects >> of behaviour that the calling code is actually interested in. >> >> The uses in x86-specific code could in theory be further broken >> down into kvm_ioapic(), kvm_pit(), etc, but I leave that for >> one of the x86 maintainers if they think it's worthwhile. > > For the whole series: > > Acked-by: Jan Kiszka Just a ping to check this will get into qemu before the hardfreeze next week... -- PMM
Re: [Qemu-devel] For all targets and machine types: "start to monitor" smoke test
On 7 August 2012 20:26, Markus Armbruster wrote: > qemu-system-arm lm3s811evb > qemu-system-arm lm3s6965evb > qemu-system-arm: /work/armbru/qemu/hw/qdev.c:310: qdev_get_gpio_in: > Assertion `n >= 0 && n < dev->num_gpio_in' failed. This is fixed by http://patchwork.ozlabs.org/patch/172820/ (which should be in my arm-devs.next queue, I just haven't got round to flushing it yet.) -- PMM
Re: [Qemu-devel] For all targets and machine types: "start to monitor" smoke test
On 7 August 2012 20:55, Markus Armbruster wrote: > Anthony Liguori writes: >> Perhaps we could add a QEMUMachine parameter that indicates that the >> machine doesn't start without special options. > > Recommend to make it a string that lists the mandatory options. How are you going to say "need either option foo or option bar" ? I'm pretty sure we have some of those (eg "either you need to pass a flash image or a kernel"). -- PMM
Re: [Qemu-devel] [PATCH 1/2] qom: Reimplement Interfaces
Hi All,
We seem to be having difficulty getting a review/merge on this patch. I
have sent two series, two pings and a PULL, with only a single reply
from P. Maydell asking for other reviewers to weigh in:
- on July 17 P. Maydell wrote ---
I guess I should mention that I'm assuming somebody else is going to
review
this patchset. It looks generally OK to me but whether the stream API
has the details of QOM interface use right is a bit out of my area
of expertise.
-- PMM
What needs to happen (and what can I do) to make this series go through?
Edgar has reviewed P2 this morning and I have already remade the series,
but my primary concern is this patch, considering it touches the core
QOM infrastructure.
I have tested this patch on target i386 with no obvious breakages along
with testing on the the embedded platforms we work with (Zynq and MB).
Regards,
Peter
On Mon, 2012-08-06 at 13:07 +1000, Peter A. G. Crosthwaite wrote:
> From: Anthony Liguori
>
> The current implementation of Interfaces is poorly designed. Each interface
> that an object implements ends up being an object that's tracked by the
> implementing object. There's all sorts of gymnastics to deal with casting
> between these objects.
>
> But an interface shouldn't be associated with an Object. Interfaces are
> global
> to a class. This patch moves all Interface knowledge to ObjectClass
> eliminating
> the relationship between Object and Interfaces.
>
> Interfaces are now abstract (as they should be) but this is okay. Interfaces
> essentially act as additional parents for the classes and are treated as such.
>
> With this new implementation, we should fully support derived interfaces
> including reimplementing an inherited interface.
>
> PC: Rebased against qom-next merge Jun-2012.
>
> PC: Removed replication of cast logic for interfaces, i.e. there is only
> one cast function - object_dynamic_cast() (and object_dynamic_cast_assert())
>
> Signed-off-by: Anthony Liguori
> Signed-off-by: Peter A. G. Crosthwaite
> ---
> include/qemu/object.h | 46 +++
> qom/object.c | 220
> +++--
> 2 files changed, 116 insertions(+), 150 deletions(-)
>
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 8b17776..cc75fee 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -239,6 +239,7 @@ struct ObjectClass
> {
> /*< private >*/
> Type type;
> +GSList *interfaces;
> };
>
> /**
> @@ -260,7 +261,6 @@ struct Object
> {
> /*< private >*/
> ObjectClass *class;
> -GSList *interfaces;
> QTAILQ_HEAD(, ObjectProperty) properties;
> uint32_t ref;
> Object *parent;
> @@ -387,6 +387,16 @@ struct TypeInfo
> OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name)
>
> /**
> + * InterfaceInfo:
> + * @type: The name of the interface.
> + *
> + * The information associated with an interface.
> + */
> +struct InterfaceInfo {
> +const char *type;
> +};
> +
> +/**
> * InterfaceClass:
> * @parent_class: the base class
> *
> @@ -396,26 +406,30 @@ struct TypeInfo
> struct InterfaceClass
> {
> ObjectClass parent_class;
> +/*< private >*/
> +ObjectClass *concrete_class;
> };
>
> +#define TYPE_INTERFACE "interface"
> +
> /**
> - * InterfaceInfo:
> - * @type: The name of the interface.
> - * @interface_initfn: This method is called during class initialization and
> is
> - * used to initialize an interface associated with a class. This function
> - * should initialize any default virtual functions for a class and/or
> override
> - * virtual functions in a parent class.
> - *
> - * The information associated with an interface.
> + * INTERFACE_CLASS:
> + * @klass: class to cast from
> + * Returns: An #InterfaceClass or raise an error if cast is invalid
> */
> -struct InterfaceInfo
> -{
> -const char *type;
> +#define INTERFACE_CLASS(klass) \
> +OBJECT_CLASS_CHECK(InterfaceClass, klass, TYPE_INTERFACE)
>
> -void (*interface_initfn)(ObjectClass *class, void *data);
> -};
> -
> -#define TYPE_INTERFACE "interface"
> +/**
> + * INTERFACE_CHECK:
> + * @interface: the type to return
> + * @obj: the object to convert to an interface
> + * @name: the interface type name
> + *
> + * Returns: @obj casted to @interface if cast is valid, otherwise raise
> error.
> + */
> +#define INTERFACE_CHECK(interface, obj, name) \
> +((interface *)object_dynamic_cast_assert(OBJECT((obj)), (name)))
>
> /
Re: [Qemu-devel] [PATCH 2/2] pseries: Use new hook to correct reset sequence
On 8 August 2012 01:00, Anthony Liguori wrote: > > They need a per machine hook before and after devices are created. This is > okay and it turns out it can be handy for other machines too that do > similiar could not exist outside of a simulator features. If it's 'before and after device creation' maybe we could call it something less confusing than "reset" ? -- PMM
Re: [Qemu-devel] For all targets and machine types: "start to monitor" smoke test
On 8 August 2012 08:50, Markus Armbruster wrote: > Markus Armbruster writes: >> The string should be suitable for inserting into -help. > > Sufficiently common cases can also be delegated to generic code: > > * Maximum number of CPUs > > Got that: QEMUMachine member max_cpus, main() enforces it. > > * Require -kernel > > Have a flag in QEMUMachine, enforce in main(). > > * Require certain drives > > Have QEMUMachine declare minimum and maximum number drives of each > BlockInterfaceType? I'm not entirely sure what the aim here is? Allow some automated test program to programmatically determine required arguments for a smoke test? Make machines consistent about how they inform the user about required arguments etc? Let us print this info in --help? Something else? -- PMM
Re: [Qemu-devel] Is it possible to detect guest OS modifying pte inside QEMU?
On 8 August 2012 08:38, 陳韋任 (Wei-Ren Chen) wrote: > Just for research, we are studying if we can leave the guest page > table walk to underlying hardware rather than using software emulation > (like current approach). So, maybe (if *doable*) we can use x86 hardware > to help us to walk guest (like ARM) page table. The rough idea is we > have to maintain a x86-format shadow page table for the corresponding > ARM page table, point host cr3 to the shadow page table, and let x86 > hardware do its job. The problem is, we have to aware that guest is > modifying its guest page table entry so that we can sync its corrsponding > shadow page table. But, we still haven't find a good way to know when > the guest OS is modifying guest page table entry. Make it read-only for the guest, and then when you get the exception when the guest tries to write it, you can (a) do what you need to do and (b) emulate the failing write insn. -- PMM
Re: [Qemu-devel] [PATCH 2/5] s390: Virtual channel subsystem support.
On 7 August 2012 15:52, Cornelia Huck wrote:
> +static void sch_handle_clear_func(SubchDev *sch)
> +{
> +struct pmcw *p = &sch->curr_status.pmcw;
> +struct scsw *s = &sch->curr_status.scsw;
> +int path;
> +
> +/* Path management: In our simple css, we always choose the only path. */
> +path = 0x80;
> +
> +/* Reset values prior to 'issueing the clear signal'. */
> +p->lpum = 0;
This is unnecessary since we're going to set p->lpum to something else about
ten lines later, right?
> +p->pom = 0xff;
> +s->pno = 0;
> +
> +/* We always 'attempt to issue the clear signal', and we always succeed.
> */
> +sch->orb = NULL;
> +sch->channel_prog = NULL;
> +sch->last_cmd = NULL;
> +s->actl &= ~SCSW_ACTL_CLEAR_PEND;
> +s->stctl |= SCSW_STCTL_STATUS_PEND;
> +
> +s->dstat = 0;
> +s->cstat = 0;
> +p->lpum = path;
> +
> +}
-- PMM
Re: [Qemu-devel] Funny -m arguments can crash
On 8 August 2012 10:04, Markus Armbruster wrote: > Next problem: minimum RAM size. > > For instance, -M pc -m X, where X < 32KiB dies "qemu: fatal: Trying to > execute code outside RAM or ROM at [...] Aborted (core dumped)" with > TCG, and "KVM internal error. Suberror: 1" with KVM. > > Should a minimum RAM size be enforced? Board-specific? Good luck. Last time I tried to add RAM size checks it got shot down in review... (http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02907.html) -- PMM
Re: [Qemu-devel] [PATCH 01/15] atomic: introduce atomic operations
On 8 August 2012 07:25, Liu Ping Fan wrote:
> +static inline void atomic_sub(int i, Atomic *v)
> +{
> +asm volatile("lock; subl %1,%0"
> + : "+m" (v->counter)
> + : "ir" (i));
> +}
NAK. We don't want random inline assembly implementations of locking
primitives in QEMU, they are way too hard to keep working with all the
possible host architectures we support. I spent some time a while back
getting rid of the (variously busted) versions we had previously.
If you absolutely must use atomic ops, use the gcc builtins. For
preference, stick to higher level and less error-prone abstractions.
-- PMM
Re: [Qemu-devel] [PATCH 12/15] qdev: using devtree lock to protect device's accessing
On 8 August 2012 07:25, Liu Ping Fan wrote: > From: Liu Ping Fan > > lock: > qemu_device_tree_mutex Looking at where it's used, this doesn't seem to have anything to do with device trees (ie dtb, see www.devicetree.org) : poorly named lock? -- PMM
Re: [Qemu-devel] [PATCH] update-linux-headers.sh: Pull in asm-generic/kvm_para.h
Ping? patchwork url: http://patchwork.ozlabs.org/patch/173202/ -- PMM On 25 July 2012 16:29, Peter Maydell wrote: > Add asm-generic/kvm_para.h to the set of non-architecture specific > KVM kernel headers we copy into QEMU. This header may be included > by an architecture's kvm_para.h header. > > Signed-off-by: Peter Maydell > --- > scripts/update-linux-headers.sh |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh > index 9d2a4bc..a639c5b 100755 > --- a/scripts/update-linux-headers.sh > +++ b/scripts/update-linux-headers.sh > @@ -46,6 +46,11 @@ mkdir -p "$output/linux-headers/linux" > for header in kvm.h kvm_para.h vhost.h virtio_config.h virtio_ring.h; do > cp "$tmpdir/include/linux/$header" "$output/linux-headers/linux" > done > +rm -rf "$output/linux-headers/asm-generic" > +mkdir -p "$output/linux-headers/asm-generic" > +for header in kvm_para.h; do > +cp "$tmpdir/include/asm-generic/$header" > "$output/linux-headers/asm-generic" > +done > if [ -L "$linux/source" ]; then > cp "$linux/source/COPYING" "$output/linux-headers" > else > -- > 1.7.5.4
Re: [Qemu-devel] [PATCH 01/15] atomic: introduce atomic operations
On 8 August 2012 14:18, Paolo Bonzini wrote: > Il 08/08/2012 15:09, Stefan Hajnoczi ha scritto: >> No need to roll our own or copy the implementation from the kernel. > > To some extent we need to because: > > 1. GCC atomics look ugly, :) do not provide rmb/wmb, and in some > versions of GCC mb is known to be (wrongly) a no-op. > > 2. glib atomics do not provide mb/rmb/wmb either, and > g_atomic_int_get/g_atomic_int_set are inefficient: they add barriers > everywhere, while it is clearer if you put barriers manually, and you > often do not need barriers in the get side. glib atomics also do not > provide xchg. These are arguments in favour of "don't try to use atomic ops" -- if serious large projects like GCC and glib can't produce working efficient implementations for all target architectures, what chance do we have? -- PMM
Re: [Qemu-devel] [PATCH V4 08/12] hw/sd.c: add SD card save/load support
On 31 July 2012 19:18, Igor Mitsyanko wrote: > On 07/31/2012 06:56 PM, Peter Maydell wrote: >> On 27 July 2012 20:29, Igor Mitsyanko wrote: >>> >>> +VMSTATE_BUFFER_MULTIPLY(wp_groups, SDState, 1, NULL, 0, >>> wpgrps_size, >>> +sizeof(unsigned long)), >> >> >> Isn't this trying to use wpgrps_size as the number of unsigned longs in >> the bitmap, when it's actually the size of the bitmap in bits? >> >> (Does this correctly work in migration between 32 and 64 bit systems >> where 'unsigned long' is a different size? How about between a little >> endian 32 bit system and a big endian 64 bit system? I don't know >> enough about the vmstate macros to be confident here...) > You're right, bitmap_new() can allocated buffers of different size for the > same number of bits but different sizeof(long) value. Maybe always align > allocated buffer size to 8 byte? Endianess seems like even bigger issue.. > Looks like we need to think of something else here. Having talked with Juan on IRC, I think the right thing here is to add support for transmitting bitmaps to vmstate, so we can say VMSTATE_BITMAP(wp_groups, SDState, wpgrps_size) and the vmstate code takes care of marshalling the bitmap to a standard wire format (probably 32 bit little endian) and back. I'll have a go at a patch... -- PMM
Re: [Qemu-devel] [PATCH 2/5] s390: Virtual channel subsystem support.
On 8 August 2012 20:16, Blue Swirl wrote: > On Wed, Aug 8, 2012 at 8:17 AM, Cornelia Huck > wrote: >> On Tue, 7 Aug 2012 21:00:59 + >> Blue Swirl wrote: >>> Please use more descriptive names instead of acronyms, for example >>> SubChStatus. >> >> I'd rather leave these at the well-known scsw, pmcw, etc. names. These >> have been around for decades, and somebody familiar with channel I/O >> will instantly know what a struct scsw is, but will need to look hard >> at the code to figure out the meaning of SubChStatus. > > If they are well-known and have been around for so long time, are > there any suitable header files (with compatible licenses) where they > are defined which could be reused? > > Otherwise, please follow CODING_STYLE. I think we should follow CODING_STYLE for capitalisation issues but generally if the device's documentation has standard abbreviations for register names, structures, etc, etc we should use them. Often this code has to be maintained later by somebody else who might not be familiar with the general operation of the hardware and who is trying to match up the code with whatever the data sheet says. Following the naming used in the h/w docs makes that job easier. (for instance I took the opportunity of making a bunch of structure member names in target-arm line up with the ARM ARM names as part of the refactoring that went on a while back.) -- PMM
Re: [Qemu-devel] [PATCH 56/72] PPC: e500: Use new MPIC dt format
On 24 June 2012 00:07, Alexander Graf wrote: > Due to popular demand, we're updating the way we generate the MPIC > node and interrupt lines based on what the current state of art is. Any chance of a slightly more detailed commit message? thanks -- PMM
Re: [Qemu-devel] [PATCH 56/72] PPC: e500: Use new MPIC dt format
On 8 August 2012 23:40, Peter Maydell wrote: > On 24 June 2012 00:07, Alexander Graf wrote: >> Due to popular demand, we're updating the way we generate the MPIC >> node and interrupt lines based on what the current state of art is. > > Any chance of a slightly more detailed commit message? Whoops, didn't realise this was an old already-committed patch. Never mind... -- PMM
[Qemu-devel] [PATCH] vmstate: Add support for saving/loading bitmaps
Add support for saving/loading bitmap.h bitmaps in vmstate.
Signed-off-by: Peter Maydell
---
This will be needed for saving/restoring the bitmap in sd.c which
is introduced by Igor's latest patchset; the relevant VMSTATE line is:
VMSTATE_BITMAP(wp_groups, SDState, 1, wpgrps_size),
(and you'll need to make wpgrps_size an int32_t, not uint32_t).
Igor: I've only tested this fairly lightly, you'll probably want to
do things like testing save on 32 bit and load on 64 bit and
vice-versa.
savevm.c | 41 +
vmstate.h | 13 +
2 files changed, 54 insertions(+)
diff --git a/savevm.c b/savevm.c
index 6e82b2d..0e2de97 100644
--- a/savevm.c
+++ b/savevm.c
@@ -86,6 +86,7 @@
#include "memory.h"
#include "qmp-commands.h"
#include "trace.h"
+#include "bitops.h"
#define SELF_ANNOUNCE_ROUNDS 5
@@ -1159,6 +1160,46 @@ const VMStateInfo vmstate_info_unused_buffer = {
.put = put_unused_buffer,
};
+/* bitmaps (as defined by bitmap.h). Note that size here is the size
+ * of the bitmap in bits. The on-the-wire format of a bitmap is 64
+ * bit words with the bits in big endian order. The in-memory format
+ * is an array of 'unsigned long', which may be either 32 or 64 bits.
+ */
+/* This is the number of 64 bit words sent over the wire */
+#define BITS_TO_U64S(nr) DIV_ROUND_UP(nr, 64)
+static int get_bitmap(QEMUFile *f, void *pv, size_t size)
+{
+unsigned long *bmp = pv;
+int i, idx = 0;
+for (i = 0; i < BITS_TO_U64S(size); i++) {
+uint64_t w = qemu_get_be64(f);
+bmp[idx++] = w;
+if (sizeof(unsigned long) == 4 && idx < BITS_TO_LONGS(size)) {
+bmp[idx++] = w >> 32;
+}
+}
+return 0;
+}
+
+static void put_bitmap(QEMUFile *f, void *pv, size_t size)
+{
+unsigned long *bmp = pv;
+int i, idx = 0;
+for (i = 0; i < BITS_TO_U64S(size); i++) {
+uint64_t w = bmp[idx++];
+if (sizeof(unsigned long) == 4 && idx < BITS_TO_LONGS(size)) {
+w |= ((uint64_t)bmp[idx++]) << 32;
+}
+qemu_put_be64(f, w);
+}
+}
+
+const VMStateInfo vmstate_info_bitmap = {
+.name = "bitmap",
+.get = get_bitmap,
+.put = put_bitmap,
+};
+
typedef struct CompatEntry {
char idstr[256];
int instance_id;
diff --git a/vmstate.h b/vmstate.h
index 5bd2b76..c45f46e 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -139,6 +139,7 @@ extern const VMStateInfo vmstate_info_uint64;
extern const VMStateInfo vmstate_info_timer;
extern const VMStateInfo vmstate_info_buffer;
extern const VMStateInfo vmstate_info_unused_buffer;
+extern const VMStateInfo vmstate_info_bitmap;
#define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
#define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0)
@@ -411,6 +412,18 @@ extern const VMStateInfo vmstate_info_unused_buffer;
.flags= VMS_BUFFER, \
}
+/* _field_size should be a uint32_t field in the _state struct giving the
+ * size of the bitmap _field in bits.
+ */
+#define VMSTATE_BITMAP(_field, _state, _version, _field_size) { \
+.name = (stringify(_field)), \
+.version_id = (_version), \
+.size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
+.info = &vmstate_info_bitmap,\
+.flags= VMS_VBUFFER|VMS_POINTER, \
+.offset = offsetof(_state, _field),\
+}
+
/* _f : field name
_f_n : num of elements field_name
_n : num of elements
--
1.7.9.5
[Qemu-devel] [RFC 2/5] ARM: KVM: Add support for KVM on ARM architecture
From: Christoffer Dall
Add basic support for KVM on ARM architecture.
Signed-off-by: Christoffer Dall
[Rusty: updates to use KVM_ARM_VCPU_INIT, KVM_GET/SET_MSRS]
Signed-off-by: Rusty Russell
[PMM: Minor tweaks and code cleanup]
Signed-off-by: Peter Maydell
---
hw/arm_pic.c | 28 +
target-arm/Makefile.objs |1 +
target-arm/cpu.h |1 +
target-arm/helper.c |2 +-
target-arm/kvm.c | 274 ++
5 files changed, 305 insertions(+), 1 deletion(-)
create mode 100644 target-arm/kvm.c
diff --git a/hw/arm_pic.c b/hw/arm_pic.c
index ffb4d41..950e6c4 100644
--- a/hw/arm_pic.c
+++ b/hw/arm_pic.c
@@ -9,6 +9,7 @@
#include "hw.h"
#include "arm-misc.h"
+#include "kvm.h"
/* Input 0 is IRQ and input 1 is FIQ. */
static void arm_pic_cpu_handler(void *opaque, int irq, int level)
@@ -34,7 +35,34 @@ static void arm_pic_cpu_handler(void *opaque, int irq, int
level)
}
}
+#ifdef CONFIG_KVM
+static void kvm_arm_pic_cpu_handler(void *opaque, int irq, int level)
+{
+ARMCPU *cpu = opaque;
+CPUARMState *env = &cpu->env;
+int kvm_irq;
+
+switch (irq) {
+case ARM_PIC_CPU_IRQ:
+kvm_irq = KVM_ARM_IRQ_LINE;
+break;
+case ARM_PIC_CPU_FIQ:
+kvm_irq = KVM_ARM_FIQ_LINE;
+break;
+default:
+hw_error("kvm_arm_pic_cpu_handler: Bad interrupt line %d\n", irq);
+}
+kvm_irq |= (env->cpu_index << 1);
+kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
+}
+#endif
+
qemu_irq *arm_pic_init_cpu(ARMCPU *cpu)
{
+#ifdef CONFIG_KVM
+if (kvm_enabled()) {
+return qemu_allocate_irqs(kvm_arm_pic_cpu_handler, cpu, 2);
+}
+#endif
return qemu_allocate_irqs(arm_pic_cpu_handler, cpu, 2);
}
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index f447c4f..f906d20 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -1,5 +1,6 @@
obj-y += arm-semi.o
obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_KVM) += kvm.o
obj-y += translate.o op_helper.o helper.o cpu.o
obj-y += neon_helper.o iwmmxt_helper.o
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 191895c..5194bad 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -236,6 +236,7 @@ ARMCPU *cpu_arm_init(const char *cpu_model);
void arm_translate_init(void);
int cpu_arm_exec(CPUARMState *s);
void do_interrupt(CPUARMState *);
+int bank_number(CPUARMState *env, int mode);
void switch_mode(CPUARMState *, int);
uint32_t do_arm_semihosting(CPUARMState *env);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5727da2..632a366 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1619,7 +1619,7 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env,
uint32_t mode)
#else
/* Map CPU modes onto saved register banks. */
-static inline int bank_number(CPUARMState *env, int mode)
+int bank_number(CPUARMState *env, int mode)
{
switch (mode) {
case ARM_CPU_MODE_USR:
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
new file mode 100644
index 000..b9abee0
--- /dev/null
+++ b/target-arm/kvm.c
@@ -0,0 +1,274 @@
+/*
+ * ARM implementation of KVM hooks
+ *
+ * Copyright Christoffer Dall 2009-2010
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#include "qemu-common.h"
+#include "qemu-timer.h"
+#include "sysemu.h"
+#include "kvm.h"
+#include "cpu.h"
+#include "device_tree.h"
+#include "hw/arm-misc.h"
+
+const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+KVM_CAP_LAST_INFO
+};
+
+int kvm_arch_init(KVMState *s)
+{
+/* For ARM interrupt delivery is always asynchronous,
+ * whether we are using an in-kernel VGIC or not.
+ */
+kvm_async_interrupts_allowed = true;
+return 0;
+}
+
+int kvm_arch_init_vcpu(CPUARMState *env)
+{
+struct kvm_vcpu_init init;
+
+init.target = KVM_ARM_TARGET_CORTEX_A15;
+memset(init.features, 0, sizeof(init.features));
+return kvm_vcpu_ioctl(env, KVM_ARM_VCPU_INIT, &init);
+}
+
+#define MSR32_INDEX_OF(coproc, crn, opc1, crm, opc2) \
+(((coproc)<<16) | ((opc1)<<11) | ((crn)<<7) | ((opc2)<<4) | (crm))
+
+int kvm_arch_put_registers(CPUARMState *env, int level)
+{
+struct kvm_regs regs;
+int mode, bn;
+struct cp15 {
+struct kvm_msrs hdr;
+struct kvm_msr_entry e[2];
+} cp15;
+int ret;
+
+ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
+if (ret < 0) {
+return ret;
+}
+
+/* We make sure the banked regs are properly set */
+mode = env->uncached_cpsr & CPSR_M;
+bn = bank_number(env, mode);
+if (mode == ARM_CPU_MODE_FIQ) {
+memcpy(env->fiq_regs, env->regs + 8, 5 * sizeof(uint32_t));
+
[Qemu-devel] [RFC 3/5] hw/arm_gic: Add presave/postload hooks
Add presave/postload hooks to the ARM GIC common base class.
These will be used by the KVM in-kernel GIC subclass to sync
state between kernel and userspace when migrating.
Signed-off-by: Peter Maydell
---
hw/arm_gic_common.c | 10 ++
hw/arm_gic_internal.h |2 ++
2 files changed, 12 insertions(+)
diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
index 360e782..d972755 100644
--- a/hw/arm_gic_common.c
+++ b/hw/arm_gic_common.c
@@ -23,9 +23,14 @@
static void gic_save(QEMUFile *f, void *opaque)
{
gic_state *s = (gic_state *)opaque;
+ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
int i;
int j;
+if (c->pre_save) {
+c->pre_save(s);
+}
+
qemu_put_be32(f, s->enabled);
for (i = 0; i < s->num_cpu; i++) {
qemu_put_be32(f, s->cpu_enabled[i]);
@@ -57,6 +62,7 @@ static void gic_save(QEMUFile *f, void *opaque)
static int gic_load(QEMUFile *f, void *opaque, int version_id)
{
gic_state *s = (gic_state *)opaque;
+ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
int i;
int j;
@@ -91,6 +97,10 @@ static int gic_load(QEMUFile *f, void *opaque, int
version_id)
s->irq_state[i].trigger = qemu_get_byte(f);
}
+if (c->post_load) {
+c->post_load(s);
+}
+
return 0;
}
diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h
index db4fad5..183dca6 100644
--- a/hw/arm_gic_internal.h
+++ b/hw/arm_gic_internal.h
@@ -118,6 +118,8 @@ void gic_init_irqs_and_distributor(gic_state *s, int
num_irq);
typedef struct ARMGICCommonClass {
SysBusDeviceClass parent_class;
+void (*pre_save)(gic_state *s);
+void (*post_load)(gic_state *s);
} ARMGICCommonClass;
#define TYPE_ARM_GIC "arm_gic"
--
1.7.9.5
[Qemu-devel] [RFC 0/5] Support KVM on ARM
This patch series is an early RFC for the QEMU patches adding support for KVM on ARM Cortex-A15 hardware. It's intended for use with the kernel tree at git://github.com/virtualopensystems/linux-kvm-arm.git kvm-a15-v10-stage There are two aims here: * early review for qemu-devel folk * resend a complete set of patches to kvmarm, since I've done a number of incremental changes and tweaks since Christoffer's original QEMU code These patches depend on various cleanups to KVM and configure which I've posted in the last couple of weeks: configure: Don't implicitly hardcode list of KVM architectures update-linux-headers.sh: Pull in asm-generic/kvm_para.h update-linux-headers.sh: Don't hard code list of architectures kvm-all.c: Move init of irqchip_inject_ioctl out of kvm_irqchip_create() kvm: Add documentation comment for kvm_irqchip_in_kernel() kvm: Decouple 'GSI routing' from 'kernel irqchip' kvm: Decouple 'MSI routing via irqfds' from 'kernel irqchip' kvm: Decouple 'irqfds usable' from 'kernel irqchip' kvm: Move kvm_allows_irq0_override() to target-i386, fix return type kvm: Rename kvm_irqchip_set_irq() to kvm_set_irq() kvm: Decouple 'async interrupt delivery' from 'kernel irqchip' A git branch consisting of qemu master + these preliminary fixes + the ARM patches is available here: git://git.linaro.org/people/pmaydell/qemu-arm.git kvm-arm with pointy-clicky version here: http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/kvm-arm There are still a number of TODOs scattered through the code; a quick summary: * a15mpcore should enforce vgic use (currently not done pending the VGIC patches landing in Christoffer's kernel tree) * the makefile change for the hw/kvm/arm_gic object is not right (see discussions on qemu-devel in the past about how to handle "only if architecture foo and KVM" object files) * kvm_arch_put/get_registers should drive register set/get from the cp15 hashtable * we should use an accessor function for c2_mask/base/control * breakpoint support is unimplemented * vgic register save/load from kernel is unimplemented (no kernel ABI) * fpu save/load unimplemented (no kernel ABI yet) * tell kernel the A15 peripheral base address (no kernel ABI) * the kernel ABI for sending per-CPU interrupts for VGIC vs non-VGIC is inconsistent (the former uses a vcpu ioctl, the latter encodes cpu number in the irq number), and we should standardise on one approach or the other Christoffer Dall (1): ARM: KVM: Add support for KVM on ARM architecture Peter Maydell (4): linux-headers: Add ARM KVM headers (not for upstream) hw/arm_gic: Add presave/postload hooks hw/kvm/arm_gic: Implement support for KVM in-kernel ARM GIC configure: Enable KVM on ARM configure|2 +- hw/a15mpcore.c | 11 +- hw/arm/Makefile.objs |1 + hw/arm_gic_common.c | 10 ++ hw/arm_gic_internal.h|2 + hw/arm_pic.c | 28 hw/kvm/arm_gic.c | 153 +++ linux-headers/asm-arm/kvm.h | 119 +++ linux-headers/asm-arm/kvm_para.h |1 + linux-headers/asm-generic/kvm_para.h |5 + linux-headers/linux/kvm.h|3 + target-arm/Makefile.objs |1 + target-arm/cpu.h |1 + target-arm/helper.c |2 +- target-arm/kvm.c | 274 ++ 15 files changed, 610 insertions(+), 3 deletions(-) create mode 100644 hw/kvm/arm_gic.c create mode 100644 linux-headers/asm-arm/kvm.h create mode 100644 linux-headers/asm-arm/kvm_para.h create mode 100644 linux-headers/asm-generic/kvm_para.h create mode 100644 target-arm/kvm.c -- 1.7.9.5
[Qemu-devel] [RFC 4/5] hw/kvm/arm_gic: Implement support for KVM in-kernel ARM GIC
Implement support for using the KVM in-kernel GIC for ARM.
Signed-off-by: Peter Maydell
---
hw/a15mpcore.c | 11 +++-
hw/arm/Makefile.objs |1 +
hw/kvm/arm_gic.c | 153 ++
3 files changed, 164 insertions(+), 1 deletion(-)
create mode 100644 hw/kvm/arm_gic.c
diff --git a/hw/a15mpcore.c b/hw/a15mpcore.c
index fc0a02a..e075849 100644
--- a/hw/a15mpcore.c
+++ b/hw/a15mpcore.c
@@ -19,6 +19,7 @@
*/
#include "sysbus.h"
+#include "kvm.h"
/* A15MP private memory region. */
@@ -41,7 +42,15 @@ static int a15mp_priv_init(SysBusDevice *dev)
A15MPPrivState *s = FROM_SYSBUS(A15MPPrivState, dev);
SysBusDevice *busdev;
-s->gic = qdev_create(NULL, "arm_gic");
+/* TODO when the VGIC patches make it to Christoffer's kernel
+ * tree we can make !kvm_irqchip_in_kernel() a fatal error.
+ */
+if (kvm_irqchip_in_kernel()) {
+s->gic = qdev_create(NULL, "kvm-arm_gic");
+} else {
+s->gic = qdev_create(NULL, "arm_gic");
+}
+
qdev_prop_set_uint32(s->gic, "num-cpu", s->num_cpu);
qdev_prop_set_uint32(s->gic, "num-irq", s->num_irq);
qdev_prop_set_uint32(s->gic, "revision", 2);
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index c413780..881fffd 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -39,5 +39,6 @@ obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
obj-y += kzm.o
obj-y += pl041.o lm4549.o
obj-$(CONFIG_FDT) += ../device_tree.o
+obj-$(CONFIG_KVM) += kvm/arm_gic.o
obj-y := $(addprefix ../,$(obj-y))
diff --git a/hw/kvm/arm_gic.c b/hw/kvm/arm_gic.c
new file mode 100644
index 000..4535f90
--- /dev/null
+++ b/hw/kvm/arm_gic.c
@@ -0,0 +1,153 @@
+/*
+ * ARM Generic Interrupt Controller using KVM in-kernel support
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Written by Peter Maydell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/sysbus.h"
+#include "kvm.h"
+#include "hw/arm_gic_internal.h"
+
+#define TYPE_KVM_ARM_GIC "kvm-arm_gic"
+#define KVM_ARM_GIC(obj) \
+ OBJECT_CHECK(gic_state, (obj), TYPE_KVM_ARM_GIC)
+#define KVM_ARM_GIC_CLASS(klass) \
+ OBJECT_CLASS_CHECK(KVMARMGICClass, (klass), TYPE_KVM_ARM_GIC)
+#define KVM_ARM_GIC_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(KVMARMGICClass, (obj), TYPE_KVM_ARM_GIC)
+
+typedef struct KVMARMGICClass {
+ARMGICCommonClass parent_class;
+int (*parent_init)(SysBusDevice *dev);
+void (*parent_reset)(DeviceState *dev);
+} KVMARMGICClass;
+
+static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
+{
+/* Meaning of the 'irq' parameter:
+ * [0..N-1] : external interrupts
+ * [N..N+31] : PPI (internal) interrupts for CPU 0
+ * [N+32..N+63] : PPI (internal interrupts for CPU 1
+ * ...
+ */
+gic_state *s = (gic_state *)opaque;
+
+
+if (irq < (s->num_irq - GIC_INTERNAL)) {
+/* External interrupt number 'irq' */
+kvm_set_irq(kvm_state, irq + GIC_INTERNAL, !!level);
+} else {
+struct kvm_irq_level irq_level;
+int cpu;
+irq -= (s->num_irq - GIC_INTERNAL);
+cpu = irq / GIC_INTERNAL;
+irq %= GIC_INTERNAL;
+/* Internal interrupt 'irq' for CPU 'cpu' */
+irq_level.irq = irq;
+irq_level.level = !!level;
+kvm_vcpu_ioctl(qemu_get_cpu(cpu), KVM_IRQ_LINE, &irq_level);
+}
+}
+
+static void kvm_arm_gic_put(gic_state *s)
+{
+/* TODO: there isn't currently a kernel interface to set the GIC state */
+}
+
+static void kvm_arm_gic_get(gic_state *s)
+{
+/* TODO: there isn't currently a kernel interface to get the GIC state */
+}
+
+static void kvm_arm_gic_reset(DeviceState *dev)
+{
+gic_state *s = ARM_GIC_COMMON(dev);
+KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s);
+kgc->parent_reset(dev);
+kvm_arm_gic_put(s);
+}
+
+static int kvm_arm_gic_init(SysBusDevice *dev)
+{
+/* Device instance init function for the GIC sysbus device */
+int i;
+gic_state *s = FROM_SYSBUS(gic_state, dev);
+KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s);
+
+kgc->parent_init(dev);
+
+i = s->n
[Qemu-devel] [RFC 1/5] linux-headers: Add ARM KVM headers (not for upstream)
This commit adds the ARM KVM headers. This is not to go to QEMU
upstream -- the correct path there is that the KVM code will be
committed to a mainline upstream kernel, and then upstream QEMU
can do a bulk header update from the upstream kernel, which will
allow us to drop this temporary commit.
This commit updates to the KVM ARM kernel tree commit
5196b1b58c, including the changes to the cp15 access
ioctls. It is the result of an update-linux-headers.sh run
with the non-ARM changes removed.
---
linux-headers/asm-arm/kvm.h | 119 ++
linux-headers/asm-arm/kvm_para.h |1 +
linux-headers/asm-generic/kvm_para.h |5 ++
linux-headers/linux/kvm.h|3 +
4 files changed, 128 insertions(+)
create mode 100644 linux-headers/asm-arm/kvm.h
create mode 100644 linux-headers/asm-arm/kvm_para.h
create mode 100644 linux-headers/asm-generic/kvm_para.h
diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
new file mode 100644
index 000..d040a2a
--- /dev/null
+++ b/linux-headers/asm-arm/kvm.h
@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2012 - Virtual Open Systems and Columbia University
+ * Author: Christoffer Dall
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef __ARM_KVM_H__
+#define __ARM_KVM_H__
+
+#include
+
+#define __KVM_HAVE_GUEST_DEBUG
+#define __KVM_HAVE_IRQ_LINE
+
+/*
+ * KVM_IRQ_LINE macros to set/read IRQ/FIQ for specific VCPU index.
+ */
+enum KVM_ARM_IRQ_LINE_TYPE {
+ KVM_ARM_IRQ_LINE = 0,
+ KVM_ARM_FIQ_LINE = 1,
+};
+
+/*
+ * Modes used for short-hand mode determinition in the world-switch code and
+ * in emulation code.
+ *
+ * Note: These indices do NOT correspond to the value of the CPSR mode bits!
+ */
+enum vcpu_mode {
+ MODE_FIQ = 0,
+ MODE_IRQ,
+ MODE_SVC,
+ MODE_ABT,
+ MODE_UND,
+ MODE_USR,
+ MODE_SYS
+};
+
+struct kvm_regs {
+ __u32 regs0_7[8]; /* Unbanked regs. (r0 - r7)*/
+ __u32 fiq_regs8_12[5]; /* Banked fiq regs. (r8 - r12) */
+ __u32 usr_regs8_12[5]; /* Banked usr registers (r8 - r12) */
+ __u32 reg13[6]; /* Banked r13, indexed by MODE_*/
+ __u32 reg14[6]; /* Banked r13, indexed by MODE_*/
+ __u32 reg15;
+ __u32 cpsr;
+ __u32 spsr[5]; /* Banked SPSR, indexed by MODE_ */
+};
+
+/* Supported Processor Types */
+#define KVM_ARM_TARGET_CORTEX_A15 (0xC0F)
+
+struct kvm_vcpu_init {
+ __u32 target;
+ __u32 features[7];
+};
+
+struct kvm_sregs {
+};
+
+struct kvm_fpu {
+};
+
+struct kvm_guest_debug_arch {
+};
+
+struct kvm_debug_exit_arch {
+};
+
+struct kvm_sync_regs {
+};
+
+struct kvm_arch_memory_slot {
+};
+
+/* Based on x86, but we use KVM_GET_VCPU_MSR_INDEX_LIST. */
+struct kvm_msr_entry {
+ __u32 index;
+ __u32 reserved;
+ __u64 data;
+};
+
+/* for KVM_GET_MSRS and KVM_SET_MSRS */
+struct kvm_msrs {
+ __u32 nmsrs; /* number of msrs in entries */
+ __u32 pad;
+
+ struct kvm_msr_entry entries[0];
+};
+
+/* for KVM_VCPU_GET_MSR_INDEX_LIST */
+struct kvm_msr_list {
+ __u32 nmsrs; /* number of msrs in entries */
+ __u32 indices[0];
+};
+
+/* If you need to interpret the index values, here's the key. */
+#define KVM_ARM_MSR_COPROC_MASK0x
+#define KVM_ARM_MSR_64_BIT_MASK0x8000
+#define KVM_ARM_MSR_64_OPC1_MASK 0x00F0
+#define KVM_ARM_MSR_64_CRM_MASK0x000F
+#define KVM_ARM_MSR_32_CRM_MASK0x000F
+#define KVM_ARM_MSR_32_OPC2_MASK 0x0070
+#define KVM_ARM_MSR_32_CRN_MASK0x0780
+#define KVM_ARM_MSR_32_OPC1_MASK 0x3800
+
+#endif /* __ARM_KVM_H__ */
diff --git a/linux-headers/asm-arm/kvm_para.h b/linux-headers/asm-arm/kvm_para.h
new file mode 100644
index 000..14fab8f
--- /dev/null
+++ b/linux-headers/asm-arm/kvm_para.h
@@ -0,0 +1 @@
+#include
diff --git a/linux-headers/asm-generic/kvm_para.h
b/linux-headers/asm-generic/kvm_para.h
new file mode 100644
index 000..63df88b
--- /dev/null
+++ b/linux-headers/asm-generic/kvm_para.h
@@ -0,0 +1,5 @@
+#ifndef _ASM_GENERIC_KVM_PARA_H
+#define _ASM_GENERIC_KVM_PARA_H
+
+
+#endif
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 5a9d4e3..0afbbab 100644
--- a/linux-headers/linux/kv
[Qemu-devel] [RFC 5/5] configure: Enable KVM on ARM
Enable KVM on ARM hosts, now that all the necessary components for it exist. Signed-off-by: Peter Maydell --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index b9a0b27..f7a825a 100755 --- a/configure +++ b/configure @@ -3797,7 +3797,7 @@ case "$target_arch2" in echo "CONFIG_NO_XEN=y" >> $config_target_mak esac case "$target_arch2" in - i386|x86_64|ppcemb|ppc|ppc64|s390x) + arm|i386|x86_64|ppcemb|ppc|ppc64|s390x) # Make sure the target and host cpus are compatible if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \ \( "$target_arch2" = "$cpu" -o \ -- 1.7.9.5
Re: [Qemu-devel] [PATCH 00/23] Suppress unused default drives
On 9 August 2012 14:31, Markus Armbruster wrote: > We create a number of default drives for machines to use: floppy, > CD-ROM, SD card. Machines can suppress the ones they don't use, but > few do. Fix that. For clarity: what are the negative effects that result from machines not saying no_floppy &c ? -- PMM
Re: [Qemu-devel] [PATCH 00/23] Suppress unused default drives
On 9 August 2012 15:08, Markus Armbruster wrote: > Peter Maydell writes: >> For clarity: what are the negative effects that result from >> machines not saying no_floppy &c ? > > "info block" shows the unused default drives. For instance, > > $ qemu-system-x86_64 -vnc :0 -monitor stdio > QEMU 1.1.50 monitor - type 'help' for more information > (qemu) info block > ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok [not inserted] > floppy0: removable=1 locked=0 tray-open=0 [not inserted] > sd0: removable=1 locked=0 tray-open=0 [not inserted] > pflash0: removable=0 file=/work/armbru/qemu/bld-x86/pc-bios/bios.bin ro=1 > drv=raw encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0 > > Drive sd0 is unused here. Might confuse the uninitiated. Moreover, you > *can* use it for something entirely else, if=sd notwithstanding: > > (qemu) device_add lsi > (qemu) device_add scsi-cd,drive=sd0 If/when we get a PCI SD card controller model, would all the PCI using machines need to be added to take the 'no default sd card' setting out again, or does it get overridden anyway if you say "and I'd like an sd controller"? -- PMM
Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests
On 4 August 2012 18:57, Blue Swirl wrote: > On Sat, Jul 28, 2012 at 1:48 PM, Peter Maydell > wrote: >> On 28 July 2012 13:31, Blue Swirl wrote: >>> I'm getting this error, probably because now Valgrind support is enabled: >>> CCcoroutine-ucontext.o >>> cc1: warnings being treated as errors >>> /src/qemu/coroutine-ucontext.c:204: error: unknown option after >>> '#pragma GCC diagnostic' kind >>> /src/qemu/coroutine-ucontext.c:209: error: unknown option after >>> '#pragma GCC diagnostic' kind >>> >>> Maybe the compiler does not understand this pragma and Valgrind check >>> should also fail in that case. >> >> Yeah, I think this is one of the few tests which want to explicitly >> check "is this construct going to provoke a compiler warning" -- >> fix is for that test to explictly put -Werror in the cflags in >> the compile_prog line. > > Now with your Xen configure patches in place, I'm not getting errors > with this applied except for Clang (which I didn't test earlier). > Maybe this should be applied. Yes, I think that (assuming we are going to go down this route at all) it would be good to apply this 11/11 patch now in good time before the freeze. Does anybody want to object? thanks -- PMM
Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
On 9 August 2012 21:01, Phil Staub wrote: > On 08/09/2012 12:57 PM, Blue Swirl wrote: >> On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell >> wrote: >>> For this purpose the usual approach is to follow up to the patch >>> mail saying "Ping" and giving a url to the patch in patchwork, >>> like this one: >>> http://patchwork.ozlabs.org/patch/163705/ >>> >>> Eventually somebody will take pity on it and apply it, but >>> it does require a bit more persistence than for more actively >>> maintained areas of the codebase. >> >> Thanks, applied. > Thank you! Maciej submitted some other MIPS patches at about the same time: http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977 at least some of which got reviewed by Richard Henderson and are therefore good candidates to get committed, if you can fish them out of the list and ping them... -- PMM
Re: [Qemu-devel] [PATCH] configure: fix double check tests with Clang
On 9 August 2012 21:31, Blue Swirl wrote:
> Configuring with Clang compiler with -Werror would not work after
> improved checks:
> /tmp/qemu-conf--25992-.c:4:32: error: self-comparison always evaluates
> to true [-Werror,-Wtautological-compare]
> int main(void) { return preadv == preadv; }
> /tmp/qemu-conf--25992-.c:13:26: error: self-comparison always
> evaluates to true [-Werror,-Wtautological-compare]
> return epoll_create1 == epoll_create1;
> /tmp/qemu-conf--25992-.c:3:13: error: explicitly assigning a variable
> of type 'char **' to itself [-Werror,-Wself-assign]
> environ = environ;
>
> Avoid the errors by adjusting the tests.
>
> Signed-off-by: Blue Swirl
> ---
> configure |6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index d8ec050..c2955f8 100755
> --- a/configure
> +++ b/configure
> @@ -2255,7 +2255,7 @@ cat > $TMPC < #include
> #include
> #include
> -int main(void) { return preadv == preadv; }
> +int main(void) { return preadv(0, (void *)0, 0, 0); }
This shouldn't need the cast, should it?
> EOF
> preadv=no
> if compile_prog "" "" ; then
> @@ -2551,7 +2551,7 @@ int main(void)
> * warning but not an error, and will proceed to fail the
> * qemu compile where we compile with -Werror.)
> */
> -return epoll_create1 == epoll_create1;
> +return (int)(uintptr_t)&epoll_create1;
> }
> EOF
> if compile_prog "" "" ; then
> @@ -2944,7 +2944,7 @@ has_environ=no
> cat > $TMPC << EOF
> #include
> int main(void) {
> -environ = environ;
> +environ = (void *)0;
...and nor should this.
> return 0;
> }
> EOF
> --
> 1.7.2.5
>
-- PMM
Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options
On 9 August 2012 20:25, Eduardo Habkost wrote:
> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>> Peter Maydell writes:
>> > For command line options which permit '?' meaning 'please list the
>> > permitted values', add support for 'help' as a synonym, by abstracting
>> > the check out into a helper function.
>> Applied. Thanks.
>
> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
> "-cpu ?model":
These options appear to be completely undocumented. They're also pretty
ugly syntax and seem to be x86 specific. However we can unbreak them
if we must with a patch like this:
--- a/vl.c
+++ b/vl.c
@@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
*/
cpudef_init();
-if (cpu_model && is_help_option(cpu_model)) {
+/* We have to check for "starts with '?' as well as is_help_option
+ * to support targets which implement various weird help options
+ * via '?thingy' syntax.
+ */
+if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
list_cpus(stdout, &fprintf, cpu_model);
exit(0);
}
(will send as a proper patch with commit message and signoff tomorrow).
Any suggestions for what the sane syntax for these options would be?
(ie the analogous change to having '?' go to 'help').
-- PMM
Re: [Qemu-devel] For all targets and machine types: "start to monitor" smoke test
On Wed, Aug 8, 2012 at 5:22 PM, Markus Armbruster wrote: > Peter Maydell writes: > >> On 7 August 2012 20:26, Markus Armbruster wrote: >>> qemu-system-arm lm3s811evb >>> qemu-system-arm lm3s6965evb >>> qemu-system-arm: /work/armbru/qemu/hw/qdev.c:310: >>> qdev_get_gpio_in: Assertion `n >= 0 && n < dev->num_gpio_in' failed. >> >> This is fixed by http://patchwork.ozlabs.org/patch/172820/ >> (which should be in my arm-devs.next queue, I just haven't >> got round to flushing it yet.) > > It does. > > Next bug: stellaris_init() passes kernel_filename via armv7_init() to > load_elf(), even when it's null. load_elf() fails with the "helpful" > error message "Bad address". > Fix on list. > $ QEMU_AUDIO_DRV=none ../qemu/bld/arm-softmmu/qemu-system-arm -vnc :0 -M > lm3s6965evb -monitor stdio -pflash /dev/null -S > Bad address > QEMU 1.1.50 monitor - type 'help' for more information > (qemu) q >
Re: [Qemu-devel] [PULL 0/2] QOMify AXI stream for Xilinx AXI ethernet/DMA
Resend of pull, Edgars review addressed. On Fri, Aug 10, 2012 at 12:30 PM, Peter A. G. Crosthwaite wrote: > are available in the git repository at: > > git://developer.petalogix.com/public/qemu.git ..BRANCH.NOT.VERIFIED.. > > Anthony Liguori (1): > qom: Reimplement Interfaces > > Peter A. G. Crosthwaite (1): > xilinx_axi*: Re-implemented interconnect > > hw/Makefile.objs |1 + > hw/petalogix_ml605_mmu.c | 24 +++-- > hw/stream.c | 23 + > hw/stream.h | 31 +++ > hw/xilinx.h | 22 ++--- > hw/xilinx_axidma.c | 74 +--- > hw/xilinx_axidma.h | 39 > hw/xilinx_axienet.c | 32 --- > include/qemu/object.h| 46 ++ > qom/object.c | 220 > ++ > 10 files changed, 255 insertions(+), 257 deletions(-) > create mode 100644 hw/stream.c > create mode 100644 hw/stream.h > delete mode 100644 hw/xilinx_axidma.h
Re: [Qemu-devel] [PULL 0/2] QOMify AXI stream for Xilinx AXI ethernet/DMA
Apoligies, bad remote, please disregard. On Fri, Aug 10, 2012 at 12:32 PM, Peter Crosthwaite wrote: > Resend of pull, > > Edgars review addressed. > > On Fri, Aug 10, 2012 at 12:30 PM, Peter A. G. Crosthwaite > wrote: >> are available in the git repository at: >> >> git://developer.petalogix.com/public/qemu.git ..BRANCH.NOT.VERIFIED.. >> >> Anthony Liguori (1): >> qom: Reimplement Interfaces >> >> Peter A. G. Crosthwaite (1): >> xilinx_axi*: Re-implemented interconnect >> >> hw/Makefile.objs |1 + >> hw/petalogix_ml605_mmu.c | 24 +++-- >> hw/stream.c | 23 + >> hw/stream.h | 31 +++ >> hw/xilinx.h | 22 ++--- >> hw/xilinx_axidma.c | 74 +--- >> hw/xilinx_axidma.h | 39 >> hw/xilinx_axienet.c | 32 --- >> include/qemu/object.h| 46 ++ >> qom/object.c | 220 >> ++ >> 10 files changed, 255 insertions(+), 257 deletions(-) >> create mode 100644 hw/stream.c >> create mode 100644 hw/stream.h >> delete mode 100644 hw/xilinx_axidma.h
Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
On Mon, 2012-08-06 at 10:13 +0100, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> wrote:
> > Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
> > SSI slave state (e.g. the CS line state).
>
> This is more me being confused about how this should work than a
> review comment, but it seems a bit odd that we have a hierarchy
> Device->SSI->ADS7846[etc], where the VMState for the ADS7846
> includes a field for the SSI VMState, but the SSI VMState doesn't
> include a field for the Device VMState.
>
> What you've done here matches how i2c works currently, but I've
> just cc'd Anthony and Juan to check whether there's a better way
> of doing it.
>
So VMSD is handled by casting to TYPE_DEVICE and populating dc->vmsd.
This is only going to work for one layer of heirachy. But, I notice a
few device models here and there just call vmstate_register() directly.
Question is, can we call vmstate_register() from the object init
function for SSI_SLAVE, seperately? This would mean that a single
TYPE_DEVICE object is going to get two vmsds instead of one. The
original, and one for SSI_SLAVE (which has the cs state). Sound legit?
Any implementation details and pitfalls to be aware of?
Would be nice to get rid of this change pattern applied to all the
existing SSI devices.
Regards,
Peter
>
> > Signed-off-by: Peter A. G. Crosthwaite
> > ---
> > hw/ads7846.c |1 +
> > hw/max111x.c |1 +
> > hw/spitz.c |2 ++
> > hw/ssi.c | 10 ++
> > hw/ssi.h | 10 ++
> > hw/stellaris.c |1 +
> > hw/z2.c|1 +
> > 7 files changed, 26 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/ads7846.c b/hw/ads7846.c
> > index 41c7f10..6a523f6 100644
> > --- a/hw/ads7846.c
> > +++ b/hw/ads7846.c
> > @@ -124,6 +124,7 @@ static const VMStateDescription vmstate_ads7846 = {
> > .minimum_version_id_old = 0,
> > .post_load = ads7856_post_load,
> > .fields = (VMStateField[]) {
> > +VMSTATE_SSI_SLAVE(ssidev, ADS7846State),
> > VMSTATE_INT32_ARRAY(input, ADS7846State, 8),
> > VMSTATE_INT32(noise, ADS7846State),
> > VMSTATE_INT32(cycle, ADS7846State),
> > diff --git a/hw/max111x.c b/hw/max111x.c
> > index 706d89f..948fd97 100644
> > --- a/hw/max111x.c
> > +++ b/hw/max111x.c
> > @@ -103,6 +103,7 @@ static const VMStateDescription vmstate_max111x = {
> > .minimum_version_id = 0,
> > .minimum_version_id_old = 0,
> > .fields = (VMStateField[]) {
> > +VMSTATE_SSI_SLAVE(ssidev, MAX111xState),
> > VMSTATE_UINT8(tb1, MAX111xState),
> > VMSTATE_UINT8(rb2, MAX111xState),
> > VMSTATE_UINT8(rb3, MAX111xState),
> > diff --git a/hw/spitz.c b/hw/spitz.c
> > index 20e7835..f5d502d 100644
> > --- a/hw/spitz.c
> > +++ b/hw/spitz.c
> > @@ -1087,6 +1087,7 @@ static const VMStateDescription
> > vmstate_corgi_ssp_regs = {
> > .minimum_version_id = 1,
> > .minimum_version_id_old = 1,
> > .fields = (VMStateField []) {
> > +VMSTATE_SSI_SLAVE(ssidev, CorgiSSPState),
> > VMSTATE_UINT32_ARRAY(enable, CorgiSSPState, 3),
> > VMSTATE_END_OF_LIST(),
> > }
> > @@ -1115,6 +1116,7 @@ static const VMStateDescription
> > vmstate_spitz_lcdtg_regs = {
> > .minimum_version_id = 1,
> > .minimum_version_id_old = 1,
> > .fields = (VMStateField []) {
> > +VMSTATE_SSI_SLAVE(ssidev, SpitzLCDTG),
> > VMSTATE_UINT32(bl_intensity, SpitzLCDTG),
> > VMSTATE_UINT32(bl_power, SpitzLCDTG),
> > VMSTATE_END_OF_LIST(),
> > diff --git a/hw/ssi.c b/hw/ssi.c
> > index 35d0a04..2db88fc 100644
> > --- a/hw/ssi.c
> > +++ b/hw/ssi.c
> > @@ -80,6 +80,16 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
> > return r;
> > }
> >
> > +const VMStateDescription vmstate_ssi_slave = {
> > +.name = "SSISlave",
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.minimum_version_id_old = 1,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > static void ssi_slave_register_types(void)
> > {
> > type_register_static(&ssi_bus_info);
> > diff --git a/hw/ssi.h b/hw/ssi.h
> > index 06657d7..975f9fb 100644
> > --- a/hw/ssi.h
> > +++ b/hw/ssi.h
> > @@ -38,6 +38,16 @@ struct SSISlave {
> > #define SSI_SLAVE_FROM_QDEV(de
Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
On 10 August 2012 09:48, Andreas Färber wrote: > Am 09.08.2012 22:36, schrieb Peter Maydell: >> Maciej submitted some other MIPS patches at about the same time: >> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977 >> at least some of which got reviewed by Richard Henderson and are >> therefore good candidates to get committed, if you can fish them out >> of the list and ping them... > > Actually there were better patches for the same bug by Meador, including > git-style rather than SVN patches and adding a helper to initialize it > consistently at all call sites. > > There's also DSP, Octeon, mips64 and signal handling patches around that > someone needs to volunteer to update, test, clean up and queue. > That a patch is on the list doesn't imply that it "just" needs to be > applied though. So please be careful which patches you ping. Yes, hence my suggestion to look for patches which got reviewed. (Although speaking as somebody who has in the past submitted patches which got neither reviewed nor rejected, I have some sympathy for the idea that if nobody among us cares enough to look at a patch at all the default should be to apply it.) -- PMM
Re: [Qemu-devel] [PATCH] armv7m: Guard against no -kernel argument
On 10 August 2012 09:53, Andreas Färber wrote: > Am 10.08.2012 03:27, schrieb Peter A. G. Crosthwaite: >> A -kernel argument must be specified for this machine. Gaurd against no >> -kernel > > "Guard" > >> argument. Previously gave an unhelpful "bad address" error message. >> >> Signed-off-by: Peter A. G. Crosthwaite > > Otherwise looks good - PMM, can you fix? Yes, I have fixed the typo and put this patch into arm-devs.next. I'm planning to put together a pullreq later today. -- PMM
Re: [Qemu-devel] [PATCH 1/2] qom: Reimplement Interfaces
On Fri, Aug 10, 2012 at 7:15 PM, Andreas Färber wrote: > Am 10.08.2012 05:16, schrieb Peter A. G. Crosthwaite: >> From: Anthony Liguori >> >> The current implementation of Interfaces is poorly designed. Each interface >> that an object implements ends up being an object that's tracked by the >> implementing object. There's all sorts of gymnastics to deal with casting >> between these objects. >> >> But an interface shouldn't be associated with an Object. Interfaces are >> global >> to a class. This patch moves all Interface knowledge to ObjectClass >> eliminating >> the relationship between Object and Interfaces. >> >> Interfaces are now abstract (as they should be) but this is okay. Interfaces >> essentially act as additional parents for the classes and are treated as >> such. >> >> With this new implementation, we should fully support derived interfaces >> including reimplementing an inherited interface. >> >> PC: Rebased against qom-next merge Jun-2012. >> >> PC: Removed replication of cast logic for interfaces, i.e. there is only >> one cast function - object_dynamic_cast() (and object_dynamic_cast_assert()) >> >> Signed-off-by: Anthony Liguori >> Signed-off-by: Peter A. G. Crosthwaite >> Acked-by: Paolo Bonzini > > Anthony, didn't you have a whole series to refactor interfaces and add > test cases? > This is patch 1 from that series. I have refactored it a little (as indicated in the comments). Unless theres been another series since I missed? > /-F > > -- > 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 v6 1/4] hw: introduce standard SD host controller
Ping! Any further thoughts here? There seem to be a few minor correction for PPM, but the sore-thumb issue is the long/infinite ADMA. Is there an (easy) AIO based solution to be had or do we need to do some sort of ptimer hack? Regards, Peter On Tue, Aug 7, 2012 at 4:31 PM, Peter Crosthwaite wrote: >>>> +sdhci_update_irq(s); >>>> +break; >>>> +} >>>> +} >>>> +} >>> >>> So I think the guest can make this loop never terminate if it sets up >>> a loop of ACT_LINK descriptors, right? I don't know how we should >>> handle this but I'm pretty sure "make qemu sit there forever not >>> responding >>> to anything and not resettable" isn't it. >>> > > Can I suggest that this is a general problem, that needs to be solved > by the AIO layer. The AIO+block layer uses coroutines to do > chunk-by-chunk non-blocking IO. I dont see how this is different. The > problem is solved by setting up asynchronous DMA transactions or even > better, arbitrary asynchronous hardware events. AIO DMA would have a > similar api to the block layer and would solve this problem once, > rather than having to put ptimers or coroutine-foo in every SGDMA > capable devices to watchdog for infinite loops. > >>> -- PMM >>> >> >> That could only happen if guest would do this on purpose, but I see your >> point. There's no way for us to tell if SD card read/write succeeded or not, > > I think we are talking about corner cases here. If there is major > infrastructural developments needed to do this properly (which I think > there might very well be), then can we declare this issue out of scope > of this series and come back to this as an incremental development. > > To summarise, its a hard problem to solve with minimal reward. > > Regards, > Peter > >> so I think the only way to to this is to suspend transfer after some >> reasonable amount of loops and resume it by qemu_timer, giving CPU time to >> do something useful. >> I've never seen long ADMA loops, so it wouldn't reflect on performance in >> any way.
[Qemu-devel] [PATCH 1/2] target-i386: Fold -cpu ?cpuid, ?model output into -cpu help, drop ?dump
Commit c8057f95 (accidentally) disabled the ability to pass
option strings starting with '?' to the target-specific
cpu_list function, so the target-i386 specific "-cpu ?dump",
"-cpu ?cpuid" and "-cpu ?model" stopped working.
Since these options are undocumented and not used by libvirt,
simply drop them completely rather than reinstating them
with new style syntax. Instead, we fold the ?model and ?cpuid
output into the output of the plain "-cpu help" output. The
detailed output produced by ?dump is dropped.
Signed-off-by: Peter Maydell
---
target-i386/cpu.c | 64 +
1 file changed, 11 insertions(+), 53 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 880cfea..d3a2b47 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1059,70 +1059,28 @@ static void listflags(char *buf, int bufsize, uint32_t
fbits,
}
}
-/* generate CPU information:
- * -?list model names
- * -?model list model names/IDs
- * -?dumpoutput all model (x86_def_t) data
- * -?cpuid list all recognized cpuid flag names
- */
+/* generate CPU information */
void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
-unsigned char model = !strcmp("?model", optarg);
-unsigned char dump = !strcmp("?dump", optarg);
-unsigned char cpuid = !strcmp("?cpuid", optarg);
x86_def_t *def;
char buf[256];
-if (cpuid) {
-(*cpu_fprintf)(f, "Recognized CPUID flags:\n");
-listflags(buf, sizeof (buf), (uint32_t)~0, feature_name, 1);
-(*cpu_fprintf)(f, " f_edx: %s\n", buf);
-listflags(buf, sizeof (buf), (uint32_t)~0, ext_feature_name, 1);
-(*cpu_fprintf)(f, " f_ecx: %s\n", buf);
-listflags(buf, sizeof (buf), (uint32_t)~0, ext2_feature_name, 1);
-(*cpu_fprintf)(f, " extf_edx: %s\n", buf);
-listflags(buf, sizeof (buf), (uint32_t)~0, ext3_feature_name, 1);
-(*cpu_fprintf)(f, " extf_ecx: %s\n", buf);
-return;
-}
for (def = x86_defs; def; def = def->next) {
snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
-if (model || dump) {
-(*cpu_fprintf)(f, "x86 %16s %-48s\n", buf, def->model_id);
-} else {
-(*cpu_fprintf)(f, "x86 %16s\n", buf);
-}
-if (dump) {
-memcpy(buf, &def->vendor1, sizeof (def->vendor1));
-memcpy(buf + 4, &def->vendor2, sizeof (def->vendor2));
-memcpy(buf + 8, &def->vendor3, sizeof (def->vendor3));
-buf[12] = '\0';
-(*cpu_fprintf)(f,
-" family %d model %d stepping %d level %d xlevel 0x%x"
-" vendor \"%s\"\n",
-def->family, def->model, def->stepping, def->level,
-def->xlevel, buf);
-listflags(buf, sizeof (buf), def->features, feature_name, 0);
-(*cpu_fprintf)(f, " feature_edx %08x (%s)\n", def->features,
-buf);
-listflags(buf, sizeof (buf), def->ext_features, ext_feature_name,
-0);
-(*cpu_fprintf)(f, " feature_ecx %08x (%s)\n", def->ext_features,
-buf);
-listflags(buf, sizeof (buf), def->ext2_features, ext2_feature_name,
-0);
-(*cpu_fprintf)(f, " extfeature_edx %08x (%s)\n",
-def->ext2_features, buf);
-listflags(buf, sizeof (buf), def->ext3_features, ext3_feature_name,
-0);
-(*cpu_fprintf)(f, " extfeature_ecx %08x (%s)\n",
-def->ext3_features, buf);
-(*cpu_fprintf)(f, "\n");
-}
+(*cpu_fprintf)(f, "x86 %16s %-48s\n", buf, def->model_id);
}
if (kvm_enabled()) {
(*cpu_fprintf)(f, "x86 %16s\n", "[host]");
}
+(*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
+listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
+(*cpu_fprintf)(f, " f_edx: %s\n", buf);
+listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
+(*cpu_fprintf)(f, " f_ecx: %s\n", buf);
+listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
+(*cpu_fprintf)(f, " extf_edx: %s\n", buf);
+listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
+(*cpu_fprintf)(f, " extf_ecx: %s\n", buf);
}
int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
--
1.7.9.5
[Qemu-devel] [PATCH 2/2] Drop cpu_list_id macro
Since the only user of the extended cpu_list_id() format
was the x86 ?model/?dump/?cpuid output, we can drop it
completely.
Signed-off-by: Peter Maydell
---
cpus.c|6 ++
linux-user/main.c |6 ++
target-i386/cpu.c |4 ++--
target-i386/cpu.h |4 ++--
4 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/cpus.c b/cpus.c
index 3de2e27..c965b3a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1191,10 +1191,8 @@ void set_cpu_log_filename(const char *optarg)
void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
/* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list_id)
-cpu_list_id(f, cpu_fprintf, optarg);
-#elif defined(cpu_list)
-cpu_list(f, cpu_fprintf); /* deprecated */
+#if defined(cpu_list)
+cpu_list(f, cpu_fprintf);
#endif
}
diff --git a/linux-user/main.c b/linux-user/main.c
index 53714de..8b50163 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3143,10 +3143,8 @@ static void handle_arg_cpu(const char *arg)
cpu_model = strdup(arg);
if (cpu_model == NULL || is_help_option(cpu_model)) {
/* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list_id)
-cpu_list_id(stdout, &fprintf, "");
-#elif defined(cpu_list)
-cpu_list(stdout, &fprintf); /* deprecated */
+#if defined(cpu_list)
+cpu_list(stdout, &fprintf);
#endif
exit(1);
}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d3a2b47..a902502 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1059,8 +1059,8 @@ static void listflags(char *buf, int bufsize, uint32_t
fbits,
}
}
-/* generate CPU information */
-void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
+/* generate CPU information. */
+void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
{
x86_def_t *def;
char buf[256];
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 60f9e97..751f44d 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -791,7 +791,7 @@ typedef struct CPUX86State {
X86CPU *cpu_x86_init(const char *cpu_model);
int cpu_x86_exec(CPUX86State *s);
-void x86_cpu_list (FILE *f, fprintf_function cpu_fprintf, const char *optarg);
+void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
void x86_cpudef_setup(void);
int cpu_x86_support_mca_broadcast(CPUX86State *env);
@@ -975,7 +975,7 @@ static inline CPUX86State *cpu_init(const char *cpu_model)
#define cpu_exec cpu_x86_exec
#define cpu_gen_code cpu_x86_gen_code
#define cpu_signal_handler cpu_x86_signal_handler
-#define cpu_list_id x86_cpu_list
+#define cpu_list x86_cpu_list
#define cpudef_setup x86_cpudef_setup
#define CPU_SAVE_VERSION 12
--
1.7.9.5
[Qemu-devel] [PATCH 0/2] target-i386: roll -cpu ?help, ?cpuid into help list, drop ?dump
Commit c8057f95 (accidentally) disabled the ability to pass option strings starting with '?' to the target-specific cpu_list function, so the target-i386 specific "-cpu ?dump", "-cpu ?cpuid" and "-cpu ?model" stopped working. Since these options are undocumented and not used by libvirt, simply drop them completely rather than reinstating them with new style syntax. Instead, we fold the ?model and ?cpuid output into the output of the plain "-cpu help" output. The detailed output produced by ?dump is dropped. Peter Maydell (2): target-i386: Fold -cpu ?cpuid, ?model output into -cpu help, drop ?dump Drop cpu_list_id macro cpus.c|6 ++--- linux-user/main.c |6 ++--- target-i386/cpu.c | 66 ++--- target-i386/cpu.h |4 ++-- 4 files changed, 18 insertions(+), 64 deletions(-) -- 1.7.9.5
Re: [Qemu-devel] [PATCH V4 00/12] SD save/load support, SD qomification and bug fixes
On 27 July 2012 20:29, Igor Mitsyanko wrote: > Igor Mitsyanko (12): > hw/sd.c: convert wp_groups in SDState to bitfield > hw/sd.c: make sd_wp_addr() accept 64 bit address argument > hw/sd.c: introduce wrapper for conversion address to wp group > hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase > hw/sd.c: convert binary variables to bool > hw/sd.c: make sd_dataready() return bool > hw/sd.c: make sd_wp_addr() return bool > hw/sd.c: add SD card save/load support > hw/sd.c: convert SD state to QOM object > SD card users: optimize access to SDClass methods > SD card: introduce "spi" property for SD card objects > hw/sd.c: introduce SD card "drive" property I'm taking patches 1,2,3,5,6,7 into arm-devs.next because they're uncontroversial standalone cleanup patches. -- PMM
Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?
On 10 August 2012 03:11, Steven wrote: > The function definition has a return address type tb_page_addr_t. > tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) > > I am wondering is this address the guest physical address or the host > virtual address. In linux-user mode the returned address is the guest virtual address. In system mode it is a ram_addr_t. (the comment above the implementation says "the returned address is not exactly the physical address: it is the offset relative to phys_ram_base" but this is out of date I think). A ram_addr_t is neither a host address nor a guest physical address but it's closely related to a guest physaddr (you can think of it as if all the RAM in the system was put into a straight line and then the ram_addr_t is an index into that). > If it it is the guest physical address, why does Qemu waste guest > physical space to store these address for tb? Thanks. I'm not sure what you're asking here. This function returns a physical address because we store TCG translated code blocks in a hash table indexed by guest physaddr. Given the information "the CPU is trying to execute code from this physaddr" we need to be able to find out whether we already have a code block translated for that. (there is also a fast code path so we can avoid doing a complete lookup from physaddr most of the time.) -- PMM
[Qemu-devel] [PATCH] cputlb.c: Fix out of date comment
The comment about the return address from get_page_addr_code() was
well out of date as phys_ram_base has not existed for some time.
Signed-off-by: Peter Maydell
---
cputlb.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cputlb.c b/cputlb.c
index 0d1e252..d3e7b25 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -312,7 +312,9 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
/* NOTE: this function can trigger an exception */
/* NOTE2: the returned address is not exactly the physical address: it
- is the offset relative to phys_ram_base */
+ * is actually a ram_addr_t (in system mode; the user mode emulation
+ * version of this function returns a guest virtual address).
+ */
tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
{
int mmu_idx, page_index, pd;
--
1.7.9.5
[Qemu-devel] ARM patches for QEMU 1.2: final call
Last call for any ARM related patches to go into 1.2. My current queue looks like this: 59cbd70 hw/sd.c: make sd_wp_addr() return bool 8b4cc14 hw/sd.c: make sd_dataready() return bool 025caa6 hw/sd.c: convert binary variables to bool 38d24e6 hw/sd.c: introduce wrapper for conversion address to wp group 1835455 hw/sd.c: make sd_wp_addr() accept 64 bit address argument 34f99a8 hw/sd.c: convert wp_groups in SDState to bitfield 0b7ede9 armv7m: Guard against no -kernel argument 62140f8 hw/armv7m_nvic: Fix incorrect default for num-irqs property http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/arm-devs.next and I plan to send a pullreq Monday. (My target-arm queue is currently empty.) -- PMM
