Re: [Qemu-devel] [PATCH 02/33] gdbserver: Don't deliver TIMER interrupts when SSTEP_NOIRQ either.

2011-04-29 Thread Blue Swirl
On Sat, Apr 30, 2011 at 1:39 AM, Richard Henderson  wrote:
> On 04/29/2011 01:53 PM, Blue Swirl wrote:
>>> +                                               CPU_INTERRUPT_TIMER |
>>
>> Grepping for CPU_INTERRUPT_TIMER shows that the flag isn't ever set,
>> only cleared or checked. How
>> about removing the flag instead?
>
> Certainly Sparc and MIPS shouldn't check it if they don't set it.

It's not used by any architecture now.

> For Alpha, I do want to easily discriminate between device, timer,
> and inter-processor interrupts.  I'm currently (ab)using the i386
> CPU_INTERRUPT_SMI name for the IPI; I could abuse, say, FIQ for
> my timer bit, but it seems nicer to simply keep the TIMER name.

Since Alpha would be the first user, you can even rename it if you like.



Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-04-29 Thread Blue Swirl
On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino  wrote:
> This series introduces the inject-nmi command for QMP, which sends an
> NMI to _all_ guest's CPUs.
>
> Also note that this series changes the human monitor nmi command to use
> the QMP implementation, which means that it now has a DIFFERENT behavior.
> Please, check patch 3/3 for details.

As discussed earlier, please change the QMP version for future
expandability so that instead of single command 'inject-nmi', 'inject'
takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
'inject' should be added.



Re: [Qemu-devel] [PATCH 02/33] gdbserver: Don't deliver TIMER interrupts when SSTEP_NOIRQ either.

2011-04-29 Thread Richard Henderson
On 04/29/2011 01:53 PM, Blue Swirl wrote:
>> +   CPU_INTERRUPT_TIMER |
> 
> Grepping for CPU_INTERRUPT_TIMER shows that the flag isn't ever set,
> only cleared or checked. How
> about removing the flag instead?

Certainly Sparc and MIPS shouldn't check it if they don't set it.

For Alpha, I do want to easily discriminate between device, timer,
and inter-processor interrupts.  I'm currently (ab)using the i386
CPU_INTERRUPT_SMI name for the IPI; I could abuse, say, FIQ for
my timer bit, but it seems nicer to simply keep the TIMER name.


r~



Re: [Qemu-devel] [PATCH 1/2] Support for MIPS64 user mode emulation

2011-04-29 Thread Khansa Butt
Please see comments highlighted in green.

On Fri, Apr 29, 2011 at 2:01 PM, Aurelien Jarno wrote:

> On Mon, Apr 25, 2011 at 04:54:19PM +0500, Khansa Butt wrote:
> > please see inline comments highlighted in red color.
> >
> > On Wed, Apr 13, 2011 at 2:32 AM, Aurelien Jarno  >wrote:
> >
> > > [I don't know very well linux-user, it would be nice to Cc: Riku
> Voipio,
> > >  the linux-user maintainer for the next version.]
> > >
> > > On Sat, Apr 09, 2011 at 04:02:31PM +0500, Khansa Butt wrote:
> > > > From e96e20e50cada1c9e1b65de5925281cdd5659746 Mon Sep 17 00:00:00
> 2001
> > > > From: Ehsan-ul-Haq & Khansa Butt 
> > > > Date: Sat, 9 Apr 2011 10:51:22 +0500
> > > > Subject: [PATCH 1/2] Support for MIPS64 user mode emulation
> > > >
> > > >
> > > > Signed-off-by: Ehsan-ul-Haq, Abdul Qadeer, Abdul Waheed, Khansa Butt
> <
> > > > kha...@kics.edu.pk>
> > > > ---
> > > >  configure |1 +
> > > >  default-configs/mips64-linux-user.mak |1 +
> > > >  linux-user/elfload.c  |2 +-
> > > >  linux-user/main.c |   29
> > > +++--
> > > >  linux-user/mips64/syscall.h   |3 +++
> > > >  linux-user/signal.c   |3 ++-
> > > >  target-mips/translate.c   |1 +
> > > >  7 files changed, 36 insertions(+), 4 deletions(-)
> > > >  create mode 100644 default-configs/mips64-linux-user.mak
> > > >
> > > > diff --git a/configure b/configure
> > > > index ae97e11..d1f7867 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -1039,6 +1039,7 @@ m68k-linux-user \
> > > >  microblaze-linux-user \
> > > >  microblazeel-linux-user \
> > > >  mips-linux-user \
> > > > +mips64-linux-user \
> > > >  mipsel-linux-user \
> > > >  ppc-linux-user \
> > > >  ppc64-linux-user \
> > > > diff --git a/default-configs/mips64-linux-user.mak
> > > > b/default-configs/mips64-linux-user.mak
> > > > new file mode 100644
> > > > index 000..1598bfc
> > > > --- /dev/null
> > > > +++ b/default-configs/mips64-linux-user.mak
> > > > @@ -0,0 +1 @@
> > > > +# Default configuration for mips64-linux-user
> > > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > > > index fe5410e..2832a33 100644
> > > > --- a/linux-user/elfload.c
> > > > +++ b/linux-user/elfload.c
> > > > @@ -1384,7 +1384,7 @@ static void load_elf_image(const char
> *image_name,
> > > int
> > > > image_fd,
> > > >  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > > >  vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> > > >
> > > > -error = target_mmap(vaddr_ps, eppnt->p_filesz +
> vaddr_po,
> > > > +error = target_mmap(vaddr_ps, eppnt->p_memsz + vaddr_po,
> > >
> > > What is the goal of this change? If the mmapped aread is bigger than
> the
> > > file size rounded up to te page size, it will cause a SIGBUS.
> > >
> > > >  elf_prot, MAP_PRIVATE | MAP_FIXED,
> > > >  image_fd, eppnt->p_offset -
> vaddr_po);
> > > >  if (error == -1) {
> > > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > > index e651bfd..a7f4955 100644
> > > > --- a/linux-user/main.c
> > > > +++ b/linux-user/main.c
> > > > @@ -1937,6 +1937,14 @@ static int do_store_exclusive(CPUMIPSState
> *env)
> > > >  int d;
> > > >
> > > >  addr = env->lladdr;
> > > > +#if defined(TARGET_MIPS64)
> > > > +/* For MIPS64 on 32 bit host there is a need to make
> > > > +* the page accessible to which the above 'addr' is belonged */
> > > > +#if HOST_LONG_BITS == 32
> > > > +int flag = PAGE_VALID | PAGE_READ | PAGE_WRITE | PAGE_WRITE_ORG;
> > > > +page_set_flags(addr, addr + 4096, flag);
> > > > +#endif
> > > > +#endif
> > >
> > > I don't really see the reason why this should be done that way. Are you
> > > trying to run MIPS32 binaries compiled for 8kB page size?
> > >
> >
> >
> >
> > this change is needed when we run MIPS64 ELF on 32 bit x86 host. MIPS64
> ELF
> > contains 36 bit address.
>
> Actually it can contains up to 62-bit address there (all the user mapped
> space).
>
> >  load_elf_image() at /home/khansa/testpatch/qemu/linux-user/elfload.c:
> QEMU
> >  contains these lines
> >/* Round addresses to page boundaries.  */
> > loaddr &= qemu_host_page_mask;
> > hiaddr = HOST_PAGE_ALIGN(hiaddr);
> > when QEMU run on 32 bit x86 the above two variables are rounded to 32 bit
> > value while these should be 36 bits as these come from MIPS64 ELF.and
> then
>
> It is correct to truncate them, as the address space of the host is
> smaller. It's just based on the fact that programs only need a subset of
> the 62 bit address space.
>
> > for these rounded address l1_map is initialized in page_find_alloc().
> > in case of SCD(store condition double ) instruction of MIPS64r2 when we
> have
> > to check load linked address its again 36 bit so it will make an
> index(addr
> > >> TARGET_PAGE_BITS) for which l1_map is no valid

[Qemu-devel] xtensa: new target architecture for qemu

2011-04-29 Thread Max Filippov
Hello.

I'm developing support for new qemu target architecture: xtensa [1],
primarily because AFAIK there's no free/open simulator for this
architecture.

Essential ISA parts (like core opcodes, special registers, windowed
registers, exceptions and interrupts) are implemented, other (like
TLB, MMU, caches, coprocessors, rare opcodes) are not, although I'm
planning to implement them if/when needed.

I'm wondering if this target could be eligible for inclusion into qemu mainline.
If it is, could anyone please review the code [2]?

There are several known issues which I'm planning to address:
- mixed coding style;
- no copyrights/license (it is BSD);
- no direct TB linking;
- dummy cpu_halted/cpu_has_work.

If you see more, please report, especially if you know how to fix them (:

[1] http://en.wikipedia.org/wiki/Tensilica
[2] 
http://jcmvbkbc.spb.ru/git/?p=dumb/qemu-xtensa.git;a=shortlog;h=refs/heads/xtensa

Thanks.
-- Max



Re: [Qemu-devel] [PATCHv2 3/4] qxl: add debug_cs and cmdlog_cs

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 02:09:54PM +0200, Gerd Hoffmann wrote:
> On 04/28/11 10:29, Alon Levy wrote:
> >With this you can output the command log and/or the guest debug (driver)
> >output to a chardev instead of stderr:
> >
> >-global qxl-vga.cmdlog_chardev=qxl_cmdlog_chardev
> >-global qxl-vga.debug_chardev=qxl_debug_chardev
> >
> >useful for debugging. if no chardev is specified prints to stderr like the 
> >old
> >code.
> 
> Hmm.  That is a bit too much ad-hoc debug hacking IMHO.  Also I'm
> not sure I like the idea to send debug stuff through chardev instead
> of just writing them to stderr or some logfile.
> 
> I do see why you are doing that though.  Unfortunaly qemu has no
> sane debug logging infrastructure.  This is long overdue IMHO, so we
> don't need hacks like this one and also don't reinvent stuff over
> and over.

The bigger hack called "qxl_terse" I didn't even send :) ok, so now I get
to either keep this in my closet or think of how to do a minimal acceptable
qemu logging infrastructure that would let me register a logging handle and
use that to redirect to a chardev (they would all default to being muxed over
stdio?)

QemuLogger *qemu_create_logger(const char *logger_id);
 - logger_id is used to match to the chardev given on the command line
 - need to prevent collision, so probably easier to have a logger_id be an int
 and have that looked up to a string in an automatically generated table?

struct QemuLogger {
  CharDriverState *chr;
};

And command line would be:
 -chardev bla,id=myid -debug qxl,myid

qxl->logger = qemu_create_logger("qxl");
qemu_chr_write(qxl->logger->chr, ..)

logger creation would have to happen before command line parsing, so in the 
device initialization.

Sounds right?

> 
> cheers,
>   Gerd
> 



[Qemu-devel] [PATCH] vfio: Fix free in dma mapping error path

2011-04-29 Thread Alex Williamson
This is allocated via vmalloc, so needs vfree, not kfree.

Signed-off-by: Alex Williamson 
---

 drivers/vfio/vfio_dma.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/vfio/vfio_dma.c b/drivers/vfio/vfio_dma.c
index cee1a25..4a488b6 100644
--- a/drivers/vfio/vfio_dma.c
+++ b/drivers/vfio/vfio_dma.c
@@ -322,7 +322,7 @@ int vfio_dma_map_common(struct vfio_listener *listener,
if (ret != npage) {
printk(KERN_ERR "%s: get_user_pages_fast returns %d, not %d\n",
__func__, ret, npage);
-   kfree(pages);
+   vfree(pages);
ret = -EFAULT;
goto out_lock;
}




[Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-04-29 Thread Scott Wood
Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
and display them in "info registers".

Also get CR and PID from the existing KVM_GET_REGS.

Signed-off-by: Scott Wood 
---
v2: fix a couple style oversights, and cache kvm caps as requested

This version depends on http://patchwork.ozlabs.org/patch/90688/
as code it adds is converted to use the cached capability.

 hw/ppc.c   |   12 
 monitor.c  |   71 -
 target-ppc/cpu.h   |1 +
 target-ppc/kvm.c   |  137 ++--
 target-ppc/translate.c |   82 +++-
 5 files changed, 294 insertions(+), 9 deletions(-)

diff --git a/hw/ppc.c b/hw/ppc.c
index 1873328..9157719 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -452,6 +452,10 @@ uint64_t cpu_ppc_load_tbl (CPUState *env)
 ppc_tb_t *tb_env = env->tb_env;
 uint64_t tb;
 
+if (kvm_enabled()) {
+return env->spr[SPR_TBL];
+}
+
 tb = cpu_ppc_get_tb(tb_env, qemu_get_clock_ns(vm_clock), 
tb_env->tb_offset);
 LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
 
@@ -471,6 +475,10 @@ static inline uint32_t _cpu_ppc_load_tbu(CPUState *env)
 
 uint32_t cpu_ppc_load_tbu (CPUState *env)
 {
+if (kvm_enabled()) {
+return env->spr[SPR_TBU];
+}
+
 return _cpu_ppc_load_tbu(env);
 }
 
@@ -616,6 +624,10 @@ uint32_t cpu_ppc_load_decr (CPUState *env)
 {
 ppc_tb_t *tb_env = env->tb_env;
 
+if (kvm_enabled()) {
+return env->spr[SPR_DECR];
+}
+
 return _cpu_ppc_load_decr(env, tb_env->decr_next);
 }
 
diff --git a/monitor.c b/monitor.c
index 5f3bc72..f63cce0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3466,7 +3466,76 @@ static const MonitorDef monitor_defs[] = {
 { "sr13", offsetof(CPUState, sr[13]) },
 { "sr14", offsetof(CPUState, sr[14]) },
 { "sr15", offsetof(CPUState, sr[15]) },
-/* Too lazy to put BATs and SPRs ... */
+/* Too lazy to put BATs... */
+{ "pvr", offsetof(CPUState, spr[SPR_PVR]) },
+
+{ "srr0", offsetof(CPUState, spr[SPR_SRR0]) },
+{ "srr1", offsetof(CPUState, spr[SPR_SRR1]) },
+{ "sprg0", offsetof(CPUState, spr[SPR_SPRG0]) },
+{ "sprg1", offsetof(CPUState, spr[SPR_SPRG1]) },
+{ "sprg2", offsetof(CPUState, spr[SPR_SPRG2]) },
+{ "sprg3", offsetof(CPUState, spr[SPR_SPRG3]) },
+{ "sprg4", offsetof(CPUState, spr[SPR_SPRG4]) },
+{ "sprg5", offsetof(CPUState, spr[SPR_SPRG5]) },
+{ "sprg6", offsetof(CPUState, spr[SPR_SPRG6]) },
+{ "sprg7", offsetof(CPUState, spr[SPR_SPRG7]) },
+{ "pid", offsetof(CPUState, spr[SPR_BOOKE_PID]) },
+{ "csrr0", offsetof(CPUState, spr[SPR_BOOKE_CSRR0]) },
+{ "csrr1", offsetof(CPUState, spr[SPR_BOOKE_CSRR1]) },
+{ "esr", offsetof(CPUState, spr[SPR_BOOKE_ESR]) },
+{ "dear", offsetof(CPUState, spr[SPR_BOOKE_DEAR]) },
+{ "mcsr", offsetof(CPUState, spr[SPR_BOOKE_MCSR]) },
+{ "tsr", offsetof(CPUState, spr[SPR_BOOKE_TSR]) },
+{ "tcr", offsetof(CPUState, spr[SPR_BOOKE_TCR]) },
+{ "vrsave", offsetof(CPUState, spr[SPR_VRSAVE]) },
+{ "pir", offsetof(CPUState, spr[SPR_BOOKE_PIR]) },
+{ "mcsrr0", offsetof(CPUState, spr[SPR_BOOKE_MCSRR0]) },
+{ "mcsrr1", offsetof(CPUState, spr[SPR_BOOKE_MCSRR1]) },
+{ "decar", offsetof(CPUState, spr[SPR_BOOKE_DECAR]) },
+{ "ivpr", offsetof(CPUState, spr[SPR_BOOKE_IVPR]) },
+{ "epcr", offsetof(CPUState, spr[SPR_BOOKE_EPCR]) },
+{ "sprg8", offsetof(CPUState, spr[SPR_BOOKE_SPRG8]) },
+{ "ivor0", offsetof(CPUState, spr[SPR_BOOKE_IVOR0]) },
+{ "ivor1", offsetof(CPUState, spr[SPR_BOOKE_IVOR1]) },
+{ "ivor2", offsetof(CPUState, spr[SPR_BOOKE_IVOR2]) },
+{ "ivor3", offsetof(CPUState, spr[SPR_BOOKE_IVOR3]) },
+{ "ivor4", offsetof(CPUState, spr[SPR_BOOKE_IVOR4]) },
+{ "ivor5", offsetof(CPUState, spr[SPR_BOOKE_IVOR5]) },
+{ "ivor6", offsetof(CPUState, spr[SPR_BOOKE_IVOR6]) },
+{ "ivor7", offsetof(CPUState, spr[SPR_BOOKE_IVOR7]) },
+{ "ivor8", offsetof(CPUState, spr[SPR_BOOKE_IVOR8]) },
+{ "ivor9", offsetof(CPUState, spr[SPR_BOOKE_IVOR9]) },
+{ "ivor10", offsetof(CPUState, spr[SPR_BOOKE_IVOR10]) },
+{ "ivor11", offsetof(CPUState, spr[SPR_BOOKE_IVOR11]) },
+{ "ivor12", offsetof(CPUState, spr[SPR_BOOKE_IVOR12]) },
+{ "ivor13", offsetof(CPUState, spr[SPR_BOOKE_IVOR13]) },
+{ "ivor14", offsetof(CPUState, spr[SPR_BOOKE_IVOR14]) },
+{ "ivor15", offsetof(CPUState, spr[SPR_BOOKE_IVOR15]) },
+{ "ivor32", offsetof(CPUState, spr[SPR_BOOKE_IVOR32]) },
+{ "ivor33", offsetof(CPUState, spr[SPR_BOOKE_IVOR33]) },
+{ "ivor34", offsetof(CPUState, spr[SPR_BOOKE_IVOR34]) },
+{ "ivor35", offsetof(CPUState, spr[SPR_BOOKE_IVOR35]) },
+{ "ivor36", offsetof(CPUState, spr[SPR_BOOKE_IVOR36]) },
+{ "ivor37", offsetof(CPUState, spr[SPR_BOOKE_IVOR37]) },
+{ "mas0", offsetof(CPUState, spr[SPR_BOOKE_MAS0]) },
+{ "mas1", offsetof(CPUState, spr[SPR_BOOKE_MAS1]) },
+{ "mas2", offsetof(CPUState, spr[SPR_BOOKE_MAS2]

Re: [Qemu-devel] [PATCH] monitor: avoid moving cursor during "mouse_button" command

2011-04-29 Thread Brad Hards
On Thursday 28 April 2011 20:46:25 Gerd Hoffmann wrote:
> I think it would be much better to keep track of the mouse position (and
> button state while being at it) in input.c instead of monitor.c.
> 
> Once this is in place it should be easy to add kbd_mouse_* functions
> which update position or buttons only, which the monitor code can use
> then to avoid the unwanted pointer warp.
This turns out to be a bit more difficult than we discussed.

The new functions work well for the monitor code side (not unexpected, since 
its essentially the same as the original code I proposed for monitor-only 
changes).

The problem is that almost all input code (in absolute mode) keeps track of 
the position itself - monitor was the exception.

So a sequence like the following:
1. Move cursor in SDL
2. Use mouse_move in monitor
3. Use mouse_button 2 in monitor
4. Click mouse in SDL
works ok up to step 3, but step 4 causes the pointer to warp back to where it 
was at the end of step 1.

So it looks like we'd have to modify all callers of kbd_mouse_event(), and the 
code paths are already a bit convoluted. As discussed on IRC, I'm a bit 
concerned about testing cocoa and spice.

Thoughts?

Brad



[Qemu-devel] [PATCH 1/3] QMP: QError: New QERR_UNSUPPORTED

2011-04-29 Thread Luiz Capitulino
From: Lai Jiangshan 

New QERR_UNSUPPORTED for unsupported commands or requests.

Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 4855604..4f3b7ca 100644
--- a/qerror.c
+++ b/qerror.c
@@ -201,6 +201,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "An undefined error has ocurred",
 },
 {
+.error_fmt = QERR_UNSUPPORTED,
+.desc  = "this feature or command is not currently supported",
+},
+{
 .error_fmt = QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
 .desc  = "'%(device)' uses a %(format) feature which is not "
  "supported by this qemu version: %(feature)",
diff --git a/qerror.h b/qerror.h
index df61d2c..582b5ef 100644
--- a/qerror.h
+++ b/qerror.h
@@ -165,6 +165,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_UNDEFINED_ERROR \
 "{ 'class': 'UndefinedError', 'data': {} }"
 
+#define QERR_UNSUPPORTED \
+"{ 'class': 'Unsupported', 'data': {} }"
+
 #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
 "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': 
%s, 'feature': %s } }"
 
-- 
1.7.5.GIT




[Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-04-29 Thread Luiz Capitulino
This series introduces the inject-nmi command for QMP, which sends an
NMI to _all_ guest's CPUs.

Also note that this series changes the human monitor nmi command to use
the QMP implementation, which means that it now has a DIFFERENT behavior.
Please, check patch 3/3 for details.




[Qemu-devel] [PATCH 3/3] HMP: Use QMP inject nmi implementation

2011-04-29 Thread Luiz Capitulino
This **CHANGES** the human monitor "nmi" command behavior.

Currently it accepts an CPU argument which, when provided, will send
the NMI to the specified CPU. This feature is of discussable value
though and HMP shouldn't have more features than QMP, so let's use
QMP's instead (it's also simpler).

Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx |9 +
 monitor.c   |   16 ++--
 qmp-commands.hx |2 +-
 3 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 834e6a8..6ad8806 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -740,10 +740,11 @@ ETEXI
 #if defined(TARGET_I386)
 {
 .name   = "nmi",
-.args_type  = "cpu_index:i",
-.params = "cpu",
-.help   = "inject an NMI on the given CPU",
-.mhandler.cmd = do_inject_nmi,
+.args_type  = "",
+.params = "",
+.help   = "inject an NMI on all guest's CPUs",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_inject_nmi,
 },
 #endif
 STEXI
diff --git a/monitor.c b/monitor.c
index 455a528..1ba98b0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2544,19 +2544,7 @@ static void do_wav_capture(Monitor *mon, const QDict 
*qdict)
 #endif
 
 #if defined(TARGET_I386)
-static void do_inject_nmi(Monitor *mon, const QDict *qdict)
-{
-CPUState *env;
-int cpu_index = qdict_get_int(qdict, "cpu_index");
-
-for (env = first_cpu; env != NULL; env = env->next_cpu)
-if (env->cpu_index == cpu_index) {
-cpu_interrupt(env, CPU_INTERRUPT_NMI);
-break;
-}
-}
-
-static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 CPUState *env;
 
@@ -2567,7 +2555,7 @@ static int do_inject_nmi_all(Monitor *mon, const QDict 
*qdict, QObject **ret_dat
 return 0;
 }
 #else
-static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 qerror_report(QERR_UNSUPPORTED);
 return -1;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1058c38..1957730 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -435,7 +435,7 @@ EQMP
 .params = "",
 .help   = "",
 .user_print = monitor_user_noop,
-.mhandler.cmd_new = do_inject_nmi_all,
+.mhandler.cmd_new = do_inject_nmi,
 },
 
 SQMP
-- 
1.7.5.GIT




[Qemu-devel] [PATCH 2/3] QMP: add inject-nmi qmp command

2011-04-29 Thread Luiz Capitulino
From: Lai Jiangshan 

inject-nmi command injects an NMI on all CPUs of guest.
It is only supported for x86 guest currently, it will
returns "Unsupported" error for non-x86 guest.

Signed-off-by: Luiz Capitulino 
---
 monitor.c   |   17 +
 qmp-commands.hx |   27 +++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 5f3bc72..455a528 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2555,6 +2555,23 @@ static void do_inject_nmi(Monitor *mon, const QDict 
*qdict)
 break;
 }
 }
+
+static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
+{
+CPUState *env;
+
+for (env = first_cpu; env != NULL; env = env->next_cpu) {
+cpu_interrupt(env, CPU_INTERRUPT_NMI);
+}
+
+return 0;
+}
+#else
+static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
+{
+qerror_report(QERR_UNSUPPORTED);
+return -1;
+}
 #endif
 
 static void do_info_status_print(Monitor *mon, const QObject *data)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fbd98ee..1058c38 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -430,6 +430,33 @@ Example:
 EQMP
 
 {
+.name   = "inject-nmi",
+.args_type  = "",
+.params = "",
+.help   = "",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_inject_nmi_all,
+},
+
+SQMP
+inject-nmi
+--
+
+Inject an NMI on guest's CPUs.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "inject-nmi" }
+<- { "return": {} }
+
+Note: inject-nmi is only supported for x86 guest currently, it will
+  returns "Unsupported" error for non-x86 guest.
+
+EQMP
+
+{
 .name   = "migrate",
 .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
 .params = "[-d] [-b] [-i] uri",
-- 
1.7.5.GIT




Re: [Qemu-devel] [PATCH 0/4] spice: fix locking

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 02:20:09PM +0300, Alon Levy wrote:
> On Fri, Apr 29, 2011 at 11:38:28AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > This patch series moves a bunch of work spice has to do from spice
> > server thread context to iothread context, which in turn allows to
> > drop the current locking mess as we don't touch qemu internals from
> > spice server thread any more.
> > 
> > A long-standing warning fix from Jes is included too.
> > 
> 
> Tested with winxp, works well with changes between vga and qxl (tested
> by setting the debug to 2 in spice-display.c). Just a single comment about
> the debug=1 change in the second patch.
> 

Forgot:

ACK series

> Alon
> 
> > cheers,
> >   Gerd
> > 
> > The following changes since commit 430a3c18064fd3c007048d757e8bd0fff45fcc99:
> > 
> >   configure: reenable opengl by default (2011-04-26 23:26:49 +0200)
> > 
> > are available in the git repository at:
> >   git://anongit.freedesktop.org/spice/qemu spice.v34
> > 
> > Gerd Hoffmann (3):
> >   spice: don't create updates in spice server context.
> >   spice: don't call displaystate callbacks from spice server context.
> >   spice: drop obsolete iothread locking
> > 
> > Jes Sorensen (1):
> >   Make spice dummy functions inline to fix calls not checking return 
> > values
> > 
> >  hw/qxl-render.c|   25 ++--
> >  hw/qxl.c   |   27 ++---
> >  ui/qemu-spice.h|   12 -
> >  ui/spice-display.c |   63 
> > +---
> >  ui/spice-display.h |   24 +++
> >  5 files changed, 94 insertions(+), 57 deletions(-)
> > 
> 



Re: [Qemu-devel] [PATCH 0/2 V9] hmp,qmp: add inject-nmi

2011-04-29 Thread Luiz Capitulino
On Thu, 28 Apr 2011 11:35:20 +0800
Lai Jiangshan  wrote:

> 
> 
> Adds new QERR_UNSUPPORTED, converts "nmi" to "inject-nmi" and
> make it supports qmp.

Lai, unfortunately this series still have some issues (like changing
the HMP command name). I think V7 was the best submission so far, so
I decided to do this: I've incorporated your v7 patches in a new series
and fixed a few issues. I'll submit it for review.



Re: [Qemu-devel] [PATCHv2 2/4] qxl: add mode to debugprint on destroy primary

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 02:01:57PM +0200, Gerd Hoffmann wrote:
> On 04/28/11 10:29, Alon Levy wrote:
> >---
> >  hw/qxl.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/hw/qxl.c b/hw/qxl.c
> >index 63e295b..ccd820c 100644
> >--- a/hw/qxl.c
> >+++ b/hw/qxl.c
> >@@ -1009,7 +1009,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
> >uint32_t val)
> >  break;
> >  case QXL_IO_DESTROY_PRIMARY:
> >  PANIC_ON(val != 0);
> >-dprint(d, 1, "QXL_IO_DESTROY_PRIMARY\n");
> >+dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", 
> >qxl_mode_to_string(d->mode));
> >  qxl_destroy_primary(d);
> >  break;
> >  case QXL_IO_DESTROY_SURFACE_WAIT:
> 
> Squash into the first?  Maybe there are a few more places where a
> pretty-printed mode would be useful?
ok, I'll look for more usage ops and do the squash.

> 
> cheers,
>   Gerd



Re: [Qemu-devel] [PATCHv2 1/4] qxl: interface_get_command: fix reported mode

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 02:01:16PM +0200, Gerd Hoffmann wrote:
> >+static const char *qxl_mode_to_string(int mode)
> >+{
> >+switch (mode) {
> >+case QXL_MODE_COMPAT:
> >+return "compat";
> >+case QXL_MODE_NATIVE:
> >+return "native";
> >+case QXL_MODE_UNDEFINED:
> >+return "undefined";
> >+case QXL_MODE_VGA:
> >+return "vga";
> >+}
> >+return "unknown";
> 
> Make that "INVALID"?  There never ever should be another move value ...
True, will do.

> 
> cheers,
>   Gerd



Re: [Qemu-devel] [PATCH v3 1/5] hpet 'driftfix': add hooks required to detect coalesced interrupts (x86 apic only)

2011-04-29 Thread Blue Swirl
On Fri, Apr 29, 2011 at 12:45 PM, Ulrich Obergfell  wrote:
>
>> On 2011-04-28 20:51, Blue Swirl wrote:
>>> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote:
 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain
 entry addresses of functions that are utilized by update_irq() to
 detect coalesced interrupts. apic code loads these pointers during
 initialization.
>>>
>>> I'm not so happy with this approach, but probably then the i386
>>> dependencies can be removed from RTC and it can be compiled only once
>>> for all targets.
>>
>> This whole series is really the minimalistic approach. The callbacks
>> defined here must remain a temporary "shortcut". Just like proper
>> abstraction of periodic tick compensation for reuse in other timers has
>> to be added later on. And the limitation to edge-triggered legacy HPET
>> INTs has to be removed.
>
> Since QEMU doesn't have a generic infrastructure to track interrupt
> delivery, I decided to reuse something that is currently available.
> The only mechanism that I'm aware of is the one that is utilized by
> RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc.

I think your approach is slightly better (while not the best possible
one), so it should be used also for RTC.

Some more comments:
- instead of global variables, there could be a setter function (maybe inline)
- pc.h would be a better place instead of sysemu.h

> The changes that would be introduced by part 1/5 and part 4/5 of
> this patch series could be replaced if a generic infrastructure
> to track interrupt delivery becomes available.

OK.



Re: [Qemu-devel] QEMU testing methodology & results

2011-04-29 Thread Blue Swirl
On Fri, Apr 29, 2011 at 11:33 AM, Paolo Bonzini  wrote:
> On 04/29/2011 02:17 AM, Peter Maydell wrote:
>>
>> The theoretical aim there as far
>> as I'm concerned is architectural correctness -- in other words we
>> should be a valid implementation of the architecture,
>
> That's not even the case for x86.  It should be a goal, however, that with
> mainstream kernels user programs shouldn't be able to see the difference.
>  There are some known issues besides bugs, for example the "instruction
> pointer of the last FP instruction" register (!) is not implemented.

For Sparc32 userland, I think the emulator is pretty close to correct
(or at least it could be very precise) since the instruction set is so
simple and there are very few corner cases. Maybe also for Sparc64,
where the set is not so simple anymore.

Still, things like order of exceptions may be tricky to implement
correctly. FPUs in QEMU execute synchronously to integer units.

Another case is that TCG needs to keep TBs and the instructions which
were used to generate the TBs in synch to support PC search (usually
this is needed anyway for example to support SMC on x86), but on Sparc
it should be possible to execute old code from cache while the
instructions have been modified in memory and no barrier instructions
have been issued.



Re: [Qemu-devel] [PULL] qemu-timer: Add and use new function qemu_timer_expired_ns and other patches

2011-04-29 Thread Blue Swirl
On Thu, Apr 28, 2011 at 3:18 PM, Stefan Weil  wrote:
> Am 20.04.2011 16:26, schrieb Stefan Weil:
>>
>> Hello,
>>
>> the four qemu-timer related patches which I sent to qemu-devel can now be
>> pulled.
>> Maybe this makes the commit to git master easier.
>>
>> There was no feedback for the first three patches.
>>
>> The fourth patch changes windows code only and is needed for native
>> windows.
>>
>> Cheers,
>> Stefan Weil
>>
>>
>> The following changes since commit
>> ec52b8753a372de30b22d9b4765a799db612:
>>
>>  target-arm: Set Invalid flag for NaN in float-to-int conversions
>> (2011-04-20 13:01:05 +0200)
>>
>> are available in the git repository at:
>>  git://qemu.weilnetz.de/git/qemu.git/ patches
>>
>> Stefan Weil (4):
>>      qemu-timer: Add and use new function qemu_timer_expired_ns
>>      qemu-timer: Remove unneeded include statement (w32)
>>      qemu-timer: Avoid type casts
>>      qemu-timer: Fix timers for w32
>>
>>  qemu-timer.c |  155
>> --
>>  qemu-timer.h |    1 -
>>  2 files changed, 128 insertions(+), 28 deletions(-)
>
>
> Ping? Is there anything missing?
> The patches were sent to qemu-devel on 2011-04-10.
>
> The second one is trivial. The last one is essential for w32 hosts
> (it only changes w32 code). The other two patches try to improve
> code quality a little bit.

Thanks, pulled.



Re: [Qemu-devel] [PATCH 02/33] gdbserver: Don't deliver TIMER interrupts when SSTEP_NOIRQ either.

2011-04-29 Thread Blue Swirl
On Thu, Apr 28, 2011 at 11:50 PM, Richard Henderson  wrote:
> This would affect Sparc as well.
>
> Signed-off-by: Richard Henderson 
> Cc: Blue Swirl 
> ---
>  cpu-exec.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 395cd8c..e1b85d6 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -361,6 +361,7 @@ int cpu_exec(CPUState *env1)
>                     if (unlikely(env->singlestep_enabled & SSTEP_NOIRQ)) {
>                         /* Mask out external interrupts for this step. */
>                         interrupt_request &= ~(CPU_INTERRUPT_HARD |
> +                                               CPU_INTERRUPT_TIMER |

Grepping for CPU_INTERRUPT_TIMER shows that the flag isn't ever set,
only cleared or checked. How
about removing the flag instead?



Re: [Qemu-devel] [RFC PATCH 0/2] Multiqueue support for qemu(virtio-net)

2011-04-29 Thread Anthony Liguori

On 04/20/2011 10:33 PM, Jason Wang wrote:

Krishna Kumar2 writes:
  >  Thanks Jason!
  >
  >  So I can use my virtio-net guest driver and test with this patch?
  >  Please provide the script you use to start MQ guest.
  >

Yes and thanks. Following is a simple script may help you start macvtap mq
guest.

qemu_path=./qemu-system-x86_64
img_path=/home/kvm_autotest_root/images/mq.qcow2
vtap_dev=/dev/tap104
mac=96:88:12:1C:27:83
smp=2
mq=4

for i in `seq $mq`
do
 vtap+=" -netdev tap,id=hn$i,fd=$((i+100)) $((i+100))<>$vtap_dev"


So you are basically dup()'ing the tap device.

Does this actually improve performance at all?

Regards,

Anthony Liguori


 netdev+="hn$i#"
done

eval "$qemu_path $img_path $vtap -device 
virtio-net-pci,queues=$mq,netdev=$netdev,mac=$mac,vectors=32 -enable-kvm -smp $smp"


  >  Regards,
  >
  >  - KK
  >
  >  Jason Wang  wrote on 04/20/2011 02:03:07 PM:
  >
  >  >  Jason Wang
  >  >  04/20/2011 02:03 PM
  >  >
  >  >  To
  >  >
  >  >  Krishna Kumar2/India/IBM@IBMIN, k...@vger.kernel.org, m...@redhat.com,
  >  >  net...@vger.kernel.org, ru...@rustcorp.com.au, qemu-
  >  >  de...@nongnu.org, anth...@codemonkey.ws
  >  >
  >  >  cc
  >  >
  >  >  Subject
  >  >
  >  >  [RFC PATCH 0/2] Multiqueue support for qemu(virtio-net)
  >  >
  >  >  Inspired by Krishna's patch
  >  (http://www.spinics.net/lists/kvm/msg52098.html
  >  >  ) and
  >  >  Michael's suggestions.  The following series adds the multiqueue support
  >  for
  >  >  qemu and enable it for virtio-net (both userspace and vhost).
  >  >
  >  >  The aim for this series is to simplified the management and achieve the
  >  same
  >  >  performacne with less codes.
  >  >
  >  >  Follows are the differences between this series and Krishna's:
  >  >
  >  >  - Add the multiqueue support for qemu and also for userspace virtio-net
  >  >  - Instead of hacking the vhost module to manipulate kthreads, this patch
  >  just
  >  >  implement the userspace based multiqueues and thus can re-use the
  >  >  existed vhost kernel-side codes without any modification.
  >  >  - Use 1:1 mapping between TX/RX pairs and vhost kthread because the
  >  >  implementation is based on usersapce.
  >  >  - The cli is also changed to make the mgmt easier, the -netdev option of
  >  qdev
  >  >  can now accpet more than one ids. You can start a multiqueue virtio-net
  >  device
  >  >  through:
  >  >  ./qemu-system-x86_64 -netdev tap,id=hn0,vhost=on,fd=X -netdev
  >  >  tap,id=hn0,vhost=on,fd=Y -device
  >  virtio-net-pci,netdev=hn0#hn1,queues=2 ...
  >  >
  >  >  The series is very primitive and still need polished.
  >  >
  >  >  Suggestions are welcomed.
  >  >  ---
  >  >
  >  >  Jason Wang (2):
  >  >net: Add multiqueue support
  >  >virtio-net: add multiqueue support
  >  >
  >  >
  >  >   hw/qdev-properties.c |   37 -
  >  >   hw/qdev.h|3
  >  >   hw/vhost.c   |   26 ++-
  >  >   hw/vhost.h   |1
  >  >   hw/vhost_net.c   |7 +
  >  >   hw/vhost_net.h   |2
  >  >   hw/virtio-net.c  |  409 +++
  >  >  +--
  >  >   hw/virtio-net.h  |2
  >  >   hw/virtio-pci.c  |1
  >  >   hw/virtio.h  |1
  >  >   net.c|   34 +++-
  >  >   net.h|   15 +-
  >  >   12 files changed, 353 insertions(+), 185 deletions(-)
  >  >
  >  >  --
  >  >  Jason Wang
  >






Re: [Qemu-devel] [RFC PATCH 1/2] net: Add multiqueue support

2011-04-29 Thread Anthony Liguori

On 04/20/2011 03:33 AM, Jason Wang wrote:

This patch adds the multiqueues support for emulated nics. Each VLANClientState
pairs are now abstract as a queue instead of a nic, and multiple VLANClientState
pointers were stored in the NICState and treated as the multiple queues of a
single nic. The netdev options of qdev were now expanded to accept more than one
netdev ids. A queue_index were also introduced to let the emulated nics know
which queue the packet were came from or sent out. Virtio-net would be the first
user.

The legacy single queue nics can still run happily without modification as the
the compatibility were kept.

Signed-off-by: Jason Wang
---
  hw/qdev-properties.c |   37 ++---
  hw/qdev.h|3 ++-
  net.c|   34 ++
  net.h|   15 +++
  4 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..dd371e1 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -384,14 +384,37 @@ PropertyInfo qdev_prop_chr = {

  static int parse_netdev(DeviceState *dev, Property *prop, const char *str)
  {
-VLANClientState **ptr = qdev_get_prop_ptr(dev, prop);
+VLANClientState ***nc = qdev_get_prop_ptr(dev, prop);
+const char *ptr = str;
+int i = 0;
+size_t len = strlen(str);
+*nc = qemu_malloc(MAX_QUEUE_NUM * sizeof(VLANClientState *));
+
+while (i<  MAX_QUEUE_NUM&&  ptr<  str + len) {
+char *name = NULL;
+char *this = strchr(ptr, '#');


However this is being used is not going to be right.  Is this taking 
netdev=a#b#c#d?


I sort of agree with Michael about using multiple netdevs for this but I 
don't yet understand how this gets sets up from userspace.


Can you give an example of usage that involves the full tap device setup?

Ideally, a user/management tool would never need to know about any of this.

In a perfect world, we could just dup() the tap fd a few times to create 
multiple queues.


Regards,

Anthony Liguori



Re: [Qemu-devel] [RESEND][PATCH 1/9] pxa2xx_pcmcia: qdevify

2011-04-29 Thread Dmitry Eremin-Solenikov
Hello,

Any chance to get any response for this patches serie?

On 4/25/11, Dmitry Eremin-Solenikov  wrote:
> Use qdev insfrastructure for managing PXA PCMCIA devices. PCMCIA bus
> itself isn't converted to QBus (yet). pxa2xx_pcmcia_init() function is
> moved to pcmcia.h as it will be used by other cores/devices (like
> StrongARM).
>
> Signed-off-by: Dmitry Eremin-Solenikov 

-- 
With best wishes
Dmitry



[Qemu-devel] [Bug 568614] Re: x86_64 host curses interface: spacing/garbling

2011-04-29 Thread Devin J. Pohly
I just compiled from git and the problem persists.

I will reiterate that the issue appears to be with the word size of the
types used, not with endianness; see comment 4.  I have not dug any
further into the QEMU code to see if I find a more "correct-looking"
solution than the quick patch I have attached to this bug.

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

Title:
  x86_64 host curses interface: spacing/garbling

Status in QEMU:
  In Progress

Bug description:
  Environment:
  Arch Linux x86_64, kernel 2.6.33, qemu 0.12.3

  Steps to reproduce:
  1. Have a host system running 64-bit Linux.
  2. Start a qemu VM with the -curses flag.

  Expected results:
  Text displayed looks as it would on a real text-mode display, and VM is 
therefore usable.

  Actual results:
  Text displayed contains an extra space between characters, causing text to 
flow off the right and bottom sides of the screen. This makes the curses 
interface unintelligible.

  The attached patch fixes this problem on 0.12.3 on my installation
  without changing behavior on a 32-bit machine.  I don't know enough of
  the semantics of console_ch_t to know if this is the "correct" fix or
  if there should be, say, an extra cast somewhere instead.



Re: [Qemu-devel] [PATCH 1/2 v4] Support for multiple keyboard devices

2011-04-29 Thread Shahar Havivi
Fine with me



On Apr 29, 2011, at 19:24, Dmitry Zhurikhin  wrote:

> On 05/11/2010 12:18 AM, Anthony Liguori wrote:
>> On 04/18/2010 02:21 PM, Shahar Havivi wrote:
>>> Patch add QEMUPutKbdEntry structure - handling each keyboard entry, the 
>>> structure handled
>>> by qemu tail queue.
>>> Adding a new keyboard add to the list and select it, removing keyboard 
>>> select the previous
>>> keyboard in list.
>>> 
>>> Signed-off-by: Shahar Havivi
>> 
>> Applied all.  Thanks.
> Hello.  What happened to this patchset?  Seems like it didn't make to the 
> trunk.  Were there any objections not discussed on the list?  If there are no 
> objections I'll update these patches and resend them.  Is it OK?
> 
>Regards,
>Dmitry
> 
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>> ---
>>>  console.h|   14 -
>>>  hw/adb.c |2 +-
>>>  hw/escc.c|3 +-
>>>  hw/musicpal.c|2 +-
>>>  hw/nseries.c |4 +-
>>>  hw/palm.c|2 +-
>>>  hw/ps2.c |2 +-
>>>  hw/pxa2xx_keypad.c   |3 +-
>>>  hw/spitz.c   |3 +-
>>>  hw/stellaris_input.c |2 +-
>>>  hw/syborg_keyboard.c |2 +-
>>>  hw/usb-hid.c |   10 ++--
>>>  hw/xenfb.c   |5 ++-
>>>  input.c  |   51 
>>> -
>>>  14 files changed, 78 insertions(+), 27 deletions(-)
>>> 
>>> diff --git a/console.h b/console.h
>>> index 6def115..91b66ea 100644
>>> --- a/console.h
>>> +++ b/console.h
>>> @@ -41,7 +41,19 @@ typedef struct QEMUPutLEDEntry {
>>>  QTAILQ_ENTRY(QEMUPutLEDEntry) next;
>>>  } QEMUPutLEDEntry;
>>> 
>>> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
>>> +typedef struct QEMUPutKbdEntry {
>>> +char *qemu_put_kbd_name;
>>> +QEMUPutKBDEvent *qemu_put_kbd_event;
>>> +void *qemu_put_kbd_event_opaque;
>>> +int index;
>>> +
>>> +QTAILQ_ENTRY(QEMUPutKbdEntry) node;
>>> +} QEMUPutKbdEntry;
>>> +
>>> +QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
>>> +void *opaque,
>>> +const char *name);
>>> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
>>>  QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
>>>  void *opaque, int absolute,
>>>  const char *name);
>>> diff --git a/hw/adb.c b/hw/adb.c
>>> index 4fb7a62..09afcf9 100644
>>> --- a/hw/adb.c
>>> +++ b/hw/adb.c
>>> @@ -304,7 +304,7 @@ void adb_kbd_init(ADBBusState *bus)
>>>  s = qemu_mallocz(sizeof(KBDState));
>>>  d = adb_register_device(bus, ADB_KEYBOARD, adb_kbd_request,
>>>  adb_kbd_reset, s);
>>> -qemu_add_kbd_event_handler(adb_kbd_put_keycode, d);
>>> +qemu_add_kbd_event_handler(adb_kbd_put_keycode, d, "adb");
>>>  register_savevm("adb_kbd", -1, 1, adb_kbd_save,
>>>  adb_kbd_load, s);
>>>  }
>>> diff --git a/hw/escc.c b/hw/escc.c
>>> index 6d2fd36..2b21d98 100644
>>> --- a/hw/escc.c
>>> +++ b/hw/escc.c
>>> @@ -919,7 +919,8 @@ static int escc_init1(SysBusDevice *dev)
>>>   "QEMU Sun Mouse");
>>>  }
>>>  if (s->chn[1].type == kbd) {
>>> -qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1]);
>>> +qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1],
>>> +   "QEMU Sun Keyboard");
>>>  }
>>> 
>>>  return 0;
>>> diff --git a/hw/musicpal.c b/hw/musicpal.c
>>> index ebd933e..e1a3b6a 100644
>>> --- a/hw/musicpal.c
>>> +++ b/hw/musicpal.c
>>> @@ -1447,7 +1447,7 @@ static int musicpal_key_init(SysBusDevice *dev)
>>> 
>>>  qdev_init_gpio_out(&dev->qdev, s->out, ARRAY_SIZE(s->out));
>>> 
>>> -qemu_add_kbd_event_handler(musicpal_key_event, s);
>>> +qemu_add_kbd_event_handler(musicpal_key_event, s, "Musicpal");
>>> 
>>>  return 0;
>>>  }
>>> diff --git a/hw/nseries.c b/hw/nseries.c
>>> index 0273eee..abfcec3 100644
>>> --- a/hw/nseries.c
>>> +++ b/hw/nseries.c
>>> @@ -262,7 +262,7 @@ static void n800_tsc_kbd_setup(struct n800_s *s)
>>>  if (n800_keys[i]>= 0)
>>>  s->keymap[n800_keys[i]] = i;
>>> 
>>> -qemu_add_kbd_event_handler(n800_key_event, s);
>>> +qemu_add_kbd_event_handler(n800_key_event, s, "Nokia n800");
>>> 
>>>  tsc210x_set_transform(s->ts.chip,&n800_pointercal);
>>>  }
>>> @@ -371,7 +371,7 @@ static void n810_kbd_setup(struct n800_s *s)
>>>  if (n810_keys[i]>  0)
>>>  s->keymap[n810_keys[i]] = i;
>>> 
>>> -qemu_add_kbd_event_handler(n810_key_event, s);
>>> +qemu_add_kbd_event_handler(n810_key_event, s, "Nokia n810");
>>> 
>>>  /* Attach the LM8322 keyboard to the I2C bus,
>>>   * should happen in n8x0_i2c_setup and s->kbd be initialised here.  */
>>> diff --git a/hw/palm.c b/hw/palm.c
>>> index 6d19167..1b405

[Qemu-devel] [PATCH] CPUPhysMemoryClient: Batch contiguous addresses when playing catchup

2011-04-29 Thread Alex Williamson
When a phys memory client registers and we play catchup by walking
the page tables, we can make a huge improvement in the number of
times the set_memory callback is called by batching contiguous
pages together.  With a 4G guest, this reduces the number of callbacks
at registration from 1048866 to 296.

Signed-off-by: Alex Williamson 
---

 exec.c |   38 --
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index e670929..a0f2954 100644
--- a/exec.c
+++ b/exec.c
@@ -1741,8 +1741,15 @@ static int cpu_notify_migration_log(int enable)
 return 0;
 }
 
-static void phys_page_for_each_1(CPUPhysMemoryClient *client,
- int level, void **lp, target_phys_addr_t addr)
+struct last_map {
+target_phys_addr_t start_addr;
+ram_addr_t size;
+ram_addr_t phys_offset;
+};
+
+static void phys_page_for_each_1(CPUPhysMemoryClient *client, int level,
+ void **lp, target_phys_addr_t addr,
+ struct last_map *map)
 {
 int i;
 
@@ -1754,15 +1761,29 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
*client,
 addr <<= L2_BITS + TARGET_PAGE_BITS;
 for (i = 0; i < L2_SIZE; ++i) {
 if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
-client->set_memory(client, addr | i << TARGET_PAGE_BITS,
-   TARGET_PAGE_SIZE, pd[i].phys_offset);
+target_phys_addr_t start_addr = addr | i << TARGET_PAGE_BITS;
+
+if (map->size &&
+start_addr == map->start_addr + map->size &&
+pd[i].phys_offset == map->phys_offset + map->size) {
+
+map->size += TARGET_PAGE_SIZE;
+continue;
+} else if (map->size) {
+client->set_memory(client, map->start_addr,
+   map->size, map->phys_offset);
+}
+
+map->start_addr = start_addr;
+map->size = TARGET_PAGE_SIZE;
+map->phys_offset = pd[i].phys_offset;
 }
 }
 } else {
 void **pp = *lp;
 for (i = 0; i < L2_SIZE; ++i) {
 phys_page_for_each_1(client, level - 1, pp + i,
- (addr << L2_BITS) | i);
+ (addr << L2_BITS) | i, map);
 }
 }
 }
@@ -1770,9 +1791,14 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
*client,
 static void phys_page_for_each(CPUPhysMemoryClient *client)
 {
 int i;
+struct last_map map = { 0 };
+
 for (i = 0; i < P_L1_SIZE; ++i) {
 phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
- l1_phys_map + i, i);
+ l1_phys_map + i, i, &map);
+}
+if (map.size) {
+client->set_memory(client, map.start_addr, map.size, map.phys_offset);
 }
 }
 




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Alex Williamson
On Fri, 2011-04-29 at 09:38 -0600, Alex Williamson wrote:
> On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
> > On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> > > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> > >> When we're trying to get a newly registered phys memory client updated
> > >> with the current page mappings, we end up passing the region offset
> > >> (a ram_addr_t) as the start address rather than the actual guest
> > >> physical memory address (target_phys_addr_t).  If your guest has less
> > >> than 3.5G of memory, these are coincidentally the same thing.  If
> > 
> > I think this broke even with < 3.5G as phys_offset also encodes the
> > memory type while region_offset does not. So everything became RAMthis
> > way, no MMIO was announced.
> > 
> > >> there's more, the region offset for the memory above 4G starts over
> > >> at 0, so the set_memory client will overwrite it's lower memory entries.
> > >>
> > >> Instead, keep track of the guest phsyical address as we're walking the
> > >> tables and pass that to the set_memory client.
> > >>
> > >> Signed-off-by: Alex Williamson 
> > > 
> > > Acked-by: Michael S. Tsirkin 
> > > 
> > > Given all this, can yo tell how much time does
> > > it take to hotplug a device with, say, a 40G RAM guest?
> > 
> > Why not collect pages of identical types and report them as one chunk
> > once the type changes?
> 
> Good idea, I'll see if I can code that up.  I don't have a terribly
> large system to test with, but with an 8G guest, it's surprisingly not
> very noticeable.  For vfio, I intend to only have one memory client, so
> adding additional devices won't have to rescan everything.  The memory
> overhead of keeping the list that the memory client creates is probably
> also low enough that it isn't worthwhile to tear it all down if all the
> devices are removed.  Thanks,

Here's a first patch at a patch to do this.  For a 4G guest, it reduces
the number of registration induced set_memory callbacks from 1048866 to
296.

Signed-off-by: Alex Williamson 
---
diff --git a/exec.c b/exec.c
index e670929..5510b0b 100644
--- a/exec.c
+++ b/exec.c
@@ -1741,8 +1741,15 @@ static int cpu_notify_migration_log(int enable)
 return 0;
 }
 
+struct last_map {
+target_phys_addr_t start_addr;
+ram_addr_t size;
+ram_addr_t phys_offset;
+};
+
 static void phys_page_for_each_1(CPUPhysMemoryClient *client,
- int level, void **lp, target_phys_addr_t addr)
+ int level, void **lp,
+ target_phys_addr_t addr, struct last_map *map)
 {
 int i;
 
@@ -1754,15 +1761,28 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
*client,
 addr <<= L2_BITS + TARGET_PAGE_BITS;
 for (i = 0; i < L2_SIZE; ++i) {
 if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
-client->set_memory(client, addr | i << TARGET_PAGE_BITS,
-   TARGET_PAGE_SIZE, pd[i].phys_offset);
+target_phys_addr_t cur = addr | i << TARGET_PAGE_BITS;
+if (map->size &&
+cur == map->start_addr + map->size &&
+pd[i].phys_offset == map->phys_offset + map->size) {
+
+map->size += TARGET_PAGE_SIZE;
+continue;
+} else if (map->size) {
+client->set_memory(client, map->start_addr,
+   map->size, map->phys_offset);
+}
+
+map->start_addr = addr | i << TARGET_PAGE_BITS;
+map->size = TARGET_PAGE_SIZE;
+map->phys_offset = pd[i].phys_offset;
 }
 }
 } else {
 void **pp = *lp;
 for (i = 0; i < L2_SIZE; ++i) {
 phys_page_for_each_1(client, level - 1, pp + i,
- (addr << L2_BITS) | i);
+ (addr << L2_BITS) | i, map);
 }
 }
 }
@@ -1770,9 +1790,15 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
*client,
 static void phys_page_for_each(CPUPhysMemoryClient *client)
 {
 int i;
+struct last_map map = { 0 };
+
 for (i = 0; i < P_L1_SIZE; ++i) {
 phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
- l1_phys_map + i, i);
+ l1_phys_map + i, i, &map);
+}
+if (map.size) {
+client->set_memory(client, map.start_addr,
+   map.size, map.phys_offset);
 }
 }
 






Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Jan Kiszka
On 2011-04-29 18:20, Alex Williamson wrote:
> On Fri, 2011-04-29 at 18:07 +0200, Jan Kiszka wrote:
>> On 2011-04-29 17:55, Alex Williamson wrote:
>>> On Fri, 2011-04-29 at 17:45 +0200, Jan Kiszka wrote:
 On 2011-04-29 17:38, Alex Williamson wrote:
> On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
>> On 2011-04-29 17:06, Michael S. Tsirkin wrote:
>>> On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
 When we're trying to get a newly registered phys memory client updated
 with the current page mappings, we end up passing the region offset
 (a ram_addr_t) as the start address rather than the actual guest
 physical memory address (target_phys_addr_t).  If your guest has less
 than 3.5G of memory, these are coincidentally the same thing.  If
>>
>> I think this broke even with < 3.5G as phys_offset also encodes the
>> memory type while region_offset does not. So everything became RAMthis
>> way, no MMIO was announced.
>>
 there's more, the region offset for the memory above 4G starts over
 at 0, so the set_memory client will overwrite it's lower memory 
 entries.

 Instead, keep track of the guest phsyical address as we're walking the
 tables and pass that to the set_memory client.

 Signed-off-by: Alex Williamson 
>>>
>>> Acked-by: Michael S. Tsirkin 
>>>
>>> Given all this, can yo tell how much time does
>>> it take to hotplug a device with, say, a 40G RAM guest?
>>
>> Why not collect pages of identical types and report them as one chunk
>> once the type changes?
>
> Good idea, I'll see if I can code that up.  I don't have a terribly
> large system to test with, but with an 8G guest, it's surprisingly not
> very noticeable.  For vfio, I intend to only have one memory client, so
> adding additional devices won't have to rescan everything.  The memory
> overhead of keeping the list that the memory client creates is probably
> also low enough that it isn't worthwhile to tear it all down if all the
> devices are removed.  Thanks,

 What other clients register late? Do the need to know to whole memory
 layout?

 This full page table walk is likely a latency killer as it happens under
 global lock. Ugly.
>>>
>>> vhost and kvm are the only current users.  kvm registers it's client
>>> early enough that there's no memory registered, so doesn't really need
>>> this replay through the page table walk.  I'm not sure how vhost works
>>> currently.  I'm also looking at using this for vfio to register pages
>>> for the iommu.
>>
>> Hmm, it looks like vhost is basically recreating the condensed, slotted
>> memory layout from the per-page reports now. A bit inefficient,
>> specifically as this happens per vhost device, no? And if vfio preferred
>> a slotted format as well, you would end up copying vhost logic.
>>
>> That sounds to me like the qemu core should start tracking slots and
>> report slot changes, not memory region registrations.
> 
> I was thinking the same thing, but I think Michael is concerned if we'll
> each need slightly different lists.  This is also where kvm is mapping
> to a fixed array of slots, which is know to blow-up with too many
> assigned devices.  Needs to be fixed on both kernel and qemu side.
> Runtime overhead of the phys memory client is pretty minimal, it's just
> the startup that thrashes set_memory.

I'm not just concerned about the runtime overhead. This is code
duplication. Even if the format of the lists differ, their structure
should not: one entry per continuous memory region, and some lists may
track sparsely based on their interests.

I'm sure the core could be taught to help the clients creating and
maintaining such lists. We already have two types of users in tree, you
are about to create another one, and Xen should have some need for it as
well.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 1/2 v4] Support for multiple keyboard devices

2011-04-29 Thread Dmitry Zhurikhin

On 05/11/2010 12:18 AM, Anthony Liguori wrote:

On 04/18/2010 02:21 PM, Shahar Havivi wrote:
Patch add QEMUPutKbdEntry structure - handling each keyboard entry, the 
structure handled

by qemu tail queue.
Adding a new keyboard add to the list and select it, removing keyboard select 
the previous

keyboard in list.

Signed-off-by: Shahar Havivi


Applied all.  Thanks.
Hello.  What happened to this patchset?  Seems like it didn't make to the 
trunk.  Were there any objections not discussed on the list?  If there are no 
objections I'll update these patches and resend them.  Is it OK?


Regards,
Dmitry



Regards,

Anthony Liguori


---
  console.h|   14 -
  hw/adb.c |2 +-
  hw/escc.c|3 +-
  hw/musicpal.c|2 +-
  hw/nseries.c |4 +-
  hw/palm.c|2 +-
  hw/ps2.c |2 +-
  hw/pxa2xx_keypad.c   |3 +-
  hw/spitz.c   |3 +-
  hw/stellaris_input.c |2 +-
  hw/syborg_keyboard.c |2 +-
  hw/usb-hid.c |   10 ++--
  hw/xenfb.c   |5 ++-
  input.c  |   51 -
  14 files changed, 78 insertions(+), 27 deletions(-)

diff --git a/console.h b/console.h
index 6def115..91b66ea 100644
--- a/console.h
+++ b/console.h
@@ -41,7 +41,19 @@ typedef struct QEMUPutLEDEntry {
  QTAILQ_ENTRY(QEMUPutLEDEntry) next;
  } QEMUPutLEDEntry;

-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
+typedef struct QEMUPutKbdEntry {
+char *qemu_put_kbd_name;
+QEMUPutKBDEvent *qemu_put_kbd_event;
+void *qemu_put_kbd_event_opaque;
+int index;
+
+QTAILQ_ENTRY(QEMUPutKbdEntry) node;
+} QEMUPutKbdEntry;
+
+QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
+void *opaque,
+const char *name);
+void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
  QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
  void *opaque, int absolute,
  const char *name);
diff --git a/hw/adb.c b/hw/adb.c
index 4fb7a62..09afcf9 100644
--- a/hw/adb.c
+++ b/hw/adb.c
@@ -304,7 +304,7 @@ void adb_kbd_init(ADBBusState *bus)
  s = qemu_mallocz(sizeof(KBDState));
  d = adb_register_device(bus, ADB_KEYBOARD, adb_kbd_request,
  adb_kbd_reset, s);
-qemu_add_kbd_event_handler(adb_kbd_put_keycode, d);
+qemu_add_kbd_event_handler(adb_kbd_put_keycode, d, "adb");
  register_savevm("adb_kbd", -1, 1, adb_kbd_save,
  adb_kbd_load, s);
  }
diff --git a/hw/escc.c b/hw/escc.c
index 6d2fd36..2b21d98 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -919,7 +919,8 @@ static int escc_init1(SysBusDevice *dev)
   "QEMU Sun Mouse");
  }
  if (s->chn[1].type == kbd) {
-qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1]);
+qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1],
+   "QEMU Sun Keyboard");
  }

  return 0;
diff --git a/hw/musicpal.c b/hw/musicpal.c
index ebd933e..e1a3b6a 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -1447,7 +1447,7 @@ static int musicpal_key_init(SysBusDevice *dev)

  qdev_init_gpio_out(&dev->qdev, s->out, ARRAY_SIZE(s->out));

-qemu_add_kbd_event_handler(musicpal_key_event, s);
+qemu_add_kbd_event_handler(musicpal_key_event, s, "Musicpal");

  return 0;
  }
diff --git a/hw/nseries.c b/hw/nseries.c
index 0273eee..abfcec3 100644
--- a/hw/nseries.c
+++ b/hw/nseries.c
@@ -262,7 +262,7 @@ static void n800_tsc_kbd_setup(struct n800_s *s)
  if (n800_keys[i]>= 0)
  s->keymap[n800_keys[i]] = i;

-qemu_add_kbd_event_handler(n800_key_event, s);
+qemu_add_kbd_event_handler(n800_key_event, s, "Nokia n800");

  tsc210x_set_transform(s->ts.chip,&n800_pointercal);
  }
@@ -371,7 +371,7 @@ static void n810_kbd_setup(struct n800_s *s)
  if (n810_keys[i]>  0)
  s->keymap[n810_keys[i]] = i;

-qemu_add_kbd_event_handler(n810_key_event, s);
+qemu_add_kbd_event_handler(n810_key_event, s, "Nokia n810");

  /* Attach the LM8322 keyboard to the I2C bus,
   * should happen in n8x0_i2c_setup and s->kbd be initialised here.  */
diff --git a/hw/palm.c b/hw/palm.c
index 6d19167..1b405d4 100644
--- a/hw/palm.c
+++ b/hw/palm.c
@@ -228,7 +228,7 @@ static void palmte_init(ram_addr_t ram_size,

  palmte_microwire_setup(cpu);

-qemu_add_kbd_event_handler(palmte_button_event, cpu);
+qemu_add_kbd_event_handler(palmte_button_event, cpu, "Palm Keyboard");

  palmte_gpio_setup(cpu);

diff --git a/hw/ps2.c b/hw/ps2.c
index f0b206a..886da37 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -596,7 +596,7 @@ void *ps2_kbd_init(void (*update_irq)(void *, int), void 
*update_arg)

  s->common.update

Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Alex Williamson
On Fri, 2011-04-29 at 18:07 +0200, Jan Kiszka wrote:
> On 2011-04-29 17:55, Alex Williamson wrote:
> > On Fri, 2011-04-29 at 17:45 +0200, Jan Kiszka wrote:
> >> On 2011-04-29 17:38, Alex Williamson wrote:
> >>> On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
>  On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> >> When we're trying to get a newly registered phys memory client updated
> >> with the current page mappings, we end up passing the region offset
> >> (a ram_addr_t) as the start address rather than the actual guest
> >> physical memory address (target_phys_addr_t).  If your guest has less
> >> than 3.5G of memory, these are coincidentally the same thing.  If
> 
>  I think this broke even with < 3.5G as phys_offset also encodes the
>  memory type while region_offset does not. So everything became RAMthis
>  way, no MMIO was announced.
> 
> >> there's more, the region offset for the memory above 4G starts over
> >> at 0, so the set_memory client will overwrite it's lower memory 
> >> entries.
> >>
> >> Instead, keep track of the guest phsyical address as we're walking the
> >> tables and pass that to the set_memory client.
> >>
> >> Signed-off-by: Alex Williamson 
> >
> > Acked-by: Michael S. Tsirkin 
> >
> > Given all this, can yo tell how much time does
> > it take to hotplug a device with, say, a 40G RAM guest?
> 
>  Why not collect pages of identical types and report them as one chunk
>  once the type changes?
> >>>
> >>> Good idea, I'll see if I can code that up.  I don't have a terribly
> >>> large system to test with, but with an 8G guest, it's surprisingly not
> >>> very noticeable.  For vfio, I intend to only have one memory client, so
> >>> adding additional devices won't have to rescan everything.  The memory
> >>> overhead of keeping the list that the memory client creates is probably
> >>> also low enough that it isn't worthwhile to tear it all down if all the
> >>> devices are removed.  Thanks,
> >>
> >> What other clients register late? Do the need to know to whole memory
> >> layout?
> >>
> >> This full page table walk is likely a latency killer as it happens under
> >> global lock. Ugly.
> > 
> > vhost and kvm are the only current users.  kvm registers it's client
> > early enough that there's no memory registered, so doesn't really need
> > this replay through the page table walk.  I'm not sure how vhost works
> > currently.  I'm also looking at using this for vfio to register pages
> > for the iommu.
> 
> Hmm, it looks like vhost is basically recreating the condensed, slotted
> memory layout from the per-page reports now. A bit inefficient,
> specifically as this happens per vhost device, no? And if vfio preferred
> a slotted format as well, you would end up copying vhost logic.
> 
> That sounds to me like the qemu core should start tracking slots and
> report slot changes, not memory region registrations.

I was thinking the same thing, but I think Michael is concerned if we'll
each need slightly different lists.  This is also where kvm is mapping
to a fixed array of slots, which is know to blow-up with too many
assigned devices.  Needs to be fixed on both kernel and qemu side.
Runtime overhead of the phys memory client is pretty minimal, it's just
the startup that thrashes set_memory.

Alex






Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Jan Kiszka
On 2011-04-29 17:55, Alex Williamson wrote:
> On Fri, 2011-04-29 at 17:45 +0200, Jan Kiszka wrote:
>> On 2011-04-29 17:38, Alex Williamson wrote:
>>> On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
 On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
>> When we're trying to get a newly registered phys memory client updated
>> with the current page mappings, we end up passing the region offset
>> (a ram_addr_t) as the start address rather than the actual guest
>> physical memory address (target_phys_addr_t).  If your guest has less
>> than 3.5G of memory, these are coincidentally the same thing.  If

 I think this broke even with < 3.5G as phys_offset also encodes the
 memory type while region_offset does not. So everything became RAMthis
 way, no MMIO was announced.

>> there's more, the region offset for the memory above 4G starts over
>> at 0, so the set_memory client will overwrite it's lower memory entries.
>>
>> Instead, keep track of the guest phsyical address as we're walking the
>> tables and pass that to the set_memory client.
>>
>> Signed-off-by: Alex Williamson 
>
> Acked-by: Michael S. Tsirkin 
>
> Given all this, can yo tell how much time does
> it take to hotplug a device with, say, a 40G RAM guest?

 Why not collect pages of identical types and report them as one chunk
 once the type changes?
>>>
>>> Good idea, I'll see if I can code that up.  I don't have a terribly
>>> large system to test with, but with an 8G guest, it's surprisingly not
>>> very noticeable.  For vfio, I intend to only have one memory client, so
>>> adding additional devices won't have to rescan everything.  The memory
>>> overhead of keeping the list that the memory client creates is probably
>>> also low enough that it isn't worthwhile to tear it all down if all the
>>> devices are removed.  Thanks,
>>
>> What other clients register late? Do the need to know to whole memory
>> layout?
>>
>> This full page table walk is likely a latency killer as it happens under
>> global lock. Ugly.
> 
> vhost and kvm are the only current users.  kvm registers it's client
> early enough that there's no memory registered, so doesn't really need
> this replay through the page table walk.  I'm not sure how vhost works
> currently.  I'm also looking at using this for vfio to register pages
> for the iommu.

Hmm, it looks like vhost is basically recreating the condensed, slotted
memory layout from the per-page reports now. A bit inefficient,
specifically as this happens per vhost device, no? And if vfio preferred
a slotted format as well, you would end up copying vhost logic.

That sounds to me like the qemu core should start tracking slots and
report slot changes, not memory region registrations.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH] qemu-kvm: fix pulseaudio detection in configure

2011-04-29 Thread Marc-Antoine Perennou
pulse/simple.h does not include stdlib.h
We cannot use NULL since it may not be defined
Use 0 instead

Signed-off-by: Marc-Antoine Perennou 
---
 configure |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index ea8b676..d67c3ce 100755
--- a/configure
+++ b/configure
@@ -1567,7 +1567,7 @@ for drv in $audio_drv_list; do

 pa)
 audio_drv_probe $drv pulse/simple.h "-lpulse-simple -lpulse" \
-"pa_simple *s = NULL; pa_simple_free(s); return 0;"
+"pa_simple *s = 0; pa_simple_free(s); return 0;"
 libs_softmmu="-lpulse -lpulse-simple $libs_softmmu"
 audio_pt_int="yes"
 ;;
-- 
1.7.5.52.ge839f.dirty



Re: [Qemu-devel] [PULL] Request for Pull

2011-04-29 Thread Aneesh Kumar K.V
On Fri, 29 Apr 2011 14:10:43 +0530, "Aneesh Kumar K.V" 
 wrote:
> On Thu, 28 Apr 2011 12:39:45 -0500, Anthony Liguori  
> wrote:
> > On 04/28/2011 12:21 PM, Stefan Weil wrote:
> > > Am 28.04.2011 15:56, schrieb Anthony Liguori:
> > >> On 04/27/2011 11:16 AM, Venkateswararao Jujjuri wrote:
> > >>> The following changes since commit
> > >>> 661bfc80e876d32da8befe53ba0234d87fc0bcc2:
> > >>> Jan Kiszka (1):
> > >>> pflash: Restore & fix lazy ROMD switching
> > >>>
> > >>> are available in the git repository at:
> > >>>
> > >>> git://repo.or.cz/qemu/aliguori/jvrao.git for-anthony
> > >>
> > >> Pulled. Thanks.
> > >>
> > >> Regards,
> > >>
> > >> Anthony Liguori
> > >
> > >
> > > This pull breaks compilation and linkage of latest QEMU:
> > >
> > > * Compilation fails because of wrong include paths (caused by moved 
> > > files).
> > > I just sent a patch to fix this issue.
> > >
> > > * The linker fails for all system emulations without virtio. Example:
> > >
> > > LINK cris-softmmu/qemu-system-cris
> > > ../fsdev/qemu-fsdev.o:(.data+0xc): undefined reference to `local_ops'
> > 
> > The use of CONFIG_REALLY_VIRTFS was wrong in the rename patch.  I'll 
> > push a fix.
> > 
> 
> The change you committed will pull in lot of 9p related code even
> when virtfs is not really enabled right ? why not ?
> 

Updated one which is tested.

>From 454d36bf1bd7605637897bf87a93cd18c8d29b1b Mon Sep 17 00:00:00 2001
From: Aneesh Kumar K.V 
Date: Fri, 29 Apr 2011 14:32:10 +0530
Subject: [PATCH] virtio-9p: Don't link to 9p if virtio is not enabled

If virtio is not enabled then we should not pull in
virtfs files

Signed-off-by: Aneesh Kumar K.V 
---
 Makefile.objs|8 +---
 fsdev/qemu-fsdev-dummy.c |   21 +
 2 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 fsdev/qemu-fsdev-dummy.c

diff --git a/Makefile.objs b/Makefile.objs
index 9d8851e..0668e0a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -49,8 +49,10 @@ ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS),yy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
 # only pull in the actual virtio-9p device if we also enabled virtio.
 CONFIG_REALLY_VIRTFS=y
+fsdev-nested-y = qemu-fsdev.o
+else
+fsdev-nested-y = qemu-fsdev-dummy.o
 endif
-fsdev-nested-$(CONFIG_VIRTFS) = qemu-fsdev.o
 fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y))
 
 ##
@@ -285,11 +287,11 @@ sound-obj-$(CONFIG_HDA) += intel-hda.o hda-audio.o
 adlib.o fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0
 hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 
-9pfs-nested-$(CONFIG_REALLY_VIRTFS) = virtio-9p-debug.o
+9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 
-hw-obj-$(CONFIG_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
+hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
 
 
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
new file mode 100644
index 000..6a51f60
--- /dev/null
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -0,0 +1,21 @@
+/*
+ * Virtio 9p
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Gautham R Shenoy 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#include 
+#include 
+#include "qemu-fsdev.h"
+
+int qemu_fsdev_add(QemuOpts *opts)
+{
+return 0;
+}
+
-- 
1.7.4.1




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Alex Williamson
On Fri, 2011-04-29 at 17:45 +0200, Jan Kiszka wrote:
> On 2011-04-29 17:38, Alex Williamson wrote:
> > On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
> >> On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> >>> On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
>  When we're trying to get a newly registered phys memory client updated
>  with the current page mappings, we end up passing the region offset
>  (a ram_addr_t) as the start address rather than the actual guest
>  physical memory address (target_phys_addr_t).  If your guest has less
>  than 3.5G of memory, these are coincidentally the same thing.  If
> >>
> >> I think this broke even with < 3.5G as phys_offset also encodes the
> >> memory type while region_offset does not. So everything became RAMthis
> >> way, no MMIO was announced.
> >>
>  there's more, the region offset for the memory above 4G starts over
>  at 0, so the set_memory client will overwrite it's lower memory entries.
> 
>  Instead, keep track of the guest phsyical address as we're walking the
>  tables and pass that to the set_memory client.
> 
>  Signed-off-by: Alex Williamson 
> >>>
> >>> Acked-by: Michael S. Tsirkin 
> >>>
> >>> Given all this, can yo tell how much time does
> >>> it take to hotplug a device with, say, a 40G RAM guest?
> >>
> >> Why not collect pages of identical types and report them as one chunk
> >> once the type changes?
> > 
> > Good idea, I'll see if I can code that up.  I don't have a terribly
> > large system to test with, but with an 8G guest, it's surprisingly not
> > very noticeable.  For vfio, I intend to only have one memory client, so
> > adding additional devices won't have to rescan everything.  The memory
> > overhead of keeping the list that the memory client creates is probably
> > also low enough that it isn't worthwhile to tear it all down if all the
> > devices are removed.  Thanks,
> 
> What other clients register late? Do the need to know to whole memory
> layout?
> 
> This full page table walk is likely a latency killer as it happens under
> global lock. Ugly.

vhost and kvm are the only current users.  kvm registers it's client
early enough that there's no memory registered, so doesn't really need
this replay through the page table walk.  I'm not sure how vhost works
currently.  I'm also looking at using this for vfio to register pages
for the iommu.

Alex




Re: [Qemu-devel] [PATCH] monitor: add PPC BookE SPRs

2011-04-29 Thread Scott Wood
On Fri, 29 Apr 2011 14:05:23 +0200
Jan Kiszka  wrote:

> On 2011-04-28 23:01, Scott Wood wrote:
> > +#ifdef KVM_CAP_PPC_BOOKE_SREGS
> > +if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_BOOKE_SREGS)) {
> 
> You probably want to cache the result of this syscall during init and
> check that here. There are plenty examples for this pattern around.

OK.

> > +ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> > +if (ret < 0)
> > +return ret;
> 
> Please use chechpatch.pl before submitting.

Doh, will fix.

-Scott




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Jan Kiszka
On 2011-04-29 17:38, Alex Williamson wrote:
> On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
>> On 2011-04-29 17:06, Michael S. Tsirkin wrote:
>>> On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
 When we're trying to get a newly registered phys memory client updated
 with the current page mappings, we end up passing the region offset
 (a ram_addr_t) as the start address rather than the actual guest
 physical memory address (target_phys_addr_t).  If your guest has less
 than 3.5G of memory, these are coincidentally the same thing.  If
>>
>> I think this broke even with < 3.5G as phys_offset also encodes the
>> memory type while region_offset does not. So everything became RAMthis
>> way, no MMIO was announced.
>>
 there's more, the region offset for the memory above 4G starts over
 at 0, so the set_memory client will overwrite it's lower memory entries.

 Instead, keep track of the guest phsyical address as we're walking the
 tables and pass that to the set_memory client.

 Signed-off-by: Alex Williamson 
>>>
>>> Acked-by: Michael S. Tsirkin 
>>>
>>> Given all this, can yo tell how much time does
>>> it take to hotplug a device with, say, a 40G RAM guest?
>>
>> Why not collect pages of identical types and report them as one chunk
>> once the type changes?
> 
> Good idea, I'll see if I can code that up.  I don't have a terribly
> large system to test with, but with an 8G guest, it's surprisingly not
> very noticeable.  For vfio, I intend to only have one memory client, so
> adding additional devices won't have to rescan everything.  The memory
> overhead of keeping the list that the memory client creates is probably
> also low enough that it isn't worthwhile to tear it all down if all the
> devices are removed.  Thanks,

What other clients register late? Do the need to know to whole memory
layout?

This full page table walk is likely a latency killer as it happens under
global lock. Ugly.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Alex Williamson
On Fri, 2011-04-29 at 18:34 +0300, Michael S. Tsirkin wrote:
> On Fri, Apr 29, 2011 at 05:29:06PM +0200, Jan Kiszka wrote:
> > On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> > > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> > >> When we're trying to get a newly registered phys memory client updated
> > >> with the current page mappings, we end up passing the region offset
> > >> (a ram_addr_t) as the start address rather than the actual guest
> > >> physical memory address (target_phys_addr_t).  If your guest has less
> > >> than 3.5G of memory, these are coincidentally the same thing.  If
> > 
> > I think this broke even with < 3.5G as phys_offset also encodes the
> > memory type while region_offset does not. So everything became RAMthis
> > way, no MMIO was announced.
> > 
> > >> there's more, the region offset for the memory above 4G starts over
> > >> at 0, so the set_memory client will overwrite it's lower memory entries.
> > >>
> > >> Instead, keep track of the guest phsyical address as we're walking the
> > >> tables and pass that to the set_memory client.
> > >>
> > >> Signed-off-by: Alex Williamson 
> > > 
> > > Acked-by: Michael S. Tsirkin 
> > > 
> > > Given all this, can yo tell how much time does
> > > it take to hotplug a device with, say, a 40G RAM guest?
> > 
> > Why not collect pages of identical types and report them as one chunk
> > once the type changes?
> 
> Sure, but before we bother to optimize this, is this too slow?

At a set_memory call per 4k page, it's probably worthwhile to factor in
some simply optimizations.  My set_memory callback was being hit 10^6
times.  Thanks,

Alex




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Alex Williamson
On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote:
> On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> >> When we're trying to get a newly registered phys memory client updated
> >> with the current page mappings, we end up passing the region offset
> >> (a ram_addr_t) as the start address rather than the actual guest
> >> physical memory address (target_phys_addr_t).  If your guest has less
> >> than 3.5G of memory, these are coincidentally the same thing.  If
> 
> I think this broke even with < 3.5G as phys_offset also encodes the
> memory type while region_offset does not. So everything became RAMthis
> way, no MMIO was announced.
> 
> >> there's more, the region offset for the memory above 4G starts over
> >> at 0, so the set_memory client will overwrite it's lower memory entries.
> >>
> >> Instead, keep track of the guest phsyical address as we're walking the
> >> tables and pass that to the set_memory client.
> >>
> >> Signed-off-by: Alex Williamson 
> > 
> > Acked-by: Michael S. Tsirkin 
> > 
> > Given all this, can yo tell how much time does
> > it take to hotplug a device with, say, a 40G RAM guest?
> 
> Why not collect pages of identical types and report them as one chunk
> once the type changes?

Good idea, I'll see if I can code that up.  I don't have a terribly
large system to test with, but with an 8G guest, it's surprisingly not
very noticeable.  For vfio, I intend to only have one memory client, so
adding additional devices won't have to rescan everything.  The memory
overhead of keeping the list that the memory client creates is probably
also low enough that it isn't worthwhile to tear it all down if all the
devices are removed.  Thanks,

Alex




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Michael S. Tsirkin
On Fri, Apr 29, 2011 at 05:29:06PM +0200, Jan Kiszka wrote:
> On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> >> When we're trying to get a newly registered phys memory client updated
> >> with the current page mappings, we end up passing the region offset
> >> (a ram_addr_t) as the start address rather than the actual guest
> >> physical memory address (target_phys_addr_t).  If your guest has less
> >> than 3.5G of memory, these are coincidentally the same thing.  If
> 
> I think this broke even with < 3.5G as phys_offset also encodes the
> memory type while region_offset does not. So everything became RAMthis
> way, no MMIO was announced.
> 
> >> there's more, the region offset for the memory above 4G starts over
> >> at 0, so the set_memory client will overwrite it's lower memory entries.
> >>
> >> Instead, keep track of the guest phsyical address as we're walking the
> >> tables and pass that to the set_memory client.
> >>
> >> Signed-off-by: Alex Williamson 
> > 
> > Acked-by: Michael S. Tsirkin 
> > 
> > Given all this, can yo tell how much time does
> > it take to hotplug a device with, say, a 40G RAM guest?
> 
> Why not collect pages of identical types and report them as one chunk
> once the type changes?

Sure, but before we bother to optimize this, is this too slow?

> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Jan Kiszka
On 2011-04-29 17:06, Michael S. Tsirkin wrote:
> On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
>> When we're trying to get a newly registered phys memory client updated
>> with the current page mappings, we end up passing the region offset
>> (a ram_addr_t) as the start address rather than the actual guest
>> physical memory address (target_phys_addr_t).  If your guest has less
>> than 3.5G of memory, these are coincidentally the same thing.  If

I think this broke even with < 3.5G as phys_offset also encodes the
memory type while region_offset does not. So everything became RAMthis
way, no MMIO was announced.

>> there's more, the region offset for the memory above 4G starts over
>> at 0, so the set_memory client will overwrite it's lower memory entries.
>>
>> Instead, keep track of the guest phsyical address as we're walking the
>> tables and pass that to the set_memory client.
>>
>> Signed-off-by: Alex Williamson 
> 
> Acked-by: Michael S. Tsirkin 
> 
> Given all this, can yo tell how much time does
> it take to hotplug a device with, say, a 40G RAM guest?

Why not collect pages of identical types and report them as one chunk
once the type changes?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH 7/9] qapi: test schema for test-visiter unit tests

2011-04-29 Thread Michael Roth

Signed-off-by: Michael Roth 
---
 qapi-schema-test.json |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 qapi-schema-test.json

diff --git a/qapi-schema-test.json b/qapi-schema-test.json
new file mode 100644
index 000..4717dee
--- /dev/null
+++ b/qapi-schema-test.json
@@ -0,0 +1,11 @@
+# *-*- Mode: Python -*-*
+
+# for testing nested structs
+{ 'type': 'UserDefOne',
+  'data': { 'integer': 'int', 'string': 'str' } }
+
+{ 'type': 'UserDefTwo',
+  'data': { 'string': 'str',
+'dict': { 'string': 'str',
+  'dict': { 'userdef': 'UserDefOne', 'string': 'str' },
+  '*dict2': { 'userdef': 'UserDefOne', 'string': 'str' } } 
} }
-- 
1.7.0.4




[Qemu-devel] [PATCH 9/9] qapi: test-visiter, pull in gen code, tests for nested structures

2011-04-29 Thread Michael Roth
Pull into test files generated from qapi-schema-test.json by
visiter/types generators so we can do unit tests against the generated
code.

Also, add test cases for handling of nested structs/qobjects. Note:
optional members are implemented yet so test case is slightly gimped.

Signed-off-by: Michael Roth 
---
 test-visiter.c |   98 
 1 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/test-visiter.c b/test-visiter.c
index 4aec8ab..28810d2 100644
--- a/test-visiter.c
+++ b/test-visiter.c
@@ -1,6 +1,8 @@
 #include 
 #include "qapi/qmp-output-visiter.h"
 #include "qapi/qmp-input-visiter.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
 #include "qemu-objects.h"
 
 typedef struct TestStruct
@@ -37,6 +39,100 @@ static void visit_type_TestStructList(Visiter *m, 
TestStructList ** obj, const c
 visit_end_list(m, errp);
 }
 
+/* test deep nesting with refs to other user-defined types */
+static void test_nested_structs(void)
+{
+QmpOutputVisiter *mo;
+QmpInputVisiter *mi;
+Visiter *v;
+UserDefOne ud1;
+UserDefOne *ud1_p = &ud1, *ud1c_p = NULL;
+UserDefTwo ud2;
+UserDefTwo *ud2_p = &ud2, *ud2c_p = NULL;
+Error *err = NULL;
+QObject *obj;
+QString *str;
+
+ud1.integer = 42;
+ud1.string = strdup("fourty two");
+
+/* sanity check */
+mo = qmp_output_visiter_new();
+v = qmp_output_get_visiter(mo);
+visit_type_UserDefOne(v, &ud1_p, "o_O", &err);
+if (err) {
+g_error("%s", error_get_pretty(err));
+}
+obj = qmp_output_get_qobject(mo);
+g_assert(obj);
+qobject_decref(obj);
+
+ud2.string = strdup("fourty three");
+ud2.dict.string = strdup("fourty four");
+ud2.dict.dict.userdef = ud1_p;
+ud2.dict.dict.string = strdup("fourty five");
+ud2.dict.has_dict2 = true;
+ud2.dict.dict2.userdef = ud1_p;
+ud2.dict.dict2.string = strdup("fourty six");
+
+/* c type -> qobject */
+mo = qmp_output_visiter_new();
+v = qmp_output_get_visiter(mo);
+visit_type_UserDefTwo(v, &ud2_p, "unused", &err);
+if (err) {
+g_error("%s", error_get_pretty(err));
+}
+obj = qmp_output_get_qobject(mo);
+g_assert(obj);
+str = qobject_to_json_pretty(obj);
+g_print("%s\n", qstring_get_str(str));
+QDECREF(str);
+
+/* qobject -> c type, should match original struct */
+mi = qmp_input_visiter_new(obj);
+v = qmp_input_get_visiter(mi);
+visit_type_UserDefTwo(v, &ud2c_p, "unused", &err);
+if (err) {
+g_error("%s", error_get_pretty(err));
+}
+
+g_assert(!g_strcmp0(ud2c_p->string, ud2.string));
+g_assert(!g_strcmp0(ud2c_p->dict.string, ud2.dict.string));
+
+ud1c_p = ud2c_p->dict.dict.userdef;
+g_assert(ud1c_p->integer == ud1_p->integer);
+g_assert(!g_strcmp0(ud1c_p->string, ud1_p->string));
+
+g_assert(!g_strcmp0(ud2c_p->dict.dict.string, ud2.dict.dict.string));
+
+/* TODO: ud2.dict.dict2 is optional dict, so these require
+ * visit_start_optional() to be implemented for input visiter
+ */
+/*
+ud1c_p = ud2c_p->dict.dict2.userdef;
+g_assert(ud1c_p->integer == ud1_p->integer);
+g_assert(!g_strcmp0(ud1c_p->string, ud1_p->string));
+*/
+
+g_assert(g_strcmp0(ud2c_p->dict.dict2.string, ud2.dict.dict2.string));
+
+qemu_free(ud1.string);
+qemu_free(ud2.string);
+qemu_free(ud2.dict.string);
+qemu_free(ud2.dict.dict.string);
+qemu_free(ud2.dict.dict2.string);
+
+qemu_free(ud1c_p->string);
+qemu_free(ud1c_p);
+qemu_free(ud2c_p->string);
+qemu_free(ud2c_p->dict.string);
+qemu_free(ud2c_p->dict.dict.string);
+qemu_free(ud2c_p->dict.dict2.string);
+qemu_free(ud2c_p);
+
+qobject_decref(obj);
+}
+
 int main(int argc, char **argv)
 {
 QmpOutputVisiter *mo;
@@ -122,6 +218,8 @@ int main(int argc, char **argv)
 
 qobject_decref(obj);
 
+g_test_add_func("/0.15/nested_structs", test_nested_structs);
+
 g_test_run();
 
 return 0;
-- 
1.7.0.4




[Qemu-devel] [PATCH 5/9] qapi: add --prefix option to type generator

2011-04-29 Thread Michael Roth
This is mainly so we can generate a header file with the filename
{prefix}qapi-types.h by passing in a test schema for use with unit
tests.

Signed-off-by: Michael Roth 
---
 scripts/qapi-types.py |   30 --
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index d645bad..e38a651 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -1,6 +1,7 @@
 from ordereddict import OrderedDict
 from qapi import *
 import sys
+import getopt
 
 def generate_fwd_struct(name, members):
 return mcgen('''
@@ -108,16 +109,33 @@ struct %(name)s
 
 return ret
 
-fdecl = open('qapi-types.h', 'w')
+try:
+opts, args = getopt.gnu_getopt(sys.argv[1:], "p:", ["prefix="])
+except getopt.GetoptError, err:
+print str(err)
+sys.exit(1)
 
-exprs = parse_schema(sys.stdin)
+prefix = ""
+h_file = 'qapi-types.h'
+
+for o, a in opts:
+if o in ("-p", "--prefix"):
+prefix = a
+
+h_file = prefix + h_file
+
+fdecl = open(h_file, 'w')
 
-fdecl.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
-#ifndef QAPI_TYPES_H
-#define QAPI_TYPES_H
+fdecl.write(mcgen('''
+/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+#ifndef %(guard)s
+#define %(guard)s
 
 #include "qapi-types-core.h"
-''')
+''',
+  guard=guardname(h_file)))
+
+exprs = parse_schema(sys.stdin)
 
 for expr in exprs:
 ret = "\n"
-- 
1.7.0.4




[Qemu-devel] [PATCH 6/9] qapi: add --prefix option for visiter generator

2011-04-29 Thread Michael Roth
Generated code assumes the required types header was generated using the
same prefix.

Signed-off-by: Michael Roth 
---
 scripts/qapi-visit.py |   41 -
 1 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index bf005c6..ee6b031 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -1,6 +1,7 @@
 from ordereddict import OrderedDict
 from qapi import *
 import sys
+import getopt
 
 def generate_visit_struct_body(field_prefix, members):
 ret = ""
@@ -135,21 +136,43 @@ void visit_type_%(name)s(Visiter *m, %(name)s * obj, 
const char *name, Error **e
 ''',
 name=name)
 
-fdef = open('qapi-visit.c', 'w')
-fdecl = open('qapi-visit.h', 'w')
+try:
+opts, args = getopt.gnu_getopt(sys.argv[1:], "p:", ["prefix="])
+except getopt.GetoptError, err:
+print str(err)
+sys.exit(1)
 
-fdef.write('''/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+prefix = ""
+c_file = 'qapi-visit.c'
+h_file = 'qapi-visit.h'
 
-#include "qapi-visit.h"
-''')
+for o, a in opts:
+if o in ("-p", "--prefix"):
+prefix = a
+
+c_file = prefix + c_file
+h_file = prefix + h_file
+
+fdef = open(c_file, 'w')
+fdecl = open(h_file, 'w')
+
+fdef.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
 
-fdecl.write('''/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+#include "%(header)s"
+''',
+ header=basename(h_file)))
+
+fdecl.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
 
-#ifndef QAPI_VISIT_H
-#define QAPI_VISIT_H
+#ifndef %(guard)s
+#define %(guard)s
 
 #include "qapi-visit-core.h"
-''')
+#include "%(prefix)sqapi-types.h"
+''',
+  prefix=prefix, guard=guardname(h_file)))
 
 exprs = parse_schema(sys.stdin)
 
-- 
1.7.0.4




[Qemu-devel] [PATCH 4/9] qapi: some basename/guardname py utility functions

2011-04-29 Thread Michael Roth
Some utility functions to get basename/guardname from a
filename/filepath. These will be used to allow arbitrarilly named test
files to be generated by code generators for use in unit tests.

Signed-off-by: Michael Roth 
---
 scripts/qapi.py |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1bc4604..2786430 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -157,3 +157,8 @@ def cgen(code, **kwds):
 def mcgen(code, **kwds):
 return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
 
+def basename(filename):
+return filename.split("/")[-1]
+
+def guardname(filename):
+return filename.replace("/", "_").replace("-", "_").split(".")[0].upper()
-- 
1.7.0.4




[Qemu-devel] [PATCH 8/9] qapi: Makefile, build test-visiter with generated test code

2011-04-29 Thread Michael Roth
This pulls in test-qapi-visit.c/.h and test-qapi-types.h, which are
generated from qapi-schema-test.json using the --prefix arguments for
the various code generators. Useful for targetted testing of the schema
parser/code generators.

Signed-off-by: Michael Roth 
---
 Makefile |   15 ---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index d510779..d05cf74 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 # Makefile for QEMU.
 
 GENERATED_HEADERS = config-host.h trace.h qemu-options.def qmp.h libqmp.h 
qdev-marshal.h
-GENERATED_HEADERS += qapi-types.h qmp-marshal-types.h qcfg-marshal.h
+GENERATED_HEADERS += qapi-types.h qmp-marshal-types.h qcfg-marshal.h 
test-qapi-types.h
 ifeq ($(TRACE_BACKEND),dtrace)
 GENERATED_HEADERS += trace-dtrace.h
 endif
@@ -72,7 +72,7 @@ defconfig:
 
 -include config-all-devices.mak
 
-TOOLS += test-libqmp test-qcfg qsh
+TOOLS += test-libqmp test-qcfg qsh test-visiter
 
 build-all: $(DOCS) $(TOOLS) recurse-all
 
@@ -109,6 +109,9 @@ QEMU_CFLAGS+=$(CURL_CFLAGS)
 
 QEMU_CFLAGS+=$(GLIB_CFLAGS)
 
+QEMU_CFLAGS+="-I."
+QEMU_CFLAGS+="-Iqapi"
+
 ui/cocoa.o: ui/cocoa.m
 
 ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS)
@@ -252,7 +255,13 @@ test-qcfg: test-qcfg.o $(QCFG_OBJS) qemu-timer-common.o
 
 qapi-obj-y := qapi/qmp-output-visiter.o qapi/qmp-input-visiter.o
 
-test-visiter: test-visiter.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o 
$(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o 
json-streamer.o json-lexer.o json-parser.o qerror.o
+test-qapi-types.h: $(SRC_PATH)/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-types.py
+   $(call quiet-command,python $(SRC_PATH)/scripts/qapi-types.py 
--prefix="test-" < $<, "  GEN   $@")
+test-qapi-visit.c: test-qapi-visit.h
+test-qapi-visit.h: $(SRC_PATH)/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-visit.py
+   $(call quiet-command,python $(SRC_PATH)/scripts/qapi-visit.py 
--prefix="test-" < $<, "  GEN   $@")
+test-visiter.o: test-qapi-types.h test-qapi-visit.c
+test-visiter: test-visiter.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o 
$(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o 
json-streamer.o json-lexer.o json-parser.o qerror.o test-qapi-visit.o
 
 qmp-check: build-all
$(call quiet-command, ./test-libqmp, "  CHECK   $@")
-- 
1.7.0.4




[Qemu-devel] [PATCH 1/9] qapi: Fix type generator for structured type members

2011-04-29 Thread Michael Roth
This generalizes the generate_struct() function to support generating
structs for top-level named structs as well anonymous nested structs.
generate_struct() now calls itself recursively to support arbitrary
nesting by checking if a fields "type" is actually an OrderedDict.

Signed-off-by: Michael Roth 
---
 scripts/qapi-types.py |   39 +++
 scripts/qapi.py   |7 +--
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 42ad6a5..d645bad 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -4,7 +4,6 @@ import sys
 
 def generate_fwd_struct(name, members):
 return mcgen('''
-
 typedef struct %(name)s %(name)s;
 
 typedef struct %(name)sList
@@ -15,34 +14,40 @@ typedef struct %(name)sList
 ''',
  name=name)
 
-def generate_struct(name, members):
+def generate_struct(structname, fieldname, members):
 ret = mcgen('''
-
 struct %(name)s
 {
 ''',
-  name=name)
+  name=structname)
 
-for argname, argtype, optional in parse_args(members):
+for argname, argentry, optional, structured in parse_args(members):
 if optional:
 ret += mcgen('''
 bool has_%(c_name)s;
 ''',
  c_name=c_var(argname))
-ret += mcgen('''
+if structured:
+push_indent()
+ret += generate_struct("", argname, argentry)
+pop_indent()
+else:
+ret += mcgen('''
 %(c_type)s %(c_name)s;
 ''',
- c_type=c_type(argtype), c_name=c_var(argname))
+ c_type=c_type(argentry), c_name=c_var(argname))
 
+if len(fieldname):
+fieldname = " " + fieldname
 ret += mcgen('''
-};
-''')
+}%(field)s;
+''',
+field=fieldname)
 
 return ret
 
 def generate_handle(name, typeinfo):
 return mcgen('''
-
 typedef struct %(name)s
 {
 %(c_type)s handle;
@@ -58,7 +63,6 @@ typedef struct %(name)sList
 
 def generate_enum(name, values):
 ret = mcgen('''
-
 typedef enum %(name)s
 {
 ''',
@@ -83,7 +87,6 @@ typedef enum %(name)s
 
 def generate_union(name, typeinfo):
 ret = mcgen('''
-
 struct %(name)s
 {
 %(name)sKind kind;
@@ -117,7 +120,7 @@ fdecl.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
 ''')
 
 for expr in exprs:
-ret = ''
+ret = "\n"
 if expr.has_key('type'):
 ret += generate_fwd_struct(expr['type'], expr['data'])
 elif expr.has_key('enum'):
@@ -125,18 +128,22 @@ for expr in exprs:
 ret += generate_enum(expr['enum'], expr['data'])
 elif expr.has_key('union'):
 add_enum('%sKind' % expr['union'])
-ret += generate_fwd_struct(expr['union'], expr['data'])
+ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
 ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
+else:
+continue
 fdecl.write(ret)
 
 for expr in exprs:
-ret = ''
+ret = "\n"
 if expr.has_key('type'):
-ret += generate_struct(expr['type'], expr['data'])
+ret += generate_struct(expr['type'], "", expr['data'])
 elif expr.has_key('handle'):
 ret += generate_handle(expr['handle'], expr['data'])
 elif expr.has_key('union'):
 ret += generate_union(expr['union'], expr['data'])
+else:
+continue
 fdecl.write(ret)
 
 fdecl.write('''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 74533d0..1bc4604 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -74,12 +74,15 @@ def parse_schema(fp):
 def parse_args(typeinfo):
 for member in typeinfo:
 argname = member
-argtype = typeinfo[member]
+argentry = typeinfo[member]
 optional = False
+structured = False
 if member.startswith('*'):
 argname = member[1:]
 optional = True
-yield (argname, argtype, optional)
+if isinstance(argentry, OrderedDict):
+structured = True
+yield (argname, argentry, optional, structured)
 
 def de_camel_case(name):
 new_name = ''
-- 
1.7.0.4




[Qemu-devel] [PATCH 3/9] qapi: Fix visiter generator for nested structs/qobjects

2011-04-29 Thread Michael Roth
Recursively handle structured types as identified by the schema parser.
Generated code Uses visiter stack's push/pop logic to traverse the
structure's tree.

Signed-off-by: Michael Roth 
---
 scripts/qapi-visit.py |   55 +---
 1 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 6bcd84d..bf005c6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,35 +2,56 @@ from ordereddict import OrderedDict
 from qapi import *
 import sys
 
-def generate_visit_struct(name, members):
-ret = mcgen('''
-
-void visit_type_%(name)s(Visiter *m, %(name)s ** obj, const char *name, Error 
**errp)
-{
-visit_start_struct(m, (void **)obj, "%(name)s", name, errp);
-''',
-name=name)
-
-for argname, argtype, optional in parse_args(members):
+def generate_visit_struct_body(field_prefix, members):
+ret = ""
+if len(field_prefix):
+field_prefix = field_prefix + "."
+for argname, argentry, optional, structured in parse_args(members):
 if optional:
 ret += mcgen('''
-visit_start_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", errp);
-if ((*obj)->has_%(c_name)s) {
+visit_start_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", errp);
+if ((*obj)->%(prefix)shas_%(c_name)s) {
 ''',
+ c_prefix=c_var(field_prefix), prefix=field_prefix,
  c_name=c_var(argname), name=argname)
 push_indent()
 
-ret += mcgen('''
-visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", errp);
+if structured:
+ret += mcgen('''
+visit_start_struct(m, NULL, "", "%(name)s", errp);
 ''',
- type=type_name(argtype), c_name=c_var(argname), 
name=argname)
+ name=argname)
+ret += generate_visit_struct_body(field_prefix + argname, argentry)
+ret += mcgen('''
+visit_end_struct(m, errp);
+''')
+else:
+ret += mcgen('''
+visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", errp);
+''',
+ c_prefix=c_var(field_prefix), prefix=field_prefix,
+ type=type_name(argentry), c_name=c_var(argname),
+ name=argname)
 
 if optional:
 pop_indent()
 ret += mcgen('''
-}
-visit_end_optional(m, errp);
+}
+visit_end_optional(m, errp);
 ''')
+return ret
+
+def generate_visit_struct(name, members):
+ret = mcgen('''
+
+void visit_type_%(name)s(Visiter *m, %(name)s ** obj, const char *name, Error 
**errp)
+{
+visit_start_struct(m, (void **)obj, "%(name)s", name, errp);
+''',
+name=name)
+push_indent()
+ret += generate_visit_struct_body("", members)
+pop_indent()
 
 ret += mcgen('''
 visit_end_struct(m, errp);
-- 
1.7.0.4




[Qemu-devel] [PATCH 2/9] qapi: input visiter, don't always allocate memory for structs

2011-04-29 Thread Michael Roth
Sometimes we have anonymous nested structs rather than references. In
these cases we call visit_start_struct() primarilly to push the nested
qobject onto the stack, and specify a NULL obj value to avoid doing any
memory allocation. So add a simple check for this NULL value.

Signed-off-by: Michael Roth 
---
 qapi/qmp-input-visiter.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/qapi/qmp-input-visiter.c b/qapi/qmp-input-visiter.c
index 53379fd..4bae85e 100644
--- a/qapi/qmp-input-visiter.c
+++ b/qapi/qmp-input-visiter.c
@@ -75,7 +75,9 @@ static void qmp_input_start_struct(Visiter *v, void **obj, 
const char *kind, con
 
 qmp_input_push(qiv, qobj);
 
-*obj = qemu_mallocz(QAPI_OBJECT_SIZE);
+if (obj) {
+*obj = qemu_mallocz(QAPI_OBJECT_SIZE);
+}
 }
 
 static void qmp_input_end_struct(Visiter *v, Error **errp)
-- 
1.7.0.4




[Qemu-devel] [PULL] QAPI code generator fix-ups

2011-04-29 Thread Michael Roth
The following changes since commit 4e277a7c641698faf8f01599043b622f80237b42:
  Anthony Liguori (1):
Add input marshaller

are available in the git repository at:

  git://repo.or.cz/qemu/mdroth.git qapi-for-anthony

Michael Roth (9):
  qapi: Fix type generator for structured type members
  qapi: input visiter, don't always allocate memory for structs
  qapi: Fix visiter generator for nested structs/qobjects
  qapi: some basename/guardname py utility functions
  qapi: add --prefix option to type generator
  qapi: add --prefix option for visiter generator
  qapi: test schema for test-visiter unit tests
  qapi: Makefile, build test-visiter with generated test code
  qapi: test-visiter, pull in gen code, tests for nested structures

 Makefile |   15 ++-
 qapi-schema-test.json|   11 +
 qapi/qmp-input-visiter.c |4 +-
 scripts/qapi-types.py|   69 ++--
 scripts/qapi-visit.py|   96 +
 scripts/qapi.py  |   12 +-
 test-visiter.c   |   98 ++
 7 files changed, 251 insertions(+), 54 deletions(-)
 create mode 100644 qapi-schema-test.json




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-04-29 Thread Michael S. Tsirkin
On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> When we're trying to get a newly registered phys memory client updated
> with the current page mappings, we end up passing the region offset
> (a ram_addr_t) as the start address rather than the actual guest
> physical memory address (target_phys_addr_t).  If your guest has less
> than 3.5G of memory, these are coincidentally the same thing.  If
> there's more, the region offset for the memory above 4G starts over
> at 0, so the set_memory client will overwrite it's lower memory entries.
> 
> Instead, keep track of the guest phsyical address as we're walking the
> tables and pass that to the set_memory client.
> 
> Signed-off-by: Alex Williamson 

Acked-by: Michael S. Tsirkin 

Given all this, can yo tell how much time does
it take to hotplug a device with, say, a 40G RAM guest?
> ---
> 
>  exec.c |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 4752af1..e670929 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1742,7 +1742,7 @@ static int cpu_notify_migration_log(int enable)
>  }
>  
>  static void phys_page_for_each_1(CPUPhysMemoryClient *client,
> - int level, void **lp)
> + int level, void **lp, target_phys_addr_t 
> addr)
>  {
>  int i;
>  
> @@ -1751,16 +1751,18 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
> *client,
>  }
>  if (level == 0) {
>  PhysPageDesc *pd = *lp;
> +addr <<= L2_BITS + TARGET_PAGE_BITS;
>  for (i = 0; i < L2_SIZE; ++i) {
>  if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
> -client->set_memory(client, pd[i].region_offset,
> +client->set_memory(client, addr | i << TARGET_PAGE_BITS,
> TARGET_PAGE_SIZE, pd[i].phys_offset);
>  }
>  }
>  } else {
>  void **pp = *lp;
>  for (i = 0; i < L2_SIZE; ++i) {
> -phys_page_for_each_1(client, level - 1, pp + i);
> +phys_page_for_each_1(client, level - 1, pp + i,
> + (addr << L2_BITS) | i);
>  }
>  }
>  }
> @@ -1770,7 +1772,7 @@ static void phys_page_for_each(CPUPhysMemoryClient 
> *client)
>  int i;
>  for (i = 0; i < P_L1_SIZE; ++i) {
>  phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
> - l1_phys_map + i);
> + l1_phys_map + i, i);
>  }
>  }
>  



Re: [Qemu-devel] Supporting emulation of IOMMUs

2011-04-29 Thread Richard Henderson
On 04/28/2011 02:57 PM, Richard Henderson wrote:
> I've had a read through the patches posted in January.  It all does
> seem relatively sane.  At least, I can readily see how I would apply
> these interfaces to my Alpha port without trouble.

I take that back, I see one rather annoying problem: the assumption
that the translate function operates on some sort of standalone device.

This assumption is present in two places:

> void pci_register_iommu(PCIDevice *dev, DMATranslateFunc *translate);

Here, you're assuming that the IOMMU is a device on the PCI bus itself.

While this may indeed be how the AMD-IOMMU presents itself for the 
convenience of the pc-minded operating system, that's certainly not how
the hardware is implemented.  In practice it's a function of the PCI
host controller.  And indeed, that's how it is presented to the system
for the Sparc and Alpha controllers with which I am familiar.  I assume
it's similar for PowerPC, but I've never looked.

> struct DMAMmu {
> DeviceState *iommu;
> DMATranslateFunc *translate;
> QLIST_HEAD(memory_maps, DMAMemoryMap) memory_maps;
> };

Here, you're assuming that the "iommu state" is a standalone qdev.

This is probably true most of the time, given that 

  FROM_SYSBUS(PCIHostState, sysbus_from_qdev(dev))

is true.  However, the Alpha Typhoon chipset has two pci host
controllers that are tied together in ways that would be 
difficult, or at least irritating, to represent as two separate
qdev entities.

I suggest that, like many other places we have callbacks, this
should be an opaque value private to the translate function.

In your AMD-IOMMU case you can then do

  pci_register_iommu(dev->bus, amd_iommu_translate, st);

and avoid 

> PCIDevice *pci_dev = container_of(dev, PCIDevice, dma);
> PCIDevice *iommu_dev = DO_UPCAST(PCIDevice, qdev, dev->mmu->iommu);
> AMDIOMMUState *st = DO_UPCAST(AMDIOMMUState, dev, iommu_dev);

at the beginning of your translate function.  You currently have
three levels of casting and pointer chasing; surely you can agree
that having a single cast from void* is much easier to follow.

In my Alpha case I can then do

  pci_register_iommu(pci_bus0, typhoon_iommu_translate, &state->pchip0);
  pci_register_iommu(pci_bus1, typhoon_iommu_translate, &state->pchip1);

and be off to the races.


r~



Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

2011-04-29 Thread Anthony Liguori

On 04/29/2011 08:38 AM, Jes Sorensen wrote:

On 04/28/11 17:10, Anthony Liguori wrote:

No, the command does too many things and as such, makes it impossible
for a management tool to gracefully recover.


It is exactly the same for the management tool:
- Creation of the new image either succeeds or fails
- Switchover either succeeds or fails



Creating an image can be treated as an atomic operation.  It either 
fully succeeds or you assume it failed and throw the image away since 
you can always try again.


When you combine creating an image with another operation, you create a 
a single operation that cannot be treated as atomic which makes recovery 
from failure much more difficult.



But the new image is always valid once it's been created as pending
writes are lost no matter what in a hard power failure.  That suggests
the following flow:

1) Create new image with a backing file
2) Completion ACK

At this stage, the management tool should update it's internal state to
point to the new image.


This can still be done with the existing code - it's not the ideal
setup, but it is possible due to evil things such as selinux labeling



I don't follow.



3) Begin switch over to new image
4) Switch over image.
5) Receive notification when it's complete


Sorry but this isn't an accurate description of the process. Once you
have a new image ready, you need to halt writes, flush buffers, and then
you can do the switch, which has to be atomic.



You need to queue writes, you can still let the guest run while the 
remaining writes are sent to disk.


But if this is a background task, then as a management tool, don't I 
want a notification when it's been completed?


Don't I want to have a little spinning box in my GUI or something like 
that while this is happening?



If (4) is happening asynchronously, things get kind of complicated.  You
can either wait for things to quiesce on their own or you can queue
pending requests.  It's unclear to me what the right strategy is or if
there's a mixed strategy that's needed.  I think it makes sense to treat
3/4 as a command with (5) being an event notification.


The actual guest application freeze (what some strange people use the
quiicannotspell word for) is done prior to the switchover, you cannot do
it in parallel with it.



I'm not even talking about application quiescing here.  And yeah, I have 
to frequently google that word because Thunderbird doesn't have it in 
it's dictionary :-)



But combining 1-5 in a single interface creates a command that while
convenient on the command line, is not usable for a robust management tool.


As I explained you can already use an externally created image with the
current interface, the only issue that may be worth doing async is the
buffer flushing. However once you do that, guest writes are going to
stall anyway, and eventually the guest applications will all stall.


The interface shouldn't have the option to create an image... because if 
you have that, the interface is difficult to use.


Why present an option to do something that we know is broken?  We can't 
blame management tools for not doing a good job managing KVM if we're 
giving them poor interfaces to work with.


Regards,

Anthony Liguori


The single command is both better from a consistency perspective, and it
will do the right job. Things are not going to get any easier from the
management tool's perspective than they are with the current interface.

Cheers,
Jes







Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

2011-04-29 Thread Jes Sorensen
On 04/28/11 17:10, Anthony Liguori wrote:
> On 04/28/2011 09:57 AM, Jes Sorensen wrote:
>> On 04/28/11 16:46, Anthony Liguori wrote:
>> Sorry this is inherently broken. The management tool should not be
>> keeping state in this process. I agree an async interface would be nice,
>> but the above process is plain wrong.
>>
>> The async snapshot process needs to be doing the exact same as in the
>> current implementation, the main difference is that it would be running
>> asynchronously and that QMP would be able to query the state of it.
> 
> No, the command does too many things and as such, makes it impossible
> for a management tool to gracefully recover.

It is exactly the same for the management tool:
- Creation of the new image either succeeds or fails
- Switchover either succeeds or fails

If you have a crash in the process, you still can only know by checking
the consistency of the new image after rebooting.

>> There is no issue here, you have the exact same problem if you get a
>> crash during d) in your example. It is the same with the existing
>> command, the crash is only an issue if it happens right in the middle of
>> the switch over. Until then, only the original image remains valid.
> 
> But the new image is always valid once it's been created as pending
> writes are lost no matter what in a hard power failure.  That suggests
> the following flow:
> 
> 1) Create new image with a backing file
> 2) Completion ACK
> 
> At this stage, the management tool should update it's internal state to
> point to the new image.

This can still be done with the existing code - it's not the ideal
setup, but it is possible due to evil things such as selinux labeling.

> 3) Begin switch over to new image
> 4) Switch over image.
> 5) Receive notification when it's complete

Sorry but this isn't an accurate description of the process. Once you
have a new image ready, you need to halt writes, flush buffers, and then
you can do the switch, which has to be atomic. The only thing that can
really be async in all of this is the buffer flushing. There isn't any
long switch-over process.

> If (4) is happening asynchronously, things get kind of complicated.  You
> can either wait for things to quiesce on their own or you can queue
> pending requests.  It's unclear to me what the right strategy is or if
> there's a mixed strategy that's needed.  I think it makes sense to treat
> 3/4 as a command with (5) being an event notification.

The actual guest application freeze (what some strange people use the
quiicannotspell word for) is done prior to the switchover, you cannot do
it in parallel with it.

> But combining 1-5 in a single interface creates a command that while
> convenient on the command line, is not usable for a robust management tool.

As I explained you can already use an externally created image with the
current interface, the only issue that may be worth doing async is the
buffer flushing. However once you do that, guest writes are going to
stall anyway, and eventually the guest applications will all stall.

The single command is both better from a consistency perspective, and it
will do the right job. Things are not going to get any easier from the
management tool's perspective than they are with the current interface.

Cheers,
Jes




Re: [Qemu-devel] [PATCH 29/33] target-alpha: Add custom PALcode image for CLIPPER emulation.

2011-04-29 Thread Richard Henderson
On 04/29/2011 02:13 AM, Peter Maydell wrote:
> 2011/4/28 Richard Henderson :
>> +[submodule "roms/qemu-palcode"]
>> +   path = roms/qemu-palcode
>> +   url = git://repo.or.cz/qemu-palcode.git
> 
> What's the license on this binary blob? There isn't
> a COPYING file in the qemu-palcode git repo and most of
> the source files seem to be missing copyright/licensing
> headers.

GPL.  I should take care of that legalese soon, I suppose.


r~



Re: [Qemu-devel] [PATCHv3] virtio-serial-bus: use bh for unthrottling

2011-04-29 Thread Amit Shah
On (Fri) 29 Apr 2011 [14:25:06], Alon Levy wrote:
> Instead of calling flush_queued_data when unthrottling, schedule
> a bh. That way we can return immediately to the caller, and the
> flush uses the same call path as a have_data for callbackee.
> 
> No migration change is required because bh are called from vm_stop.

No sign-off?

The initial tests look good, no fireworks yet.

Amit



Re: [Qemu-devel] [PATCH 6/8] blockdev: Allow image files to auto-enable streaming

2011-04-29 Thread Kevin Wolf
Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
> Image files that having streaming enabled will automatically begin
> streaming when opened.
> 
> Signed-off-by: Stefan Hajnoczi 

Hm... I wasn't really happy about images that do copy on read even if I
didn't tell qemu so on the command line. Now they can start to copy data
and use up my internet connection even without the guest really
accessing the data. This seems to be one step more that I find rather
questionable.

Anyway, same as for copy on read: While we can discuss _allowing_ it in
backing files, it's definitely not suitable as a primary interface.

Kevin



[Qemu-devel] [Bug 458521] Re: kvm crash when using virtio for network, hardy guest

2011-04-29 Thread Jamie Strandboge
** Changed in: linux (Ubuntu Hardy)
   Status: Incomplete => In Progress

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

Title:
  kvm crash when using virtio for network, hardy guest

Status in QEMU:
  Fix Released
Status in Release Notes for Ubuntu:
  Fix Released
Status in “linux” package in Ubuntu:
  Fix Released
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “linux” source package in Hardy:
  In Progress
Status in “qemu-kvm” source package in Hardy:
  Invalid
Status in “linux” source package in Karmic:
  Fix Released
Status in “qemu-kvm” source package in Karmic:
  Fix Released

Bug description:
  Binary package hint: kvm

  I was running a kvm host with 5 virtual machines using jaunty. 1
  running centos 5.4, 3 running karmic and 1 running hardy.

  I upgraded the host to karmic to test it and one of my virtual
  machines(the vm with hardy) crashs after some seconds of boot! I stack
  traced the kvm process and found a "virtio-net truncating packet"
  before process crash.

  No error in any log file. The VM just crash.

  I changed the model of network interface(the disk is still virtio) in
  VM.xml to rtl8139, redefined and started. The problem goes away.

  I don't know if it's a kvm bug or libvirt(regarding the bridge
  network) bug but definitely is a bug. All other VMs are running OK
  with virtio for both network and disk.

  ===
  Karmic Release Notes:

  KVM Guest Crashes when Guest is Hardy and using Virtio Networking

  Ubuntu 8.04 LTS (Hardy) KVM guests using virtio networking may crash,
  when running on top of Ubuntu 9.10 (Karmic) hosts.

  As a workaround, such guests should use either e1000 or rtl839 as the
  networking model.  A fix for the bug is currently in progress and
  should be addressed in an update to the qemu-kvm package in Karmic.

  ===

  ===
  SRU Justification

  This bug is a regression from the kvm-84 package in 9.04.  Karmic users 
hosting 8.04 KVM guests and using virtio networking *will* crash their VM when 
the network connection is saturated.  As the virtual machine crashes without 
sync buffers, there could very well be data lost in the guest filesystem or 
memory.
  I have posted the patch for review, and acked upstream:
   * http://lists.gnu.org/archive/html/qemu-devel/2009-10/msg02495.html
  I am bundling this fix with two other minor fixes, for Bug #452323  and Bug 
#453441.

  TEST CASE:
   1) Download and uncompress
* http://rookery.canonical.com/~kirkland/hardy.img.bz2
   2) Download a bridged networking script
* http://rookery.canonical.com/~kirkland/bridge.sh
   3) Start the vm with virtio and bridged networking
* sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net tap,script=bridge.sh 
-drive file=hardy.img,if=virtio,index=0,boot=on
   4) The VM's username and password are both "ubuntu".
   5) In the guest, have nc listen on a port
* nc -lp 1234 > /dev/null
   6) On your host, flood the guest with network traffic on that port
* cat /dev/urandom | nc -w 3 guest_ip 1234
   7) Without this fix, the guest will crash immediately, saying:
*   virtio-net truncating packet
   8) With the fix, you will be able to send the guest with data over the 
virtio network ad nauseum.  I get about 6MB/s throughput.

  ===



Re: [Qemu-devel] [PATCH 3/8] qed: add support for Copy-on-Read

2011-04-29 Thread Kevin Wolf
Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
> From: Anthony Liguori 
> 
> When creating an image using qemu-img, just pass '-o copy_on_read' and then
> whenever QED reads from a backing file, it will write the block to the QED
> file after the read completes ensuring that you only fetch from the backing
> device once.

As discussed previously, this is not the right primary interface.

Also, the patch still seems to be broken with respect to concurrent writes.

Kevin



Re: [Qemu-devel] [PATCHv2 3/4] qxl: add debug_cs and cmdlog_cs

2011-04-29 Thread Gerd Hoffmann

On 04/28/11 10:29, Alon Levy wrote:

With this you can output the command log and/or the guest debug (driver)
output to a chardev instead of stderr:

-global qxl-vga.cmdlog_chardev=qxl_cmdlog_chardev
-global qxl-vga.debug_chardev=qxl_debug_chardev

useful for debugging. if no chardev is specified prints to stderr like the old
code.


Hmm.  That is a bit too much ad-hoc debug hacking IMHO.  Also I'm not 
sure I like the idea to send debug stuff through chardev instead of just 
writing them to stderr or some logfile.


I do see why you are doing that though.  Unfortunaly qemu has no sane 
debug logging infrastructure.  This is long overdue IMHO, so we don't 
need hacks like this one and also don't reinvent stuff over and over.


cheers,
  Gerd




Re: [Qemu-devel] [PATCH 2/8] qmp: Add QMP support for stream commands

2011-04-29 Thread Kevin Wolf
Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
> From: Anthony Liguori 
> 
> For leaf images with copy on read semantics, the stream commands allow the 
> user
> to populate local blocks by manually streaming them from the backing image.
> Once all blocks have been streamed, the dependency on the original backing
> image can be removed.  Therefore, stream commands can be used to implement
> post-copy live block migration and rapid deployment.
> 
> The stream command can be used to stream a single sector, to start streaming
> the entire device, and to cancel an active stream.  It is easiest to allow the
> stream command to manage streaming for the entire device but a managent tool
> could use single sector mode to throttle the I/O rate.  When a single sector 
> is
> streamed, the command returns an offset that can be used for a subsequent 
> call.

You mean literally single sectors? You're not interested in completing
the job in finite time, are you? ;-)

I would suggest adding a length argument for the all=false case, so that
management tools can choose more reasonable sizes.

Kevin



Re: [Qemu-devel] [PATCH] monitor: add PPC BookE SPRs

2011-04-29 Thread Jan Kiszka
On 2011-04-28 23:01, Scott Wood wrote:
> Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
> and display them in "info registers".
> 
> Also get CR and PID from the existing KVM_GET_REGS.
> 
> Signed-off-by: Scott Wood 
> ---
>  hw/ppc.c   |   12 +
>  monitor.c  |   71 +++-
>  target-ppc/cpu.h   |1 +
>  target-ppc/kvm.c   |  123 ++-
>  target-ppc/translate.c |   82 +++-
>  5 files changed, 282 insertions(+), 7 deletions(-)
> 

...

> @@ -191,11 +197,122 @@ int kvm_arch_get_registers(CPUState *env)
>  env->spr[SPR_SPRG6] = regs.sprg6;
>  env->spr[SPR_SPRG7] = regs.sprg7;
>  
> +env->spr[SPR_BOOKE_PID] = regs.pid;
> +
>  for (i = 0;i < 32; i++)
>  env->gpr[i] = regs.gpr[i];
>  
> +#ifdef KVM_CAP_PPC_BOOKE_SREGS
> +if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_BOOKE_SREGS)) {

You probably want to cache the result of this syscall during init and
check that here. There are plenty examples for this pattern around.

> +ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> +if (ret < 0)
> +return ret;

Please use chechpatch.pl before submitting.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCHv2 2/4] qxl: add mode to debugprint on destroy primary

2011-04-29 Thread Gerd Hoffmann

On 04/28/11 10:29, Alon Levy wrote:

---
  hw/qxl.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 63e295b..ccd820c 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1009,7 +1009,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
  break;
  case QXL_IO_DESTROY_PRIMARY:
  PANIC_ON(val != 0);
-dprint(d, 1, "QXL_IO_DESTROY_PRIMARY\n");
+dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", 
qxl_mode_to_string(d->mode));
  qxl_destroy_primary(d);
  break;
  case QXL_IO_DESTROY_SURFACE_WAIT:


Squash into the first?  Maybe there are a few more places where a 
pretty-printed mode would be useful?


cheers,
  Gerd



Re: [Qemu-devel] [PATCHv2 1/4] qxl: interface_get_command: fix reported mode

2011-04-29 Thread Gerd Hoffmann

+static const char *qxl_mode_to_string(int mode)
+{
+switch (mode) {
+case QXL_MODE_COMPAT:
+return "compat";
+case QXL_MODE_NATIVE:
+return "native";
+case QXL_MODE_UNDEFINED:
+return "undefined";
+case QXL_MODE_VGA:
+return "vga";
+}
+return "unknown";


Make that "INVALID"?  There never ever should be another move value ...

cheers,
  Gerd



Re: [Qemu-devel] [PATCH 2/4] spice: don't create updates in spice server context.

2011-04-29 Thread Gerd Hoffmann

  Hi,


-static int debug = 0;
+static int debug = 1;

Accident?


Oops.  That one wasn't intentional, will fix for the pull request.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream

2011-04-29 Thread Kevin Wolf
Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
> From: Anthony Liguori 
> 
> Signed-off-by: Anthony Liguori 
> ---
>  block.c |   32 
>  block.h |2 ++
>  block_int.h |3 +++
>  3 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f731c7a..5e3476c 100644
> --- a/block.c
> +++ b/block.c
> @@ -2248,6 +2248,38 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState 
> *bs, int64_t sector_num,
>  return ret;
>  }
>  
> +/**
> + * Attempt to stream an image starting from sector_num.
> + *
> + * @sector_num - the first sector to start streaming from
> + * @cb - block completion callback
> + * @opaque - data to pass completion callback
> + *
> + * Returns NULL if the image format not support streaming, the image is
> + * read-only, or no image is open.
> + *
> + * The intention of this function is for a user to execute it once with a
> + * sector_num of 0 and then upon receiving a completion callback, to remember
> + * the number of sectors "streamed", and then to call this function again 
> with
> + * an offset adjusted by the number of sectors previously streamed.
> + *
> + * This allows a user to progressive stream in an image at a pace that makes
> + * sense.  In general, this function tries to do the smallest amount of I/O
> + * possible to do some useful work.
> + *
> + * This function only really makes sense in combination with a block format
> + * that supports copy on read and has it enabled.  If copy on read is not
> + * enabled, a block format driver may return NULL.
> + */
> +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t sector_num,
> +  BlockDriverCompletionFunc *cb, void 
> *opaque)

I think bdrv_aio_stream is a bad name for this. It only becomes image
streaming because the caller repeatedly calls this function. What the
function really does is copying some data from the backing file into the
overlay image.

I'm not sure how the caller would know how many sectors have been
copied. A BlockDriverCompletionFunc usually returns 0 on success, did
you change it here to use positive numbers for something else? At least
this must be documented somewhere, but I would suggest to add a
nb_sectors argument instead so that the caller decides how many sectors
to copy.

If you say that it only makes sense with copy on read, should one think
of it as a read that throws the read data away? I think considering it a
copy function makes more sense and is independent of copy on read.

Kevin



[Qemu-devel] [PATCH] doc: Add explanation that -alt-grab and -ctrl-grab affect special keys

2011-04-29 Thread Brad Hards
Phillip Merensky reported that the special keys (e.g. Ctrl-Alt-f for full
screen) did not work correctly if -alt-grab is used.

BUG: 696530

Review of ui/sdl.c:sdl_refresh indicates that this is the intended behaviour,
so we should update the documentation to match the actual behaviour, as
suggested by Phillip in the bug report.

Signed-off-by: Brad Hards 
---
 qemu-doc.texi   |6 +-
 qemu-options.hx |6 --
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 86e017c..d96a6ab 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -278,7 +278,11 @@ targets do not need a disk image.
 
 @c man begin OPTIONS
 
-During the graphical emulation, you can use the following keys:
+During the graphical emulation, you can use special key combinations to change
+modes. The default key mappings are shown below, but if you use 
@code{-alt-grab}
+then the modifier is Ctrl-Alt-Shift (instead of Ctrl-Alt) and if you use 
+@code{-ctrl-grab} then the modifier is the right Ctrl key (instead of 
Ctrl-Alt):
+
 @table @key
 @item Ctrl-Alt-f
 @kindex Ctrl-Alt-f
diff --git a/qemu-options.hx b/qemu-options.hx
index 489df10..8d83aa1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -662,7 +662,8 @@ DEF("alt-grab", 0, QEMU_OPTION_alt_grab,
 STEXI
 @item -alt-grab
 @findex -alt-grab
-Use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt).
+Use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt). Note that this also
+affects the special keys (for fullscreen, monitor-mode switching, etc).
 ETEXI
 
 DEF("ctrl-grab", 0, QEMU_OPTION_ctrl_grab,
@@ -671,7 +672,8 @@ DEF("ctrl-grab", 0, QEMU_OPTION_ctrl_grab,
 STEXI
 @item -ctrl-grab
 @findex -ctrl-grab
-Use Right-Ctrl to grab mouse (instead of Ctrl-Alt).
+Use Right-Ctrl to grab mouse (instead of Ctrl-Alt). Note that this also
+affects the special keys (for fullscreen, monitor-mode switching, etc).
 ETEXI
 
 DEF("no-quit", 0, QEMU_OPTION_no_quit,
-- 
1.7.1



Re: [Qemu-devel] [PATCHv3] virtio-serial-bus: use bh for unthrottling

2011-04-29 Thread Amit Shah
On (Fri) 29 Apr 2011 [14:25:06], Alon Levy wrote:
> Instead of calling flush_queued_data when unthrottling, schedule
> a bh. That way we can return immediately to the caller, and the
> flush uses the same call path as a have_data for callbackee.
> 
> No migration change is required because bh are called from vm_stop.

This one looks OK; I'll put it through some testing.

However, I'd like confirmation that accessing (and modifying) vq
elements is fine in bh context.  Stefan / Michael?

> ---
>  hw/virtio-serial-bus.c |   12 ++--
>  hw/virtio-serial.h |5 +
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index f10d48f..ca0581b 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -285,6 +285,13 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
>  return 0;
>  }
>  
> +static void flush_queued_data_bh(void *opaque)
> +{
> +VirtIOSerialPort *port = opaque;
> +
> +flush_queued_data(port);
> +}
> +
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
>  {
>  if (!port) {
> @@ -295,8 +302,7 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, 
> bool throttle)
>  if (throttle) {
>  return;
>  }
> -
> -flush_queued_data(port);
> +qemu_bh_schedule(port->bh);
>  }
>  
>  /* Guest wants to notify us of some event */
> @@ -726,6 +732,7 @@ static int virtser_port_qdev_init(DeviceState *qdev, 
> DeviceInfo *base)
>  bool plugging_port0;
>  
>  port->vser = bus->vser;
> +port->bh = qemu_bh_new(flush_queued_data_bh, port);
>  
>  /*
>   * Is the first console port we're seeing? If so, put it up at
> @@ -792,6 +799,7 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
>  VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
>  VirtIOSerial *vser = port->vser;
>  
> +qemu_bh_delete(port->bh);
>  remove_port(port->vser, port->id);
>  
>  QTAILQ_REMOVE(&vser->ports, port, next);
> diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
> index 5eb948e..0fa03d1 100644
> --- a/hw/virtio-serial.h
> +++ b/hw/virtio-serial.h
> @@ -119,6 +119,11 @@ struct VirtIOSerialPort {
>  uint32_t iov_idx;
>  uint64_t iov_offset;
>  
> +/*
> + * When unthrottling we use a buttomhalf to call flush_queued_data.
> + */
> +QEMUBH *bh;
> +
>  /* Identify if this is a port that binds with hvc in the guest */
>  uint8_t is_console;
>  
> -- 
> 1.7.5
> 

Amit



[Qemu-devel] [PATCHv3] virtio-serial-bus: use bh for unthrottling

2011-04-29 Thread Alon Levy
Instead of calling flush_queued_data when unthrottling, schedule
a bh. That way we can return immediately to the caller, and the
flush uses the same call path as a have_data for callbackee.

No migration change is required because bh are called from vm_stop.
---
 hw/virtio-serial-bus.c |   12 ++--
 hw/virtio-serial.h |5 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index f10d48f..ca0581b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -285,6 +285,13 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
 return 0;
 }
 
+static void flush_queued_data_bh(void *opaque)
+{
+VirtIOSerialPort *port = opaque;
+
+flush_queued_data(port);
+}
+
 void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
 {
 if (!port) {
@@ -295,8 +302,7 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, 
bool throttle)
 if (throttle) {
 return;
 }
-
-flush_queued_data(port);
+qemu_bh_schedule(port->bh);
 }
 
 /* Guest wants to notify us of some event */
@@ -726,6 +732,7 @@ static int virtser_port_qdev_init(DeviceState *qdev, 
DeviceInfo *base)
 bool plugging_port0;
 
 port->vser = bus->vser;
+port->bh = qemu_bh_new(flush_queued_data_bh, port);
 
 /*
  * Is the first console port we're seeing? If so, put it up at
@@ -792,6 +799,7 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
 VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
 VirtIOSerial *vser = port->vser;
 
+qemu_bh_delete(port->bh);
 remove_port(port->vser, port->id);
 
 QTAILQ_REMOVE(&vser->ports, port, next);
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 5eb948e..0fa03d1 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -119,6 +119,11 @@ struct VirtIOSerialPort {
 uint32_t iov_idx;
 uint64_t iov_offset;
 
+/*
+ * When unthrottling we use a buttomhalf to call flush_queued_data.
+ */
+QEMUBH *bh;
+
 /* Identify if this is a port that binds with hvc in the guest */
 uint8_t is_console;
 
-- 
1.7.5




Re: [Qemu-devel] [PATCH 0/4] spice: fix locking

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 11:38:28AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> This patch series moves a bunch of work spice has to do from spice
> server thread context to iothread context, which in turn allows to
> drop the current locking mess as we don't touch qemu internals from
> spice server thread any more.
> 
> A long-standing warning fix from Jes is included too.
> 

Tested with winxp, works well with changes between vga and qxl (tested
by setting the debug to 2 in spice-display.c). Just a single comment about
the debug=1 change in the second patch.

Alon

> cheers,
>   Gerd
> 
> The following changes since commit 430a3c18064fd3c007048d757e8bd0fff45fcc99:
> 
>   configure: reenable opengl by default (2011-04-26 23:26:49 +0200)
> 
> are available in the git repository at:
>   git://anongit.freedesktop.org/spice/qemu spice.v34
> 
> Gerd Hoffmann (3):
>   spice: don't create updates in spice server context.
>   spice: don't call displaystate callbacks from spice server context.
>   spice: drop obsolete iothread locking
> 
> Jes Sorensen (1):
>   Make spice dummy functions inline to fix calls not checking return 
> values
> 
>  hw/qxl-render.c|   25 ++--
>  hw/qxl.c   |   27 ++---
>  ui/qemu-spice.h|   12 -
>  ui/spice-display.c |   63 
> +---
>  ui/spice-display.h |   24 +++
>  5 files changed, 94 insertions(+), 57 deletions(-)
> 



Re: [Qemu-devel] [PATCH v2] qemu-progress.c: printf isn't signal safe

2011-04-29 Thread Kevin Wolf
Am 28.04.2011 13:58, schrieb jes.soren...@redhat.com:
> From: Jes Sorensen 
> 
> Change the signal handling to indicate a signal is pending, rather
> then printing directly from the signal handler.
> 
> In addition make the signal prints go to stderr, rather than stdout.
> 
> Signed-off-by: Jes Sorensen 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH 2/4] spice: don't create updates in spice server context.

2011-04-29 Thread Alon Levy
On Fri, Apr 29, 2011 at 11:38:30AM +0200, Gerd Hoffmann wrote:
> This patch moves the creation of spice screen updates from the spice
> server context to qemu iothread context (display refresh timer to be
> exact).  This way we avoid accessing qemu internals (display surface)
> from spice thread context which in turn allows us to simplify locking.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  hw/qxl.c   |   17 +++--
>  ui/spice-display.c |   45 -
>  ui/spice-display.h |   21 -
>  3 files changed, 55 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index fe4212b..bd250db 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -343,18 +343,22 @@ static int interface_get_command(QXLInstance *sin, 
> struct QXLCommandExt *ext)
>  SimpleSpiceUpdate *update;
>  QXLCommandRing *ring;
>  QXLCommand *cmd;
> -int notify;
> +int notify, ret;
>  
>  switch (qxl->mode) {
>  case QXL_MODE_VGA:
>  dprint(qxl, 2, "%s: vga\n", __FUNCTION__);
> -update = qemu_spice_create_update(&qxl->ssd);
> -if (update == NULL) {
> -return false;
> +ret = false;
> +qemu_mutex_lock(&qxl->ssd.lock);
> +if (qxl->ssd.update != NULL) {
> +update = qxl->ssd.update;
> +qxl->ssd.update = NULL;
> +*ext = update->ext;
> +ret = true;
>  }
> -*ext = update->ext;
> +qemu_mutex_unlock(&qxl->ssd.lock);
>  qxl_log_command(qxl, "vga", ext);
> -return true;
> +return ret;
>  case QXL_MODE_COMPAT:
>  case QXL_MODE_NATIVE:
>  case QXL_MODE_UNDEFINED:
> @@ -1304,6 +1308,7 @@ static int qxl_init_primary(PCIDevice *dev)
>  vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
> qxl_hw_screen_dump, qxl_hw_text_update, 
> qxl);
>  qxl->ssd.ds = vga->ds;
> +qemu_mutex_init(&qxl->ssd.lock);
>  qxl->ssd.bufsize = (16 * 1024 * 1024);
>  qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
>  
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 020b423..39c0ba1 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -27,7 +27,7 @@
>  
>  #include "spice-display.h"
>  
> -static int debug = 0;
> +static int debug = 1;
Accident?

>  
>  static void GCC_FMT_ATTR(2, 3) dprint(int level, const char *fmt, ...)
>  {
> @@ -62,14 +62,7 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
>  dest->right = MAX(dest->right, r->right);
>  }
>  
> -/*
> - * Called from spice server thread context (via interface_get_command).
> - *
> - * We must aquire the global qemu mutex here to make sure the
> - * DisplayState (+DisplaySurface) we are accessing doesn't change
> - * underneath us.
> - */
> -SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
> +static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>  {
>  SimpleSpiceUpdate *update;
>  QXLDrawable *drawable;
> @@ -78,9 +71,7 @@ SimpleSpiceUpdate 
> *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>  uint8_t *src, *dst;
>  int by, bw, bh;
>  
> -qemu_mutex_lock_iothread();
>  if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> -qemu_mutex_unlock_iothread();
>  return NULL;
>  };
>  
> @@ -141,7 +132,6 @@ SimpleSpiceUpdate 
> *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>  cmd->data = (intptr_t)drawable;
>  
>  memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> -qemu_mutex_unlock_iothread();
>  return update;
>  }
>  
> @@ -241,6 +231,12 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
>  qemu_pf_conv_put(ssd->conv);
>  ssd->conv = NULL;
>  
> +qemu_mutex_lock(&ssd->lock);
> +if (ssd->update != NULL) {
> +qemu_spice_destroy_update(ssd, ssd->update);
> +ssd->update = NULL;
> +}
> +qemu_mutex_unlock(&ssd->lock);
>  qemu_spice_destroy_host_primary(ssd);
>  qemu_spice_create_host_primary(ssd);
>  
> @@ -252,6 +248,14 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
>  {
>  dprint(3, "%s:\n", __FUNCTION__);
>  vga_hw_update();
> +
> +qemu_mutex_lock(&ssd->lock);
> +if (ssd->update == NULL) {
> +ssd->update = qemu_spice_create_update(ssd);
> +ssd->notify++;
> +}
> +qemu_mutex_unlock(&ssd->lock);
> +
>  if (ssd->notify) {
>  ssd->notify = 0;
>  ssd->worker->wakeup(ssd->worker);
> @@ -298,14 +302,20 @@ static int interface_get_command(QXLInstance *sin, 
> struct QXLCommandExt *ext)
>  {
>  SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
>  SimpleSpiceUpdate *update;
> +int ret = false;
>  
>  dprint(3, "%s:\n", __FUNCTION__);
> -update = qemu_spice_create_update(ssd);
> -if (update == NULL) {
> -return false;
> +
> +qemu_mutex_lock(&ssd->lock);
> +if (ssd->update != NULL) {
> +   

Re: [Qemu-devel] [PATCHv2] ide/atapi: fix set but unused

2011-04-29 Thread Kevin Wolf
Am 28.04.2011 15:34, schrieb Alon Levy:
> Signed-off-by: Alon Levy 
> ---
>  hw/ide/atapi.c |4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH 1/2] atapi: Move comment to proper place

2011-04-29 Thread Kevin Wolf
Am 28.04.2011 16:34, schrieb Amit Shah:
> Move misplaced comment for media_is_dvd()
> 
> Signed-off-by: Amit Shah 

Thanks, applied both to the block branch.

Kevin



Re: [Qemu-devel] [PATCH] Make qemu-img convert properly consider backing file contents when used with -o backing_file

2011-04-29 Thread Kevin Wolf
Am 29.04.2011 03:42, schrieb Brad Campbell:
> G'day all,
> 
> This patch makes qemu-img properly consider the contents of the output 
> backing file when performing a convert operation. All things considered 
> it would also perform similar to rebase, where you could specify a 
> completely different backing file and it would just de-dup.
> 
> I've poked this in as an attachment as apparently my last attempt at an 
> in-line patch munged the formatting.
> 
> Comments, pokes or flames welcome.

Please include a commit comment and a Signed-off-by line. Instead of
sending a plain diff, you'll get a better result if you have a git
commit locally and use git format-patch to create the patch file.

Also please make sure to avoid trailing whitespace.

> > @@ -719,6 +720,16 @@ static int img_convert(int argc, char **argv)
>  out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
>  if (out_baseimg_param) {
>  out_baseimg = out_baseimg_param->value.s;
> +
> +/* out_baseimg_parm != NULL even if there is no base img specified! 
> */
> +if (out_baseimg) {
> +out_bf = bdrv_new_open(out_baseimg, NULL, BDRV_O_FLAGS);
> +if (!out_bf) {
> +error_report("Could not open backing file '%s'", 
> out_baseimg);
> +ret = -1;
> +goto out;
> 

bdrv_new_open already prints an error message in case of failure, so we
shouldn't duplicate it here.

> @@ -889,8 +903,15 @@ static int img_convert(int argc, char **argv)
> are present in both the output's and input's base images 
> (no
> need to copy them). */
>  if (out_baseimg) {
> -if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
> -   n, &n1)) {
> +if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) 
> < 0) {
> +error_report("error while reading input file");
> +goto out;
> +}
> +if (bdrv_read(out_bf, sector_num - bs_offset, buf3, n) < 
> 0) {
> +error_report("error while reading backing file");
> +goto out;
> +}

Did you test what happens with a backing file that is smaller than the
overlay? I suspect it will fail here after this change.

Also I think you need to set ret = -1 before jumping to out:

Kevin



Re: [Qemu-devel] [PATCH v3 1/5] hpet 'driftfix': add hooks required to detect coalesced interrupts (x86 apic only)

2011-04-29 Thread Jan Kiszka
On 2011-04-29 11:45, Ulrich Obergfell wrote:
> 
>> On 2011-04-28 20:51, Blue Swirl wrote:
>>> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote:
 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain
 entry addresses of functions that are utilized by update_irq() to
 detect coalesced interrupts. apic code loads these pointers during
 initialization.
>>>
>>> I'm not so happy with this approach, but probably then the i386
>>> dependencies can be removed from RTC and it can be compiled only once
>>> for all targets.
>>
>> This whole series is really the minimalistic approach. The callbacks
>> defined here must remain a temporary "shortcut". Just like proper
>> abstraction of periodic tick compensation for reuse in other timers has
>> to be added later on. And the limitation to edge-triggered legacy HPET
>> INTs has to be removed.
> 
> Since QEMU doesn't have a generic infrastructure to track interrupt
> delivery, I decided to reuse something that is currently available.
> The only mechanism that I'm aware of is the one that is utilized by
> RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc.
> 
> The changes that would be introduced by part 1/5 and part 4/5 of
> this patch series could be replaced if a generic infrastructure
> to track interrupt delivery becomes available.

Right, and I hope you have the resources to continue working on this
topic in the foreseeable future.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [Bug 754591] Re: NIC doesn't work when it had been used before

2011-04-29 Thread Yongjie Ren
I can reproduce it stably. I use igb driver for my NIC. And Alex
Williamson is on the way to fix it.

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

Title:
  NIC doesn't work when it had been used before

Status in QEMU:
  New

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64): All
  Guest OS (ia32/ia32e/IA64): All
  Guest OS Type (Linux/Windows):All
  kvm.git Commit:3b840cc27e5b831a26e3303096b88e112d1cf59f
  qemu-kvm Commit:df85c051d780bca0ee2462cfeb8ef6d9552a19b0
  Host Kernel Version:2.6.38+
  Hardware:Westmere-EP / Sandy Bridge

  
  Bug detailed description:
  --
  NIC doesn't work when it had been used before.
  Statically assign a NIC to guest, it works well. Then shutdown the guest, and
  assign the same NIC to a guest again. The NIC doesn't work. The dmesg of the
  guest is as following.
  --
  Intel(R) Gigabit Ethernet Network Driver - version 2.1.0-k2
  Copyright (c) 2007-2009 Intel Corporation.
  ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11
  igb :00:03.0: PCI INT A -> Link[LNKC] -> GSI 11 (level, high) -> IRQ 11
  igb :00:03.0: setting latency timer to 64
alloc irq_desc for 24 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 24 for MSI/MSI-X
alloc irq_desc for 25 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 25 for MSI/MSI-X
alloc irq_desc for 26 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 26 for MSI/MSI-X
  igb :00:03.0: PHY reset is blocked due to SOL/IDER session.
  igb :00:03.0: The NVM Checksum Is Not Valid
  igb :00:03.0: PCI INT A disabled
  igb: probe of :00:03.0 failed with error -5

  
  Reproduce steps:
  
  1.statically assign a NIC to a guest, check if it works well
  2.shutdown the guest.
  3.statically assign the same NIC to a guest (NIC doesn't work now).



Re: [Qemu-devel] [PATCH v3 1/5] hpet 'driftfix': add hooks required to detect coalesced interrupts (x86 apic only)

2011-04-29 Thread Ulrich Obergfell

> On 2011-04-28 20:51, Blue Swirl wrote:
>> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote:
>>> 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain
>>> entry addresses of functions that are utilized by update_irq() to
>>> detect coalesced interrupts. apic code loads these pointers during
>>> initialization.
>>
>> I'm not so happy with this approach, but probably then the i386
>> dependencies can be removed from RTC and it can be compiled only once
>> for all targets.
> 
> This whole series is really the minimalistic approach. The callbacks
> defined here must remain a temporary "shortcut". Just like proper
> abstraction of periodic tick compensation for reuse in other timers has
> to be added later on. And the limitation to edge-triggered legacy HPET
> INTs has to be removed.

Since QEMU doesn't have a generic infrastructure to track interrupt
delivery, I decided to reuse something that is currently available.
The only mechanism that I'm aware of is the one that is utilized by
RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc.

The changes that would be introduced by part 1/5 and part 4/5 of
this patch series could be replaced if a generic infrastructure
to track interrupt delivery becomes available.

Regards,

Uli



[Qemu-devel] [PATCH 4/4] spice: drop obsolete iothread locking

2011-04-29 Thread Gerd Hoffmann
We don't use qemu internals from spice server context any more.
Thus we don't also need to grab the iothread mutex from spice
server context.  And we don't have to temporarely release the
lock to avoid deadlocks.  Drop all the calls.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl.c   |8 
 ui/spice-display.c |6 --
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 4dfddf0..2bb36c6 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -666,10 +666,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
 dprint(d, 1, "%s: start%s\n", __FUNCTION__,
loadvm ? " (loadvm)" : "");
 
-qemu_mutex_unlock_iothread();
 d->ssd.worker->reset_cursor(d->ssd.worker);
 d->ssd.worker->reset_image_cache(d->ssd.worker);
-qemu_mutex_lock_iothread();
 qxl_reset_surfaces(d);
 qxl_reset_memslots(d);
 
@@ -799,9 +797,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
 {
 dprint(d, 1, "%s:\n", __FUNCTION__);
 d->mode = QXL_MODE_UNDEFINED;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->destroy_surfaces(d->ssd.worker);
-qemu_mutex_lock_iothread();
 memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
 }
 
@@ -870,9 +866,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
 dprint(d, 1, "%s\n", __FUNCTION__);
 
 d->mode = QXL_MODE_UNDEFINED;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0);
-qemu_mutex_lock_iothread();
 }
 
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -942,10 +936,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_UPDATE_AREA:
 {
 QXLRect update = d->ram->update_area;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface,
&update, NULL, 0, 0);
-qemu_mutex_lock_iothread();
 break;
 }
 case QXL_IO_NOTIFY_CMD:
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 1e1a35d..34201cd 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -176,18 +176,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay 
*ssd)
 surface.mem= (intptr_t)ssd->buf;
 surface.group_id   = MEMSLOT_GROUP_HOST;
 
-qemu_mutex_unlock_iothread();
 ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
-qemu_mutex_lock_iothread();
 }
 
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 {
 dprint(1, "%s:\n", __FUNCTION__);
 
-qemu_mutex_unlock_iothread();
 ssd->worker->destroy_primary_surface(ssd->worker, 0);
-qemu_mutex_lock_iothread();
 }
 
 void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
@@ -197,9 +193,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int 
running, int reason)
 if (running) {
 ssd->worker->start(ssd->worker);
 } else {
-qemu_mutex_unlock_iothread();
 ssd->worker->stop(ssd->worker);
-qemu_mutex_lock_iothread();
 }
 ssd->running = running;
 }
-- 
1.7.1




[Qemu-devel] [PATCH 3/4] spice: don't call displaystate callbacks from spice server context.

2011-04-29 Thread Gerd Hoffmann
This patch moves the displaystate callback calls for setting the cursor
and the mouse pointer from spice server to qemu (iothread) context.
This allows us to simplify locking.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl-render.c|   25 -
 hw/qxl.c   |2 ++
 ui/spice-display.c |   12 
 ui/spice-display.h |3 +++
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 58965e0..1316066 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -185,7 +185,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
 QXLCursor *cursor;
 QEMUCursor *c;
-int x = -1, y = -1;
 
 if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) {
 return;
@@ -198,8 +197,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 }
 switch (cmd->type) {
 case QXL_CURSOR_SET:
-x = cmd->u.set.position.x;
-y = cmd->u.set.position.y;
 cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id);
 if (cursor->chunk.data_size != cursor->data_size) {
 fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__);
@@ -209,18 +206,20 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 if (c == NULL) {
 c = cursor_builtin_left_ptr();
 }
-qemu_mutex_lock_iothread();
-qxl->ssd.ds->cursor_define(c);
-qxl->ssd.ds->mouse_set(x, y, 1);
-qemu_mutex_unlock_iothread();
-cursor_put(c);
+qemu_mutex_lock(&qxl->ssd.lock);
+if (qxl->ssd.cursor) {
+cursor_put(qxl->ssd.cursor);
+}
+qxl->ssd.cursor = c;
+qxl->ssd.mouse_x = cmd->u.set.position.x;
+qxl->ssd.mouse_y = cmd->u.set.position.y;
+qemu_mutex_unlock(&qxl->ssd.lock);
 break;
 case QXL_CURSOR_MOVE:
-x = cmd->u.position.x;
-y = cmd->u.position.y;
-qemu_mutex_lock_iothread();
-qxl->ssd.ds->mouse_set(x, y, 1);
-qemu_mutex_unlock_iothread();
+qemu_mutex_lock(&qxl->ssd.lock);
+qxl->ssd.mouse_x = cmd->u.position.x;
+qxl->ssd.mouse_y = cmd->u.position.y;
+qemu_mutex_unlock(&qxl->ssd.lock);
 break;
 }
 }
diff --git a/hw/qxl.c b/hw/qxl.c
index bd250db..4dfddf0 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1309,6 +1309,8 @@ static int qxl_init_primary(PCIDevice *dev)
qxl_hw_screen_dump, qxl_hw_text_update, 
qxl);
 qxl->ssd.ds = vga->ds;
 qemu_mutex_init(&qxl->ssd.lock);
+qxl->ssd.mouse_x = -1;
+qxl->ssd.mouse_y = -1;
 qxl->ssd.bufsize = (16 * 1024 * 1024);
 qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 39c0ba1..1e1a35d 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -254,6 +254,16 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 ssd->update = qemu_spice_create_update(ssd);
 ssd->notify++;
 }
+if (ssd->cursor) {
+ssd->ds->cursor_define(ssd->cursor);
+cursor_put(ssd->cursor);
+ssd->cursor = NULL;
+}
+if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+ssd->ds->mouse_set(ssd->mouse_x, ssd->mouse_y, 1);
+ssd->mouse_x = -1;
+ssd->mouse_y = -1;
+}
 qemu_mutex_unlock(&ssd->lock);
 
 if (ssd->notify) {
@@ -409,6 +419,8 @@ void qemu_spice_display_init(DisplayState *ds)
 assert(sdpy.ds == NULL);
 sdpy.ds = ds;
 qemu_mutex_init(&sdpy.lock);
+sdpy.mouse_x = -1;
+sdpy.mouse_y = -1;
 sdpy.bufsize = (16 * 1024 * 1024);
 sdpy.buf = qemu_malloc(sdpy.bufsize);
 register_displaychangelistener(ds, &display_listener);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index e0cc46e..2f95f68 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -20,6 +20,7 @@
 #include 
 
 #include "qemu-thread.h"
+#include "console.h"
 #include "pflib.h"
 
 #define NUM_MEMSLOTS 8
@@ -55,6 +56,8 @@ struct SimpleSpiceDisplay {
  */
 QemuMutex lock;
 SimpleSpiceUpdate *update;
+QEMUCursor *cursor;
+int mouse_x, mouse_y;
 };
 
 struct SimpleSpiceUpdate {
-- 
1.7.1




[Qemu-devel] [PATCH 2/4] spice: don't create updates in spice server context.

2011-04-29 Thread Gerd Hoffmann
This patch moves the creation of spice screen updates from the spice
server context to qemu iothread context (display refresh timer to be
exact).  This way we avoid accessing qemu internals (display surface)
from spice thread context which in turn allows us to simplify locking.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl.c   |   17 +++--
 ui/spice-display.c |   45 -
 ui/spice-display.h |   21 -
 3 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index fe4212b..bd250db 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -343,18 +343,22 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 SimpleSpiceUpdate *update;
 QXLCommandRing *ring;
 QXLCommand *cmd;
-int notify;
+int notify, ret;
 
 switch (qxl->mode) {
 case QXL_MODE_VGA:
 dprint(qxl, 2, "%s: vga\n", __FUNCTION__);
-update = qemu_spice_create_update(&qxl->ssd);
-if (update == NULL) {
-return false;
+ret = false;
+qemu_mutex_lock(&qxl->ssd.lock);
+if (qxl->ssd.update != NULL) {
+update = qxl->ssd.update;
+qxl->ssd.update = NULL;
+*ext = update->ext;
+ret = true;
 }
-*ext = update->ext;
+qemu_mutex_unlock(&qxl->ssd.lock);
 qxl_log_command(qxl, "vga", ext);
-return true;
+return ret;
 case QXL_MODE_COMPAT:
 case QXL_MODE_NATIVE:
 case QXL_MODE_UNDEFINED:
@@ -1304,6 +1308,7 @@ static int qxl_init_primary(PCIDevice *dev)
 vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
qxl_hw_screen_dump, qxl_hw_text_update, 
qxl);
 qxl->ssd.ds = vga->ds;
+qemu_mutex_init(&qxl->ssd.lock);
 qxl->ssd.bufsize = (16 * 1024 * 1024);
 qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 020b423..39c0ba1 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -27,7 +27,7 @@
 
 #include "spice-display.h"
 
-static int debug = 0;
+static int debug = 1;
 
 static void GCC_FMT_ATTR(2, 3) dprint(int level, const char *fmt, ...)
 {
@@ -62,14 +62,7 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
 dest->right = MAX(dest->right, r->right);
 }
 
-/*
- * Called from spice server thread context (via interface_get_command).
- *
- * We must aquire the global qemu mutex here to make sure the
- * DisplayState (+DisplaySurface) we are accessing doesn't change
- * underneath us.
- */
-SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
 SimpleSpiceUpdate *update;
 QXLDrawable *drawable;
@@ -78,9 +71,7 @@ SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 uint8_t *src, *dst;
 int by, bw, bh;
 
-qemu_mutex_lock_iothread();
 if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-qemu_mutex_unlock_iothread();
 return NULL;
 };
 
@@ -141,7 +132,6 @@ SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 cmd->data = (intptr_t)drawable;
 
 memset(&ssd->dirty, 0, sizeof(ssd->dirty));
-qemu_mutex_unlock_iothread();
 return update;
 }
 
@@ -241,6 +231,12 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
 qemu_pf_conv_put(ssd->conv);
 ssd->conv = NULL;
 
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update != NULL) {
+qemu_spice_destroy_update(ssd, ssd->update);
+ssd->update = NULL;
+}
+qemu_mutex_unlock(&ssd->lock);
 qemu_spice_destroy_host_primary(ssd);
 qemu_spice_create_host_primary(ssd);
 
@@ -252,6 +248,14 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 {
 dprint(3, "%s:\n", __FUNCTION__);
 vga_hw_update();
+
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update == NULL) {
+ssd->update = qemu_spice_create_update(ssd);
+ssd->notify++;
+}
+qemu_mutex_unlock(&ssd->lock);
+
 if (ssd->notify) {
 ssd->notify = 0;
 ssd->worker->wakeup(ssd->worker);
@@ -298,14 +302,20 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 SimpleSpiceUpdate *update;
+int ret = false;
 
 dprint(3, "%s:\n", __FUNCTION__);
-update = qemu_spice_create_update(ssd);
-if (update == NULL) {
-return false;
+
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update != NULL) {
+update = ssd->update;
+ssd->update = NULL;
+*ext = update->ext;
+ret = true;
 }
-*ext = update->ext;
-return true;
+qemu_mutex_unlock(&ssd->lock);
+
+return ret;
 }
 
 static int interface_req_cmd_notification(QXLInstance *sin)
@@ -398,6 +408,7 @@ void qemu_spice_display_init(DisplayState *ds)
 {
 assert(sdpy.ds =

[Qemu-devel] [PATCH 0/4] spice: fix locking

2011-04-29 Thread Gerd Hoffmann
  Hi,

This patch series moves a bunch of work spice has to do from spice
server thread context to iothread context, which in turn allows to
drop the current locking mess as we don't touch qemu internals from
spice server thread any more.

A long-standing warning fix from Jes is included too.

cheers,
  Gerd

The following changes since commit 430a3c18064fd3c007048d757e8bd0fff45fcc99:

  configure: reenable opengl by default (2011-04-26 23:26:49 +0200)

are available in the git repository at:
  git://anongit.freedesktop.org/spice/qemu spice.v34

Gerd Hoffmann (3):
  spice: don't create updates in spice server context.
  spice: don't call displaystate callbacks from spice server context.
  spice: drop obsolete iothread locking

Jes Sorensen (1):
  Make spice dummy functions inline to fix calls not checking return values

 hw/qxl-render.c|   25 ++--
 hw/qxl.c   |   27 ++---
 ui/qemu-spice.h|   12 -
 ui/spice-display.c |   63 +---
 ui/spice-display.h |   24 +++
 5 files changed, 94 insertions(+), 57 deletions(-)



[Qemu-devel] [PATCH 1/4] Make spice dummy functions inline to fix calls not checking return values

2011-04-29 Thread Gerd Hoffmann
From: Jes Sorensen 

qemu_spice_set_passwd() and qemu_spice_set_pw_expire() dummy functions
needs to be inline, in order to handle the case where they are called
without checking the return value.

Signed-off-by: Jes Sorensen 
Signed-off-by: Gerd Hoffmann 
---
 ui/qemu-spice.h |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 916e5dc..3c6f1fe 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -47,8 +47,16 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
 #else  /* CONFIG_SPICE */
 
 #define using_spice 0
-#define qemu_spice_set_passwd(_p, _f1, _f2) (-1)
-#define qemu_spice_set_pw_expire(_e) (-1)
+static inline int qemu_spice_set_passwd(const char *passwd,
+bool fail_if_connected,
+bool disconnect_if_connected)
+{
+return -1;
+}
+static inline int qemu_spice_set_pw_expire(time_t expires)
+{
+return -1;
+}
 static inline int qemu_spice_migrate_info(const char *h, int p, int t, const 
char *s)
 { return -1; }
 
-- 
1.7.1




Re: [Qemu-devel] [PATCH 29/33] target-alpha: Add custom PALcode image for CLIPPER emulation.

2011-04-29 Thread Peter Maydell
2011/4/28 Richard Henderson :
> +[submodule "roms/qemu-palcode"]
> +       path = roms/qemu-palcode
> +       url = git://repo.or.cz/qemu-palcode.git

What's the license on this binary blob? There isn't
a COPYING file in the qemu-palcode git repo and most of
the source files seem to be missing copyright/licensing
headers.

thanks
-- PMM



[Qemu-devel] [PATCH] qemu-img resize: Fix option parsing

2011-04-29 Thread Kevin Wolf
For shrinking images, you're supposed to use a negative size. However, the
leading minus makes getopt think that it's an option and so you get the help
text if you don't use -- like in 'qemu-img resize test.img -- -1G'.

This patch handles the size first and removes it from the argument list so that
getopt won't even try to interpret it and you don't need -- any more.

Signed-off-by: Kevin Wolf 
---
 qemu-img.c |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ed5ba91..e825123 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1442,6 +1442,16 @@ static int img_resize(int argc, char **argv)
 { NULL }
 };
 
+/* Remove size from argv manually so that negative numbers are not treated
+ * as options by getopt. */
+if (argc < 3) {
+help();
+return 1;
+}
+
+size = argv[--argc];
+
+/* Parse getopt arguments */
 fmt = NULL;
 for(;;) {
 c = getopt(argc, argv, "f:h");
@@ -1458,11 +1468,10 @@ static int img_resize(int argc, char **argv)
 break;
 }
 }
-if (optind + 1 >= argc) {
+if (optind >= argc) {
 help();
 }
 filename = argv[optind++];
-size = argv[optind++];
 
 /* Choose grow, shrink, or absolute resize mode */
 switch (size[0]) {
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH 1/2] Support for MIPS64 user mode emulation

2011-04-29 Thread Aurelien Jarno
On Mon, Apr 25, 2011 at 04:54:19PM +0500, Khansa Butt wrote:
> please see inline comments highlighted in red color.
> 
> On Wed, Apr 13, 2011 at 2:32 AM, Aurelien Jarno wrote:
> 
> > [I don't know very well linux-user, it would be nice to Cc: Riku Voipio,
> >  the linux-user maintainer for the next version.]
> >
> > On Sat, Apr 09, 2011 at 04:02:31PM +0500, Khansa Butt wrote:
> > > From e96e20e50cada1c9e1b65de5925281cdd5659746 Mon Sep 17 00:00:00 2001
> > > From: Ehsan-ul-Haq & Khansa Butt 
> > > Date: Sat, 9 Apr 2011 10:51:22 +0500
> > > Subject: [PATCH 1/2] Support for MIPS64 user mode emulation
> > >
> > >
> > > Signed-off-by: Ehsan-ul-Haq, Abdul Qadeer, Abdul Waheed, Khansa Butt <
> > > kha...@kics.edu.pk>
> > > ---
> > >  configure |1 +
> > >  default-configs/mips64-linux-user.mak |1 +
> > >  linux-user/elfload.c  |2 +-
> > >  linux-user/main.c |   29
> > +++--
> > >  linux-user/mips64/syscall.h   |3 +++
> > >  linux-user/signal.c   |3 ++-
> > >  target-mips/translate.c   |1 +
> > >  7 files changed, 36 insertions(+), 4 deletions(-)
> > >  create mode 100644 default-configs/mips64-linux-user.mak
> > >
> > > diff --git a/configure b/configure
> > > index ae97e11..d1f7867 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1039,6 +1039,7 @@ m68k-linux-user \
> > >  microblaze-linux-user \
> > >  microblazeel-linux-user \
> > >  mips-linux-user \
> > > +mips64-linux-user \
> > >  mipsel-linux-user \
> > >  ppc-linux-user \
> > >  ppc64-linux-user \
> > > diff --git a/default-configs/mips64-linux-user.mak
> > > b/default-configs/mips64-linux-user.mak
> > > new file mode 100644
> > > index 000..1598bfc
> > > --- /dev/null
> > > +++ b/default-configs/mips64-linux-user.mak
> > > @@ -0,0 +1 @@
> > > +# Default configuration for mips64-linux-user
> > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > > index fe5410e..2832a33 100644
> > > --- a/linux-user/elfload.c
> > > +++ b/linux-user/elfload.c
> > > @@ -1384,7 +1384,7 @@ static void load_elf_image(const char *image_name,
> > int
> > > image_fd,
> > >  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > >  vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> > >
> > > -error = target_mmap(vaddr_ps, eppnt->p_filesz + vaddr_po,
> > > +error = target_mmap(vaddr_ps, eppnt->p_memsz + vaddr_po,
> >
> > What is the goal of this change? If the mmapped aread is bigger than the
> > file size rounded up to te page size, it will cause a SIGBUS.
> >
> > >  elf_prot, MAP_PRIVATE | MAP_FIXED,
> > >  image_fd, eppnt->p_offset - vaddr_po);
> > >  if (error == -1) {
> > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > index e651bfd..a7f4955 100644
> > > --- a/linux-user/main.c
> > > +++ b/linux-user/main.c
> > > @@ -1937,6 +1937,14 @@ static int do_store_exclusive(CPUMIPSState *env)
> > >  int d;
> > >
> > >  addr = env->lladdr;
> > > +#if defined(TARGET_MIPS64)
> > > +/* For MIPS64 on 32 bit host there is a need to make
> > > +* the page accessible to which the above 'addr' is belonged */
> > > +#if HOST_LONG_BITS == 32
> > > +int flag = PAGE_VALID | PAGE_READ | PAGE_WRITE | PAGE_WRITE_ORG;
> > > +page_set_flags(addr, addr + 4096, flag);
> > > +#endif
> > > +#endif
> >
> > I don't really see the reason why this should be done that way. Are you
> > trying to run MIPS32 binaries compiled for 8kB page size?
> >
> 
> 
> 
> this change is needed when we run MIPS64 ELF on 32 bit x86 host. MIPS64 ELF
> contains 36 bit address.

Actually it can contains up to 62-bit address there (all the user mapped
space).

>  load_elf_image() at /home/khansa/testpatch/qemu/linux-user/elfload.c: QEMU
>  contains these lines
>/* Round addresses to page boundaries.  */
> loaddr &= qemu_host_page_mask;
> hiaddr = HOST_PAGE_ALIGN(hiaddr);
> when QEMU run on 32 bit x86 the above two variables are rounded to 32 bit
> value while these should be 36 bits as these come from MIPS64 ELF.and then

It is correct to truncate them, as the address space of the host is
smaller. It's just based on the fact that programs only need a subset of
the 62 bit address space.

> for these rounded address l1_map is initialized in page_find_alloc().
> in case of SCD(store condition double ) instruction of MIPS64r2 when we have
> to check load linked address its again 36 bit so it will make an index(addr
> >> TARGET_PAGE_BITS) for which l1_map is no valid entry, returning 0 value
> and we got segmentation fault. this is the reason we did following changes
> in main.c do_store_exclusive()

No, the load linked address register size, as well as the shift is
actually implementation dependent. On old 64-bit MIPS implementation 
and MIPS32 core it is a 32-bit register and a 4-bit shift, where as 
o

Re: [Qemu-devel] [PULL] Request for Pull

2011-04-29 Thread Aneesh Kumar K.V
On Thu, 28 Apr 2011 12:39:45 -0500, Anthony Liguori  
wrote:
> On 04/28/2011 12:21 PM, Stefan Weil wrote:
> > Am 28.04.2011 15:56, schrieb Anthony Liguori:
> >> On 04/27/2011 11:16 AM, Venkateswararao Jujjuri wrote:
> >>> The following changes since commit
> >>> 661bfc80e876d32da8befe53ba0234d87fc0bcc2:
> >>> Jan Kiszka (1):
> >>> pflash: Restore & fix lazy ROMD switching
> >>>
> >>> are available in the git repository at:
> >>>
> >>> git://repo.or.cz/qemu/aliguori/jvrao.git for-anthony
> >>
> >> Pulled. Thanks.
> >>
> >> Regards,
> >>
> >> Anthony Liguori
> >
> >
> > This pull breaks compilation and linkage of latest QEMU:
> >
> > * Compilation fails because of wrong include paths (caused by moved files).
> > I just sent a patch to fix this issue.
> >
> > * The linker fails for all system emulations without virtio. Example:
> >
> > LINK cris-softmmu/qemu-system-cris
> > ../fsdev/qemu-fsdev.o:(.data+0xc): undefined reference to `local_ops'
> 
> The use of CONFIG_REALLY_VIRTFS was wrong in the rename patch.  I'll 
> push a fix.
> 

The change you committed will pull in lot of 9p related code even
when virtfs is not really enabled right ? why not ?

diff --git a/Makefile.objs b/Makefile.objs
index 3833314..bbae9a4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -51,7 +51,7 @@ ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS),yy)
 CONFIG_REALLY_VIRTFS=y
 endif
 fsdev-nested-$(CONFIG_VIRTFS) = qemu-fsdev.o
-fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y))
+fsdev-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y))
 
 ##
 # libqemu_common.a: Target independent part of system emulation. The
diff --git a/vl.c b/vl.c
index 14255c4..5a7810f 100644
--- a/vl.c
+++ b/vl.c
@@ -1607,7 +1607,7 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
 return 0;
 }
 
-#ifdef CONFIG_VIRTFS
+#ifdef CONFIG_REALLY_VIRTFS
 static int fsdev_init_func(QemuOpts *opts, void *opaque)
 {
 int ret;
@@ -2822,7 +2822,7 @@ int main(int argc, char **argv, char **envp)
 
 if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, 
1) != 0)
 exit(1);
-#ifdef CONFIG_VIRTFS
+#ifdef CONFIG_REALLY_VIRTFS
 if (qemu_opts_foreach(qemu_find_opts("fsdev"), fsdev_init_func, NULL, 1) 
!= 0) {
 exit(1);
 }





[Qemu-devel] [Bug 754591] Re: NIC doesn't work when it had been used before

2011-04-29 Thread flypen
I found this problem, too. A VM used a NIC with PCI-passthrough mode.
Then I shutdown the VM, and tried to give the NIC back to the host. But
I found I couldn't assign the NIC to the host again.  This problem
didn't occur each time. If I did this steps for ten times, I could see
it at least once.

The kernel version was 2.6.32. I used Intel e1000e driver for the NIC. Here are 
some of the logs:
kernel: e1000e :02:00.1: enabling device ( -> 0002)
kernel: e1000e :02:00.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
kernel: :02:00.1: :02:00.1: PHY reset is blocked due to SOL/IDER 
session.
kernel: :02:00.1: :02:00.1: The NVM Checksum Is Not Valid
 kernel: e1000e :02:00.1: PCI INT B disabled

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

Title:
  NIC doesn't work when it had been used before

Status in QEMU:
  New

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64): All
  Guest OS (ia32/ia32e/IA64): All
  Guest OS Type (Linux/Windows):All
  kvm.git Commit:3b840cc27e5b831a26e3303096b88e112d1cf59f
  qemu-kvm Commit:df85c051d780bca0ee2462cfeb8ef6d9552a19b0
  Host Kernel Version:2.6.38+
  Hardware:Westmere-EP / Sandy Bridge

  
  Bug detailed description:
  --
  NIC doesn't work when it had been used before.
  Statically assign a NIC to guest, it works well. Then shutdown the guest, and
  assign the same NIC to a guest again. The NIC doesn't work. The dmesg of the
  guest is as following.
  --
  Intel(R) Gigabit Ethernet Network Driver - version 2.1.0-k2
  Copyright (c) 2007-2009 Intel Corporation.
  ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11
  igb :00:03.0: PCI INT A -> Link[LNKC] -> GSI 11 (level, high) -> IRQ 11
  igb :00:03.0: setting latency timer to 64
alloc irq_desc for 24 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 24 for MSI/MSI-X
alloc irq_desc for 25 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 25 for MSI/MSI-X
alloc irq_desc for 26 on node -1
alloc kstat_irqs on node -1
  igb :00:03.0: irq 26 for MSI/MSI-X
  igb :00:03.0: PHY reset is blocked due to SOL/IDER session.
  igb :00:03.0: The NVM Checksum Is Not Valid
  igb :00:03.0: PCI INT A disabled
  igb: probe of :00:03.0 failed with error -5

  
  Reproduce steps:
  
  1.statically assign a NIC to a guest, check if it works well
  2.shutdown the guest.
  3.statically assign the same NIC to a guest (NIC doesn't work now).



Re: [Qemu-devel] [PATCH 00/33] Alpha system emulation, v3

2011-04-29 Thread Paolo Bonzini

On 04/28/2011 10:50 PM, Richard Henderson wrote:

   * I still don't think I know how git submodules really work.  When I
 updated the one patch containing the PALcode blob, I couldn't figure
 out how to change the submodule revision hash number to HEAD.


cd roms/palcode
git pull origin
# ensure no merge happened, etc. etc.
cd ../..
# update blob
git commit roms/palcode path/to/blob

Paolo



Re: [Qemu-devel] QEMU testing methodology & results

2011-04-29 Thread Paolo Bonzini

On 04/29/2011 02:17 AM, Peter Maydell wrote:

The theoretical aim there as far
as I'm concerned is architectural correctness -- in other words we
should be a valid implementation of the architecture,


That's not even the case for x86.  It should be a goal, however, that 
with mainstream kernels user programs shouldn't be able to see the 
difference.  There are some known issues besides bugs, for example the 
"instruction pointer of the last FP instruction" register (!) is not 
implemented.


Paolo